Thread: possible bug seen with -DCLOBBER_CACHE_ALWAYS and changing GUCs
SQL:
set datestyle to postgres,us;
prepare stmt as select '02-01-2011'::date::text;
execute stmt;
set datestyle to postgres,euro;
execute stmt;
deallocate stmt;
The results I get with normal debug compilation are:
SET
PREPARE
=A0=A0=A0 text
------------
=A002-01-2011
(1 row)
SET
=A0=A0=A0 text
------------
=A001-02-2011
(1 row)
DEALLOCATE
But with -DCLOBBER_CACHE_ALWAYS and -DRELCACHE_FORCE_RELEASE, I get:
SET
PREPARE
=A0 =A0=A0 text
------------
=A002-01-2011
(1 row)
SET
=A0=A0=A0 text
------------
=A002-01-2011
(1 row)
DEALLOCATE
Which one of those results is correct?
Regards,
Jeff Davis
Jeff Davis <pgsql@j-davis.com> writes:
> SQL:
> set datestyle to postgres,us;
> prepare stmt as select '02-01-2011'::date::text;
> execute stmt;
> set datestyle to postgres,euro;
> execute stmt;
> deallocate stmt;
> The results I get with normal debug compilation are:
> SET
> PREPARE
> text
> ------------
> 02-01-2011
> (1 row)
> SET
> text
> ------------
> 01-02-2011
> (1 row)
> DEALLOCATE
The result of parse analysis for that query is a stored date constant
(in a Const node) with a cast-to-text on top of it. The system is aware
that cast-date-to-text isn't immutable, so it doesn't try to fold the
cast operation. When you execute the query, it displays the date
constant using the now-current datestyle.
> But with -DCLOBBER_CACHE_ALWAYS and -DRELCACHE_FORCE_RELEASE, I get:
> SET
> PREPARE
> text
> ------------
> 02-01-2011
> (1 row)
> SET
> text
> ------------
> 02-01-2011
> (1 row)
> DEALLOCATE
> Which one of those results is correct?
I believe what is happening in the second case is that the query is
getting re-parse-analyzed, from scratch, and since now datestyle is
different (DMY not MDY), the date literal gets interpreted differently.
You could argue it either way as to which result is "more correct",
but I doubt we're going to try to do something about that. Best advice
is to avoid ambiguous input, or if you can't, at least avoid flipping
your datestyle on the fly.
regards, tom lane
On Wed, 2011-11-30 at 20:10 -0500, Tom Lane wrote:
> I believe what is happening in the second case is that the query is
> getting re-parse-analyzed, from scratch, and since now datestyle is
> different (DMY not MDY), the date literal gets interpreted differently.
> You could argue it either way as to which result is "more correct",
> but I doubt we're going to try to do something about that. Best advice
> is to avoid ambiguous input, or if you can't, at least avoid flipping
> your datestyle on the fly.
I'm fine calling that "not a bug", though it appears to work in 8.3.
Regards,
Jeff Davis
On Wed, Nov 30, 2011 at 08:10:22PM -0500, Tom Lane wrote: > Jeff Davis <pgsql@j-davis.com> writes: > > SQL: > > set datestyle to postgres,us; > > prepare stmt as select '02-01-2011'::date::text; > > execute stmt; > > set datestyle to postgres,euro; > > execute stmt; > > deallocate stmt; > > > The results I get with normal debug compilation are: > > > SET > > PREPARE > > text > > ------------ > > 02-01-2011 > > (1 row) > > > SET > > text > > ------------ > > 01-02-2011 > > (1 row) > > > DEALLOCATE > > But with -DCLOBBER_CACHE_ALWAYS and -DRELCACHE_FORCE_RELEASE, I get: > > > SET > > PREPARE > > text > > ------------ > > 02-01-2011 > > (1 row) > > > SET > > text > > ------------ > > 02-01-2011 > > (1 row) > > > DEALLOCATE > > > Which one of those results is correct? > > I believe what is happening in the second case is that the query is > getting re-parse-analyzed, from scratch, and since now datestyle is > different (DMY not MDY), the date literal gets interpreted differently. > You could argue it either way as to which result is "more correct", > but I doubt we're going to try to do something about that. Best advice > is to avoid ambiguous input, or if you can't, at least avoid flipping > your datestyle on the fly. One could defend consistent use of either the PREPARE-time DateStyle or the EXECUTE-time DateStyle to interpret literals. However, using the value as of the last RevalidateCachedQuery(), its timing independent of any GUC change, is an implementation artifact with no redeeming value for the user. This hazard also arises around IntervalStyle, TimeZone, sql_inheritance, transform_null_equals, and array_nulls. Implementation challenges aside, I'd contend for always using PREPARE-time values during parse analysis. That's more consistent with the user-visible consequences of changing search_path or standard_conforming_strings. That said, I don't have in mind a cure clearly less ugly than the disease. nm
On Wed, Nov 30, 2011 at 5:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> The result of parse analysis for that query is a stored date constant
> (in a Const node) with a cast-to-text on top of it. =A0The system is aware
> that cast-date-to-text isn't immutable, so it doesn't try to fold the
> cast operation. =A0When you execute the query, it displays the date
> constant using the now-current datestyle.
Another thought: why does it execute the type input function (which is
dependent on a GUC), but not the cast?
Regards,
Jeff Davis
Jeff Davis <pgsql@j-davis.com> writes:
> On Wed, Nov 30, 2011 at 5:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The result of parse analysis for that query is a stored date constant
>> (in a Const node) with a cast-to-text on top of it. The system is aware
>> that cast-date-to-text isn't immutable, so it doesn't try to fold the
>> cast operation. When you execute the query, it displays the date
>> constant using the now-current datestyle.
> Another thought: why does it execute the type input function (which is
> dependent on a GUC), but not the cast?
It's historical, see parse_coerce.c around line 210:
* XXX if the typinput function is not immutable, we really ought to
* postpone evaluation of the function call until runtime. But there
* is no way to represent a typinput function call as an expression
* tree, because C-string values are not Datums. (XXX This *is*
* possible as of 7.3, do we want to do it?)
(Of course, the parenthetical comment is a good deal newer than what
precedes it.)
However, I don't think this is the core issue. What is the core issue
is how many GUC settings we would like to save and reinstate for every
cached plan. We do that now for search_path because of the potential
security implications, but the costs of doing it for all of 'em seem
a tad daunting; not to mention that somebody is going to complain that
that's not the behavior he wanted.
regards, tom lane