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[email protected]

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 8278e742f8dcfd9b27d632d39230d808bd80a113..a02a77a03a532c4f3819be698eebb4e458d2e67a 100644 (file)
--- 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 d9777099aa6f298652a47ec98f4066dc6baa5e43..7be7fe96892e3b4370e3a2d17246d140e0ea8baf 100644 (file)
--- 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 73521b93fcab498b3ffb3149183047bff7315bd6..add3be6566031cb35c70b914ff2e42fa9a950d8e 100644 (file)
--- 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 12979602e5968200f334e98f4a3ccc5386a0cadb..a52261f2e92e7058e222bb54c71f7b3e7d6ee8a2 100644 (file)
--- 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 7cdf142df630e4a9700afcc1f26fdf424cd94502..bc3eea9ba5a2eba3d20ec3127e9dbd0b784f4ba5 100644 (file)
--- 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 8c54f717df83096c3505b6ea8dbed0f6d6ad3480..7bb6d17fcb0aa5f7a39b35823435f557a6b133ee 100644 (file)
--- 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