On Fri, Feb 02, 2018 at 04:07:28PM +0900, Michael Paquier wrote: > You need to read that as "only a SubPlan can be executed after a SubLink > has been processed by the planner", so please replace the last "latter" > by "planner".
(I forgot to add Peter and Andrew in CC: previously, so done now.) e4128ee7 is making is clear that SubLink are authorized when transforming it in transformSubLink(), however I cannot think about a use case so should we just forbid them, and this is actually untested. So the patch attached does so. The second problem involves a cache lookup failure for a type when trying to use pg_get_functiondef on a procedure. Luckily, it is possible to make the difference between a procedure and a function by checking if prorettype is InvalidOid or not. There is room for a new patch which supports pg_get_proceduredef() to generate the definition of a procedure, with perhaps a dedicated psql shortcut, but that could always be done later on. -- Michael
From a33e015fa06c118c7430506ca2c42c1146deb439 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Fri, 9 Feb 2018 15:49:37 +0900 Subject: [PATCH] Fix minor issues with procedure calls Procedures invoked with subqueries as arguments fail to treat them correctly as those are generated as SubLink nodes which need to be processed through the planner first to be correctly executed. The CALL infrastructure lacks the logic, and actually it may not make much sense to support such cases as any application can use a proper SELECT query to do the same, so block them. A second problem is related to the use of pg_get_functiondef which complains about a cache lookup failure when called on a procedure. It is possible to make the difference between a procedure and a function by looking at prorettype, so block properly the case where this function is called on a procedure. There is room for support of a system function which generates definitions for procedures, and which could share much with pg_get_functiondef, but this is left as future work. Bug report by Pavel Stehule, patch by Michael Paquier. Discussion: https://postgr.es/m/CAFj8pRDxOwPPzpA8i+AQeDQFj7bhVw-dR2==rfwz3zmgkm5...@mail.gmail.com --- src/backend/parser/parse_expr.c | 4 +++- src/backend/utils/adt/ruleutils.c | 5 +++++ src/test/regress/expected/create_procedure.out | 8 ++++++++ src/test/regress/sql/create_procedure.sql | 8 ++++++++ 4 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index d45926f27f..031f1b72fb 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -1818,7 +1818,6 @@ transformSubLink(ParseState *pstate, SubLink *sublink) case EXPR_KIND_RETURNING: case EXPR_KIND_VALUES: case EXPR_KIND_VALUES_SINGLE: - case EXPR_KIND_CALL: /* okay */ break; case EXPR_KIND_CHECK_CONSTRAINT: @@ -1847,6 +1846,9 @@ transformSubLink(ParseState *pstate, SubLink *sublink) case EXPR_KIND_PARTITION_EXPRESSION: err = _("cannot use subquery in partition key expression"); break; + case EXPR_KIND_CALL: + err = _("cannot use subquery in CALL arguments"); + break; /* * There is intentionally no default: case here, so that the diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 28767a129a..dc3d3c7752 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -2472,6 +2472,11 @@ pg_get_functiondef(PG_FUNCTION_ARGS) (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("\"%s\" is an aggregate function", name))); + if (!OidIsValid(proc->prorettype)) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is a procedure", name))); + /* * We always qualify the function name, to ensure the right function gets * replaced. diff --git a/src/test/regress/expected/create_procedure.out b/src/test/regress/expected/create_procedure.out index ccad5c87d5..41e0921b33 100644 --- a/src/test/regress/expected/create_procedure.out +++ b/src/test/regress/expected/create_procedure.out @@ -94,6 +94,14 @@ ALTER ROUTINE testfunc1a RENAME TO testfunc1; ALTER ROUTINE ptest1(text) RENAME TO ptest1a; ALTER ROUTINE ptest1a RENAME TO ptest1; DROP ROUTINE testfunc1(int); +-- subqueries +CALL ptest2((SELECT 5)); +ERROR: cannot use subquery in CALL arguments +LINE 1: CALL ptest2((SELECT 5)); + ^ +-- Function definition +SELECT pg_get_functiondef('ptest2'::regproc); +ERROR: "ptest2" is a procedure -- cleanup DROP PROCEDURE ptest1; DROP PROCEDURE ptest2; diff --git a/src/test/regress/sql/create_procedure.sql b/src/test/regress/sql/create_procedure.sql index 8c47b7e9ef..a140fef928 100644 --- a/src/test/regress/sql/create_procedure.sql +++ b/src/test/regress/sql/create_procedure.sql @@ -72,6 +72,14 @@ ALTER ROUTINE ptest1a RENAME TO ptest1; DROP ROUTINE testfunc1(int); +-- subqueries +CALL ptest2((SELECT 5)); + + +-- Function definition +SELECT pg_get_functiondef('ptest2'::regproc); + + -- cleanup DROP PROCEDURE ptest1; -- 2.16.1
signature.asc
Description: PGP signature
