From c9d5298485b78a37923a23f9af9aa0ade06762db Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 4 Jan 2021 11:52:00 -0500 Subject: [PATCH] Re-implement pl/pgsql's expression and assignment parsing. Invent new RawParseModes that allow the core grammar to handle pl/pgsql expressions and assignments directly, and thereby get rid of a lot of hackery in pl/pgsql's parser. This moves a good deal of knowledge about pl/pgsql into the core code: notably, we have to invent a CoercionContext that matches pl/pgsql's (rather dubious) historical behavior for assignment coercions. That's getting away from the original idea of pl/pgsql as an arm's-length extension of the core, but really we crossed that bridge a long time ago. The main advantage of doing this is that we can now use the core parser to generate FieldStore and/or SubscriptingRef nodes to handle assignments to pl/pgsql variables that are records or arrays. That fixes a number of cases that had never been implemented in pl/pgsql assignment, such as nested records and array slicing, and it allows pl/pgsql assignment to support the datatype-specific subscripting behaviors introduced in commit c7aba7c14. There are cosmetic benefits too: when a syntax error occurs in a pl/pgsql expression, the error report no longer includes the confusing "SELECT" keyword that used to get prefixed to the expression text. Also, there seem to be some small speed gains. Discussion: https://postgr.es/m/4165684.1607707277@sss.pgh.pa.us --- contrib/hstore/expected/hstore.out | 4 + contrib/hstore/sql/hstore.sql | 4 + doc/src/sgml/plpgsql.sgml | 21 +- src/backend/commands/functioncmds.c | 1 + src/backend/executor/spi.c | 54 +++- src/backend/nodes/copyfuncs.c | 17 ++ src/backend/nodes/equalfuncs.c | 15 ++ src/backend/nodes/nodeFuncs.c | 10 + src/backend/nodes/outfuncs.c | 15 ++ src/backend/parser/analyze.c | 240 ++++++++++++++++++ src/backend/parser/gram.y | 99 +++++++- src/backend/parser/parse_coerce.c | 8 + src/backend/parser/parse_target.c | 41 +-- src/backend/parser/parser.c | 6 +- src/backend/tcop/utility.c | 8 + src/include/nodes/nodes.h | 1 + src/include/nodes/parsenodes.h | 19 ++ src/include/nodes/primnodes.h | 1 + src/include/parser/parse_target.h | 12 + src/include/parser/parser.h | 13 +- src/interfaces/ecpg/preproc/parse.pl | 6 +- src/pl/plpgsql/src/Makefile | 2 +- src/pl/plpgsql/src/expected/plpgsql_array.out | 94 +++++++ .../plpgsql/src/expected/plpgsql_record.out | 91 ++++++- .../plpgsql/src/expected/plpgsql_varprops.out | 2 +- src/pl/plpgsql/src/pl_comp.c | 49 ++-- src/pl/plpgsql/src/pl_exec.c | 49 +++- src/pl/plpgsql/src/pl_gram.y | 196 ++++++++------ src/pl/plpgsql/src/plpgsql.h | 5 +- src/pl/plpgsql/src/sql/plpgsql_array.sql | 79 ++++++ src/pl/plpgsql/src/sql/plpgsql_record.sql | 42 +++ src/test/regress/expected/plpgsql.out | 36 +-- 32 files changed, 1081 insertions(+), 159 deletions(-) create mode 100644 src/pl/plpgsql/src/expected/plpgsql_array.out create mode 100644 src/pl/plpgsql/src/sql/plpgsql_array.sql diff --git a/contrib/hstore/expected/hstore.out b/contrib/hstore/expected/hstore.out index fdcc3920ce..64a3272b9c 100644 --- a/contrib/hstore/expected/hstore.out +++ b/contrib/hstore/expected/hstore.out @@ -1583,6 +1583,10 @@ select f2 from test_json_agg; "d"=>NULL, "x"=>"xyzzy" (3 rows) +-- Test subscripting in plpgsql +do $$ declare h hstore; +begin h['a'] := 'b'; raise notice 'h = %, h[a] = %', h, h['a']; end $$; +NOTICE: h = "a"=>"b", h[a] = b -- Check the hstore_hash() and hstore_hash_extended() function explicitly. SELECT v as value, hstore_hash(v)::bit(32) as standard, hstore_hash_extended(v, 0)::bit(32) as extended0, diff --git a/contrib/hstore/sql/hstore.sql b/contrib/hstore/sql/hstore.sql index 8d96e30403..a59db66b0a 100644 --- a/contrib/hstore/sql/hstore.sql +++ b/contrib/hstore/sql/hstore.sql @@ -372,6 +372,10 @@ select f2['d':'e'] from test_json_agg; -- error update test_json_agg set f2['d'] = f2['e'], f2['x'] = 'xyzzy'; select f2 from test_json_agg; +-- Test subscripting in plpgsql +do $$ declare h hstore; +begin h['a'] := 'b'; raise notice 'h = %, h[a] = %', h, h['a']; end $$; + -- Check the hstore_hash() and hstore_hash_extended() function explicitly. SELECT v as value, hstore_hash(v)::bit(32) as standard, hstore_hash_extended(v, 0)::bit(32) as extended0, diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index 11246aa653..45d3e43ed1 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -946,8 +946,8 @@ PREPARE statement_name(integer, integer) AS SELECT $1 database engine. The expression must yield a single value (possibly a row value, if the variable is a row or record variable). The target variable can be a simple variable (optionally qualified with a block - name), a field of a row or record variable, or an element of an array - that is a simple variable or field. Equal (=) can be + name), a field of a row or record target, or an element or slice of + an array target. Equal (=) can be used instead of PL/SQL-compliant :=. @@ -968,8 +968,25 @@ PREPARE statement_name(integer, integer) AS SELECT $1 tax := subtotal * 0.06; my_record.user_id := 20; +my_array[j] := 20; +my_array[1:3] := array[1,2,3]; +complex_array[n].realpart = 12.3; + + + It's useful to know that what follows the assignment operator is + essentially treated as a SELECT command; as long + as it returns a single row and column, it will work. Thus for example + one can write something like + +total_sales := sum(quantity) from sales; + + This provides an effect similar to the single-row SELECT + ... INTO syntax described in + . However, that syntax + is more portable. + diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index 55950e754d..7a4e104623 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -1628,6 +1628,7 @@ CreateCast(CreateCastStmt *stmt) case COERCION_ASSIGNMENT: castcontext = COERCION_CODE_ASSIGNMENT; break; + /* COERCION_PLPGSQL is intentionally not covered here */ case COERCION_EXPLICIT: castcontext = COERCION_CODE_EXPLICIT; break; diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 6c0593686a..e28d242922 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -51,6 +51,12 @@ static _SPI_connection *_SPI_current = NULL; static int _SPI_stack_depth = 0; /* allocated size of _SPI_stack */ static int _SPI_connected = -1; /* current stack index */ +typedef struct SPICallbackArg +{ + const char *query; + RawParseMode mode; +} SPICallbackArg; + static Portal SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, ParamListInfo paramLI, bool read_only); @@ -1479,6 +1485,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, Snapshot snapshot; MemoryContext oldcontext; Portal portal; + SPICallbackArg spicallbackarg; ErrorContextCallback spierrcontext; /* @@ -1533,8 +1540,10 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, * Setup error traceback support for ereport(), in case GetCachedPlan * throws an error. */ + spicallbackarg.query = plansource->query_string; + spicallbackarg.mode = plan->parse_mode; spierrcontext.callback = _SPI_error_callback; - spierrcontext.arg = unconstify(char *, plansource->query_string); + spierrcontext.arg = &spicallbackarg; spierrcontext.previous = error_context_stack; error_context_stack = &spierrcontext; @@ -1952,6 +1961,7 @@ SPI_plan_get_cached_plan(SPIPlanPtr plan) { CachedPlanSource *plansource; CachedPlan *cplan; + SPICallbackArg spicallbackarg; ErrorContextCallback spierrcontext; Assert(plan->magic == _SPI_PLAN_MAGIC); @@ -1966,8 +1976,10 @@ SPI_plan_get_cached_plan(SPIPlanPtr plan) plansource = (CachedPlanSource *) linitial(plan->plancache_list); /* Setup error traceback support for ereport() */ + spicallbackarg.query = plansource->query_string; + spicallbackarg.mode = plan->parse_mode; spierrcontext.callback = _SPI_error_callback; - spierrcontext.arg = unconstify(char *, plansource->query_string); + spierrcontext.arg = &spicallbackarg; spierrcontext.previous = error_context_stack; error_context_stack = &spierrcontext; @@ -2094,13 +2106,16 @@ _SPI_prepare_plan(const char *src, SPIPlanPtr plan) List *raw_parsetree_list; List *plancache_list; ListCell *list_item; + SPICallbackArg spicallbackarg; ErrorContextCallback spierrcontext; /* * Setup error traceback support for ereport() */ + spicallbackarg.query = src; + spicallbackarg.mode = plan->parse_mode; spierrcontext.callback = _SPI_error_callback; - spierrcontext.arg = unconstify(char *, src); + spierrcontext.arg = &spicallbackarg; spierrcontext.previous = error_context_stack; error_context_stack = &spierrcontext; @@ -2199,13 +2214,16 @@ _SPI_prepare_oneshot_plan(const char *src, SPIPlanPtr plan) List *raw_parsetree_list; List *plancache_list; ListCell *list_item; + SPICallbackArg spicallbackarg; ErrorContextCallback spierrcontext; /* * Setup error traceback support for ereport() */ + spicallbackarg.query = src; + spicallbackarg.mode = plan->parse_mode; spierrcontext.callback = _SPI_error_callback; - spierrcontext.arg = unconstify(char *, src); + spierrcontext.arg = &spicallbackarg; spierrcontext.previous = error_context_stack; error_context_stack = &spierrcontext; @@ -2263,6 +2281,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, SPITupleTable *my_tuptable = NULL; int res = 0; bool pushed_active_snap = false; + SPICallbackArg spicallbackarg; ErrorContextCallback spierrcontext; CachedPlan *cplan = NULL; ListCell *lc1; @@ -2270,8 +2289,10 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, /* * Setup error traceback support for ereport() */ + spicallbackarg.query = NULL; /* we'll fill this below */ + spicallbackarg.mode = plan->parse_mode; spierrcontext.callback = _SPI_error_callback; - spierrcontext.arg = NULL; /* we'll fill this below */ + spierrcontext.arg = &spicallbackarg; spierrcontext.previous = error_context_stack; error_context_stack = &spierrcontext; @@ -2318,7 +2339,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, List *stmt_list; ListCell *lc2; - spierrcontext.arg = unconstify(char *, plansource->query_string); + spicallbackarg.query = plansource->query_string; /* * If this is a one-shot plan, we still need to do parse analysis. @@ -2722,7 +2743,8 @@ _SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, uint64 tcount) static void _SPI_error_callback(void *arg) { - const char *query = (const char *) arg; + SPICallbackArg *carg = (SPICallbackArg *) arg; + const char *query = carg->query; int syntaxerrposition; if (query == NULL) /* in case arg wasn't set yet */ @@ -2740,7 +2762,23 @@ _SPI_error_callback(void *arg) internalerrquery(query); } else - errcontext("SQL statement \"%s\"", query); + { + /* Use the parse mode to decide how to describe the query */ + switch (carg->mode) + { + case RAW_PARSE_PLPGSQL_EXPR: + errcontext("SQL expression \"%s\"", query); + break; + case RAW_PARSE_PLPGSQL_ASSIGN1: + case RAW_PARSE_PLPGSQL_ASSIGN2: + case RAW_PARSE_PLPGSQL_ASSIGN3: + errcontext("PL/pgSQL assignment \"%s\"", query); + break; + default: + errcontext("SQL statement \"%s\"", query); + break; + } + } } /* diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 67d4541866..ba3ccc712c 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3199,6 +3199,20 @@ _copySetOperationStmt(const SetOperationStmt *from) return newnode; } +static PLAssignStmt * +_copyPLAssignStmt(const PLAssignStmt *from) +{ + PLAssignStmt *newnode = makeNode(PLAssignStmt); + + COPY_STRING_FIELD(name); + COPY_NODE_FIELD(indirection); + COPY_SCALAR_FIELD(nnames); + COPY_NODE_FIELD(val); + COPY_LOCATION_FIELD(location); + + return newnode; +} + static AlterTableStmt * _copyAlterTableStmt(const AlterTableStmt *from) { @@ -5220,6 +5234,9 @@ copyObjectImpl(const void *from) case T_SetOperationStmt: retval = _copySetOperationStmt(from); break; + case T_PLAssignStmt: + retval = _copyPLAssignStmt(from); + break; case T_AlterTableStmt: retval = _copyAlterTableStmt(from); break; diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 4d4258b0cb..a2ef853dc2 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1085,6 +1085,18 @@ _equalSetOperationStmt(const SetOperationStmt *a, const SetOperationStmt *b) return true; } +static bool +_equalPLAssignStmt(const PLAssignStmt *a, const PLAssignStmt *b) +{ + COMPARE_STRING_FIELD(name); + COMPARE_NODE_FIELD(indirection); + COMPARE_SCALAR_FIELD(nnames); + COMPARE_NODE_FIELD(val); + COMPARE_LOCATION_FIELD(location); + + return true; +} + static bool _equalAlterTableStmt(const AlterTableStmt *a, const AlterTableStmt *b) { @@ -3275,6 +3287,9 @@ equal(const void *a, const void *b) case T_SetOperationStmt: retval = _equalSetOperationStmt(a, b); break; + case T_PLAssignStmt: + retval = _equalPLAssignStmt(a, b); + break; case T_AlterTableStmt: retval = _equalAlterTableStmt(a, b); break; diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c index a011bc7b98..6be19916fc 100644 --- a/src/backend/nodes/nodeFuncs.c +++ b/src/backend/nodes/nodeFuncs.c @@ -3669,6 +3669,16 @@ raw_expression_tree_walker(Node *node, return true; } break; + case T_PLAssignStmt: + { + PLAssignStmt *stmt = (PLAssignStmt *) node; + + if (walker(stmt->indirection, context)) + return true; + if (walker(stmt->val, context)) + return true; + } + break; case T_A_Expr: { A_Expr *expr = (A_Expr *) node; diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 6b8ec03fa7..8392be6d44 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -2775,6 +2775,18 @@ _outSelectStmt(StringInfo str, const SelectStmt *node) WRITE_NODE_FIELD(rarg); } +static void +_outPLAssignStmt(StringInfo str, const PLAssignStmt *node) +{ + WRITE_NODE_TYPE("PLASSIGN"); + + WRITE_STRING_FIELD(name); + WRITE_NODE_FIELD(indirection); + WRITE_INT_FIELD(nnames); + WRITE_NODE_FIELD(val); + WRITE_LOCATION_FIELD(location); +} + static void _outFuncCall(StringInfo str, const FuncCall *node) { @@ -4211,6 +4223,9 @@ outNode(StringInfo str, const void *obj) case T_SelectStmt: _outSelectStmt(str, obj); break; + case T_PLAssignStmt: + _outPLAssignStmt(str, obj); + break; case T_ColumnDef: _outColumnDef(str, obj); break; diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 1066f9458f..28e192f51c 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -42,8 +42,10 @@ #include "parser/parse_param.h" #include "parser/parse_relation.h" #include "parser/parse_target.h" +#include "parser/parse_type.h" #include "parser/parsetree.h" #include "rewrite/rewriteManip.h" +#include "utils/builtins.h" #include "utils/rel.h" @@ -70,6 +72,8 @@ static Query *transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt); static List *transformReturningList(ParseState *pstate, List *returningList); static List *transformUpdateTargetList(ParseState *pstate, List *targetList); +static Query *transformPLAssignStmt(ParseState *pstate, + PLAssignStmt *stmt); static Query *transformDeclareCursorStmt(ParseState *pstate, DeclareCursorStmt *stmt); static Query *transformExplainStmt(ParseState *pstate, @@ -304,6 +308,11 @@ transformStmt(ParseState *pstate, Node *parseTree) } break; + case T_PLAssignStmt: + result = transformPLAssignStmt(pstate, + (PLAssignStmt *) parseTree); + break; + /* * Special cases */ @@ -367,6 +376,7 @@ analyze_requires_snapshot(RawStmt *parseTree) case T_DeleteStmt: case T_UpdateStmt: case T_SelectStmt: + case T_PLAssignStmt: result = true; break; @@ -2393,6 +2403,236 @@ transformReturningList(ParseState *pstate, List *returningList) } +/* + * transformPLAssignStmt - + * transform a PL/pgSQL assignment statement + * + * If there is no opt_indirection, the transformed statement looks like + * "SELECT a_expr ...", except the expression has been cast to the type of + * the target. With indirection, it's still a SELECT, but the expression will + * incorporate FieldStore and/or assignment SubscriptingRef nodes to compute a + * new value for a container-type variable represented by the target. The + * expression references the target as the container source. + */ +static Query * +transformPLAssignStmt(ParseState *pstate, PLAssignStmt *stmt) +{ + Query *qry = makeNode(Query); + ColumnRef *cref = makeNode(ColumnRef); + List *indirection = stmt->indirection; + int nnames = stmt->nnames; + SelectStmt *sstmt = stmt->val; + Node *target; + Oid targettype; + int32 targettypmod; + Oid targetcollation; + List *tlist; + TargetEntry *tle; + Oid type_id; + Node *qual; + ListCell *l; + + /* + * First, construct a ColumnRef for the target variable. If the target + * has more than one dotted name, we have to pull the extra names out of + * the indirection list. + */ + cref->fields = list_make1(makeString(stmt->name)); + cref->location = stmt->location; + if (nnames > 1) + { + /* avoid munging the raw parsetree */ + indirection = list_copy(indirection); + while (--nnames > 0 && indirection != NIL) + { + Node *ind = (Node *) linitial(indirection); + + if (!IsA(ind, String)) + elog(ERROR, "invalid name count in PLAssignStmt"); + cref->fields = lappend(cref->fields, ind); + indirection = list_delete_first(indirection); + } + } + + /* + * Transform the target reference. Typically we will get back a Param + * node, but there's no reason to be too picky about its type. + */ + target = transformExpr(pstate, (Node *) cref, + EXPR_KIND_UPDATE_TARGET); + targettype = exprType(target); + targettypmod = exprTypmod(target); + targetcollation = exprCollation(target); + + /* + * The rest mostly matches transformSelectStmt, except that we needn't + * consider WITH or DISTINCT, and we build a targetlist our own way. + */ + qry->commandType = CMD_SELECT; + pstate->p_is_insert = false; + + /* make FOR UPDATE/FOR SHARE info available to addRangeTableEntry */ + pstate->p_locking_clause = sstmt->lockingClause; + + /* make WINDOW info available for window functions, too */ + pstate->p_windowdefs = sstmt->windowClause; + + /* process the FROM clause */ + transformFromClause(pstate, sstmt->fromClause); + + /* initially transform the targetlist as if in SELECT */ + tlist = transformTargetList(pstate, sstmt->targetList, + EXPR_KIND_SELECT_TARGET); + + /* we should have exactly one targetlist item */ + if (list_length(tlist) != 1) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg_plural("assignment source returned %d column", + "assignment source returned %d columns", + list_length(tlist), + list_length(tlist)))); + + tle = linitial_node(TargetEntry, tlist); + + /* + * This next bit is similar to transformAssignedExpr; the key difference + * is we use COERCION_PLPGSQL not COERCION_ASSIGNMENT. + */ + type_id = exprType((Node *) tle->expr); + + pstate->p_expr_kind = EXPR_KIND_UPDATE_TARGET; + + if (indirection) + { + tle->expr = (Expr *) + transformAssignmentIndirection(pstate, + target, + stmt->name, + false, + targettype, + targettypmod, + targetcollation, + indirection, + list_head(indirection), + (Node *) tle->expr, + COERCION_PLPGSQL, + exprLocation(target)); + } + else if (targettype != type_id && + (targettype == RECORDOID || ISCOMPLEX(targettype)) && + (type_id == RECORDOID || ISCOMPLEX(type_id))) + { + /* + * Hack: do not let coerce_to_target_type() deal with inconsistent + * composite types. Just pass the expression result through as-is, + * and let the PL/pgSQL executor do the conversion its way. This is + * rather bogus, but it's needed for backwards compatibility. + */ + } + else + { + /* + * For normal non-qualified target column, do type checking and + * coercion. + */ + Node *orig_expr = (Node *) tle->expr; + + tle->expr = (Expr *) + coerce_to_target_type(pstate, + orig_expr, type_id, + targettype, targettypmod, + COERCION_PLPGSQL, + COERCE_IMPLICIT_CAST, + -1); + /* With COERCION_PLPGSQL, this error is probably unreachable */ + if (tle->expr == NULL) + ereport(ERROR, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("variable \"%s\" is of type %s" + " but expression is of type %s", + stmt->name, + format_type_be(targettype), + format_type_be(type_id)), + errhint("You will need to rewrite or cast the expression."), + parser_errposition(pstate, exprLocation(orig_expr)))); + } + + pstate->p_expr_kind = EXPR_KIND_NONE; + + qry->targetList = list_make1(tle); + + /* transform WHERE */ + qual = transformWhereClause(pstate, sstmt->whereClause, + EXPR_KIND_WHERE, "WHERE"); + + /* initial processing of HAVING clause is much like WHERE clause */ + qry->havingQual = transformWhereClause(pstate, sstmt->havingClause, + EXPR_KIND_HAVING, "HAVING"); + + /* + * Transform sorting/grouping stuff. Do ORDER BY first because both + * transformGroupClause and transformDistinctClause need the results. Note + * that these functions can also change the targetList, so it's passed to + * them by reference. + */ + qry->sortClause = transformSortClause(pstate, + sstmt->sortClause, + &qry->targetList, + EXPR_KIND_ORDER_BY, + false /* allow SQL92 rules */ ); + + qry->groupClause = transformGroupClause(pstate, + sstmt->groupClause, + &qry->groupingSets, + &qry->targetList, + qry->sortClause, + EXPR_KIND_GROUP_BY, + false /* allow SQL92 rules */ ); + + /* No DISTINCT clause */ + Assert(!sstmt->distinctClause); + qry->distinctClause = NIL; + qry->hasDistinctOn = false; + + /* transform LIMIT */ + qry->limitOffset = transformLimitClause(pstate, sstmt->limitOffset, + EXPR_KIND_OFFSET, "OFFSET", + sstmt->limitOption); + qry->limitCount = transformLimitClause(pstate, sstmt->limitCount, + EXPR_KIND_LIMIT, "LIMIT", + sstmt->limitOption); + qry->limitOption = sstmt->limitOption; + + /* transform window clauses after we have seen all window functions */ + qry->windowClause = transformWindowDefinitions(pstate, + pstate->p_windowdefs, + &qry->targetList); + + qry->rtable = pstate->p_rtable; + qry->jointree = makeFromExpr(pstate->p_joinlist, qual); + + qry->hasSubLinks = pstate->p_hasSubLinks; + qry->hasWindowFuncs = pstate->p_hasWindowFuncs; + qry->hasTargetSRFs = pstate->p_hasTargetSRFs; + qry->hasAggs = pstate->p_hasAggs; + + foreach(l, sstmt->lockingClause) + { + transformLockingClause(pstate, qry, + (LockingClause *) lfirst(l), false); + } + + assign_query_collations(pstate, qry); + + /* this must be done after collations, for reliable comparison of exprs */ + if (pstate->p_hasAggs || qry->groupClause || qry->groupingSets || qry->havingQual) + parseCheckAggregates(pstate, qry); + + return qry; +} + + /* * transformDeclareCursorStmt - * transform a DECLARE CURSOR Statement diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index fb025f08a4..31c95443a5 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -294,6 +294,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type select_no_parens select_with_parens select_clause simple_select values_clause + PLpgSQL_Expr PLAssignStmt %type alter_column_default opclass_item opclass_drop alter_using %type add_drop opt_asc_desc opt_nulls_order @@ -535,7 +536,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type ColId ColLabel BareColLabel %type NonReservedWord NonReservedWord_or_Sconst %type var_name type_function_name param_name -%type createdb_opt_name +%type createdb_opt_name plassign_target %type var_value zone_value %type auth_ident RoleSpec opt_granted_by @@ -731,6 +732,10 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); * something other than the usual list of SQL commands. */ %token MODE_TYPE_NAME +%token MODE_PLPGSQL_EXPR +%token MODE_PLPGSQL_ASSIGN1 +%token MODE_PLPGSQL_ASSIGN2 +%token MODE_PLPGSQL_ASSIGN3 /* Precedence: lowest to highest */ @@ -810,6 +815,32 @@ parse_toplevel: { pg_yyget_extra(yyscanner)->parsetree = list_make1($2); } + | MODE_PLPGSQL_EXPR PLpgSQL_Expr + { + pg_yyget_extra(yyscanner)->parsetree = + list_make1(makeRawStmt($2, 0)); + } + | MODE_PLPGSQL_ASSIGN1 PLAssignStmt + { + PLAssignStmt *n = (PLAssignStmt *) $2; + n->nnames = 1; + pg_yyget_extra(yyscanner)->parsetree = + list_make1(makeRawStmt((Node *) n, 0)); + } + | MODE_PLPGSQL_ASSIGN2 PLAssignStmt + { + PLAssignStmt *n = (PLAssignStmt *) $2; + n->nnames = 2; + pg_yyget_extra(yyscanner)->parsetree = + list_make1(makeRawStmt((Node *) n, 0)); + } + | MODE_PLPGSQL_ASSIGN3 PLAssignStmt + { + PLAssignStmt *n = (PLAssignStmt *) $2; + n->nnames = 3; + pg_yyget_extra(yyscanner)->parsetree = + list_make1(makeRawStmt((Node *) n, 0)); + } ; /* @@ -15024,6 +15055,72 @@ role_list: RoleSpec { $$ = lappend($1, $3); } ; + +/***************************************************************************** + * + * PL/pgSQL extensions + * + * You'd think a PL/pgSQL "expression" should be just an a_expr, but + * historically it can include just about anything that can follow SELECT. + * Therefore the returned struct is a SelectStmt. + *****************************************************************************/ + +PLpgSQL_Expr: opt_target_list + from_clause where_clause + group_clause having_clause window_clause + opt_sort_clause opt_select_limit opt_for_locking_clause + { + SelectStmt *n = makeNode(SelectStmt); + + n->targetList = $1; + n->fromClause = $2; + n->whereClause = $3; + n->groupClause = $4; + n->havingClause = $5; + n->windowClause = $6; + n->sortClause = $7; + if ($8) + { + n->limitOffset = $8->limitOffset; + n->limitCount = $8->limitCount; + if (!n->sortClause && + $8->limitOption == LIMIT_OPTION_WITH_TIES) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("WITH TIES cannot be specified without ORDER BY clause"))); + n->limitOption = $8->limitOption; + } + n->lockingClause = $9; + $$ = (Node *) n; + } + ; + +/* + * PL/pgSQL Assignment statement: name opt_indirection := PLpgSQL_Expr + */ + +PLAssignStmt: plassign_target opt_indirection plassign_equals PLpgSQL_Expr + { + PLAssignStmt *n = makeNode(PLAssignStmt); + + n->name = $1; + n->indirection = check_indirection($2, yyscanner); + /* nnames will be filled by calling production */ + n->val = (SelectStmt *) $4; + n->location = @1; + $$ = (Node *) n; + } + ; + +plassign_target: ColId { $$ = $1; } + | PARAM { $$ = psprintf("$%d", $1); } + ; + +plassign_equals: COLON_EQUALS + | '=' + ; + + /* * Name classification hierarchy. * diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c index 74eb39c0e4..d5310f27db 100644 --- a/src/backend/parser/parse_coerce.c +++ b/src/backend/parser/parse_coerce.c @@ -3098,6 +3098,14 @@ find_coercion_pathway(Oid targetTypeId, Oid sourceTypeId, } } + /* + * When parsing PL/pgSQL assignments, allow an I/O cast to be used + * whenever no normal coercion is available. + */ + if (result == COERCION_PATH_NONE && + ccontext == COERCION_PLPGSQL) + result = COERCION_PATH_COERCEVIAIO; + return result; } diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index fdf5500bf2..7eaa076771 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -34,17 +34,6 @@ static void markTargetListOrigin(ParseState *pstate, TargetEntry *tle, Var *var, int levelsup); -static Node *transformAssignmentIndirection(ParseState *pstate, - Node *basenode, - const char *targetName, - bool targetIsSubscripting, - Oid targetTypeId, - int32 targetTypMod, - Oid targetCollation, - List *indirection, - ListCell *indirection_cell, - Node *rhs, - int location); static Node *transformAssignmentSubscripts(ParseState *pstate, Node *basenode, const char *targetName, @@ -56,6 +45,7 @@ static Node *transformAssignmentSubscripts(ParseState *pstate, List *indirection, ListCell *next_indirection, Node *rhs, + CoercionContext ccontext, int location); static List *ExpandColumnRefStar(ParseState *pstate, ColumnRef *cref, bool make_target_entry); @@ -561,6 +551,7 @@ transformAssignedExpr(ParseState *pstate, indirection, list_head(indirection), (Node *) expr, + COERCION_ASSIGNMENT, location); } else @@ -642,15 +633,15 @@ updateTargetListEntry(ParseState *pstate, /* * Process indirection (field selection or subscripting) of the target - * column in INSERT/UPDATE. This routine recurses for multiple levels - * of indirection --- but note that several adjacent A_Indices nodes in - * the indirection list are treated as a single multidimensional subscript + * column in INSERT/UPDATE/assignment. This routine recurses for multiple + * levels of indirection --- but note that several adjacent A_Indices nodes + * in the indirection list are treated as a single multidimensional subscript * operation. * * In the initial call, basenode is a Var for the target column in UPDATE, - * or a null Const of the target's type in INSERT. In recursive calls, - * basenode is NULL, indicating that a substitute node should be consed up if - * needed. + * or a null Const of the target's type in INSERT, or a Param for the target + * variable in PL/pgSQL assignment. In recursive calls, basenode is NULL, + * indicating that a substitute node should be consed up if needed. * * targetName is the name of the field or subfield we're assigning to, and * targetIsSubscripting is true if we're subscripting it. These are just for @@ -667,12 +658,16 @@ updateTargetListEntry(ParseState *pstate, * rhs is the already-transformed value to be assigned; note it has not been * coerced to any particular type. * + * ccontext is the coercion level to use while coercing the rhs. For + * normal statements it'll be COERCION_ASSIGNMENT, but PL/pgSQL uses + * a special value. + * * location is the cursor error position for any errors. (Note: this points * to the head of the target clause, eg "foo" in "foo.bar[baz]". Later we * might want to decorate indirection cells with their own location info, * in which case the location argument could probably be dropped.) */ -static Node * +Node * transformAssignmentIndirection(ParseState *pstate, Node *basenode, const char *targetName, @@ -683,6 +678,7 @@ transformAssignmentIndirection(ParseState *pstate, List *indirection, ListCell *indirection_cell, Node *rhs, + CoercionContext ccontext, int location) { Node *result; @@ -757,6 +753,7 @@ transformAssignmentIndirection(ParseState *pstate, indirection, i, rhs, + ccontext, location); } @@ -807,6 +804,7 @@ transformAssignmentIndirection(ParseState *pstate, indirection, lnext(indirection, i), rhs, + ccontext, location); /* and build a FieldStore node */ @@ -845,6 +843,7 @@ transformAssignmentIndirection(ParseState *pstate, indirection, NULL, rhs, + ccontext, location); } @@ -853,7 +852,7 @@ transformAssignmentIndirection(ParseState *pstate, result = coerce_to_target_type(pstate, rhs, exprType(rhs), targetTypeId, targetTypMod, - COERCION_ASSIGNMENT, + ccontext, COERCE_IMPLICIT_CAST, -1); if (result == NULL) @@ -898,6 +897,7 @@ transformAssignmentSubscripts(ParseState *pstate, List *indirection, ListCell *next_indirection, Node *rhs, + CoercionContext ccontext, int location) { Node *result; @@ -949,6 +949,7 @@ transformAssignmentSubscripts(ParseState *pstate, indirection, next_indirection, rhs, + ccontext, location); /* @@ -969,7 +970,7 @@ transformAssignmentSubscripts(ParseState *pstate, result = coerce_to_target_type(pstate, result, resulttype, targetTypeId, targetTypMod, - COERCION_ASSIGNMENT, + ccontext, COERCE_IMPLICIT_CAST, -1); /* can fail if we had int2vector/oidvector, but not for true domains */ diff --git a/src/backend/parser/parser.c b/src/backend/parser/parser.c index 8eb8feb372..875de7ba28 100644 --- a/src/backend/parser/parser.c +++ b/src/backend/parser/parser.c @@ -57,7 +57,11 @@ raw_parser(const char *str, RawParseMode mode) /* this array is indexed by RawParseMode enum */ static const int mode_token[] = { 0, /* RAW_PARSE_DEFAULT */ - MODE_TYPE_NAME /* RAW_PARSE_TYPE_NAME */ + MODE_TYPE_NAME, /* RAW_PARSE_TYPE_NAME */ + MODE_PLPGSQL_EXPR, /* RAW_PARSE_PLPGSQL_EXPR */ + MODE_PLPGSQL_ASSIGN1, /* RAW_PARSE_PLPGSQL_ASSIGN1 */ + MODE_PLPGSQL_ASSIGN2, /* RAW_PARSE_PLPGSQL_ASSIGN2 */ + MODE_PLPGSQL_ASSIGN3 /* RAW_PARSE_PLPGSQL_ASSIGN3 */ }; yyextra.have_lookahead = true; diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 5825d3d814..53a511f1da 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -2313,6 +2313,10 @@ CreateCommandTag(Node *parsetree) tag = CMDTAG_SELECT; break; + case T_PLAssignStmt: + tag = CMDTAG_SELECT; + break; + /* utility statements --- same whether raw or cooked */ case T_TransactionStmt: { @@ -3181,6 +3185,10 @@ GetCommandLogLevel(Node *parsetree) lev = LOGSTMT_ALL; break; + case T_PLAssignStmt: + lev = LOGSTMT_ALL; + break; + /* utility statements --- same whether raw or cooked */ case T_TransactionStmt: lev = LOGSTMT_ALL; diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h index fa28b21821..caed683ba9 100644 --- a/src/include/nodes/nodes.h +++ b/src/include/nodes/nodes.h @@ -314,6 +314,7 @@ typedef enum NodeTag T_DeleteStmt, T_UpdateStmt, T_SelectStmt, + T_PLAssignStmt, T_AlterTableStmt, T_AlterTableCmd, T_AlterDomainStmt, diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index a0f37e5268..dc2bb40926 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1675,6 +1675,25 @@ typedef struct SetOperationStmt } SetOperationStmt; +/* ---------------------- + * PL/pgSQL Assignment Statement + * + * Like SelectStmt, this is transformed into a SELECT Query. + * However, the targetlist of the result looks more like an UPDATE. + * ---------------------- + */ +typedef struct PLAssignStmt +{ + NodeTag type; + + char *name; /* initial column name */ + List *indirection; /* subscripts and field names, if any */ + int nnames; /* number of names to use in ColumnRef */ + SelectStmt *val; /* the PL/pgSQL expression to assign */ + int location; /* name's token location, or -1 if unknown */ +} PLAssignStmt; + + /***************************************************************************** * Other Statements (no optimizations required) * diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index e51be4bce8..d4ce037088 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -457,6 +457,7 @@ typedef enum CoercionContext { COERCION_IMPLICIT, /* coercion in context of expression */ COERCION_ASSIGNMENT, /* coercion in context of assignment */ + COERCION_PLPGSQL, /* if no assignment cast, use CoerceViaIO */ COERCION_EXPLICIT /* explicit cast operation */ } CoercionContext; diff --git a/src/include/parser/parse_target.h b/src/include/parser/parse_target.h index 9d3637747d..1a7b1a9277 100644 --- a/src/include/parser/parse_target.h +++ b/src/include/parser/parse_target.h @@ -36,6 +36,18 @@ extern void updateTargetListEntry(ParseState *pstate, TargetEntry *tle, char *colname, int attrno, List *indirection, int location); +extern Node *transformAssignmentIndirection(ParseState *pstate, + Node *basenode, + const char *targetName, + bool targetIsSubscripting, + Oid targetTypeId, + int32 targetTypMod, + Oid targetCollation, + List *indirection, + ListCell *indirection_cell, + Node *rhs, + CoercionContext ccontext, + int location); extern List *checkInsertTargets(ParseState *pstate, List *cols, List **attrnos); extern TupleDesc expandRecordVariable(ParseState *pstate, Var *var, diff --git a/src/include/parser/parser.h b/src/include/parser/parser.h index 80d90027cc..853b0f1606 100644 --- a/src/include/parser/parser.h +++ b/src/include/parser/parser.h @@ -27,12 +27,21 @@ * RAW_PARSE_TYPE_NAME: parse a type name, and return a one-element List * containing a TypeName node. * - * ... more to come ... + * RAW_PARSE_PLPGSQL_EXPR: parse a PL/pgSQL expression, and return + * a one-element List containing a RawStmt node. + * + * RAW_PARSE_PLPGSQL_ASSIGNn: parse a PL/pgSQL assignment statement, + * and return a one-element List containing a RawStmt node. "n" + * gives the number of dotted names comprising the target ColumnRef. */ typedef enum { RAW_PARSE_DEFAULT = 0, - RAW_PARSE_TYPE_NAME + RAW_PARSE_TYPE_NAME, + RAW_PARSE_PLPGSQL_EXPR, + RAW_PARSE_PLPGSQL_ASSIGN1, + RAW_PARSE_PLPGSQL_ASSIGN2, + RAW_PARSE_PLPGSQL_ASSIGN3 } RawParseMode; /* Values for the backslash_quote GUC */ diff --git a/src/interfaces/ecpg/preproc/parse.pl b/src/interfaces/ecpg/preproc/parse.pl index 7f9be85eb6..a84243fc8f 100644 --- a/src/interfaces/ecpg/preproc/parse.pl +++ b/src/interfaces/ecpg/preproc/parse.pl @@ -70,7 +70,11 @@ my %replace_types = ( 'ColId' => 'ignore', 'type_function_name' => 'ignore', 'ColLabel' => 'ignore', - 'Sconst' => 'ignore',); + 'Sconst' => 'ignore', + 'PLpgSQL_Expr' => 'ignore', + 'PLAssignStmt' => 'ignore', + 'plassign_target' => 'ignore', + 'plassign_equals' => 'ignore',); # these replace_line commands excise certain keywords from the core keyword # lists. Be sure to account for these in ColLabel and related productions. diff --git a/src/pl/plpgsql/src/Makefile b/src/pl/plpgsql/src/Makefile index 193df8a010..9946abbc1d 100644 --- a/src/pl/plpgsql/src/Makefile +++ b/src/pl/plpgsql/src/Makefile @@ -32,7 +32,7 @@ DATA = plpgsql.control plpgsql--1.0.sql REGRESS_OPTS = --dbname=$(PL_TESTDB) -REGRESS = plpgsql_call plpgsql_control plpgsql_copy plpgsql_domain \ +REGRESS = plpgsql_array plpgsql_call plpgsql_control plpgsql_copy plpgsql_domain \ plpgsql_record plpgsql_cache plpgsql_simple plpgsql_transaction \ plpgsql_trap plpgsql_trigger plpgsql_varprops diff --git a/src/pl/plpgsql/src/expected/plpgsql_array.out b/src/pl/plpgsql/src/expected/plpgsql_array.out new file mode 100644 index 0000000000..5f28b4f685 --- /dev/null +++ b/src/pl/plpgsql/src/expected/plpgsql_array.out @@ -0,0 +1,94 @@ +-- +-- Tests for PL/pgSQL handling of array variables +-- +-- We also check arrays of composites here, so this has some overlap +-- with the plpgsql_record tests. +-- +create type complex as (r float8, i float8); +create type quadarray as (c1 complex[], c2 complex); +do $$ declare a int[]; +begin a := array[1,2]; a[3] := 4; raise notice 'a = %', a; end$$; +NOTICE: a = {1,2,4} +do $$ declare a int[]; +begin a[3] := 4; raise notice 'a = %', a; end$$; +NOTICE: a = [3:3]={4} +do $$ declare a int[]; +begin a[1][4] := 4; raise notice 'a = %', a; end$$; +NOTICE: a = [1:1][4:4]={{4}} +do $$ declare a int[]; +begin a[1] := 23::text; raise notice 'a = %', a; end$$; -- lax typing +NOTICE: a = {23} +do $$ declare a int[]; +begin a := array[1,2]; a[2:3] := array[3,4]; raise notice 'a = %', a; end$$; +NOTICE: a = {1,3,4} +do $$ declare a int[]; +begin a := array[1,2]; a[2] := a[2] + 1; raise notice 'a = %', a; end$$; +NOTICE: a = {1,3} +do $$ declare a int[]; +begin a[1:2] := array[3,4]; raise notice 'a = %', a; end$$; +NOTICE: a = {3,4} +do $$ declare a int[]; +begin a[1:2] := 4; raise notice 'a = %', a; end$$; -- error +ERROR: malformed array literal: "4" +DETAIL: Array value must start with "{" or dimension information. +CONTEXT: PL/pgSQL function inline_code_block line 2 at assignment +do $$ declare a complex[]; +begin a[1] := (1,2); a[1].i := 11; raise notice 'a = %', a; end$$; +NOTICE: a = {"(1,11)"} +do $$ declare a complex[]; +begin a[1].i := 11; raise notice 'a = %, a[1].i = %', a, a[1].i; end$$; +NOTICE: a = {"(,11)"}, a[1].i = 11 +-- perhaps this ought to work, but for now it doesn't: +do $$ declare a complex[]; +begin a[1:2].i := array[11,12]; raise notice 'a = %', a; end$$; +ERROR: cannot assign to field "i" of column "a" because its type complex[] is not a composite type +LINE 1: a[1:2].i := array[11,12] + ^ +QUERY: a[1:2].i := array[11,12] +CONTEXT: PL/pgSQL function inline_code_block line 2 at assignment +do $$ declare a quadarray; +begin a.c1[1].i := 11; raise notice 'a = %, a.c1[1].i = %', a, a.c1[1].i; end$$; +NOTICE: a = ("{""(,11)""}",), a.c1[1].i = 11 +do $$ declare a int[]; +begin a := array_agg(x) from (values(1),(2),(3)) v(x); raise notice 'a = %', a; end$$; +NOTICE: a = {1,2,3} +create temp table onecol as select array[1,2] as f1; +do $$ declare a int[]; +begin a := f1 from onecol; raise notice 'a = %', a; end$$; +NOTICE: a = {1,2} +do $$ declare a int[]; +begin a := * from onecol for update; raise notice 'a = %', a; end$$; +NOTICE: a = {1,2} +-- error cases: +do $$ declare a int[]; +begin a := from onecol; raise notice 'a = %', a; end$$; +ERROR: assignment source returned 0 columns +CONTEXT: PL/pgSQL assignment "a := from onecol" +PL/pgSQL function inline_code_block line 2 at assignment +do $$ declare a int[]; +begin a := f1, f1 from onecol; raise notice 'a = %', a; end$$; +ERROR: assignment source returned 2 columns +CONTEXT: PL/pgSQL assignment "a := f1, f1 from onecol" +PL/pgSQL function inline_code_block line 2 at assignment +insert into onecol values(array[11]); +do $$ declare a int[]; +begin a := f1 from onecol; raise notice 'a = %', a; end$$; +ERROR: query "a := f1 from onecol" returned more than one row +CONTEXT: PL/pgSQL function inline_code_block line 2 at assignment +do $$ declare a int[]; +begin a := f1 from onecol limit 1; raise notice 'a = %', a; end$$; +NOTICE: a = {1,2} +do $$ declare a real; +begin a[1] := 2; raise notice 'a = %', a; end$$; +ERROR: cannot subscript type real because it does not support subscripting +LINE 1: a[1] := 2 + ^ +QUERY: a[1] := 2 +CONTEXT: PL/pgSQL function inline_code_block line 2 at assignment +do $$ declare a complex; +begin a.r[1] := 2; raise notice 'a = %', a; end$$; +ERROR: cannot subscript type double precision because it does not support subscripting +LINE 1: a.r[1] := 2 + ^ +QUERY: a.r[1] := 2 +CONTEXT: PL/pgSQL function inline_code_block line 2 at assignment diff --git a/src/pl/plpgsql/src/expected/plpgsql_record.out b/src/pl/plpgsql/src/expected/plpgsql_record.out index cf6089cbb2..6e835c0751 100644 --- a/src/pl/plpgsql/src/expected/plpgsql_record.out +++ b/src/pl/plpgsql/src/expected/plpgsql_record.out @@ -3,6 +3,7 @@ -- create type two_int4s as (f1 int4, f2 int4); create type two_int8s as (q1 int8, q2 int8); +create type nested_int8s as (c1 two_int8s, c2 two_int8s); -- base-case return of a composite type create function retc(int) returns two_int8s language plpgsql as $$ begin return row($1,1)::two_int8s; end $$; @@ -82,6 +83,88 @@ begin end$$; NOTICE: c4 = (1,2) NOTICE: c8 = (1,2) +do $$ declare c two_int8s; d nested_int8s; +begin + c := row(1,2); + d := row(c, row(c.q1, c.q2+1)); + raise notice 'c = %, d = %', c, d; + c.q1 := 10; + d.c1 := row(11,12); + d.c2.q2 := 42; + raise notice 'c = %, d = %', c, d; + raise notice 'c.q1 = %, d.c2 = %', c.q1, d.c2; + raise notice '(d).c2.q2 = %', (d).c2.q2; -- doesn't work without parens + raise notice '(d.c2).q2 = %', (d.c2).q2; -- doesn't work without parens +end$$; +NOTICE: c = (1,2), d = ("(1,2)","(1,3)") +NOTICE: c = (10,2), d = ("(11,12)","(1,42)") +NOTICE: c.q1 = 10, d.c2 = (1,42) +NOTICE: (d).c2.q2 = 42 +NOTICE: (d.c2).q2 = 42 +-- block-qualified naming +do $$ <> declare c two_int8s; d nested_int8s; +begin + b.c := row(1,2); + b.d := row(b.c, row(b.c.q1, b.c.q2+1)); + raise notice 'b.c = %, b.d = %', b.c, b.d; + b.c.q1 := 10; + b.d.c1 := row(11,12); + b.d.c2.q2 := 42; + raise notice 'b.c = %, b.d = %', b.c, b.d; + raise notice 'b.c.q1 = %, b.d.c2 = %', b.c.q1, b.d.c2; + raise notice '(b.d).c2.q2 = %', (b.d).c2.q2; -- doesn't work without parens + raise notice '(b.d.c2).q2 = %', (b.d.c2).q2; -- doesn't work without parens +end$$; +NOTICE: b.c = (1,2), b.d = ("(1,2)","(1,3)") +NOTICE: b.c = (10,2), b.d = ("(11,12)","(1,42)") +NOTICE: b.c.q1 = 10, b.d.c2 = (1,42) +NOTICE: (b.d).c2.q2 = 42 +NOTICE: (b.d.c2).q2 = 42 +-- error cases +do $$ declare c two_int8s; begin c.x = 1; end $$; +ERROR: record "c" has no field "x" +CONTEXT: PL/pgSQL assignment "c.x = 1" +PL/pgSQL function inline_code_block line 1 at assignment +do $$ declare c nested_int8s; begin c.x = 1; end $$; +ERROR: record "c" has no field "x" +CONTEXT: PL/pgSQL assignment "c.x = 1" +PL/pgSQL function inline_code_block line 1 at assignment +do $$ declare c nested_int8s; begin c.x.q1 = 1; end $$; +ERROR: record "c" has no field "x" +CONTEXT: PL/pgSQL assignment "c.x.q1 = 1" +PL/pgSQL function inline_code_block line 1 at assignment +do $$ declare c nested_int8s; begin c.c2.x = 1; end $$; +ERROR: cannot assign to field "x" of column "c" because there is no such column in data type two_int8s +LINE 1: c.c2.x = 1 + ^ +QUERY: c.c2.x = 1 +CONTEXT: PL/pgSQL function inline_code_block line 1 at assignment +do $$ declare c nested_int8s; begin d.c2.x = 1; end $$; +ERROR: "d.c2.x" is not a known variable +LINE 1: do $$ declare c nested_int8s; begin d.c2.x = 1; end $$; + ^ +do $$ <> declare c two_int8s; begin b.c.x = 1; end $$; +ERROR: record "c" has no field "x" +CONTEXT: PL/pgSQL assignment "b.c.x = 1" +PL/pgSQL function inline_code_block line 1 at assignment +do $$ <> declare c nested_int8s; begin b.c.x = 1; end $$; +ERROR: record "c" has no field "x" +CONTEXT: PL/pgSQL assignment "b.c.x = 1" +PL/pgSQL function inline_code_block line 1 at assignment +do $$ <> declare c nested_int8s; begin b.c.x.q1 = 1; end $$; +ERROR: record "c" has no field "x" +CONTEXT: PL/pgSQL assignment "b.c.x.q1 = 1" +PL/pgSQL function inline_code_block line 1 at assignment +do $$ <> declare c nested_int8s; begin b.c.c2.x = 1; end $$; +ERROR: cannot assign to field "x" of column "b" because there is no such column in data type two_int8s +LINE 1: b.c.c2.x = 1 + ^ +QUERY: b.c.c2.x = 1 +CONTEXT: PL/pgSQL function inline_code_block line 1 at assignment +do $$ <> declare c nested_int8s; begin b.d.c2.x = 1; end $$; +ERROR: "b.d.c2" is not a known variable +LINE 1: do $$ <> declare c nested_int8s; begin b.d.c2.x = 1; end ... + ^ -- check passing composite result to another function create function getq1(two_int8s) returns int8 language plpgsql as $$ declare r two_int8s; begin r := $1; return r.q1; end $$; @@ -188,7 +271,7 @@ NOTICE: r1.q1 = NOTICE: r1.q2 = NOTICE: r1 = ERROR: record "r1" has no field "nosuchfield" -CONTEXT: SQL statement "SELECT r1.nosuchfield" +CONTEXT: SQL expression "r1.nosuchfield" PL/pgSQL function inline_code_block line 7 at RAISE -- records, not so much do $$ @@ -202,7 +285,7 @@ end$$; NOTICE: r1 = ERROR: record "r1" is not assigned yet DETAIL: The tuple structure of a not-yet-assigned record is indeterminate. -CONTEXT: SQL statement "SELECT r1.f1" +CONTEXT: SQL expression "r1.f1" PL/pgSQL function inline_code_block line 5 at RAISE -- but OK if you assign first do $$ @@ -220,7 +303,7 @@ NOTICE: r1.f1 = 1 NOTICE: r1.f2 = 2 NOTICE: r1 = (1,2) ERROR: record "r1" has no field "nosuchfield" -CONTEXT: SQL statement "SELECT r1.nosuchfield" +CONTEXT: SQL expression "r1.nosuchfield" PL/pgSQL function inline_code_block line 9 at RAISE -- check repeated assignments to composite fields create table some_table (id int, data text); @@ -431,7 +514,7 @@ create function getf3(x mutable) returns int language plpgsql as $$ begin return x.f3; end $$; select getf3(null::mutable); -- doesn't work yet ERROR: record "x" has no field "f3" -CONTEXT: SQL statement "SELECT x.f3" +CONTEXT: SQL expression "x.f3" PL/pgSQL function getf3(mutable) line 1 at RETURN alter table mutable add column f3 int; select getf3(null::mutable); -- now it works diff --git a/src/pl/plpgsql/src/expected/plpgsql_varprops.out b/src/pl/plpgsql/src/expected/plpgsql_varprops.out index 18f03d75b4..3801dccc95 100644 --- a/src/pl/plpgsql/src/expected/plpgsql_varprops.out +++ b/src/pl/plpgsql/src/expected/plpgsql_varprops.out @@ -76,7 +76,7 @@ begin raise notice 'x = %', x; end$$; ERROR: division by zero -CONTEXT: SQL statement "SELECT 1/0" +CONTEXT: SQL expression "1/0" PL/pgSQL function inline_code_block line 3 during statement block local variable initialization do $$ declare x bigint[] := array[1,3,5]; diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c index 81979c2961..5336793a93 100644 --- a/src/pl/plpgsql/src/pl_comp.c +++ b/src/pl/plpgsql/src/pl_comp.c @@ -1458,7 +1458,8 @@ plpgsql_parse_dblword(char *word1, char *word2, /* * We should do nothing in DECLARE sections. In SQL expressions, we * really only need to make sure that RECFIELD datums are created when - * needed. + * needed. In all the cases handled by this function, returning a T_DATUM + * with a two-word idents string is the right thing. */ if (plpgsql_IdentifierLookup != IDENTIFIER_LOOKUP_DECLARE) { @@ -1532,40 +1533,53 @@ plpgsql_parse_tripword(char *word1, char *word2, char *word3, List *idents; int nnames; - idents = list_make3(makeString(word1), - makeString(word2), - makeString(word3)); - /* - * We should do nothing in DECLARE sections. In SQL expressions, we - * really only need to make sure that RECFIELD datums are created when - * needed. + * We should do nothing in DECLARE sections. In SQL expressions, we need + * to make sure that RECFIELD datums are created when needed, and we need + * to be careful about how many names are reported as belonging to the + * T_DATUM: the third word could be a sub-field reference, which we don't + * care about here. */ if (plpgsql_IdentifierLookup != IDENTIFIER_LOOKUP_DECLARE) { /* - * Do a lookup in the current namespace stack. Must find a qualified + * Do a lookup in the current namespace stack. Must find a record * reference, else ignore. */ ns = plpgsql_ns_lookup(plpgsql_ns_top(), false, word1, word2, word3, &nnames); - if (ns != NULL && nnames == 2) + if (ns != NULL) { switch (ns->itemtype) { case PLPGSQL_NSTYPE_REC: { - /* - * words 1/2 are a record name, so third word could be - * a field in this record. - */ PLpgSQL_rec *rec; PLpgSQL_recfield *new; rec = (PLpgSQL_rec *) (plpgsql_Datums[ns->itemno]); - new = plpgsql_build_recfield(rec, word3); - + if (nnames == 1) + { + /* + * First word is a record name, so second word + * could be a field in this record (and the third, + * a sub-field). We build a RECFIELD datum + * whether it is or not --- any error will be + * detected later. + */ + new = plpgsql_build_recfield(rec, word2); + idents = list_make2(makeString(word1), + makeString(word2)); + } + else + { + /* Block-qualified reference to record variable. */ + new = plpgsql_build_recfield(rec, word3); + idents = list_make3(makeString(word1), + makeString(word2), + makeString(word3)); + } wdatum->datum = (PLpgSQL_datum *) new; wdatum->ident = NULL; wdatum->quoted = false; /* not used */ @@ -1580,6 +1594,9 @@ plpgsql_parse_tripword(char *word1, char *word2, char *word3, } /* Nothing found */ + idents = list_make3(makeString(word1), + makeString(word2), + makeString(word3)); cword->idents = idents; return false; } diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 4a51fb6d9f..5e4dbd25a2 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -4182,7 +4182,7 @@ exec_prepare_plan(PLpgSQL_execstate *estate, memset(&options, 0, sizeof(options)); options.parserSetup = (ParserSetupHook) plpgsql_parser_setup; options.parserSetupArg = (void *) expr; - options.parseMode = RAW_PARSE_DEFAULT; + options.parseMode = expr->parseMode; options.cursorOptions = cursorOptions; plan = SPI_prepare_extended(expr->query, &options); if (plan == NULL) @@ -8006,10 +8006,14 @@ get_cast_hashentry(PLpgSQL_execstate *estate, placeholder->collation = get_typcollation(srctype); /* - * Apply coercion. We use ASSIGNMENT coercion because that's the - * closest match to plpgsql's historical behavior; in particular, - * EXPLICIT coercion would allow silent truncation to a destination - * varchar/bpchar's length, which we do not want. + * Apply coercion. We use the special coercion context + * COERCION_PLPGSQL to match plpgsql's historical behavior, namely + * that any cast not available at ASSIGNMENT level will be implemented + * as an I/O coercion. (It's somewhat dubious that we prefer I/O + * coercion over cast pathways that exist at EXPLICIT level. Changing + * that would cause assorted minor behavioral differences though, and + * a user who wants the explicit-cast behavior can always write an + * explicit cast.) * * If source type is UNKNOWN, coerce_to_target_type will fail (it only * expects to see that for Const input nodes), so don't call it; we'll @@ -8022,7 +8026,7 @@ get_cast_hashentry(PLpgSQL_execstate *estate, cast_expr = coerce_to_target_type(NULL, (Node *) placeholder, srctype, dsttype, dsttypmod, - COERCION_ASSIGNMENT, + COERCION_PLPGSQL, COERCE_IMPLICIT_CAST, -1); @@ -8030,7 +8034,8 @@ get_cast_hashentry(PLpgSQL_execstate *estate, * If there's no cast path according to the parser, fall back to using * an I/O coercion; this is semantically dubious but matches plpgsql's * historical behavior. We would need something of the sort for - * UNKNOWN literals in any case. + * UNKNOWN literals in any case. (This is probably now only reachable + * in the case where srctype is UNKNOWN/RECORD.) */ if (cast_expr == NULL) { @@ -8339,7 +8344,8 @@ exec_check_rw_parameter(PLpgSQL_expr *expr, int target_dno) return; /* - * Top level of expression must be a simple FuncExpr or OpExpr. + * Top level of expression must be a simple FuncExpr, OpExpr, or + * SubscriptingRef. */ if (IsA(expr->expr_simple_expr, FuncExpr)) { @@ -8355,6 +8361,33 @@ exec_check_rw_parameter(PLpgSQL_expr *expr, int target_dno) funcid = opexpr->opfuncid; fargs = opexpr->args; } + else if (IsA(expr->expr_simple_expr, SubscriptingRef)) + { + SubscriptingRef *sbsref = (SubscriptingRef *) expr->expr_simple_expr; + + /* We only trust standard varlena arrays to be safe */ + if (get_typsubscript(sbsref->refcontainertype, NULL) != + F_ARRAY_SUBSCRIPT_HANDLER) + return; + + /* refexpr can be a simple Param, otherwise must not contain target */ + if (!(sbsref->refexpr && IsA(sbsref->refexpr, Param)) && + contains_target_param((Node *) sbsref->refexpr, &target_dno)) + return; + + /* the other subexpressions must not contain target */ + if (contains_target_param((Node *) sbsref->refupperindexpr, + &target_dno) || + contains_target_param((Node *) sbsref->reflowerindexpr, + &target_dno) || + contains_target_param((Node *) sbsref->refassgnexpr, + &target_dno)) + return; + + /* OK, we can pass target as a read-write parameter */ + expr->rwparam = target_dno; + return; + } else return; diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y index c09576efff..ad248bc764 100644 --- a/src/pl/plpgsql/src/pl_gram.y +++ b/src/pl/plpgsql/src/pl_gram.y @@ -51,7 +51,6 @@ typedef struct { int location; - int leaderlen; } sql_error_callback_arg; #define parser_errposition(pos) plpgsql_scanner_errposition(pos) @@ -67,7 +66,7 @@ static PLpgSQL_expr *read_sql_construct(int until, int until2, int until3, const char *expected, - const char *sqlstart, + RawParseMode parsemode, bool isexpression, bool valid_sql, bool trim, @@ -78,7 +77,7 @@ static PLpgSQL_expr *read_sql_expression(int until, static PLpgSQL_expr *read_sql_expression2(int until, int until2, const char *expected, int *endtoken); -static PLpgSQL_expr *read_sql_stmt(const char *sqlstart); +static PLpgSQL_expr *read_sql_stmt(void); static PLpgSQL_type *read_datatype(int tok); static PLpgSQL_stmt *make_execsql_stmt(int firsttoken, int location); static PLpgSQL_stmt_fetch *read_fetch_direction(void); @@ -99,8 +98,8 @@ static PLpgSQL_row *read_into_scalar_list(char *initial_name, static PLpgSQL_row *make_scalar_list1(char *initial_name, PLpgSQL_datum *initial_datum, int lineno, int location); -static void check_sql_expr(const char *stmt, int location, - int leaderlen); +static void check_sql_expr(const char *stmt, + RawParseMode parseMode, int location); static void plpgsql_sql_error_callback(void *arg); static PLpgSQL_type *parse_datatype(const char *string, int location); static void check_labels(const char *start_label, @@ -540,7 +539,7 @@ decl_statement : decl_varname decl_const decl_datatype decl_collate decl_notnull { PLpgSQL_var *new; PLpgSQL_expr *curname_def; - char buf[1024]; + char buf[NAMEDATALEN * 2 + 64]; char *cp1; char *cp2; @@ -557,9 +556,9 @@ decl_statement : decl_varname decl_const decl_datatype decl_collate decl_notnull curname_def = palloc0(sizeof(PLpgSQL_expr)); - strcpy(buf, "SELECT "); + /* Note: refname has been truncated to NAMEDATALEN */ cp1 = new->refname; - cp2 = buf + strlen(buf); + cp2 = buf; /* * Don't trust standard_conforming_strings here; * it might change before we use the string. @@ -575,6 +574,7 @@ decl_statement : decl_varname decl_const decl_datatype decl_collate decl_notnull } strcpy(cp2, "'::pg_catalog.refcursor"); curname_def->query = pstrdup(buf); + curname_def->parseMode = RAW_PARSE_PLPGSQL_EXPR; new->default_val = curname_def; new->cursor_explicit_expr = $7; @@ -602,7 +602,7 @@ opt_scrollable : decl_cursor_query : { - $$ = read_sql_stmt(""); + $$ = read_sql_stmt(); } ; @@ -904,15 +904,37 @@ proc_stmt : pl_block ';' { $$ = $1; } ; -stmt_perform : K_PERFORM expr_until_semi +stmt_perform : K_PERFORM { PLpgSQL_stmt_perform *new; + int startloc; new = palloc0(sizeof(PLpgSQL_stmt_perform)); new->cmd_type = PLPGSQL_STMT_PERFORM; new->lineno = plpgsql_location_to_lineno(@1); new->stmtid = ++plpgsql_curr_compile->nstatements; - new->expr = $2; + plpgsql_push_back_token(K_PERFORM); + + /* + * Since PERFORM isn't legal SQL, we have to cheat to + * the extent of substituting "SELECT" for "PERFORM" + * in the parsed text. It does not seem worth + * inventing a separate parse mode for this one case. + * We can't do syntax-checking until after we make the + * substitution. + */ + new->expr = read_sql_construct(';', 0, 0, ";", + RAW_PARSE_DEFAULT, + false, false, true, + &startloc, NULL); + /* overwrite "perform" ... */ + memcpy(new->expr->query, " SELECT", 7); + /* left-justify to get rid of the leading space */ + memmove(new->expr->query, new->expr->query + 1, + strlen(new->expr->query)); + /* offset syntax error position to account for that */ + check_sql_expr(new->expr->query, new->expr->parseMode, + startloc + 1); $$ = (PLpgSQL_stmt *)new; } @@ -926,7 +948,8 @@ stmt_call : K_CALL new->cmd_type = PLPGSQL_STMT_CALL; new->lineno = plpgsql_location_to_lineno(@1); new->stmtid = ++plpgsql_curr_compile->nstatements; - new->expr = read_sql_stmt("CALL "); + plpgsql_push_back_token(K_CALL); + new->expr = read_sql_stmt(); new->is_call = true; $$ = (PLpgSQL_stmt *)new; @@ -941,7 +964,8 @@ stmt_call : K_CALL new->cmd_type = PLPGSQL_STMT_CALL; new->lineno = plpgsql_location_to_lineno(@1); new->stmtid = ++plpgsql_curr_compile->nstatements; - new->expr = read_sql_stmt("DO "); + plpgsql_push_back_token(K_DO); + new->expr = read_sql_stmt(); new->is_call = false; $$ = (PLpgSQL_stmt *)new; @@ -949,16 +973,40 @@ stmt_call : K_CALL } ; -stmt_assign : assign_var assign_operator expr_until_semi +stmt_assign : T_DATUM { PLpgSQL_stmt_assign *new; + RawParseMode pmode; + /* see how many names identify the datum */ + switch ($1.ident ? 1 : list_length($1.idents)) + { + case 1: + pmode = RAW_PARSE_PLPGSQL_ASSIGN1; + break; + case 2: + pmode = RAW_PARSE_PLPGSQL_ASSIGN2; + break; + case 3: + pmode = RAW_PARSE_PLPGSQL_ASSIGN3; + break; + default: + elog(ERROR, "unexpected number of names"); + pmode = 0; /* keep compiler quiet */ + } + + check_assignable($1.datum, @1); new = palloc0(sizeof(PLpgSQL_stmt_assign)); new->cmd_type = PLPGSQL_STMT_ASSIGN; new->lineno = plpgsql_location_to_lineno(@1); new->stmtid = ++plpgsql_curr_compile->nstatements; - new->varno = $1->dno; - new->expr = $3; + new->varno = $1.datum->dno; + /* Push back the head name to include it in the stmt */ + plpgsql_push_back_token(T_DATUM); + new->expr = read_sql_construct(';', 0, 0, ";", + pmode, + false, true, true, + NULL, NULL); $$ = (PLpgSQL_stmt *)new; } @@ -1452,16 +1500,16 @@ for_control : for_variable K_IN /* * Read tokens until we see either a ".." - * or a LOOP. The text we read may not - * necessarily be a well-formed SQL - * statement, so we need to invoke - * read_sql_construct directly. + * or a LOOP. The text we read may be either + * an expression or a whole SQL statement, so + * we need to invoke read_sql_construct directly, + * and tell it not to check syntax yet. */ expr1 = read_sql_construct(DOT_DOT, K_LOOP, 0, "LOOP", - "SELECT ", + RAW_PARSE_DEFAULT, true, false, true, @@ -1476,8 +1524,13 @@ for_control : for_variable K_IN PLpgSQL_var *fvar; PLpgSQL_stmt_fori *new; - /* Check first expression is well-formed */ - check_sql_expr(expr1->query, expr1loc, 7); + /* + * Relabel first expression as an expression; + * then we can check its syntax. + */ + expr1->parseMode = RAW_PARSE_PLPGSQL_EXPR; + check_sql_expr(expr1->query, expr1->parseMode, + expr1loc); /* Read and check the second one */ expr2 = read_sql_expression2(K_LOOP, K_BY, @@ -1522,12 +1575,8 @@ for_control : for_variable K_IN else { /* - * No "..", so it must be a query loop. We've - * prefixed an extra SELECT to the query text, - * so we need to remove that before performing - * syntax checking. + * No "..", so it must be a query loop. */ - char *tmp_query; PLpgSQL_stmt_fors *new; if (reverse) @@ -1536,12 +1585,9 @@ for_control : for_variable K_IN errmsg("cannot specify REVERSE in query FOR loop"), parser_errposition(tokloc))); - Assert(strncmp(expr1->query, "SELECT ", 7) == 0); - tmp_query = pstrdup(expr1->query + 7); - pfree(expr1->query); - expr1->query = tmp_query; - - check_sql_expr(expr1->query, expr1loc, 0); + /* Check syntax as a regular query */ + check_sql_expr(expr1->query, expr1->parseMode, + expr1loc); new = palloc0(sizeof(PLpgSQL_stmt_fors)); new->cmd_type = PLPGSQL_STMT_FORS; @@ -1870,7 +1916,7 @@ stmt_raise : K_RAISE expr = read_sql_construct(',', ';', K_USING, ", or ; or USING", - "SELECT ", + RAW_PARSE_PLPGSQL_EXPR, true, true, true, NULL, &tok); new->params = lappend(new->params, expr); @@ -1958,7 +2004,7 @@ loop_body : proc_sect K_END K_LOOP opt_label ';' * variable. (The composite case is probably a syntax error, but we'll let * the core parser decide that.) Normally, we should assume that such a * word is a SQL statement keyword that isn't also a plpgsql keyword. - * However, if the next token is assignment or '[', it can't be a valid + * However, if the next token is assignment or '[' or '.', it can't be a valid * SQL statement, and what we're probably looking at is an intended variable * assignment. Give an appropriate complaint for that, instead of letting * the core parser throw an unhelpful "syntax error". @@ -1977,7 +2023,8 @@ stmt_execsql : K_IMPORT tok = yylex(); plpgsql_push_back_token(tok); - if (tok == '=' || tok == COLON_EQUALS || tok == '[') + if (tok == '=' || tok == COLON_EQUALS || + tok == '[' || tok == '.') word_is_not_variable(&($1), @1); $$ = make_execsql_stmt(T_WORD, @1); } @@ -1987,7 +2034,8 @@ stmt_execsql : K_IMPORT tok = yylex(); plpgsql_push_back_token(tok); - if (tok == '=' || tok == COLON_EQUALS || tok == '[') + if (tok == '=' || tok == COLON_EQUALS || + tok == '[' || tok == '.') cword_is_not_variable(&($1), @1); $$ = make_execsql_stmt(T_CWORD, @1); } @@ -2001,7 +2049,7 @@ stmt_dynexecute : K_EXECUTE expr = read_sql_construct(K_INTO, K_USING, ';', "INTO or USING or ;", - "SELECT ", + RAW_PARSE_PLPGSQL_EXPR, true, true, true, NULL, &endtoken); @@ -2040,7 +2088,7 @@ stmt_dynexecute : K_EXECUTE { expr = read_sql_construct(',', ';', K_INTO, ", or ; or INTO", - "SELECT ", + RAW_PARSE_PLPGSQL_EXPR, true, true, true, NULL, &endtoken); new->params = lappend(new->params, expr); @@ -2122,7 +2170,7 @@ stmt_open : K_OPEN cursor_variable else { plpgsql_push_back_token(tok); - new->query = read_sql_stmt(""); + new->query = read_sql_stmt(); } } else @@ -2246,8 +2294,8 @@ stmt_set : K_SET new->cmd_type = PLPGSQL_STMT_SET; new->lineno = plpgsql_location_to_lineno(@1); new->stmtid = ++plpgsql_curr_compile->nstatements; - - new->expr = read_sql_stmt("SET "); + plpgsql_push_back_token(K_SET); + new->expr = read_sql_stmt(); $$ = (PLpgSQL_stmt *)new; } @@ -2259,7 +2307,8 @@ stmt_set : K_SET new->cmd_type = PLPGSQL_STMT_SET; new->lineno = plpgsql_location_to_lineno(@1); new->stmtid = ++plpgsql_curr_compile->nstatements; - new->expr = read_sql_stmt("RESET "); + plpgsql_push_back_token(K_RESET); + new->expr = read_sql_stmt(); $$ = (PLpgSQL_stmt *)new; } @@ -2656,7 +2705,8 @@ static PLpgSQL_expr * read_sql_expression(int until, const char *expected) { return read_sql_construct(until, 0, 0, expected, - "SELECT ", true, true, true, NULL, NULL); + RAW_PARSE_PLPGSQL_EXPR, + true, true, true, NULL, NULL); } /* Convenience routine to read an expression with two possible terminators */ @@ -2665,15 +2715,17 @@ read_sql_expression2(int until, int until2, const char *expected, int *endtoken) { return read_sql_construct(until, until2, 0, expected, - "SELECT ", true, true, true, NULL, endtoken); + RAW_PARSE_PLPGSQL_EXPR, + true, true, true, NULL, endtoken); } /* Convenience routine to read a SQL statement that must end with ';' */ static PLpgSQL_expr * -read_sql_stmt(const char *sqlstart) +read_sql_stmt(void) { return read_sql_construct(';', 0, 0, ";", - sqlstart, false, true, true, NULL, NULL); + RAW_PARSE_DEFAULT, + false, true, true, NULL, NULL); } /* @@ -2683,9 +2735,9 @@ read_sql_stmt(const char *sqlstart) * until2: token code for alternate terminator (pass 0 if none) * until3: token code for another alternate terminator (pass 0 if none) * expected: text to use in complaining that terminator was not found - * sqlstart: text to prefix to the accumulated SQL text + * parsemode: raw_parser() mode to use * isexpression: whether to say we're reading an "expression" or a "statement" - * valid_sql: whether to check the syntax of the expr (prefixed with sqlstart) + * valid_sql: whether to check the syntax of the expr * trim: trim trailing whitespace * startloc: if not NULL, location of first token is stored at *startloc * endtoken: if not NULL, ending token is stored at *endtoken @@ -2696,7 +2748,7 @@ read_sql_construct(int until, int until2, int until3, const char *expected, - const char *sqlstart, + RawParseMode parsemode, bool isexpression, bool valid_sql, bool trim, @@ -2711,7 +2763,6 @@ read_sql_construct(int until, PLpgSQL_expr *expr; initStringInfo(&ds); - appendStringInfoString(&ds, sqlstart); /* special lookup mode for identifiers within the SQL text */ save_IdentifierLookup = plpgsql_IdentifierLookup; @@ -2787,6 +2838,7 @@ read_sql_construct(int until, expr = palloc0(sizeof(PLpgSQL_expr)); expr->query = pstrdup(ds.data); + expr->parseMode = parsemode; expr->plan = NULL; expr->paramnos = NULL; expr->rwparam = -1; @@ -2794,7 +2846,7 @@ read_sql_construct(int until, pfree(ds.data); if (valid_sql) - check_sql_expr(expr->query, startlocation, strlen(sqlstart)); + check_sql_expr(expr->query, expr->parseMode, startlocation); return expr; } @@ -3033,13 +3085,14 @@ make_execsql_stmt(int firsttoken, int location) expr = palloc0(sizeof(PLpgSQL_expr)); expr->query = pstrdup(ds.data); + expr->parseMode = RAW_PARSE_DEFAULT; expr->plan = NULL; expr->paramnos = NULL; expr->rwparam = -1; expr->ns = plpgsql_ns_top(); pfree(ds.data); - check_sql_expr(expr->query, location, 0); + check_sql_expr(expr->query, expr->parseMode, location); execsql = palloc(sizeof(PLpgSQL_stmt_execsql)); execsql->cmd_type = PLPGSQL_STMT_EXECSQL; @@ -3382,7 +3435,7 @@ make_return_query_stmt(int location) { /* ordinary static query */ plpgsql_push_back_token(tok); - new->query = read_sql_stmt(""); + new->query = read_sql_stmt(); } else { @@ -3637,13 +3690,12 @@ make_scalar_list1(char *initial_name, * borders. So it is best to bail out as early as we can. * * It is assumed that "stmt" represents a copy of the function source text - * beginning at offset "location", with leader text of length "leaderlen" - * (typically "SELECT ") prefixed to the source text. We use this assumption - * to transpose any error cursor position back to the function source text. + * beginning at offset "location". We use this assumption to transpose + * any error cursor position back to the function source text. * If no error cursor is provided, we'll just point at "location". */ static void -check_sql_expr(const char *stmt, int location, int leaderlen) +check_sql_expr(const char *stmt, RawParseMode parseMode, int location) { sql_error_callback_arg cbarg; ErrorContextCallback syntax_errcontext; @@ -3653,7 +3705,6 @@ check_sql_expr(const char *stmt, int location, int leaderlen) return; cbarg.location = location; - cbarg.leaderlen = leaderlen; syntax_errcontext.callback = plpgsql_sql_error_callback; syntax_errcontext.arg = &cbarg; @@ -3661,7 +3712,7 @@ check_sql_expr(const char *stmt, int location, int leaderlen) error_context_stack = &syntax_errcontext; oldCxt = MemoryContextSwitchTo(plpgsql_compile_tmp_cxt); - (void) raw_parser(stmt, RAW_PARSE_DEFAULT); + (void) raw_parser(stmt, parseMode); MemoryContextSwitchTo(oldCxt); /* Restore former ereport callback */ @@ -3686,12 +3737,12 @@ plpgsql_sql_error_callback(void *arg) * Note we are dealing with 1-based character numbers at this point. */ errpos = geterrposition(); - if (errpos > cbarg->leaderlen) + if (errpos > 0) { int myerrpos = getinternalerrposition(); if (myerrpos > 0) /* safety check */ - internalerrposition(myerrpos + errpos - cbarg->leaderlen - 1); + internalerrposition(myerrpos + errpos - 1); } /* In any case, flush errposition --- we want internalerrposition only */ @@ -3717,7 +3768,6 @@ parse_datatype(const char *string, int location) ErrorContextCallback syntax_errcontext; cbarg.location = location; - cbarg.leaderlen = 0; syntax_errcontext.callback = plpgsql_sql_error_callback; syntax_errcontext.arg = &cbarg; @@ -3780,7 +3830,6 @@ read_cursor_args(PLpgSQL_var *cursor, int until) int argc; char **argv; StringInfoData ds; - char *sqlstart = "SELECT "; bool any_named = false; tok = yylex(); @@ -3881,12 +3930,12 @@ read_cursor_args(PLpgSQL_var *cursor, int until) */ item = read_sql_construct(',', ')', 0, ",\" or \")", - sqlstart, + RAW_PARSE_PLPGSQL_EXPR, true, true, false, /* do not trim */ NULL, &endtoken); - argv[argpos] = item->query + strlen(sqlstart); + argv[argpos] = item->query; if (endtoken == ')' && !(argc == row->nfields - 1)) ereport(ERROR, @@ -3905,7 +3954,6 @@ read_cursor_args(PLpgSQL_var *cursor, int until) /* Make positional argument list */ initStringInfo(&ds); - appendStringInfoString(&ds, sqlstart); for (argc = 0; argc < row->nfields; argc++) { Assert(argv[argc] != NULL); @@ -3921,10 +3969,10 @@ read_cursor_args(PLpgSQL_var *cursor, int until) if (argc < row->nfields - 1) appendStringInfoString(&ds, ", "); } - appendStringInfoChar(&ds, ';'); expr = palloc0(sizeof(PLpgSQL_expr)); expr->query = pstrdup(ds.data); + expr->parseMode = RAW_PARSE_PLPGSQL_EXPR; expr->plan = NULL; expr->paramnos = NULL; expr->rwparam = -1; @@ -4097,14 +4145,14 @@ make_case(int location, PLpgSQL_expr *t_expr, PLpgSQL_expr *expr = cwt->expr; StringInfoData ds; - /* copy expression query without SELECT keyword (expr->query + 7) */ - Assert(strncmp(expr->query, "SELECT ", 7) == 0); + /* We expect to have expressions not statements */ + Assert(expr->parseMode == RAW_PARSE_PLPGSQL_EXPR); - /* And do the string hacking */ + /* Do the string hacking */ initStringInfo(&ds); - appendStringInfo(&ds, "SELECT \"%s\" IN (%s)", - varname, expr->query + 7); + appendStringInfo(&ds, "\"%s\" IN (%s)", + varname, expr->query); pfree(expr->query); expr->query = pstrdup(ds.data); diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index dee175b7bf..f80d023a5a 100644 --- a/src/pl/plpgsql/src/plpgsql.h +++ b/src/pl/plpgsql/src/plpgsql.h @@ -218,8 +218,9 @@ typedef struct PLpgSQL_type */ typedef struct PLpgSQL_expr { - char *query; - SPIPlanPtr plan; + char *query; /* query string, verbatim from function body */ + RawParseMode parseMode; /* raw_parser() mode to use */ + SPIPlanPtr plan; /* plan, or NULL if not made yet */ Bitmapset *paramnos; /* all dnos referenced by this query */ int rwparam; /* dno of read/write param, or -1 if none */ diff --git a/src/pl/plpgsql/src/sql/plpgsql_array.sql b/src/pl/plpgsql/src/sql/plpgsql_array.sql new file mode 100644 index 0000000000..4c3f26be10 --- /dev/null +++ b/src/pl/plpgsql/src/sql/plpgsql_array.sql @@ -0,0 +1,79 @@ +-- +-- Tests for PL/pgSQL handling of array variables +-- +-- We also check arrays of composites here, so this has some overlap +-- with the plpgsql_record tests. +-- + +create type complex as (r float8, i float8); +create type quadarray as (c1 complex[], c2 complex); + +do $$ declare a int[]; +begin a := array[1,2]; a[3] := 4; raise notice 'a = %', a; end$$; + +do $$ declare a int[]; +begin a[3] := 4; raise notice 'a = %', a; end$$; + +do $$ declare a int[]; +begin a[1][4] := 4; raise notice 'a = %', a; end$$; + +do $$ declare a int[]; +begin a[1] := 23::text; raise notice 'a = %', a; end$$; -- lax typing + +do $$ declare a int[]; +begin a := array[1,2]; a[2:3] := array[3,4]; raise notice 'a = %', a; end$$; + +do $$ declare a int[]; +begin a := array[1,2]; a[2] := a[2] + 1; raise notice 'a = %', a; end$$; + +do $$ declare a int[]; +begin a[1:2] := array[3,4]; raise notice 'a = %', a; end$$; + +do $$ declare a int[]; +begin a[1:2] := 4; raise notice 'a = %', a; end$$; -- error + +do $$ declare a complex[]; +begin a[1] := (1,2); a[1].i := 11; raise notice 'a = %', a; end$$; + +do $$ declare a complex[]; +begin a[1].i := 11; raise notice 'a = %, a[1].i = %', a, a[1].i; end$$; + +-- perhaps this ought to work, but for now it doesn't: +do $$ declare a complex[]; +begin a[1:2].i := array[11,12]; raise notice 'a = %', a; end$$; + +do $$ declare a quadarray; +begin a.c1[1].i := 11; raise notice 'a = %, a.c1[1].i = %', a, a.c1[1].i; end$$; + +do $$ declare a int[]; +begin a := array_agg(x) from (values(1),(2),(3)) v(x); raise notice 'a = %', a; end$$; + +create temp table onecol as select array[1,2] as f1; + +do $$ declare a int[]; +begin a := f1 from onecol; raise notice 'a = %', a; end$$; + +do $$ declare a int[]; +begin a := * from onecol for update; raise notice 'a = %', a; end$$; + +-- error cases: + +do $$ declare a int[]; +begin a := from onecol; raise notice 'a = %', a; end$$; + +do $$ declare a int[]; +begin a := f1, f1 from onecol; raise notice 'a = %', a; end$$; + +insert into onecol values(array[11]); + +do $$ declare a int[]; +begin a := f1 from onecol; raise notice 'a = %', a; end$$; + +do $$ declare a int[]; +begin a := f1 from onecol limit 1; raise notice 'a = %', a; end$$; + +do $$ declare a real; +begin a[1] := 2; raise notice 'a = %', a; end$$; + +do $$ declare a complex; +begin a.r[1] := 2; raise notice 'a = %', a; end$$; diff --git a/src/pl/plpgsql/src/sql/plpgsql_record.sql b/src/pl/plpgsql/src/sql/plpgsql_record.sql index 128846e610..be10f00b1e 100644 --- a/src/pl/plpgsql/src/sql/plpgsql_record.sql +++ b/src/pl/plpgsql/src/sql/plpgsql_record.sql @@ -4,6 +4,7 @@ create type two_int4s as (f1 int4, f2 int4); create type two_int8s as (q1 int8, q2 int8); +create type nested_int8s as (c1 two_int8s, c2 two_int8s); -- base-case return of a composite type create function retc(int) returns two_int8s language plpgsql as @@ -59,6 +60,47 @@ begin raise notice 'c8 = %', c8; end$$; +do $$ declare c two_int8s; d nested_int8s; +begin + c := row(1,2); + d := row(c, row(c.q1, c.q2+1)); + raise notice 'c = %, d = %', c, d; + c.q1 := 10; + d.c1 := row(11,12); + d.c2.q2 := 42; + raise notice 'c = %, d = %', c, d; + raise notice 'c.q1 = %, d.c2 = %', c.q1, d.c2; + raise notice '(d).c2.q2 = %', (d).c2.q2; -- doesn't work without parens + raise notice '(d.c2).q2 = %', (d.c2).q2; -- doesn't work without parens +end$$; + +-- block-qualified naming +do $$ <> declare c two_int8s; d nested_int8s; +begin + b.c := row(1,2); + b.d := row(b.c, row(b.c.q1, b.c.q2+1)); + raise notice 'b.c = %, b.d = %', b.c, b.d; + b.c.q1 := 10; + b.d.c1 := row(11,12); + b.d.c2.q2 := 42; + raise notice 'b.c = %, b.d = %', b.c, b.d; + raise notice 'b.c.q1 = %, b.d.c2 = %', b.c.q1, b.d.c2; + raise notice '(b.d).c2.q2 = %', (b.d).c2.q2; -- doesn't work without parens + raise notice '(b.d.c2).q2 = %', (b.d.c2).q2; -- doesn't work without parens +end$$; + +-- error cases +do $$ declare c two_int8s; begin c.x = 1; end $$; +do $$ declare c nested_int8s; begin c.x = 1; end $$; +do $$ declare c nested_int8s; begin c.x.q1 = 1; end $$; +do $$ declare c nested_int8s; begin c.c2.x = 1; end $$; +do $$ declare c nested_int8s; begin d.c2.x = 1; end $$; +do $$ <> declare c two_int8s; begin b.c.x = 1; end $$; +do $$ <> declare c nested_int8s; begin b.c.x = 1; end $$; +do $$ <> declare c nested_int8s; begin b.c.x.q1 = 1; end $$; +do $$ <> declare c nested_int8s; begin b.c.c2.x = 1; end $$; +do $$ <> declare c nested_int8s; begin b.d.c2.x = 1; end $$; + -- check passing composite result to another function create function getq1(two_int8s) returns int8 language plpgsql as $$ declare r two_int8s; begin r := $1; return r.q1; end $$; diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index d55006d8c9..0c6c9ba1f5 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -1761,10 +1761,10 @@ select f1(42) as int, f1(4.5) as num; select f1(point(3,4)); -- fail for lack of + operator ERROR: operator does not exist: point + integer -LINE 1: SELECT x + 1 - ^ +LINE 1: x + 1 + ^ HINT: No operator matches the given name and argument types. You might need to add explicit type casts. -QUERY: SELECT x + 1 +QUERY: x + 1 CONTEXT: PL/pgSQL function f1(anyelement) line 3 at RETURN drop function f1(x anyelement); create function f1(x anyelement) returns anyarray as $$ @@ -2361,7 +2361,7 @@ begin end $$ language plpgsql; select namedparmcursor_test7(); ERROR: division by zero -CONTEXT: SQL statement "SELECT 42/0 AS p1, 77 AS p2;" +CONTEXT: SQL expression "42/0 AS p1, 77 AS p2" PL/pgSQL function namedparmcursor_test7() line 6 at OPEN -- check that line comments work correctly within the argument list (there -- is some special handling of this case in the code: the newline after the @@ -2574,9 +2574,9 @@ end; $$ language plpgsql; -- blocks select excpt_test1(); ERROR: column "sqlstate" does not exist -LINE 1: SELECT sqlstate - ^ -QUERY: SELECT sqlstate +LINE 1: sqlstate + ^ +QUERY: sqlstate CONTEXT: PL/pgSQL function excpt_test1() line 3 at RAISE create function excpt_test2() returns void as $$ begin @@ -2589,9 +2589,9 @@ end; $$ language plpgsql; -- should fail select excpt_test2(); ERROR: column "sqlstate" does not exist -LINE 1: SELECT sqlstate - ^ -QUERY: SELECT sqlstate +LINE 1: sqlstate + ^ +QUERY: sqlstate CONTEXT: PL/pgSQL function excpt_test2() line 5 at RAISE create function excpt_test3() returns void as $$ begin @@ -4467,11 +4467,11 @@ end $$; select fail(); ERROR: division by zero -CONTEXT: SQL statement "SELECT 1/0" +CONTEXT: SQL expression "1/0" PL/pgSQL function fail() line 3 at RETURN select fail(); ERROR: division by zero -CONTEXT: SQL statement "SELECT 1/0" +CONTEXT: SQL expression "1/0" PL/pgSQL function fail() line 3 at RETURN drop function fail(); -- Test handling of string literals. @@ -4497,10 +4497,10 @@ HINT: Use the escape string syntax for backslashes, e.g., E'\\'. select strtest(); NOTICE: foo\bar!baz WARNING: nonstandard use of \\ in a string literal -LINE 1: SELECT 'foo\\bar\041baz' - ^ +LINE 1: 'foo\\bar\041baz' + ^ HINT: Use the escape string syntax for backslashes, e.g., E'\\'. -QUERY: SELECT 'foo\\bar\041baz' +QUERY: 'foo\\bar\041baz' strtest ------------- foo\bar!baz @@ -5621,9 +5621,9 @@ ALTER TABLE alter_table_under_transition_tables UPDATE alter_table_under_transition_tables SET id = id; ERROR: column "name" does not exist -LINE 1: SELECT (SELECT string_agg(id || '=' || name, ',') FROM d) - ^ -QUERY: SELECT (SELECT string_agg(id || '=' || name, ',') FROM d) +LINE 1: (SELECT string_agg(id || '=' || name, ',') FROM d) + ^ +QUERY: (SELECT string_agg(id || '=' || name, ',') FROM d) CONTEXT: PL/pgSQL function alter_table_under_transition_tables_upd_func() line 3 at RAISE -- -- Test multiple reference to a transition table