From: | Peter Geoghegan <peter(at)2ndquadrant(dot)com> |
---|---|
To: | PG Hackers <pgsql-hackers(at)postgresql(dot)org> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Subject: | Re: pg_stat_statements normalisation without invasive changes to the parser (was: Next steps on pg_stat_statements normalisation) |
Date: | 2012-02-20 23:16:04 |
Message-ID: | CAEYLb_U4fiuvCchAVe2go97ayKoB82DwM=ECYYdF32RvsRt==w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 16 February 2012 21:11, Peter Geoghegan <peter(at)2ndquadrant(dot)com> wrote:
> * # XXX: This test currently fails!:
> #verify_normalizes_correctly("SELECT cast('1' as dnotnull);","SELECT
> cast(? as dnotnull);",conn, "domain literal canonicalization/cast")
>
> It appears to fail because the CoerceToDomain node gives its location
> to the constant node as starting from "cast", so we end up with
> "SELECT ?('1' as dnotnull);". I'm not quite sure if this points to
> there being a slight tension with my use of the location field in this
> way, or if this is something that could be fixed as a bug in core
> (albeit a highly obscure one), though I suspect the latter.
So I looked at this in more detail today, and it turns out that it has
nothing to do with CoerceToDomain in particular. The same effect can
be observed by doing this:
select cast('foo' as text);
In turns out that this happens for the same reason as the location of
the Const token in the following query:
select integer 5;
being given such that the string "select ?" results.
Resolving this one issue resolves some others, as it allows me to
greatly simplify the get_constant_length() logic.
Here is the single, hacky change I've made just for now to the core
parser to quickly see if it all works as expected:
*************** transformTypeCast(ParseState *pstate, Ty
*** 2108,2113 ****
--- 2108,2116 ----
if (location < 0)
location = tc->typeName->location;
+ if (IsA(expr, Const))
+ location = ((Const*)expr)->location;
+
result = coerce_to_target_type(pstate, expr, inputType,
targetType, targetTypmod,
COERCION_EXPLICIT,
After making this change, I can get all my regression tests to pass
(once I change the normalised representation of certain queries to
look like: "select integer ?" rather than "select ?", which is better
anyway), including the CAST()/CoerceToDomain one that previously
failed. So far so good.
Clearly this change is a quick and dirty workaround, and something
better is required. The question I'd pose to the maintainer of this
code is: what business does the coerce_to_target_type call have
changing the location of the Const node resulting from coercion under
the circumstances described? I understand that the location of the
CoerceToDomain should be at "CAST", but why should the underlying
Const's position be the same? Do you agree that this is a bug, and if
so, would you please facilitate me by committing a fix?
--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services
From | Date | Subject | |
---|---|---|---|
Next Message | Simon Riggs | 2012-02-20 23:23:42 | Re: 16-bit page checksums for 9.2 |
Previous Message | Bruce Momjian | 2012-02-20 23:09:01 | Re: 16-bit page checksums for 9.2 |