CALL stmt, ERROR: unrecognized node type: 113 bug
Hi
I am playing with procedures little bit
I found few bugs
create procedure test(a int)
as $$
begin
raise notice '>>>%<<<', a;
end;
$$ language plpgsql;
call test(10); -- ok
postgres=# call test((select 10));
ERROR: unrecognized node type: 113
postgres=# \sf test
ERROR: cache lookup failed for type 0
Regards
Pavel
On Thu, Feb 01, 2018 at 05:33:54PM +0100, Pavel Stehule wrote:
I am playing with procedures little bit
I found few bugs
create procedure test(a int)
as $$
begin
raise notice '>>>%<<<', a;
end;
$$ language plpgsql;call test(10); -- ok
postgres=# call test((select 10));
ERROR: unrecognized node type: 113postgres=# \sf test
ERROR: cache lookup failed for type 0
Peter, Andrew, this is missing some bits related to the conversion of
SubLink nodes to SubPlan nodes for procedures when used as argument of
a procedure as only the latter can be executed after the former is
processed by the latter (see SS_process_sublinks).
--
Michael
On Fri, Feb 02, 2018 at 04:01:13PM +0900, Michael Paquier wrote:
Peter, Andrew, this is missing some bits related to the conversion of
SubLink nodes to SubPlan nodes for procedures when used as argument of
a procedure as only the latter can be executed after the former is
processed by the latter (see SS_process_sublinks).
Meh-to-self.
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".
--
Michael
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
Attachments:
0001-Fix-minor-issues-with-procedure-calls.patchtext/x-diff; charset=us-asciiDownload
From a33e015fa06c118c7430506ca2c42c1146deb439 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@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==rfWZ3zMGkm568Q@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
Hi
2018-02-09 7:56 GMT+01:00 Michael Paquier <michael@paquier.xyz>:
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.
Blocking subqueries in CALL parameters is possible solution. But blocking
func def for procedures without any substitution doesn't look correct for
me.
Regards
Pavel
Show quoted text
--
Michael
On Fri, Feb 09, 2018 at 12:02:57PM +0100, Pavel Stehule wrote:
2018-02-09 7:56 GMT+01:00 Michael Paquier <michael@paquier.xyz>:
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.Blocking subqueries in CALL parameters is possible solution.
ExecuteCallStmt has visibly been written so as it is able to handle the
input set of arguments with a minimal infrastructure in place. SubLink
nodes require more advanced handling as those need to go through the
planner. There is also additional processing in the rewriter. At the
end I tend to think that any user would just turn their back on calling
a function for such cases anyway, so it seems to me that the potential
benefits are not worth the code complexity.
But blocking
func def for procedures without any substitution doesn't look correct for
me.
I don't disagree with you here, there is room for such a function, but
on the other side not having it does not make the existing feature less
useful. As it is Peter's and Andrew's feature, the last word should
come from them. Here is my opinion for what it's worth:
- Procedures are not functions, the code is pretty clear about that, so
a system function to generate the definition of a procedure should not
happen with pg_get_functiondef(). They share a lot of infrastructure so
you can reuse a lot of the code present.
- A different psql shortcut should be used, and I would assume that \sp
is adapted.
- Aggregates are also in pg_proc, we generate an error on them because
they are of different type, so an error for procedures when trying to
fetch a functoin definition looks like the good answer.
If folks feel that having a way to retrieve the procedure definition
easily via ruleutils.c is a must-have, then we have material for a good
open item :)
--
Michael
On Fri, Feb 9, 2018 at 6:23 AM, Michael Paquier <michael@paquier.xyz> wrote:
On Fri, Feb 09, 2018 at 12:02:57PM +0100, Pavel Stehule wrote:
2018-02-09 7:56 GMT+01:00 Michael Paquier <michael@paquier.xyz>:
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 definitionof
a procedure, with perhaps a dedicated psql shortcut, but that could
always be done later on.Blocking subqueries in CALL parameters is possible solution.
ExecuteCallStmt has visibly been written so as it is able to handle the
input set of arguments with a minimal infrastructure in place. SubLink
nodes require more advanced handling as those need to go through the
planner. There is also additional processing in the rewriter. At the
end I tend to think that any user would just turn their back on calling
a function for such cases anyway, so it seems to me that the potential
benefits are not worth the code complexity.
CALL is not just a different syntax for function invocation - if you want
the properties of CALL then falling back to SELECT function() is not a
valid alternative.
To me this feels like an interaction between two features that users are
going to expect to just work. Current discussions lead me to think that is
something we strive to provide unless a strong argument against is
provided. I'm not sure added code complexity here is going to make the
grade even if I cannot reasonably judge just how much complexity is
involved.
David J.
2018-02-09 15:15 GMT+01:00 David G. Johnston <david.g.johnston@gmail.com>:
On Fri, Feb 9, 2018 at 6:23 AM, Michael Paquier <michael@paquier.xyz>
wrote:On Fri, Feb 09, 2018 at 12:02:57PM +0100, Pavel Stehule wrote:
2018-02-09 7:56 GMT+01:00 Michael Paquier <michael@paquier.xyz>:
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 definitionof
a procedure, with perhaps a dedicated psql shortcut, but that could
always be done later on.Blocking subqueries in CALL parameters is possible solution.
ExecuteCallStmt has visibly been written so as it is able to handle the
input set of arguments with a minimal infrastructure in place. SubLink
nodes require more advanced handling as those need to go through the
planner. There is also additional processing in the rewriter. At the
end I tend to think that any user would just turn their back on calling
a function for such cases anyway, so it seems to me that the potential
benefits are not worth the code complexity.CALL is not just a different syntax for function invocation - if you want
the properties of CALL then falling back to SELECT function() is not a
valid alternative.
+1
To me this feels like an interaction between two features that users are
going to expect to just work. Current discussions lead me to think that is
something we strive to provide unless a strong argument against is
provided. I'm not sure added code complexity here is going to make the
grade even if I cannot reasonably judge just how much complexity is
involved.
when some procedure can do transaction control, or can returns possible set
or multirecord set (in future), then 100% agree, so it is different
creature then function. But if not, then it should be specified why it is
different from void function.
I don't understand, why we should to prohibit subqueries as procedure
params - with some limits. I can understand to requirement to not change
any data.
Regards
Pavel
Show quoted text
David J.
"David G. Johnston" <david.g.johnston@gmail.com> writes:
On Fri, Feb 9, 2018 at 6:23 AM, Michael Paquier <michael@paquier.xyz> wrote:
On Fri, Feb 09, 2018 at 12:02:57PM +0100, Pavel Stehule wrote:
Blocking subqueries in CALL parameters is possible solution.
To me this feels like an interaction between two features that users are
going to expect to just work.
Meh. It doesn't look significantly different to me than the restriction
that you can't have sub-selects in CHECK expressions, index expressions,
etc. Obviously we need a clean failure like you get for those cases.
But otherwise it's an OK restriction that stems from exactly the same
cause: we do not want to invoke the full planner in this context (and
even if we did, we don't want to use the full executor to execute the
result).
regards, tom lane
On Fri, Feb 9, 2018 at 7:42 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
"David G. Johnston" <david.g.johnston@gmail.com> writes:
On Fri, Feb 9, 2018 at 6:23 AM, Michael Paquier <michael@paquier.xyz>
wrote:
On Fri, Feb 09, 2018 at 12:02:57PM +0100, Pavel Stehule wrote:
Blocking subqueries in CALL parameters is possible solution.
To me this feels like an interaction between two features that users are
going to expect to just work.Meh. It doesn't look significantly different to me than the restriction
that you can't have sub-selects in CHECK expressions, index expressions,
etc. Obviously we need a clean failure like you get for those cases.
But otherwise it's an OK restriction that stems from exactly the same
cause: we do not want to invoke the full planner in this context (and
even if we did, we don't want to use the full executor to execute the
result).
Does/Should:
CALL test(func(10)); --with or without an extra set of parentheses
work here too?
David J.
On 2018-02-09 09:42:41 -0500, Tom Lane wrote:
It doesn't look significantly different to me than the restriction
that you can't have sub-selects in CHECK expressions, index
expressions, etc. Obviously we need a clean failure like you get for
those cases. But otherwise it's an OK restriction that stems from
exactly the same cause: we do not want to invoke the full planner in
this context (and even if we did, we don't want to use the full
executor to execute the result).
+1
On Fri, Feb 09, 2018 at 08:30:49AM -0800, Andres Freund wrote:
On 2018-02-09 09:42:41 -0500, Tom Lane wrote:
It doesn't look significantly different to me than the restriction
that you can't have sub-selects in CHECK expressions, index
expressions, etc. Obviously we need a clean failure like you get for
those cases. But otherwise it's an OK restriction that stems from
exactly the same cause: we do not want to invoke the full planner in
this context (and even if we did, we don't want to use the full
executor to execute the result).+1
+1.
--
MichaelMichael Paquier <michael@paquier.xyz> writes:
On Fri, Feb 09, 2018 at 08:30:49AM -0800, Andres Freund wrote:
On 2018-02-09 09:42:41 -0500, Tom Lane wrote:
... But otherwise it's an OK restriction that stems from
exactly the same cause: we do not want to invoke the full planner in
this context (and even if we did, we don't want to use the full
executor to execute the result).
So I idly looked at ExecuteCallStmt to see how, in fact, it is executing
these things, and I was right to guess that it couldn't possibly support
a sub-select, since it's just using ExecEvalExprSwitchContext. So we
need to go teach transformSubLink that EXPR_KIND_CALL is *not* okay,
which is simple enough.
However, I also wondered how ExecuteCallStmt works at all for pass-by-
reference datatypes, since it immediately destroys the execution context
for each expression. And the answer is that it doesn't, as proven here:
regression=# create procedure myp(f1 text) as $$begin
regression$# raise notice 'f1 = %', f1;
regression$# end$$ language plpgsql;
CREATE PROCEDURE
regression=# call myp('xyzzy');
NOTICE: f1 = xyzzy
CALL
regression=# call myp('xyzzy' || 'x');
NOTICE: f1 =
CALL
The call with a literal constant accidentally seems to work, because the
returned Datum is actually pointing into the original expression tree.
But if you have the expression do any actual work, then not so much.
I think this could be fixed by evaluating all the arguments in a single
execution context that is not destroyed till after the call finishes
(using a separate one for each argument seems pretty silly anyway).
However, the code could do with more than zero commentary about how come
this is safe at all --- even if we keep the execution context, it's still
a child of whatever random memory context ExecuteCallStmt was called in,
and it's not very clear that that context will survive a transaction
commit.
This does not leave me with a warm feeling about either the amount
of testing the procedure feature has gotten, or the state of its
internal documentation.
regards, tom lane
I wrote:
However, I also wondered how ExecuteCallStmt works at all for pass-by-
reference datatypes, since it immediately destroys the execution context
for each expression. And the answer is that it doesn't, as proven here:
On closer inspection, there are actually three sub-cases involved.
It accidentally works for simple constant arguments, because the
passed Datum will point at a Const node generated during transformExpr.
And it works for fully run-time-evaluated arguments, because those end
up in memory belonging to the standalone ExprContext(s), which
ExecuteCallStmt never bothers to free at all. (Which is a bug in itself,
although possibly one that wouldn't be exposed in practice given that
we disallow SRFs here; I don't know if there are any other cases that
would expect ExprContext cleanup hooks to get invoked.) Where it doesn't
work is for expressions that are const-folded during ExecPrepareExpr,
because then the values are in Const nodes that live in the EState's
per-query context, and the code is throwing that away too soon.
I pushed a fix for all that.
The failure in pg_get_functiondef() is still there. While the immediate
answer probably is to teach that function to emit correct CREATE PROCEDURE
syntax, I continue to think that it's a bad idea to be putting zeroes into
pg_proc.prorettype.
regards, tom lane
On Sat, Feb 10, 2018 at 01:46:40PM -0500, Tom Lane wrote:
I pushed a fix for all that.
Shouldn't there be a test case as well? The patch I sent upthread was
doing the whole set, except that I did not bother
The failure in pg_get_functiondef() is still there. While the immediate
answer probably is to teach that function to emit correct CREATE PROCEDURE
syntax, I continue to think that it's a bad idea to be putting zeroes into
pg_proc.prorettype.
Yeah, or an error with a new function dedicated to procedures. I also
finc confusing the use of prorettype to track this object type.
This brings the amount of objects stored in pg_proc to four. Perhaps it
would be time to bring more clarity in pg_proc by introducing a prokind
column for functions, aggregates, window functions and procedures? I
don't feel really hot for an extra boolean column like proisproc.
--
Michael
On Sun, Feb 11, 2018 at 08:17:55AM +0900, Michael Paquier wrote:
On Sat, Feb 10, 2018 at 01:46:40PM -0500, Tom Lane wrote:
I pushed a fix for all that.
Shouldn't there be a test case as well? The patch I sent upthread was
doing the whole set, except that I did not bother
... Rename EXPR_KIND_CALL to something else.
This was missing the last part of the sentence.
--
Michael
Michael Paquier <michael@paquier.xyz> writes:
On Sat, Feb 10, 2018 at 01:46:40PM -0500, Tom Lane wrote:
I pushed a fix for all that.
Shouldn't there be a test case as well?
There was one for the premature-free issue in d02d4a6d4. I didn't really
see a need for an explicit test for the subselect issue.
This brings the amount of objects stored in pg_proc to four. Perhaps it
would be time to bring more clarity in pg_proc by introducing a prokind
column for functions, aggregates, window functions and procedures?
Yeah. I was under the impression that Peter was looking into that ...
[ digs... ] see
/messages/by-id/80ee1f5c-fa9d-7285-ed07-cff53d4f4858@2ndquadrant.com
regards, tom lane
On 2/9/18 09:42, Tom Lane wrote:
Meh. It doesn't look significantly different to me than the restriction
that you can't have sub-selects in CHECK expressions, index expressions,
etc. Obviously we need a clean failure like you get for those cases.
But otherwise it's an OK restriction that stems from exactly the same
cause: we do not want to invoke the full planner in this context (and
even if we did, we don't want to use the full executor to execute the
result).
A close analogy is that EXECUTE parameters also don't accept subqueries.
It would perhaps be nice if that could be made to work, but as
discussed it would require a bunch more work.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
2018-02-12 18:17 GMT+01:00 Peter Eisentraut <
peter.eisentraut@2ndquadrant.com>:
On 2/9/18 09:42, Tom Lane wrote:
Meh. It doesn't look significantly different to me than the restriction
that you can't have sub-selects in CHECK expressions, index expressions,
etc. Obviously we need a clean failure like you get for those cases.
But otherwise it's an OK restriction that stems from exactly the same
cause: we do not want to invoke the full planner in this context (and
even if we did, we don't want to use the full executor to execute the
result).A close analogy is that EXECUTE parameters also don't accept subqueries.
It would perhaps be nice if that could be made to work, but as
discussed it would require a bunch more work.
I can live with it. Should be well documented and explained.
Regards
Pavel
Show quoted text
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2/1/18 11:33, Pavel Stehule wrote:
postgres=# \sf test
ERROR: cache lookup failed for type 0
Here is a patch set that adds procedure support to \ef and \sf.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Fix-typo.patchtext/plain; charset=UTF-8; name=0001-Fix-typo.patch; x-mac-creator=0; x-mac-type=0Download
From ffa37952aac6562f78fbed6f3d73a02913b42b89 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 12 Feb 2018 13:47:18 -0500
Subject: [PATCH 1/3] Fix typo
---
src/test/regress/expected/create_function_3.out | 144 ++++++++++++------------
src/test/regress/sql/create_function_3.sql | 88 +++++++--------
2 files changed, 116 insertions(+), 116 deletions(-)
diff --git a/src/test/regress/expected/create_function_3.out b/src/test/regress/expected/create_function_3.out
index b5e19485e5..2fd25b8593 100644
--- a/src/test/regress/expected/create_function_3.out
+++ b/src/test/regress/expected/create_function_3.out
@@ -69,124 +69,124 @@ SELECT proname, provolatile FROM pg_proc
--
-- SECURITY DEFINER | INVOKER
--
-CREATE FUNCTION functext_C_1(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_C_1(int) RETURNS bool LANGUAGE 'sql'
AS 'SELECT $1 > 0';
-CREATE FUNCTION functext_C_2(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_C_2(int) RETURNS bool LANGUAGE 'sql'
SECURITY DEFINER AS 'SELECT $1 = 0';
-CREATE FUNCTION functext_C_3(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_C_3(int) RETURNS bool LANGUAGE 'sql'
SECURITY INVOKER AS 'SELECT $1 < 0';
SELECT proname, prosecdef FROM pg_proc
- WHERE oid in ('functext_C_1'::regproc,
- 'functext_C_2'::regproc,
- 'functext_C_3'::regproc) ORDER BY proname;
+ WHERE oid in ('functest_C_1'::regproc,
+ 'functest_C_2'::regproc,
+ 'functest_C_3'::regproc) ORDER BY proname;
proname | prosecdef
--------------+-----------
- functext_c_1 | f
- functext_c_2 | t
- functext_c_3 | f
+ functest_c_1 | f
+ functest_c_2 | t
+ functest_c_3 | f
(3 rows)
-ALTER FUNCTION functext_C_1(int) IMMUTABLE; -- unrelated change, no effect
-ALTER FUNCTION functext_C_2(int) SECURITY INVOKER;
-ALTER FUNCTION functext_C_3(int) SECURITY DEFINER;
+ALTER FUNCTION functest_C_1(int) IMMUTABLE; -- unrelated change, no effect
+ALTER FUNCTION functest_C_2(int) SECURITY INVOKER;
+ALTER FUNCTION functest_C_3(int) SECURITY DEFINER;
SELECT proname, prosecdef FROM pg_proc
- WHERE oid in ('functext_C_1'::regproc,
- 'functext_C_2'::regproc,
- 'functext_C_3'::regproc) ORDER BY proname;
+ WHERE oid in ('functest_C_1'::regproc,
+ 'functest_C_2'::regproc,
+ 'functest_C_3'::regproc) ORDER BY proname;
proname | prosecdef
--------------+-----------
- functext_c_1 | f
- functext_c_2 | f
- functext_c_3 | t
+ functest_c_1 | f
+ functest_c_2 | f
+ functest_c_3 | t
(3 rows)
--
-- LEAKPROOF
--
-CREATE FUNCTION functext_E_1(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_E_1(int) RETURNS bool LANGUAGE 'sql'
AS 'SELECT $1 > 100';
-CREATE FUNCTION functext_E_2(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_E_2(int) RETURNS bool LANGUAGE 'sql'
LEAKPROOF AS 'SELECT $1 > 100';
SELECT proname, proleakproof FROM pg_proc
- WHERE oid in ('functext_E_1'::regproc,
- 'functext_E_2'::regproc) ORDER BY proname;
+ WHERE oid in ('functest_E_1'::regproc,
+ 'functest_E_2'::regproc) ORDER BY proname;
proname | proleakproof
--------------+--------------
- functext_e_1 | f
- functext_e_2 | t
+ functest_e_1 | f
+ functest_e_2 | t
(2 rows)
-ALTER FUNCTION functext_E_1(int) LEAKPROOF;
-ALTER FUNCTION functext_E_2(int) STABLE; -- unrelated change, no effect
+ALTER FUNCTION functest_E_1(int) LEAKPROOF;
+ALTER FUNCTION functest_E_2(int) STABLE; -- unrelated change, no effect
SELECT proname, proleakproof FROM pg_proc
- WHERE oid in ('functext_E_1'::regproc,
- 'functext_E_2'::regproc) ORDER BY proname;
+ WHERE oid in ('functest_E_1'::regproc,
+ 'functest_E_2'::regproc) ORDER BY proname;
proname | proleakproof
--------------+--------------
- functext_e_1 | t
- functext_e_2 | t
+ functest_e_1 | t
+ functest_e_2 | t
(2 rows)
-ALTER FUNCTION functext_E_2(int) NOT LEAKPROOF; -- remove leakproog attribute
+ALTER FUNCTION functest_E_2(int) NOT LEAKPROOF; -- remove leakproog attribute
SELECT proname, proleakproof FROM pg_proc
- WHERE oid in ('functext_E_1'::regproc,
- 'functext_E_2'::regproc) ORDER BY proname;
+ WHERE oid in ('functest_E_1'::regproc,
+ 'functest_E_2'::regproc) ORDER BY proname;
proname | proleakproof
--------------+--------------
- functext_e_1 | t
- functext_e_2 | f
+ functest_e_1 | t
+ functest_e_2 | f
(2 rows)
-- it takes superuser privilege to turn on leakproof, but not for turn off
-ALTER FUNCTION functext_E_1(int) OWNER TO regress_unpriv_user;
-ALTER FUNCTION functext_E_2(int) OWNER TO regress_unpriv_user;
+ALTER FUNCTION functest_E_1(int) OWNER TO regress_unpriv_user;
+ALTER FUNCTION functest_E_2(int) OWNER TO regress_unpriv_user;
SET SESSION AUTHORIZATION regress_unpriv_user;
SET search_path TO temp_func_test, public;
-ALTER FUNCTION functext_E_1(int) NOT LEAKPROOF;
-ALTER FUNCTION functext_E_2(int) LEAKPROOF;
+ALTER FUNCTION functest_E_1(int) NOT LEAKPROOF;
+ALTER FUNCTION functest_E_2(int) LEAKPROOF;
ERROR: only superuser can define a leakproof function
-CREATE FUNCTION functext_E_3(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_E_3(int) RETURNS bool LANGUAGE 'sql'
LEAKPROOF AS 'SELECT $1 < 200'; -- failed
ERROR: only superuser can define a leakproof function
RESET SESSION AUTHORIZATION;
--
-- CALLED ON NULL INPUT | RETURNS NULL ON NULL INPUT | STRICT
--
-CREATE FUNCTION functext_F_1(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_F_1(int) RETURNS bool LANGUAGE 'sql'
AS 'SELECT $1 > 50';
-CREATE FUNCTION functext_F_2(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_F_2(int) RETURNS bool LANGUAGE 'sql'
CALLED ON NULL INPUT AS 'SELECT $1 = 50';
-CREATE FUNCTION functext_F_3(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_F_3(int) RETURNS bool LANGUAGE 'sql'
RETURNS NULL ON NULL INPUT AS 'SELECT $1 < 50';
-CREATE FUNCTION functext_F_4(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_F_4(int) RETURNS bool LANGUAGE 'sql'
STRICT AS 'SELECT $1 = 50';
SELECT proname, proisstrict FROM pg_proc
- WHERE oid in ('functext_F_1'::regproc,
- 'functext_F_2'::regproc,
- 'functext_F_3'::regproc,
- 'functext_F_4'::regproc) ORDER BY proname;
+ WHERE oid in ('functest_F_1'::regproc,
+ 'functest_F_2'::regproc,
+ 'functest_F_3'::regproc,
+ 'functest_F_4'::regproc) ORDER BY proname;
proname | proisstrict
--------------+-------------
- functext_f_1 | f
- functext_f_2 | f
- functext_f_3 | t
- functext_f_4 | t
+ functest_f_1 | f
+ functest_f_2 | f
+ functest_f_3 | t
+ functest_f_4 | t
(4 rows)
-ALTER FUNCTION functext_F_1(int) IMMUTABLE; -- unrelated change, no effect
-ALTER FUNCTION functext_F_2(int) STRICT;
-ALTER FUNCTION functext_F_3(int) CALLED ON NULL INPUT;
+ALTER FUNCTION functest_F_1(int) IMMUTABLE; -- unrelated change, no effect
+ALTER FUNCTION functest_F_2(int) STRICT;
+ALTER FUNCTION functest_F_3(int) CALLED ON NULL INPUT;
SELECT proname, proisstrict FROM pg_proc
- WHERE oid in ('functext_F_1'::regproc,
- 'functext_F_2'::regproc,
- 'functext_F_3'::regproc,
- 'functext_F_4'::regproc) ORDER BY proname;
+ WHERE oid in ('functest_F_1'::regproc,
+ 'functest_F_2'::regproc,
+ 'functest_F_3'::regproc,
+ 'functest_F_4'::regproc) ORDER BY proname;
proname | proisstrict
--------------+-------------
- functext_f_1 | f
- functext_f_2 | t
- functext_f_3 | f
- functext_f_4 | t
+ functest_f_1 | f
+ functest_f_2 | t
+ functest_f_3 | f
+ functest_f_4 | t
(4 rows)
-- information_schema tests
@@ -236,15 +236,15 @@ drop cascades to function functest_a_3()
drop cascades to function functest_b_2(integer)
drop cascades to function functest_b_3(integer)
drop cascades to function functest_b_4(integer)
-drop cascades to function functext_c_1(integer)
-drop cascades to function functext_c_2(integer)
-drop cascades to function functext_c_3(integer)
-drop cascades to function functext_e_1(integer)
-drop cascades to function functext_e_2(integer)
-drop cascades to function functext_f_1(integer)
-drop cascades to function functext_f_2(integer)
-drop cascades to function functext_f_3(integer)
-drop cascades to function functext_f_4(integer)
+drop cascades to function functest_c_1(integer)
+drop cascades to function functest_c_2(integer)
+drop cascades to function functest_c_3(integer)
+drop cascades to function functest_e_1(integer)
+drop cascades to function functest_e_2(integer)
+drop cascades to function functest_f_1(integer)
+drop cascades to function functest_f_2(integer)
+drop cascades to function functest_f_3(integer)
+drop cascades to function functest_f_4(integer)
drop cascades to function functest_b_2(bigint)
DROP USER regress_unpriv_user;
RESET search_path;
diff --git a/src/test/regress/sql/create_function_3.sql b/src/test/regress/sql/create_function_3.sql
index 0a0e407aab..6c411bdfda 100644
--- a/src/test/regress/sql/create_function_3.sql
+++ b/src/test/regress/sql/create_function_3.sql
@@ -52,57 +52,57 @@ CREATE FUNCTION functest_B_4(int) RETURNS bool LANGUAGE 'sql'
--
-- SECURITY DEFINER | INVOKER
--
-CREATE FUNCTION functext_C_1(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_C_1(int) RETURNS bool LANGUAGE 'sql'
AS 'SELECT $1 > 0';
-CREATE FUNCTION functext_C_2(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_C_2(int) RETURNS bool LANGUAGE 'sql'
SECURITY DEFINER AS 'SELECT $1 = 0';
-CREATE FUNCTION functext_C_3(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_C_3(int) RETURNS bool LANGUAGE 'sql'
SECURITY INVOKER AS 'SELECT $1 < 0';
SELECT proname, prosecdef FROM pg_proc
- WHERE oid in ('functext_C_1'::regproc,
- 'functext_C_2'::regproc,
- 'functext_C_3'::regproc) ORDER BY proname;
+ WHERE oid in ('functest_C_1'::regproc,
+ 'functest_C_2'::regproc,
+ 'functest_C_3'::regproc) ORDER BY proname;
-ALTER FUNCTION functext_C_1(int) IMMUTABLE; -- unrelated change, no effect
-ALTER FUNCTION functext_C_2(int) SECURITY INVOKER;
-ALTER FUNCTION functext_C_3(int) SECURITY DEFINER;
+ALTER FUNCTION functest_C_1(int) IMMUTABLE; -- unrelated change, no effect
+ALTER FUNCTION functest_C_2(int) SECURITY INVOKER;
+ALTER FUNCTION functest_C_3(int) SECURITY DEFINER;
SELECT proname, prosecdef FROM pg_proc
- WHERE oid in ('functext_C_1'::regproc,
- 'functext_C_2'::regproc,
- 'functext_C_3'::regproc) ORDER BY proname;
+ WHERE oid in ('functest_C_1'::regproc,
+ 'functest_C_2'::regproc,
+ 'functest_C_3'::regproc) ORDER BY proname;
--
-- LEAKPROOF
--
-CREATE FUNCTION functext_E_1(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_E_1(int) RETURNS bool LANGUAGE 'sql'
AS 'SELECT $1 > 100';
-CREATE FUNCTION functext_E_2(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_E_2(int) RETURNS bool LANGUAGE 'sql'
LEAKPROOF AS 'SELECT $1 > 100';
SELECT proname, proleakproof FROM pg_proc
- WHERE oid in ('functext_E_1'::regproc,
- 'functext_E_2'::regproc) ORDER BY proname;
+ WHERE oid in ('functest_E_1'::regproc,
+ 'functest_E_2'::regproc) ORDER BY proname;
-ALTER FUNCTION functext_E_1(int) LEAKPROOF;
-ALTER FUNCTION functext_E_2(int) STABLE; -- unrelated change, no effect
+ALTER FUNCTION functest_E_1(int) LEAKPROOF;
+ALTER FUNCTION functest_E_2(int) STABLE; -- unrelated change, no effect
SELECT proname, proleakproof FROM pg_proc
- WHERE oid in ('functext_E_1'::regproc,
- 'functext_E_2'::regproc) ORDER BY proname;
+ WHERE oid in ('functest_E_1'::regproc,
+ 'functest_E_2'::regproc) ORDER BY proname;
-ALTER FUNCTION functext_E_2(int) NOT LEAKPROOF; -- remove leakproog attribute
+ALTER FUNCTION functest_E_2(int) NOT LEAKPROOF; -- remove leakproog attribute
SELECT proname, proleakproof FROM pg_proc
- WHERE oid in ('functext_E_1'::regproc,
- 'functext_E_2'::regproc) ORDER BY proname;
+ WHERE oid in ('functest_E_1'::regproc,
+ 'functest_E_2'::regproc) ORDER BY proname;
-- it takes superuser privilege to turn on leakproof, but not for turn off
-ALTER FUNCTION functext_E_1(int) OWNER TO regress_unpriv_user;
-ALTER FUNCTION functext_E_2(int) OWNER TO regress_unpriv_user;
+ALTER FUNCTION functest_E_1(int) OWNER TO regress_unpriv_user;
+ALTER FUNCTION functest_E_2(int) OWNER TO regress_unpriv_user;
SET SESSION AUTHORIZATION regress_unpriv_user;
SET search_path TO temp_func_test, public;
-ALTER FUNCTION functext_E_1(int) NOT LEAKPROOF;
-ALTER FUNCTION functext_E_2(int) LEAKPROOF;
+ALTER FUNCTION functest_E_1(int) NOT LEAKPROOF;
+ALTER FUNCTION functest_E_2(int) LEAKPROOF;
-CREATE FUNCTION functext_E_3(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_E_3(int) RETURNS bool LANGUAGE 'sql'
LEAKPROOF AS 'SELECT $1 < 200'; -- failed
RESET SESSION AUTHORIZATION;
@@ -110,28 +110,28 @@ CREATE FUNCTION functext_E_3(int) RETURNS bool LANGUAGE 'sql'
--
-- CALLED ON NULL INPUT | RETURNS NULL ON NULL INPUT | STRICT
--
-CREATE FUNCTION functext_F_1(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_F_1(int) RETURNS bool LANGUAGE 'sql'
AS 'SELECT $1 > 50';
-CREATE FUNCTION functext_F_2(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_F_2(int) RETURNS bool LANGUAGE 'sql'
CALLED ON NULL INPUT AS 'SELECT $1 = 50';
-CREATE FUNCTION functext_F_3(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_F_3(int) RETURNS bool LANGUAGE 'sql'
RETURNS NULL ON NULL INPUT AS 'SELECT $1 < 50';
-CREATE FUNCTION functext_F_4(int) RETURNS bool LANGUAGE 'sql'
+CREATE FUNCTION functest_F_4(int) RETURNS bool LANGUAGE 'sql'
STRICT AS 'SELECT $1 = 50';
SELECT proname, proisstrict FROM pg_proc
- WHERE oid in ('functext_F_1'::regproc,
- 'functext_F_2'::regproc,
- 'functext_F_3'::regproc,
- 'functext_F_4'::regproc) ORDER BY proname;
-
-ALTER FUNCTION functext_F_1(int) IMMUTABLE; -- unrelated change, no effect
-ALTER FUNCTION functext_F_2(int) STRICT;
-ALTER FUNCTION functext_F_3(int) CALLED ON NULL INPUT;
+ WHERE oid in ('functest_F_1'::regproc,
+ 'functest_F_2'::regproc,
+ 'functest_F_3'::regproc,
+ 'functest_F_4'::regproc) ORDER BY proname;
+
+ALTER FUNCTION functest_F_1(int) IMMUTABLE; -- unrelated change, no effect
+ALTER FUNCTION functest_F_2(int) STRICT;
+ALTER FUNCTION functest_F_3(int) CALLED ON NULL INPUT;
SELECT proname, proisstrict FROM pg_proc
- WHERE oid in ('functext_F_1'::regproc,
- 'functext_F_2'::regproc,
- 'functext_F_3'::regproc,
- 'functext_F_4'::regproc) ORDER BY proname;
+ WHERE oid in ('functest_F_1'::regproc,
+ 'functest_F_2'::regproc,
+ 'functest_F_3'::regproc,
+ 'functest_F_4'::regproc) ORDER BY proname;
-- information_schema tests
base-commit: 80f021ef139affdb219ccef71fff283e8f91f112
--
2.16.1
0002-Add-tests-for-pg_get_functiondef.patchtext/plain; charset=UTF-8; name=0002-Add-tests-for-pg_get_functiondef.patch; x-mac-creator=0; x-mac-type=0Download
From cd94f187228be3e66c741469681a5703413eecef Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 12 Feb 2018 14:03:04 -0500
Subject: [PATCH 2/3] Add tests for pg_get_functiondef
---
src/test/regress/expected/create_function_3.out | 44 +++++++++++++++++++++++++
src/test/regress/sql/create_function_3.sql | 8 +++++
2 files changed, 52 insertions(+)
diff --git a/src/test/regress/expected/create_function_3.out b/src/test/regress/expected/create_function_3.out
index 2fd25b8593..5ff1e0dd86 100644
--- a/src/test/regress/expected/create_function_3.out
+++ b/src/test/regress/expected/create_function_3.out
@@ -189,6 +189,50 @@ SELECT proname, proisstrict FROM pg_proc
functest_f_4 | t
(4 rows)
+-- pg_get_functiondef tests
+SELECT pg_get_functiondef('functest_A_1'::regproc);
+ pg_get_functiondef
+--------------------------------------------------------------------
+ CREATE OR REPLACE FUNCTION temp_func_test.functest_a_1(text, date)+
+ RETURNS boolean +
+ LANGUAGE sql +
+ AS $function$SELECT $1 = 'abcd' AND $2 > '2001-01-01'$function$ +
+
+(1 row)
+
+SELECT pg_get_functiondef('functest_B_3'::regproc);
+ pg_get_functiondef
+-----------------------------------------------------------------
+ CREATE OR REPLACE FUNCTION temp_func_test.functest_b_3(integer)+
+ RETURNS boolean +
+ LANGUAGE sql +
+ STABLE +
+ AS $function$SELECT $1 = 0$function$ +
+
+(1 row)
+
+SELECT pg_get_functiondef('functest_C_3'::regproc);
+ pg_get_functiondef
+-----------------------------------------------------------------
+ CREATE OR REPLACE FUNCTION temp_func_test.functest_c_3(integer)+
+ RETURNS boolean +
+ LANGUAGE sql +
+ SECURITY DEFINER +
+ AS $function$SELECT $1 < 0$function$ +
+
+(1 row)
+
+SELECT pg_get_functiondef('functest_F_2'::regproc);
+ pg_get_functiondef
+-----------------------------------------------------------------
+ CREATE OR REPLACE FUNCTION temp_func_test.functest_f_2(integer)+
+ RETURNS boolean +
+ LANGUAGE sql +
+ STRICT +
+ AS $function$SELECT $1 = 50$function$ +
+
+(1 row)
+
-- information_schema tests
CREATE FUNCTION functest_IS_1(a int, b int default 1, c text default 'foo')
RETURNS int
diff --git a/src/test/regress/sql/create_function_3.sql b/src/test/regress/sql/create_function_3.sql
index 6c411bdfda..fbdf8310e3 100644
--- a/src/test/regress/sql/create_function_3.sql
+++ b/src/test/regress/sql/create_function_3.sql
@@ -134,6 +134,14 @@ CREATE FUNCTION functest_F_4(int) RETURNS bool LANGUAGE 'sql'
'functest_F_4'::regproc) ORDER BY proname;
+-- pg_get_functiondef tests
+
+SELECT pg_get_functiondef('functest_A_1'::regproc);
+SELECT pg_get_functiondef('functest_B_3'::regproc);
+SELECT pg_get_functiondef('functest_C_3'::regproc);
+SELECT pg_get_functiondef('functest_F_2'::regproc);
+
+
-- information_schema tests
CREATE FUNCTION functest_IS_1(a int, b int default 1, c text default 'foo')
--
2.16.1
0003-Add-procedure-support-to-pg_get_functiondef.patchtext/plain; charset=UTF-8; name=0003-Add-procedure-support-to-pg_get_functiondef.patch; x-mac-creator=0; x-mac-type=0Download
From 3d615285d8cb500eb01f3f6c2f7f9084f01cc9a7 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 12 Feb 2018 14:33:49 -0500
Subject: [PATCH 3/3] Add procedure support to pg_get_functiondef
This also makes procedures work in psql's \ef and \sf commands.
Reported-by: Pavel Stehule <pavel.stehule@gmail.com>
---
doc/src/sgml/func.sgml | 8 ++++----
doc/src/sgml/ref/psql-ref.sgml | 10 ++++++----
src/backend/utils/adt/ruleutils.c | 19 +++++++++++++------
src/test/regress/expected/create_procedure.out | 11 +++++++++++
src/test/regress/sql/create_procedure.sql | 1 +
5 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 640ff09a7b..4be31b082a 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17008,22 +17008,22 @@ <title>System Catalog Information Functions</title>
<row>
<entry><literal><function>pg_get_functiondef(<parameter>func_oid</parameter>)</function></literal></entry>
<entry><type>text</type></entry>
- <entry>get definition of a function</entry>
+ <entry>get definition of a function or procedure</entry>
</row>
<row>
<entry><literal><function>pg_get_function_arguments(<parameter>func_oid</parameter>)</function></literal></entry>
<entry><type>text</type></entry>
- <entry>get argument list of function's definition (with default values)</entry>
+ <entry>get argument list of function's or procedure's definition (with default values)</entry>
</row>
<row>
<entry><literal><function>pg_get_function_identity_arguments(<parameter>func_oid</parameter>)</function></literal></entry>
<entry><type>text</type></entry>
- <entry>get argument list to identify a function (without default values)</entry>
+ <entry>get argument list to identify a function or procedure (without default values)</entry>
</row>
<row>
<entry><literal><function>pg_get_function_result(<parameter>func_oid</parameter>)</function></literal></entry>
<entry><type>text</type></entry>
- <entry>get <literal>RETURNS</literal> clause for function</entry>
+ <entry>get <literal>RETURNS</literal> clause for function (returns null for a procedure)</entry>
</row>
<row>
<entry><literal><function>pg_get_indexdef(<parameter>index_oid</parameter>)</function></literal></entry>
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 6f9b30b673..8bd9b9387e 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1815,8 +1815,9 @@ <title>Meta-Commands</title>
<listitem>
<para>
- This command fetches and edits the definition of the named function,
- in the form of a <command>CREATE OR REPLACE FUNCTION</command> command.
+ This command fetches and edits the definition of the named function or procedure,
+ in the form of a <command>CREATE OR REPLACE FUNCTION</command> or
+ <command>CREATE OR REPLACE PROCEDURE</command> command.
Editing is done in the same way as for <literal>\edit</literal>.
After the editor exits, the updated command waits in the query buffer;
type semicolon or <literal>\g</literal> to send it, or <literal>\r</literal>
@@ -2970,8 +2971,9 @@ <title>Meta-Commands</title>
<listitem>
<para>
- This command fetches and shows the definition of the named function,
- in the form of a <command>CREATE OR REPLACE FUNCTION</command> command.
+ This command fetches and shows the definition of the named function or procedure,
+ in the form of a <command>CREATE OR REPLACE FUNCTION</command> or
+ <command>CREATE OR REPLACE PROCEDURE</command> command.
The definition is printed to the current query output channel,
as set by <command>\o</command>.
</para>
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 28767a129a..9eec24281d 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2477,15 +2477,21 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
* replaced.
*/
nsp = get_namespace_name(proc->pronamespace);
- appendStringInfo(&buf, "CREATE OR REPLACE FUNCTION %s(",
+ appendStringInfo(&buf, "CREATE OR REPLACE %s %s(",
+ proc->prorettype ? "FUNCTION" : "PROCEDURE",
quote_qualified_identifier(nsp, name));
(void) print_function_arguments(&buf, proctup, false, true);
- appendStringInfoString(&buf, ")\n RETURNS ");
- print_function_rettype(&buf, proctup);
+ appendStringInfoString(&buf, ")\n");
+ if (proc->prorettype)
+ {
+ appendStringInfoString(&buf, " RETURNS ");
+ print_function_rettype(&buf, proctup);
+ appendStringInfoChar(&buf, '\n');
+ }
print_function_trftypes(&buf, proctup);
- appendStringInfo(&buf, "\n LANGUAGE %s\n",
+ appendStringInfo(&buf, " LANGUAGE %s\n",
quote_identifier(get_language_name(proc->prolang, false)));
/* Emit some miscellaneous options on one line */
@@ -2607,10 +2613,11 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
*
* Since the user is likely to be editing the function body string, we
* shouldn't use a short delimiter that he might easily create a conflict
- * with. Hence prefer "$function$", but extend if needed.
+ * with. Hence prefer "$function$"/"$procedure", but extend if needed.
*/
initStringInfo(&dq);
- appendStringInfoString(&dq, "$function");
+ appendStringInfoChar(&dq, '$');
+ appendStringInfoString(&dq, (proc->prorettype ? "function" : "procedure"));
while (strstr(prosrc, dq.data) != NULL)
appendStringInfoChar(&dq, 'x');
appendStringInfoChar(&dq, '$');
diff --git a/src/test/regress/expected/create_procedure.out b/src/test/regress/expected/create_procedure.out
index 873907dc43..e7bede24fa 100644
--- a/src/test/regress/expected/create_procedure.out
+++ b/src/test/regress/expected/create_procedure.out
@@ -30,6 +30,17 @@ CALL ptest1(substring(random()::text, 1, 1)); -- ok, volatile arg
public | ptest1 | | x text | proc
(1 row)
+SELECT pg_get_functiondef('ptest1'::regproc);
+ pg_get_functiondef
+---------------------------------------------------
+ CREATE OR REPLACE PROCEDURE public.ptest1(x text)+
+ LANGUAGE sql +
+ AS $procedure$ +
+ INSERT INTO cp_test VALUES (1, x); +
+ $procedure$ +
+
+(1 row)
+
SELECT * FROM cp_test ORDER BY b COLLATE "C";
a | b
---+-------
diff --git a/src/test/regress/sql/create_procedure.sql b/src/test/regress/sql/create_procedure.sql
index d65e568a64..774c12ee34 100644
--- a/src/test/regress/sql/create_procedure.sql
+++ b/src/test/regress/sql/create_procedure.sql
@@ -17,6 +17,7 @@ CREATE PROCEDURE ptest1(x text)
CALL ptest1(substring(random()::text, 1, 1)); -- ok, volatile arg
\df ptest1
+SELECT pg_get_functiondef('ptest1'::regproc);
SELECT * FROM cp_test ORDER BY b COLLATE "C";
--
2.16.1
On 2/11/18 01:10, Tom Lane wrote:
This brings the amount of objects stored in pg_proc to four. Perhaps it
would be time to bring more clarity in pg_proc by introducing a prokind
column for functions, aggregates, window functions and procedures?Yeah. I was under the impression that Peter was looking into that ...
[ digs... ] see
/messages/by-id/80ee1f5c-fa9d-7285-ed07-cff53d4f4858@2ndquadrant.com
Yeah that's still pending, but there wasn't a whole lot of reaction in
that thread.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 2/11/18 01:10, Tom Lane wrote:
Yeah. I was under the impression that Peter was looking into that ...
[ digs... ] see
/messages/by-id/80ee1f5c-fa9d-7285-ed07-cff53d4f4858@2ndquadrant.com
Yeah that's still pending, but there wasn't a whole lot of reaction in
that thread.
I think it's probably a good idea, but you hadn't finished the client
end of the patch. Since the main argument against doing this would
probably be about client-side breakage, we need to see how big that
impact will be before we make a final decision.
regards, tom lane
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 2/1/18 11:33, Pavel Stehule wrote:
postgres=# \sf test
ERROR: cache lookup failed for type 0
Here is a patch set that adds procedure support to \ef and \sf.
I've not read in detail, but it looks reasonable offhand, modulo
that I still don't like prorettype == 0 ;-).
I did notice a tiny typo:
- * with. Hence prefer "$function$", but extend if needed.
+ * with. Hence prefer "$function$"/"$procedure", but extend if needed.I think you want
+ * with. Hence prefer "$function$"/"$procedure$", but extend if needed.
regards, tom lane
On Mon, Feb 12, 2018 at 03:19:27PM -0500, Tom Lane wrote:
I've not read in detail, but it looks reasonable offhand, modulo
that I still don't like prorettype == 0 ;-).I did notice a tiny typo:
- * with. Hence prefer "$function$", but extend if needed. + * with. Hence prefer "$function$"/"$procedure", but extend if needed.I think you want
+ * with. Hence prefer "$function$"/"$procedure$", but extend if needed.
0001 and 0002 are welcome. I have a small comment on top of Tom's for 0003.
+ appendStringInfoString(&buf, ")\n");
+ if (proc->prorettype)
+ {
+ appendStringInfoString(&buf, " RETURNS ");
+ print_function_rettype(&buf, proctup);
+ appendStringInfoChar(&buf, '\n');
+ }
Could you use a separate boolean variable which is set as
OidIsValid(prorettype), say called isfunction? The goal is to avoid the
check on prorettype in more than one place. If pg_proc's shape is
changed depending on the discussion, the current patch is a recipy to
forget updating all those places. A comment in pg_get_function_result
to mention that prorettype = InvalidOid is here to track that the call
involves a procedure would be nice.Should the documentation of pg_function_is_visible also mention
procedures?
--
Michael
On 2/13/18 03:57, Michael Paquier wrote:
On Mon, Feb 12, 2018 at 03:19:27PM -0500, Tom Lane wrote:
I've not read in detail, but it looks reasonable offhand, modulo
that I still don't like prorettype == 0 ;-).I did notice a tiny typo:
- * with. Hence prefer "$function$", but extend if needed. + * with. Hence prefer "$function$"/"$procedure", but extend if needed.I think you want
+ * with. Hence prefer "$function$"/"$procedure$", but extend if needed.
done
0001 and 0002 are welcome. I have a small comment on top of Tom's for 0003.
+ appendStringInfoString(&buf, ")\n"); + if (proc->prorettype) + { + appendStringInfoString(&buf, " RETURNS "); + print_function_rettype(&buf, proctup); + appendStringInfoChar(&buf, '\n'); + } Could you use a separate boolean variable which is set as OidIsValid(prorettype), say called isfunction?
done
Should the documentation of pg_function_is_visible also mention
procedures?
done
and committed
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Mon, Feb 12, 2018 at 3:19 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
modulo that I still don't like prorettype == 0 ;-).
+1.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Tue, Feb 13, 2018 at 03:26:20PM -0500, Peter Eisentraut wrote:
and committed
Thanks, those changes are fine for me.
Perhaps you want to have print_function_rettype() drop a elog(ERROR) if
called with an invalid prorettype? I tend to be allergic to Asserts
in ruleutils.c since 0daeba0e...
--
Michael