From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Erki Eessaar <erki(dot)eessaar(at)taltech(dot)ee> |
Cc: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "pgsql-bugs(at)lists(dot)postgresql(dot)org" <pgsql-bugs(at)lists(dot)postgresql(dot)org>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> |
Subject: | Re: References to parameters by name are lost in INSERT INTO ... SELECT <parameter value> .... statements in case of routines with the SQL-standard function body |
Date: | 2021-11-16 21:52:57 |
Message-ID: | 2345766.1637099577@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs |
Erki Eessaar <erki(dot)eessaar(at)taltech(dot)ee> writes:
> Indeed, re-execution of this code without any modifications in it produces the same result.
Right, that printout is functionally equivalent to the original.
> Still I see here two problems.
> * Inconsistency - see INSERT vs. UPDATE.
Yeah, it's weird that the same parameter is printed two different ways.
I dug into it and found out that we're losing the "context->namespace"
list when recursing into the sub-SELECT from get_insert_query_def.
The fix is trivial (attached). The other places where get_query_def is
invoked quasi-recursively all pass down the parent namespace list already.
The fact that this one is out of step is a very ancient oversight (it's
at least old enough to vote, according to some quick git archaeology).
But as far as I can see, it didn't have any visible consequences until
commit e717a9a18 taught get_parameter() to pay attention to the last
entry of the list. So I'm inclined not to change it before v14.
I was also pretty unhappy that get_parameter() was so naively trusting
that an EXTERN Param must have something to do with the last entry's
argnames array, or even that there must be a last entry. So the attached
makes that a little more paranoid, too.
> * The problem of positional references in general is that changing the order of parameters requires changing the body as well, i.e., the result lost some of its qualities. Thus, one could argue that in this case the input and output are not equivalent.
I don't think this argument holds a lot of water, because you could
reverse it too: if the author had originally written $N, maybe there
was a reason why she thought that would be more maintainable than a named
reference. But we don't store any distinction between the two ways of
writing a Param reference, so we can't promise to regenerate it exactly
the way it was written.
Still, it's clear that the intent here was to print a reference-by-name
if possible, and the failure to do so within only the context of
INSERT/SELECT is therefore a bug.
regards, tom lane
Attachment | Content-Type | Size |
---|---|---|
fix-sql-function-param-decompile.patch | text/x-diff | 1.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Geoghegan | 2021-11-17 00:22:06 | Re: BUG #17255: Server crashes in index_delete_sort_cmp() due to race condition with vacuum |
Previous Message | Tom Lane | 2021-11-16 20:04:06 | Re: pg_restore depending on user functions |