Re: queryId constant squashing does not support prepared statements - Mailing list pgsql-hackers

Hello

I spent a much longer time staring at this patch than I wanted to, and
at a point I almost wanted to boot the whole thing to pg19, but because
upthread we already had an agreement that we should get it in for this
cycle, I decided that the best course of action was to just move forward
with it.

My reluctance mostly comes from this bit in generate_normalized_query:

+       /*
+        * If we have an external param at this location, but no lists are
+        * being squashed across the query, then we skip here; this will make
+        * us print print the characters found in the original query that
+        * represent the parameter in the next iteration (or after the loop is
+        * done), which is a bit odd but seems to work okay in most cases.
+        */
+       if (jstate->clocations[i].extern_param && !jstate->has_squashed_lists)
+           continue;

Sami's patch didn't have a comment here and it was not immediately
obvious what was going on; moreover I was quite surprised at what
happened if I removed it: for example, one query text (in test
level_tracking.sql) changes from

-     2 |    2 | SELECT (i + $2)::INTEGER LIMIT $3

into

+     2 |    2 | SELECT ($2 + $3)::INTEGER LIMIT $4

and if I understand this correctly, the reason is that the query is
being executed from an SQL function,

CREATE FUNCTION PLUS_ONE(i INTEGER) RETURNS INTEGER AS
$$ SELECT (i + 1.0)::INTEGER LIMIT 1 $$ LANGUAGE SQL;

so the 'i' is actually a parameter, so it makes sense that it gets
turned into a parameter in the normalized query.  With the 'if' test I
mentioned above, we print it as 'i' literally only because we
'continued' and the next time through the loop we print the text from
the original query.  This may be considered not entirely correct ...
note that the constants in the query are shown as parameters, and that
the numbers do not start from 1.  (Obviously, a few other queries also
change.)


I decided to move forward with it anyway because this weirdness seems
more contained and less damaging than the unhelpfulness of the unpatched
behavior.  We may want to revisit this in pg19 -- or, if we're really
unconfortable with this, we could even decide to revert this commit in
pg18 and try again with Param support in pg19.  But I'd like to move on
from this and my judgement was that the situation with patch is better
than without.


I added one more commit, which iterates to peel as many layers of
CoerceViaIO and RelabelType as there are around an expression.  I
noticed this lack while reading Sami's patch for that function, which
just simplified the coding therein.  For normal cases this would iterate
just once (because repeated layers of casts are likely very unusual), so
I think it's okay to do that.  There was some discussion about handling
this via recursion but it died inconclusively; I added recursive
handling of FuncExpr's arguments in 0f65f3eec478, which was different,
but I think handling this case by iterating is better than recursing.

With these commits, IMO the open item can now be closed.  Even if we
ultimately end up reverting any of this, we would probably punt support
of Params to pg19, so the open item would be gone anyway.


Lastly, I decided not to do a catversion bump.  As far as I can tell,
changes in the jumbling functions do not need them.  I tried an
'installcheck' run with a datadir initdb'd with the original code, and
it works fine.  I also tried an 'installcheck' with pg_stat_statements
installed, and was surprised to realize that the Query Id reports in
EXPLAIN make a large number of tests fail.  If I take those lines from
the original code into the expected output, and then run the tests with
the new code, I notice that a few queries have changed queryId.  I
suppose this was to be expected, and shouldn't harm anything.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"It takes less than 2 seconds to get to 78% complete; that's a good sign.
A few seconds later it's at 90%, but it seems to have stuck there.  Did
somebody make percentages logarithmic while I wasn't looking?"
                http://smylers.hates-software.com/2005/09/08/1995c749.html