Commit e240a65c authored by Tom Lane's avatar Tom Lane

Provide an error cursor for "can't call an SRF here" errors.

Since it appears that v10 is going to move the goalposts by some amount
in terms of where you can and can't invoke set-returning functions,
arrange for the executor's "set-valued function called in context that
cannot accept a set" errors to include a syntax position if possible,
pointing to the specific SRF that can't be called where it's located.

The main bit of infrastructure needed for this is to make the query source
text accessible in the executor; but it turns out that commit 4c728f38
already did that.  We just need a new function executor_errposition()
modeled on parser_errposition(), and we're ready to rock.

While experimenting with this, I noted that the error position wasn't
properly reported if it occurred in a plpgsql FOR-over-query loop,
which turned out to be because SPI_cursor_open_internal wasn't providing
an error context callback during PortalStart.  Fix that.

There's a whole lot more that could be done with this infrastructure
now that it's there, but this is not the right time in the development
cycle for that sort of work.  Hence, resist the temptation to plaster
executor_errposition() calls everywhere ... for the moment.

Discussion: https://postgr.es/m/5263.1492471571@sss.pgh.pa.us
parent 280c53ec
...@@ -2103,7 +2103,9 @@ ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args, Oid funcid, ...@@ -2103,7 +2103,9 @@ ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args, Oid funcid,
if (flinfo->fn_retset) if (flinfo->fn_retset)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("set-valued function called in context that cannot accept a set"))); errmsg("set-valued function called in context that cannot accept a set"),
parent ? executor_errposition(parent->state,
exprLocation((Node *) node)) : 0));
/* Build code to evaluate arguments directly into the fcinfo struct */ /* Build code to evaluate arguments directly into the fcinfo struct */
argno = 0; argno = 0;
......
...@@ -34,7 +34,8 @@ ...@@ -34,7 +34,8 @@
/* static function decls */ /* static function decls */
static void init_sexpr(Oid foid, Oid input_collation, SetExprState *sexpr, static void init_sexpr(Oid foid, Oid input_collation, Expr *node,
SetExprState *sexpr, PlanState *parent,
MemoryContext sexprCxt, bool allowSRF, bool needDescForSRF); MemoryContext sexprCxt, bool allowSRF, bool needDescForSRF);
static void ShutdownSetExpr(Datum arg); static void ShutdownSetExpr(Datum arg);
static void ExecEvalFuncArgs(FunctionCallInfo fcinfo, static void ExecEvalFuncArgs(FunctionCallInfo fcinfo,
...@@ -77,7 +78,7 @@ ExecInitTableFunctionResult(Expr *expr, ...@@ -77,7 +78,7 @@ ExecInitTableFunctionResult(Expr *expr,
state->funcReturnsSet = func->funcretset; state->funcReturnsSet = func->funcretset;
state->args = ExecInitExprList(func->args, parent); state->args = ExecInitExprList(func->args, parent);
init_sexpr(func->funcid, func->inputcollid, state, init_sexpr(func->funcid, func->inputcollid, expr, state, parent,
econtext->ecxt_per_query_memory, func->funcretset, false); econtext->ecxt_per_query_memory, func->funcretset, false);
} }
else else
...@@ -438,7 +439,7 @@ ExecInitFunctionResultSet(Expr *expr, ...@@ -438,7 +439,7 @@ ExecInitFunctionResultSet(Expr *expr,
FuncExpr *func = (FuncExpr *) expr; FuncExpr *func = (FuncExpr *) expr;
state->args = ExecInitExprList(func->args, parent); state->args = ExecInitExprList(func->args, parent);
init_sexpr(func->funcid, func->inputcollid, state, init_sexpr(func->funcid, func->inputcollid, expr, state, parent,
econtext->ecxt_per_query_memory, true, true); econtext->ecxt_per_query_memory, true, true);
} }
else if (IsA(expr, OpExpr)) else if (IsA(expr, OpExpr))
...@@ -446,7 +447,7 @@ ExecInitFunctionResultSet(Expr *expr, ...@@ -446,7 +447,7 @@ ExecInitFunctionResultSet(Expr *expr,
OpExpr *op = (OpExpr *) expr; OpExpr *op = (OpExpr *) expr;
state->args = ExecInitExprList(op->args, parent); state->args = ExecInitExprList(op->args, parent);
init_sexpr(op->opfuncid, op->inputcollid, state, init_sexpr(op->opfuncid, op->inputcollid, expr, state, parent,
econtext->ecxt_per_query_memory, true, true); econtext->ecxt_per_query_memory, true, true);
} }
else else
...@@ -645,7 +646,8 @@ restart: ...@@ -645,7 +646,8 @@ restart:
* init_sexpr - initialize a SetExprState node during first use * init_sexpr - initialize a SetExprState node during first use
*/ */
static void static void
init_sexpr(Oid foid, Oid input_collation, SetExprState *sexpr, init_sexpr(Oid foid, Oid input_collation, Expr *node,
SetExprState *sexpr, PlanState *parent,
MemoryContext sexprCxt, bool allowSRF, bool needDescForSRF) MemoryContext sexprCxt, bool allowSRF, bool needDescForSRF)
{ {
AclResult aclresult; AclResult aclresult;
...@@ -683,7 +685,9 @@ init_sexpr(Oid foid, Oid input_collation, SetExprState *sexpr, ...@@ -683,7 +685,9 @@ init_sexpr(Oid foid, Oid input_collation, SetExprState *sexpr,
if (sexpr->func.fn_retset && !allowSRF) if (sexpr->func.fn_retset && !allowSRF)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("set-valued function called in context that cannot accept a set"))); errmsg("set-valued function called in context that cannot accept a set"),
parent ? executor_errposition(parent->state,
exprLocation((Node *) node)) : 0));
/* Otherwise, caller should have marked the sexpr correctly */ /* Otherwise, caller should have marked the sexpr correctly */
Assert(sexpr->func.fn_retset == sexpr->funcReturnsSet); Assert(sexpr->func.fn_retset == sexpr->funcReturnsSet);
......
...@@ -28,6 +28,8 @@ ...@@ -28,6 +28,8 @@
* ExecOpenScanRelation Common code for scan node init routines. * ExecOpenScanRelation Common code for scan node init routines.
* ExecCloseScanRelation * ExecCloseScanRelation
* *
* executor_errposition Report syntactic position of an error.
*
* RegisterExprContextCallback Register function shutdown callback * RegisterExprContextCallback Register function shutdown callback
* UnregisterExprContextCallback Deregister function shutdown callback * UnregisterExprContextCallback Deregister function shutdown callback
* *
...@@ -44,6 +46,7 @@ ...@@ -44,6 +46,7 @@
#include "access/relscan.h" #include "access/relscan.h"
#include "access/transam.h" #include "access/transam.h"
#include "executor/executor.h" #include "executor/executor.h"
#include "mb/pg_wchar.h"
#include "nodes/nodeFuncs.h" #include "nodes/nodeFuncs.h"
#include "parser/parsetree.h" #include "parser/parsetree.h"
#include "storage/lmgr.h" #include "storage/lmgr.h"
...@@ -685,6 +688,36 @@ UpdateChangedParamSet(PlanState *node, Bitmapset *newchg) ...@@ -685,6 +688,36 @@ UpdateChangedParamSet(PlanState *node, Bitmapset *newchg)
bms_free(parmset); bms_free(parmset);
} }
/*
* executor_errposition
* Report an execution-time cursor position, if possible.
*
* This is expected to be used within an ereport() call. The return value
* is a dummy (always 0, in fact).
*
* The locations stored in parsetrees are byte offsets into the source string.
* We have to convert them to 1-based character indexes for reporting to
* clients. (We do things this way to avoid unnecessary overhead in the
* normal non-error case: computing character indexes would be much more
* expensive than storing token offsets.)
*/
int
executor_errposition(EState *estate, int location)
{
int pos;
/* No-op if location was not provided */
if (location < 0)
return 0;
/* Can't do anything if source text is not available */
if (estate == NULL || estate->es_sourceText == NULL)
return 0;
/* Convert offset to character number */
pos = pg_mbstrlen_with_len(estate->es_sourceText, location) + 1;
/* And pass it to the ereport mechanism */
return errposition(pos);
}
/* /*
* Register a shutdown callback in an ExprContext. * Register a shutdown callback in an ExprContext.
* *
......
...@@ -1197,9 +1197,6 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, ...@@ -1197,9 +1197,6 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
cplan = GetCachedPlan(plansource, paramLI, false, _SPI_current->queryEnv); cplan = GetCachedPlan(plansource, paramLI, false, _SPI_current->queryEnv);
stmt_list = cplan->stmt_list; stmt_list = cplan->stmt_list;
/* Pop the error context stack */
error_context_stack = spierrcontext.previous;
if (!plan->saved) if (!plan->saved)
{ {
/* /*
...@@ -1318,6 +1315,9 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, ...@@ -1318,6 +1315,9 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
Assert(portal->strategy != PORTAL_MULTI_QUERY); Assert(portal->strategy != PORTAL_MULTI_QUERY);
/* Pop the error context stack */
error_context_stack = spierrcontext.previous;
/* Pop the SPI stack */ /* Pop the SPI stack */
_SPI_end_call(true); _SPI_end_call(true);
......
...@@ -482,6 +482,8 @@ extern bool ExecRelationIsTargetRelation(EState *estate, Index scanrelid); ...@@ -482,6 +482,8 @@ extern bool ExecRelationIsTargetRelation(EState *estate, Index scanrelid);
extern Relation ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags); extern Relation ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags);
extern void ExecCloseScanRelation(Relation scanrel); extern void ExecCloseScanRelation(Relation scanrel);
extern int executor_errposition(EState *estate, int location);
extern void RegisterExprContextCallback(ExprContext *econtext, extern void RegisterExprContextCallback(ExprContext *econtext,
ExprContextCallbackFunction function, ExprContextCallbackFunction function,
Datum arg); Datum arg);
......
...@@ -193,9 +193,13 @@ SELECT few.dataa, count(*) FROM few WHERE dataa = 'a' GROUP BY few.dataa, unnest ...@@ -193,9 +193,13 @@ SELECT few.dataa, count(*) FROM few WHERE dataa = 'a' GROUP BY few.dataa, unnest
-- SRFs are not allowed in aggregate arguments -- SRFs are not allowed in aggregate arguments
SELECT min(generate_series(1, 3)) FROM few; SELECT min(generate_series(1, 3)) FROM few;
ERROR: set-valued function called in context that cannot accept a set ERROR: set-valued function called in context that cannot accept a set
LINE 1: SELECT min(generate_series(1, 3)) FROM few;
^
-- SRFs are not allowed in window function arguments, either -- SRFs are not allowed in window function arguments, either
SELECT min(generate_series(1, 3)) OVER() FROM few; SELECT min(generate_series(1, 3)) OVER() FROM few;
ERROR: set-valued function called in context that cannot accept a set ERROR: set-valued function called in context that cannot accept a set
LINE 1: SELECT min(generate_series(1, 3)) OVER() FROM few;
^
-- SRFs are normally computed after window functions -- SRFs are normally computed after window functions
SELECT id,lag(id) OVER(), count(*) OVER(), generate_series(1,3) FROM few; SELECT id,lag(id) OVER(), count(*) OVER(), generate_series(1,3) FROM few;
id | lag | count | generate_series id | lag | count | generate_series
...@@ -424,6 +428,8 @@ SELECT int4mul(generate_series(1,2), 10); ...@@ -424,6 +428,8 @@ SELECT int4mul(generate_series(1,2), 10);
-- but SRFs in function RTEs must be at top level (annoying restriction) -- but SRFs in function RTEs must be at top level (annoying restriction)
SELECT * FROM int4mul(generate_series(1,2), 10); SELECT * FROM int4mul(generate_series(1,2), 10);
ERROR: set-valued function called in context that cannot accept a set ERROR: set-valued function called in context that cannot accept a set
LINE 1: SELECT * FROM int4mul(generate_series(1,2), 10);
^
-- DISTINCT ON is evaluated before tSRF evaluation if SRF is not -- DISTINCT ON is evaluated before tSRF evaluation if SRF is not
-- referenced either in ORDER BY or in the DISTINCT ON list. The ORDER -- referenced either in ORDER BY or in the DISTINCT ON list. The ORDER
-- BY reference can be implicitly generated, if there's no other ORDER BY. -- BY reference can be implicitly generated, if there's no other ORDER BY.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment