Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1gJL5I-0003bM-Kx for pgsql-hackers@arkaria.postgresql.org; Sun, 04 Nov 2018 16:17:08 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1gJL3A-0000cZ-Sq for pgsql-hackers@arkaria.postgresql.org; Sun, 04 Nov 2018 16:14:56 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1gJL3A-0000cS-Ld for pgsql-hackers@lists.postgresql.org; Sun, 04 Nov 2018 16:14:56 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1gJL38-0004Ra-Ow for pgsql-hackers@postgresql.org; Sun, 04 Nov 2018 16:14:56 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.14.4/8.14.4) with ESMTP id wA4GEq2P010616; Sun, 4 Nov 2018 11:14:52 -0500 From: Tom Lane To: Pavel Stehule cc: Peter Eisentraut , PostgreSQL Hackers Subject: Re: bugfix: BUG #15477: Procedure call with named inout refcursor parameter - "invalid input syntax for type boolean" In-reply-to: References: <1d846492-3822-616b-7246-91b429cc5dbb@2ndquadrant.com> <26116.1541278051@sss.pgh.pa.us> <28551.1541281624@sss.pgh.pa.us> <7102.1541346845@sss.pgh.pa.us> Comments: In-reply-to Pavel Stehule message dated "Sun, 04 Nov 2018 17:01:54 +0100" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <10555.1541347990.0@sss.pgh.pa.us> Date: Sun, 04 Nov 2018 11:14:52 -0500 Message-ID: <10615.1541348092@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <10555.1541347990.1@sss.pgh.pa.us> Pavel Stehule writes: > ne 4. 11. 2018 v 16:54 odesilatel Tom Lane napsal: >> In short, I think it's a bug that we allow the above. If you >> want to keep the must-be-a-variable error then it should apply in >> this case too. > I agree. This should be prohibited from PLpgSQL. OK. In that case I'll run with my patch. The attached is what I had code-wise, but I've not changed the regression tests to match ... gotta go fix the docs too I guess. regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="call-param-matching-fix-0.1.patch"; charset="us-ascii" Content-ID: <10555.1541347990.2@sss.pgh.pa.us> Content-Description: call-param-matching-fix-0.1.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 4552638..4e6d5b1 100644 *** a/src/pl/plpgsql/src/pl_exec.c --- b/src/pl/plpgsql/src/pl_exec.c *************** *** 30,35 **** --- 30,36 ---- #include "funcapi.h" #include "miscadmin.h" #include "nodes/nodeFuncs.h" + #include "optimizer/clauses.h" #include "optimizer/planner.h" #include "parser/parse_coerce.h" #include "parser/scansup.h" *************** exec_stmt_call(PLpgSQL_execstate *estate *** 2158,2260 **** plpgsql_create_econtext(estate); } = if (SPI_processed =3D=3D 1) { SPITupleTable *tuptab =3D SPI_tuptable; = /* ! * Construct a dummy target row based on the output arguments of the ! * procedure call. */ if (!stmt->target) { Node *node; - ListCell *lc; FuncExpr *funcexpr; ! int i; ! HeapTuple tuple; Oid *argtypes; char **argnames; char *argmodes; MemoryContext oldcontext; PLpgSQL_row *row; int nfields; = /* ! * Get the original CallStmt */ ! node =3D linitial_node(Query, ((CachedPlanSource *) linitial(plan->pl= ancache_list))->query_list)->utilityStmt; ! if (!IsA(node, CallStmt)) ! elog(ERROR, "returned row from not a CallStmt"); = ! funcexpr =3D castNode(CallStmt, node)->funcexpr; = /* ! * Get the argument modes */ ! tuple =3D SearchSysCache1(PROCOID, ObjectIdGetDatum(funcexpr->funcid)= ); ! if (!HeapTupleIsValid(tuple)) ! elog(ERROR, "cache lookup failed for function %u", funcexpr->funcid)= ; ! get_func_arg_info(tuple, &argtypes, &argnames, &argmodes); ! ReleaseSysCache(tuple); = /* ! * Construct row */ oldcontext =3D MemoryContextSwitchTo(estate->func->fn_cxt); = ! row =3D palloc0(sizeof(*row)); row->dtype =3D PLPGSQL_DTYPE_ROW; row->refname =3D "(unnamed row)"; row->lineno =3D -1; ! row->varnos =3D palloc(sizeof(int) * FUNC_MAX_ARGS); = nfields =3D 0; i =3D 0; ! foreach(lc, funcexpr->args) { Node *n =3D lfirst(lc); = ! if (argmodes && argmodes[i] =3D=3D PROARGMODE_INOUT) { if (IsA(n, Param)) { ! Param *param =3D castNode(Param, n); = /* paramid is offset by 1 (see make_datum_param()) */ row->varnos[nfields++] =3D param->paramid - 1; } - else if (IsA(n, NamedArgExpr)) - { - NamedArgExpr *nexpr =3D castNode(NamedArgExpr, n); - Param *param; - = - if (!IsA(nexpr->arg, Param)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("argument %d is an output argument but is not writable"= , i + 1))); - = - param =3D castNode(Param, nexpr->arg); - = - /* - * Named arguments must be after positional arguments, - * so we can increase nfields. - */ - row->varnos[nexpr->argnumber] =3D param->paramid - 1; - nfields++; - } else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), ! errmsg("argument %d is an output argument but is not writable",= i + 1))); } i++; } = row->nfields =3D nfields; = - MemoryContextSwitchTo(oldcontext); - = stmt->target =3D (PLpgSQL_variable *) row; } = --- 2159,2268 ---- plpgsql_create_econtext(estate); } = + /* + * Check result rowcount; if there's one row, assign procedure's output + * values back to the appropriate variables. + */ if (SPI_processed =3D=3D 1) { SPITupleTable *tuptab =3D SPI_tuptable; = /* ! * If first time through, construct a DTYPE_ROW datum representing the ! * plpgsql variables associated with the procedure's output arguments. ! * Then we can use exec_move_row() to do the assignments. */ if (!stmt->target) { Node *node; FuncExpr *funcexpr; ! HeapTuple func_tuple; ! List *funcargs; Oid *argtypes; char **argnames; char *argmodes; MemoryContext oldcontext; PLpgSQL_row *row; int nfields; + ListCell *lc; + int i; = /* ! * Get the parsed CallStmt, and look up the called procedure */ ! node =3D linitial_node(Query, ! ((CachedPlanSource *) linitial(plan->plancache_list))->query_li= st)->utilityStmt; ! if (node =3D=3D NULL || !IsA(node, CallStmt)) ! elog(ERROR, "query for CALL statement is not a CallStmt"); = ! funcexpr =3D ((CallStmt *) node)->funcexpr; ! = ! func_tuple =3D SearchSysCache1(PROCOID, ! ObjectIdGetDatum(funcexpr->funcid)); ! if (!HeapTupleIsValid(func_tuple)) ! elog(ERROR, "cache lookup failed for function %u", ! funcexpr->funcid); = /* ! * Extract function arguments, and expand any named-arg notation */ ! funcargs =3D expand_function_arguments(funcexpr->args, ! funcexpr->funcresulttype, ! func_tuple); = /* ! * Get the argument modes, too ! */ ! get_func_arg_info(func_tuple, &argtypes, &argnames, &argmodes); ! = ! ReleaseSysCache(func_tuple); ! = ! /* ! * Begin constructing row Datum */ oldcontext =3D MemoryContextSwitchTo(estate->func->fn_cxt); = ! row =3D (PLpgSQL_row *) palloc0(sizeof(PLpgSQL_row)); row->dtype =3D PLPGSQL_DTYPE_ROW; row->refname =3D "(unnamed row)"; row->lineno =3D -1; ! row->varnos =3D (int *) palloc(sizeof(int) * list_length(funcargs)); = + MemoryContextSwitchTo(oldcontext); + = + /* + * Examine procedure's argument list. Each output arg position + * should be an unadorned plpgsql variable (datum), which we can + * insert into the row Datum. + */ nfields =3D 0; i =3D 0; ! foreach(lc, funcargs) { Node *n =3D lfirst(lc); = ! if (argmodes && ! (argmodes[i] =3D=3D PROARGMODE_INOUT || ! argmodes[i] =3D=3D PROARGMODE_OUT)) { if (IsA(n, Param)) { ! Param *param =3D (Param *) n; = /* paramid is offset by 1 (see make_datum_param()) */ row->varnos[nfields++] =3D param->paramid - 1; } else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), ! errmsg("CALL argument %d is an output argument but is not writa= ble", ! i + 1))); } i++; } = row->nfields =3D nfields; = stmt->target =3D (PLpgSQL_variable *) row; } = ------- =_aaaaaaaaaa0--