ERROR: type of parameter 1 (fruit2) does not match that when preparing the plan (fruit1)
Hi
This function should produce form_urlencoded data from a row.
It works on the first invoction.
but the second fails and references the table from the previous invocation.
In this case I am using pgbouncer but I have tested it without
and also without the urldecode on several platforms (pg13)
thanks,
dh
-------to reproduce --------------------------------------
CREATE OR REPLACE FUNCTION record_to_form_data(p_r record)
RETURNS text
LANGUAGE plpgsql
AS $function$
begin
return (
select string_agg(format('%s=%s',key,urlencode(value)),'&')
from
(select p_r.*) i,
hstore(i.*) as h,each(h) );
end;
$function$;
create table fruit1(id varchar not null,name varchar not null,color varchar);
create table fruit2(id varchar not null,name varchar not null);
insert into fruit1 values('1','apple','red');
insert into fruit2 values('1','apple');
select record_to_form_data(f.*) from fruit1 f;
select record_to_form_data(f.*) from fruit2 f;
--------------------------------
testit6=# select record_to_form_data(f.*) from fruit1 f;
record_to_form_data
---------------------------
id=1&name=apple&color=red
(1 row)
testit6=# select record_to_form_data(f.*) from fruit2 f;
ERROR: type of parameter 1 (fruit2) does not match that when preparing the plan (fruit1)
CONTEXT: SQL statement "SELECT (
select string_agg(format('%s=%s',key,urlencode(value)),'&')
from
(select p_r.*) i,
hstore(i.*) as h,each(h) )"
PL/pgSQL function record_to_form_data(record) line 6 at RETURN
testit6=# \c
psql (13.5 (Debian 13.5-0+deb11u1), server 13.6 (Debian 13.6-1.pgdg110+1))
SSL connection (protocol: TLSv1.3, cipher: TLS_AES_256_GCM_SHA384, bits: 256, compression: off)
You are now connected to database "testit6" as user "david".
testit6=# select record_to_form_data(f.*) from fruit2 f;
record_to_form_data
---------------------
id=1&name=apple
(1 row)
On Sun, May 1, 2022 at 8:44 AM d <dchuck@yurfish.com> wrote:
-------to reproduce --------------------------------------
CREATE OR REPLACE FUNCTION record_to_form_data(p_r record)
RETURNS text
LANGUAGE plpgsql
AS $function$
begin
return (
select string_agg(format('%s=%s',key,urlencode(value)),'&')
from
(select p_r.*) i,
hstore(i.*) as h,each(h) );
end;
$function$;
Not a bug, it is a documented limitation.
It is your use of "(select p_r.*)" that is problematic.
https://www.postgresql.org/docs/current/plpgsql-implementation.html#PLPGSQL-PLAN-CACHING
"""
The mutable nature of record variables presents another problem in this
connection. When fields of a record variable are used in expressions or
statements, the data types of the fields must not change from one call of
the function to the next, since each expression will be analyzed using the
data type that is present when the expression is first reached. EXECUTE can
be used to get around this problem when necessary.
"""
David J.
"David G. Johnston" <david.g.johnston@gmail.com> writes:
On Sun, May 1, 2022 at 8:44 AM d <dchuck@yurfish.com> wrote:
CREATE OR REPLACE FUNCTION record_to_form_data(p_r record)
Not a bug, it is a documented limitation.
FWIW, it does seem to work as desired if you declare the argument as
"anyelement".
Maybe we could improve this situation by treating a "record" parameter
as polymorphic, though that might cause some odd inconsistencies with
plpgsql's historical treatment of "record" local variables.
regards, tom lane
On Sun, May 1, 2022 at 10:08 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
"David G. Johnston" <david.g.johnston@gmail.com> writes:
On Sun, May 1, 2022 at 8:44 AM d <dchuck@yurfish.com> wrote:
CREATE OR REPLACE FUNCTION record_to_form_data(p_r record)
Not a bug, it is a documented limitation.
FWIW, it does seem to work as desired if you declare the argument as
"anyelement".
+1
Maybe we could improve this situation by treating a "record" parameter
as polymorphic, though that might cause some odd inconsistencies with
plpgsql's historical treatment of "record" local variables.
The extent of needing to treat "record" as polymorphic-like seems like it
would be limited to resolve_polymorphic_argtype in funcapi.c. Namely, in
computing the hash key for the compiled hash entry for the function.
Similar to how we append the trigger oid in compute_function_hashkey in
pl.compile (which ultimately calls the former) so trigger invocations
become per-table.
David J.
"David G. Johnston" <david.g.johnston@gmail.com> writes:
On Sun, May 1, 2022 at 10:08 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Maybe we could improve this situation by treating a "record" parameter
as polymorphic, though that might cause some odd inconsistencies with
plpgsql's historical treatment of "record" local variables.
The extent of needing to treat "record" as polymorphic-like seems like it
would be limited to resolve_polymorphic_argtype in funcapi.c. Namely, in
computing the hash key for the compiled hash entry for the function.
Similar to how we append the trigger oid in compute_function_hashkey in
pl.compile (which ultimately calls the former) so trigger invocations
become per-table.
I'm hesitant to touch funcapi.c for this; the scope of potential
side-effects becomes enormous as soon as you do.
regards, tom lane
Moving discussion to -hackers
On Sun, May 1, 2022 at 12:46 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
"David G. Johnston" <david.g.johnston@gmail.com> writes:
On Sun, May 1, 2022 at 10:08 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Maybe we could improve this situation by treating a "record" parameter
as polymorphic, though that might cause some odd inconsistencies with
plpgsql's historical treatment of "record" local variables.The extent of needing to treat "record" as polymorphic-like seems like it
would be limited to resolve_polymorphic_argtype in funcapi.c. Namely, in
computing the hash key for the compiled hash entry for the function.
Similar to how we append the trigger oid in compute_function_hashkey in
pl.compile (which ultimately calls the former) so trigger invocations
become per-table.I'm hesitant to touch funcapi.c for this; the scope of potential
side-effects becomes enormous as soon as you do.
Agreed, though the only caller of this particular function seems to be in
plpgsql/pl_comp.c anyway...one more-or-less directly from do_compile and
the other via compute_function_hashkey, so its presence in funcapi.c seems
unusual.
But, to get rid of the cache error it seems sufficient to simply ensure we
compute a hash key that considers the called types.
That do_compile doesn't see these substitutions is a positive for minimal
potential impact with no apparent downside.
I added the test case from the -bug email (I'd probably change it to match
the "scenario" for the file for a final version).
I have some questions in the code that I could probably get answers for
via tests - would including those tests be acceptable (defaults,
named-argument-calling, output argmode)?
I did check-world without issue - but I suspect this to be under-tested and
not naturally used in the course of unrelated testing.
David J.
+ /*
+ * Saved compiled functions with record-typed input args to a
hashkey
+ * that substitutes all known actual composite type OIDs in the
+ * call signature for the corresponding generic record OID from
+ * the definition signature. This avoids a probable error:
+ * "type of parameter ... does not match that when preparing the
plan"
+ * when such a record variable is used in a query within the
function.
+ */
+ for (int i = 0; i < procStruct->pronargs; i++)
+ {
+ if (hashkey->argtypes[i] != RECORDOID)
+ continue;
+
+ // ??? I presume other parts of the system synchronize these
two arrays
+ // In particular for default arguments not specified in the
function call
+ // or named-argument function call syntax.
+
+ /* Don't bother trying to substitute once we run out of input
arguments */
+ if (i > fcinfo->nargs - 1)
+ break;
+
+ hashkey->argtypes[i] =
+ get_call_expr_argtype(fcinfo->flinfo->fn_expr, i);
+ }+select record_to_form_data(fruit1) from fruit1;
+ record_to_form_data
+---------------------
+ (1,apple,red)
+(1 row)
+
+select record_to_form_data(fruit2) from fruit2;
+ record_to_form_data
+---------------------
+ (1,apple)
+(1 row)
+Attachments:
v0001-cache-record-input-function-based-on-call-arguments.patchapplication/octet-stream; name=v0001-cache-record-input-function-based-on-call-arguments.patchDownload
commit 45f52ed92f079ca91ce34343630911ca5cb4c00e
Author: David G. Johnston <david.g.johnston@gmail.com>
Date: Mon May 2 02:00:45 2022 +0000
cache record input functions based on call arguments
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index b791c23f06..5b3035c560 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -2492,6 +2492,31 @@ compute_function_hashkey(FunctionCallInfo fcinfo,
memcpy(hashkey->argtypes, procStruct->proargtypes.values,
procStruct->pronargs * sizeof(Oid));
+ /*
+ * Saved compiled functions with record-typed input args to a hashkey
+ * that substitutes all known actual composite type OIDs in the
+ * call signature for the corresponding generic record OID from
+ * the definition signature. This avoids a probable error:
+ * "type of parameter ... does not match that when preparing the plan"
+ * when such a record variable is used in a query within the function.
+ */
+ for (int i = 0; i < procStruct->pronargs; i++)
+ {
+ if (hashkey->argtypes[i] != RECORDOID)
+ continue;
+
+ // ??? I presume other parts of the system synchronize these two arrays
+ // In particular for default arguments not specified in the function call
+ // or named-argument function call syntax.
+
+ /* Don't bother trying to substitute once we run out of input arguments */
+ if (i > fcinfo->nargs - 1)
+ break;
+
+ hashkey->argtypes[i] =
+ get_call_expr_argtype(fcinfo->flinfo->fn_expr, i);
+ }
+
/* resolve any polymorphic argument types */
plpgsql_resolve_polymorphic_argtypes(procStruct->pronargs,
hashkey->argtypes,
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 08e42f17dc..c86930c12b 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -5779,3 +5779,32 @@ END; $$ LANGUAGE plpgsql;
ERROR: "x" is not a scalar variable
LINE 3: GET DIAGNOSTICS x = ROW_COUNT;
^
+CREATE OR REPLACE FUNCTION record_to_form_data(p_r record)
+ RETURNS text
+ LANGUAGE plpgsql
+AS $function$
+begin
+
+return (SELECT format('%s',p_r));
+
+end;
+$function$;
+create table fruit1(id varchar not null,name varchar not null,color varchar);
+create table fruit2(id varchar not null,name varchar not null);
+insert into fruit1 values('1','apple','red');
+insert into fruit2 values('1','apple');
+select record_to_form_data(fruit1) from fruit1;
+ record_to_form_data
+---------------------
+ (1,apple,red)
+(1 row)
+
+select record_to_form_data(fruit2) from fruit2;
+ record_to_form_data
+---------------------
+ (1,apple)
+(1 row)
+
+DROP FUNCTION record_to_form_data(record);
+DROP TABLE fruit1;
+DROP TABLE fruit2;
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 588c331033..b02b20760c 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -4711,3 +4711,28 @@ BEGIN
GET DIAGNOSTICS x = ROW_COUNT;
RETURN;
END; $$ LANGUAGE plpgsql;
+
+
+CREATE OR REPLACE FUNCTION record_to_form_data(p_r record)
+ RETURNS text
+ LANGUAGE plpgsql
+AS $function$
+begin
+
+return (SELECT format('%s',p_r));
+
+end;
+$function$;
+
+create table fruit1(id varchar not null,name varchar not null,color varchar);
+create table fruit2(id varchar not null,name varchar not null);
+
+insert into fruit1 values('1','apple','red');
+insert into fruit2 values('1','apple');
+
+select record_to_form_data(fruit1) from fruit1;
+select record_to_form_data(fruit2) from fruit2;
+
+DROP FUNCTION record_to_form_data(record);
+DROP TABLE fruit1;
+DROP TABLE fruit2;