Moving discussion to -hackers On Sun, May 1, 2022 at 12:46 PM Tom Lane <t...@sss.pgh.pa.us> wrote:
> "David G. Johnston" <david.g.johns...@gmail.com> writes: > > On Sun, May 1, 2022 at 10:08 AM Tom Lane <t...@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) +
v0001-cache-record-input-function-based-on-call-arguments.patch
Description: Binary data