ERROR: type of parameter 1 (fruit2) does not match that when preparing the plan (fruit1)

Started by dalmost 4 years ago6 messageshackersbugs
Jump to latest
#1d
dchuck@yurfish.com
hackersbugs

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)

#2David G. Johnston
david.g.johnston@gmail.com
In reply to: d (#1)
hackersbugs
Re: ERROR: type of parameter 1 (fruit2) does not match that when preparing the plan (fruit1)

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.

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: David G. Johnston (#2)
hackersbugs
Re: ERROR: type of parameter 1 (fruit2) does not match that when preparing the plan (fruit1)

"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

#4David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#3)
hackersbugs
Re: ERROR: type of parameter 1 (fruit2) does not match that when preparing the plan (fruit1)

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.

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: David G. Johnston (#4)
hackersbugs
Re: ERROR: type of parameter 1 (fruit2) does not match that when preparing the plan (fruit1)

"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

#6David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#5)
hackersbugs
Re: ERROR: type of parameter 1 (fruit2) does not match that when preparing the plan (fruit1)

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+79-0