From d43a619c60405ecda275ca9e3ac9ead242e20ecb Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 16 Jan 2017 15:23:11 -0500
Subject: [PATCH] Fix check_srf_call_placement() to handle VALUES cases
correctly.
INSERT ... VALUES with a single VALUES row is implemented quite differently
from the general VALUES case. A user-visible implication of that is that
we accept SRFs in the single-row case, but not in the multi-row case.
That's a historical artifact no doubt, but in view of the lack of field
complaints, I'm not excited about fixing it right now.
However, check_srf_call_placement() needs to know about this, first because
it should throw an error in the unsupported case, and second because it
should set p_hasTargetSRFs in the single-row case (because we treat that
like a SELECT tlist). That's an oversight in commit a4c35ea1c.
To fix, split EXPR_KIND_VALUES into two values. So far as I can see,
this is the only place where we need to distinguish the two cases at
present; but there might be more later.
Patch by me, per report from Andres Freund.
Discussion: https://postgr.es/m/20170116081548.zg63zltblwimpfgp@alap3.anarazel.de
---
src/backend/parser/analyze.c | 2 +-
src/backend/parser/parse_agg.c | 2 ++
src/backend/parser/parse_expr.c | 2 ++
src/backend/parser/parse_func.c | 7 ++++++-
src/include/parser/parse_node.h | 1 +
src/test/regress/expected/tsrf.out | 4 +++-
6 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 8278e742f8d..a02a77a03a5 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -790,7 +790,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
*/
exprList = transformExpressionList(pstate,
(List *) linitial(valuesLists),
- EXPR_KIND_VALUES,
+ EXPR_KIND_VALUES_SINGLE,
true);
/* Prepare row for assignment to target table */
diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index d9777099aa6..7be7fe96892 100644
--- a/src/backend/parser/parse_agg.c
+++ b/src/backend/parser/parse_agg.c
@@ -447,6 +447,7 @@ check_agglevels_and_constraints(ParseState *pstate, Node *expr)
errkind = true;
break;
case EXPR_KIND_VALUES:
+ case EXPR_KIND_VALUES_SINGLE:
errkind = true;
break;
case EXPR_KIND_CHECK_CONSTRAINT:
@@ -840,6 +841,7 @@ transformWindowFuncCall(ParseState *pstate, WindowFunc *wfunc,
errkind = true;
break;
case EXPR_KIND_VALUES:
+ case EXPR_KIND_VALUES_SINGLE:
errkind = true;
break;
case EXPR_KIND_CHECK_CONSTRAINT:
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 73521b93fca..add3be65660 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -1797,6 +1797,7 @@ transformSubLink(ParseState *pstate, SubLink *sublink)
case EXPR_KIND_OFFSET:
case EXPR_KIND_RETURNING:
case EXPR_KIND_VALUES:
+ case EXPR_KIND_VALUES_SINGLE:
/* okay */
break;
case EXPR_KIND_CHECK_CONSTRAINT:
@@ -3410,6 +3411,7 @@ ParseExprKindName(ParseExprKind exprKind)
case EXPR_KIND_RETURNING:
return "RETURNING";
case EXPR_KIND_VALUES:
+ case EXPR_KIND_VALUES_SINGLE:
return "VALUES";
case EXPR_KIND_CHECK_CONSTRAINT:
case EXPR_KIND_DOMAIN_CHECK:
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 12979602e59..a52261f2e92 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -2141,7 +2141,12 @@ check_srf_call_placement(ParseState *pstate, int location)
errkind = true;
break;
case EXPR_KIND_VALUES:
- /* okay */
+ /* SRFs are presently not supported by nodeValuesscan.c */
+ errkind = true;
+ break;
+ case EXPR_KIND_VALUES_SINGLE:
+ /* okay, since we process this like a SELECT tlist */
+ pstate->p_hasTargetSRFs = true;
break;
case EXPR_KIND_CHECK_CONSTRAINT:
case EXPR_KIND_DOMAIN_CHECK:
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index 7cdf142df63..bc3eea9ba5a 100644
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -55,6 +55,7 @@ typedef enum ParseExprKind
EXPR_KIND_OFFSET, /* OFFSET */
EXPR_KIND_RETURNING, /* RETURNING */
EXPR_KIND_VALUES, /* VALUES */
+ EXPR_KIND_VALUES_SINGLE, /* single-row VALUES (in INSERT only) */
EXPR_KIND_CHECK_CONSTRAINT, /* CHECK constraint for a table */
EXPR_KIND_DOMAIN_CHECK, /* CHECK constraint for a domain */
EXPR_KIND_COLUMN_DEFAULT, /* default value for a table column */
diff --git a/src/test/regress/expected/tsrf.out b/src/test/regress/expected/tsrf.out
index 8c54f717df8..7bb6d17fcb0 100644
--- a/src/test/regress/expected/tsrf.out
+++ b/src/test/regress/expected/tsrf.out
@@ -374,7 +374,9 @@ LINE 1: INSERT INTO fewmore VALUES(1) RETURNING generate_series(1,3)...
^
-- nor standalone VALUES (but surely this is a bug?)
VALUES(1, generate_series(1,2));
-ERROR: set-valued function called in context that cannot accept a set
+ERROR: set-returning functions are not allowed in VALUES
+LINE 1: VALUES(1, generate_series(1,2));
+ ^
-- We allow tSRFs that are not at top level
SELECT int4mul(generate_series(1,2), 10);
int4mul
--
GitLab