2018-03-20 15:05 GMT+01:00 Merlin Moncure <mmonc...@gmail.com>:
> On Wed, Feb 28, 2018 at 4:28 PM, Peter Eisentraut
> <peter.eisentr...@2ndquadrant.com> wrote:
> > This patch set adds support for INOUT parameters to procedures.
> > Currently, INOUT and OUT parameters are not supported.
> >
> > A top-level CALL returns the output parameters as a result row. In
> > PL/pgSQL, I have added special support to pass the output back into the
> > variables, as one would expect.
> >
> > These patches apply on top of the "prokind" patch set v2. (Tom has
> > submitted an updated version of that, which overlaps with some of the
> > changes I've made here. I will work on consolidating that soon.)
>
> I did a pull from master to play around with INOUT parameters and got
> some strange interactions with DEFAULT. Specifically, DEFAULT doesn't
> do much beyond, 'return the last supplied value given'. I'm not sure
> if this is expected behavior; it seems odd:
>
> postgres=# create or replace procedure p(a inout int default 7) as $$
> begin return; end; $$ language plpgsql;
> CREATE PROCEDURE
> postgres=# call p();
> a
> ───
>
> (1 row)
>
> postgres=# call p(3);
> a
> ───
> 3
> (1 row)
>
> postgres=# call p();
> a
> ───
> 3
> (1 row)
>
>
> I got null,3,3. I would have expected 7,3,7. Default arguments might
> remove quite some of the pain associated with having to supply bogus
> arguments to get the INOUT parameters working.
>
> Edit: In one case, after dropping the function and recreating it, I
> got the procedure to return 0 where it had not before, so this smells
> like a bug.
> postgres=# call p();
> 2018-03-20 09:04:50.543 CDT [21494] ERROR: function p() does not
> exist at character 6
> 2018-03-20 09:04:50.543 CDT [21494] HINT: No function matches the
> given name and argument types. You might need to add explicit type
> casts.
> 2018-03-20 09:04:50.543 CDT [21494] STATEMENT: call p();
> ERROR: function p() does not exist
> LINE 1: call p();
> ^
> HINT: No function matches the given name and argument types. You
> might need to add explicit type casts.
> Time: 0.297 ms
> postgres=# create or replace procedure p(a inout int default 7) as $$
> begin return; end; $$ language plpgsql;
> CREATE PROCEDURE
> Time: 1.182 ms
> postgres=# call p();
> a
> ───
> 0
> (1 row)
>
I wrote patch
Regards
Pavel
>
>
> merlin
>
>
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 86fa8c0dd7..c7a44d858b 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -55,6 +55,7 @@
#include "executor/executor.h"
#include "miscadmin.h"
#include "optimizer/var.h"
+#include "optimizer/clauses.h"
#include "parser/parse_coerce.h"
#include "parser/parse_collate.h"
#include "parser/parse_expr.h"
@@ -2254,6 +2255,9 @@ ExecuteCallStmt(CallStmt *stmt, ParamListInfo params, bool atomic, DestReceiver
elog(ERROR, "cache lookup failed for function %u", fexpr->funcid);
if (!heap_attisnull(tp, Anum_pg_proc_proconfig))
callcontext->atomic = true;
+
+ fexpr->args = expand_function_arguments(fexpr->args, fexpr->funcresulttype, tp);
+
ReleaseSysCache(tp);
/* Initialize function call structure */
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index a9a09afd2b..40eae3a835 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -130,8 +130,6 @@ static Expr *simplify_function(Oid funcid,
Oid result_collid, Oid input_collid, List **args_p,
bool funcvariadic, bool process_args, bool allow_non_const,
eval_const_expressions_context *context);
-static List *expand_function_arguments(List *args, Oid result_type,
- HeapTuple func_tuple);
static List *reorder_function_arguments(List *args, HeapTuple func_tuple);
static List *add_function_defaults(List *args, HeapTuple func_tuple);
static List *fetch_function_defaults(HeapTuple func_tuple);
@@ -4112,7 +4110,7 @@ simplify_function(Oid funcid, Oid result_type, int32 result_typmod,
* cases it handles should never occur there. This should be OK since it
* will fall through very quickly if there's nothing to do.
*/
-static List *
+List *
expand_function_arguments(List *args, Oid result_type, HeapTuple func_tuple)
{
Form_pg_proc funcform = (Form_pg_proc) GETSTRUCT(func_tuple);
diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h
index ba4fa4b68b..ed854fdd40 100644
--- a/src/include/optimizer/clauses.h
+++ b/src/include/optimizer/clauses.h
@@ -14,9 +14,9 @@
#ifndef CLAUSES_H
#define CLAUSES_H
+#include "access/htup.h"
#include "nodes/relation.h"
-
#define is_opclause(clause) ((clause) != NULL && IsA(clause, OpExpr))
#define is_funcclause(clause) ((clause) != NULL && IsA(clause, FuncExpr))
@@ -85,4 +85,7 @@ extern Node *estimate_expression_value(PlannerInfo *root, Node *node);
extern Query *inline_set_returning_function(PlannerInfo *root,
RangeTblEntry *rte);
+extern List *expand_function_arguments(List *args, Oid result_type,
+ HeapTuple func_tuple);
+
#endif /* CLAUSES_H */
diff --git a/src/pl/plpgsql/src/expected/plpgsql_call.out b/src/pl/plpgsql/src/expected/plpgsql_call.out
index 1e94a44f2b..9d7d7da5b0 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_call.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_call.out
@@ -169,3 +169,80 @@ DROP PROCEDURE test_proc1;
DROP PROCEDURE test_proc3;
DROP PROCEDURE test_proc4;
DROP TABLE test1;
+-- named parameters and defaults
+CREATE PROCEDURE test_proc(a int, b int, c int DEFAULT -1)
+AS $$
+BEGIN
+ RAISE NOTICE 'a: %, b: %, c: %', a, b, c;
+END;
+$$ LANGUAGE plpgsql;
+CALL test_proc(10,20,30);
+NOTICE: a: 10, b: 20, c: 30
+CALL test_proc(10,20);
+NOTICE: a: 10, b: 20, c: -1
+CALL test_proc(c=>1, a=>3, b=>2);
+NOTICE: a: 3, b: 2, c: 1
+DROP PROCEDURE test_proc;
+CREATE PROCEDURE test_proc1(INOUT _a int, INOUT _b int)
+AS $$
+BEGIN
+ RAISE NOTICE 'test_proc1: a: %, b: %', _a, _b;
+ _a := _a * 10;
+ _b := _b + 10;
+END;
+$$ LANGUAGE plpgsql;
+CALL test_proc1(10,20);
+NOTICE: test_proc1: a: 10, b: 20
+ _a | _b
+-----+----
+ 100 | 30
+(1 row)
+
+CALL test_proc1(_b=>20, _a=>10);
+NOTICE: test_proc1: a: 10, b: 20
+ _a | _b
+-----+----
+ 100 | 30
+(1 row)
+
+DO $$
+DECLARE a int; b int;
+BEGIN
+ a := 10; b := 30;
+ CALL test_proc1(a, b);
+ RAISE NOTICE 'a: %, b: %', a, b;
+ a := 10; b := 30;
+ CALL test_proc1(_b=>b, _a=>a);
+ RAISE NOTICE 'a: %, b: %', a, b;
+END
+$$;
+NOTICE: test_proc1: a: 10, b: 30
+NOTICE: a: 100, b: 40
+NOTICE: test_proc1: a: 10, b: 30
+NOTICE: a: 100, b: 40
+DROP PROCEDURE test_proc1;
+CREATE PROCEDURE test_proc1(INOUT _a int, INOUT _b int, INOUT _c int)
+AS $$
+BEGIN
+ RAISE NOTICE 'test_proc1: a: %, b: %, c: %', _a, _b, _c;
+ _a := _a * 10;
+ _b := _b + 10;
+ _c := _c * -10;
+END;
+$$ LANGUAGE plpgsql;
+DO $$
+DECLARE a int; b int; c int;
+BEGIN
+ a := 10; b := 30; c := 50;
+ CALL test_proc1(a, b, c);
+ RAISE NOTICE 'a: %, b: %, c: %', a, b, c;
+ a := 10; b := 30; c := 50;
+ CALL test_proc1(a, _c=>c, _b=>b);
+ RAISE NOTICE 'a: %, b: %, c: %', a, b, c;
+END
+$$;
+NOTICE: test_proc1: a: 10, b: 30, c: 50
+NOTICE: a: 100, b: 40, c: -500
+NOTICE: test_proc1: a: 10, b: 30, c: 50
+NOTICE: a: 100, b: 40, c: -500
+DROP PROCEDURE test_proc1;
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 68da7cf669..e073b15d60 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2134,14 +2134,36 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
{
Param *param;
- if (!IsA(n, Param))
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("argument %d is an output argument but is not writable", i + 1)));
+ if (IsA(n,NamedArgExpr))
+ {
+ NamedArgExpr *nexpr = (NamedArgExpr *) n;
+
+ 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 = castNode(Param, n);
- /* paramid is offset by 1 (see make_datum_param()) */
- row->varnos[nfields++] = param->paramid - 1;
+ param = castNode(Param, nexpr->arg);
+
+ /* paramid is offset by 1 (see make_datum_param()) */
+ /* should be ensured so this target varno is not used yet */
+ row->varnos[nexpr->argnumber] = param->paramid - 1;
+
+ /* named arguments must be after possition arguments, so I can increase nfields */
+ nfields++;
+ }
+ else
+ {
+ if (!IsA(n, Param))
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("argument %d is an output argument but is not writable", i + 1)));
+
+ param = castNode(Param, n);
+
+ /* paramid is offset by 1 (see make_datum_param()) */
+ row->varnos[nfields++] = param->paramid - 1;
+ }
}
i++;
}
diff --git a/src/pl/plpgsql/src/sql/plpgsql_call.sql b/src/pl/plpgsql/src/sql/plpgsql_call.sql
index f1eed9975a..3f21dd9ea6 100644
--- a/src/pl/plpgsql/src/sql/plpgsql_call.sql
+++ b/src/pl/plpgsql/src/sql/plpgsql_call.sql
@@ -167,3 +167,67 @@ DROP PROCEDURE test_proc3;
DROP PROCEDURE test_proc4;
DROP TABLE test1;
+
+-- named parameters and defaults
+CREATE PROCEDURE test_proc(a int, b int, c int DEFAULT -1)
+AS $$
+BEGIN
+ RAISE NOTICE 'a: %, b: %, c: %', a, b, c;
+END;
+$$ LANGUAGE plpgsql;
+
+CALL test_proc(10,20,30);
+CALL test_proc(10,20);
+CALL test_proc(c=>1, a=>3, b=>2);
+
+DROP PROCEDURE test_proc;
+
+CREATE PROCEDURE test_proc1(INOUT _a int, INOUT _b int)
+AS $$
+BEGIN
+ RAISE NOTICE 'test_proc1: a: %, b: %', _a, _b;
+ _a := _a * 10;
+ _b := _b + 10;
+END;
+$$ LANGUAGE plpgsql;
+
+CALL test_proc1(10,20);
+CALL test_proc1(_b=>20, _a=>10);
+
+DO $$
+DECLARE a int; b int;
+BEGIN
+ a := 10; b := 30;
+ CALL test_proc1(a, b);
+ RAISE NOTICE 'a: %, b: %', a, b;
+ a := 10; b := 30;
+ CALL test_proc1(_b=>b, _a=>a);
+ RAISE NOTICE 'a: %, b: %', a, b;
+END
+$$;
+
+DROP PROCEDURE test_proc1;
+
+CREATE PROCEDURE test_proc1(INOUT _a int, INOUT _b int, INOUT _c int)
+AS $$
+BEGIN
+ RAISE NOTICE 'test_proc1: a: %, b: %, c: %', _a, _b, _c;
+ _a := _a * 10;
+ _b := _b + 10;
+ _c := _c * -10;
+END;
+$$ LANGUAGE plpgsql;
+
+DO $$
+DECLARE a int; b int; c int;
+BEGIN
+ a := 10; b := 30; c := 50;
+ CALL test_proc1(a, b, c);
+ RAISE NOTICE 'a: %, b: %, c: %', a, b, c;
+ a := 10; b := 30; c := 50;
+ CALL test_proc1(a, _c=>c, _b=>b);
+ RAISE NOTICE 'a: %, b: %, c: %', a, b, c;
+END
+$$;
+
+DROP PROCEDURE test_proc1;
