date/time special values incorrectly cached as constant in plpgsql

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

date/time special values incorrectly cached as constant in plpgsql

Tijs van Dam
Hi,

I was badly bitten by the fact that date/time special values are parsed as constants early by the query parser. This was with DATE 'today', which is according to some older web pages the right way to get today's date (it's listed as an example in documentation before 8.0).

In my case, I used this in a plpgsql function, called at set times from a long-running connection. In that case, the value is cached as a constant in the compiled function. Imagine how long it's taken to figure out why daily procedures wouldn't run correctly, while they performed just fine when called manually from a fresh connection, and in any kind of test I could come up with.

The behavior is easily reproduced with a line like:

create function foo() returns timestamptz as $$ begin return 'now'::timestamptz; end; $$ language plpgsql;

which will return the same value over and over.

I'd consider this a bug, or at least a deficiency in the documentation. There is a hint that this might happen in the documentation section 9.9.4 (warning against using such string in a DEFAULT clause), but no mention in section 8.5.1.4 (date/time special values, which defines these literals) other than the rather vague remark that they are converted to ordinary values when read.

In my humble opinion, the whole system would be more robust if the query parser would emit function calls for these values (now(), CURRENT_DATE, CURRENT_DATE +/- 1), which would solve this for all cases (plpgsql functions, default clauses, views, cached query plans). If the sql standard says it should behave like it does, or we're worried about backwards comp, then perhaps an improvement could be made by using a different mode in certain contexts (I can remotely see some benefit in being able to denote table or view creation time, but none in the time a function is first used in a certain process, or the time at which a query is first parsed).

I don't think the current behavior is consistent, either. The conversion to time value is done by the parser only if the type can be derived from the statement context (i.e. immediate cast), otherwise it's done later (e.g. when assigning a string literal to a timestamp variable, or returning a string literal from a function with timestamp type), with a different result.

Nota bene, this page: https://www.postgresql.org/docs/current/plpgsql-implementation.html section 42.11.2 -- points out how early conversion can mess up query plan caching, and then uses a workaround with a variable as an example that *does* work correctly. But if someone would helpfully replace "curtime := 'now';" with "curtime := 'now'::timestamp;" in the example, things would go wrong again.

If no change is made to the parser, then I'd propose at least a big fat warning in section 8.5.1.4 that 'now', 'yesterday', 'today', and 'tomorrow' should only be used with the greatest caution, as these values will be converted to constants and then cached in unexpected places.

Regards, Tijs van Dam


Reply | Threaded
Open this post in threaded view
|

Re: date/time special values incorrectly cached as constant in plpgsql

David G Johnston
On Sat, Oct 17, 2020 at 4:05 AM Tijs van Dam <[hidden email]> wrote: 
If no change is made to the parser, then I'd propose at least a big fat warning in section 8.5.1.4 that 'now', 'yesterday', 'today', and 'tomorrow' should only be used with the greatest caution, as these values will be converted to constants and then cached in unexpected places.

IMO, there really isn't anything surprising that these literal inputs end up converted to constants, which are indeed cached in parts of the system that utilize a cache, or are stored as the resultant literal instead of an expression.  That's how literal input values work.  If I need something to be dynamic I have to use a volatile function.

I've flagged this one for later consideration and may decide to write a documentation patch at some point - but as the existing docs aren't wrong and do cover this dynamic, if maybe not explicitly and thoroughly enough for some readers, the effort/benefit calculation isn't that high for me.

David J.

Reply | Threaded
Open this post in threaded view
|

Re: date/time special values incorrectly cached as constant in plpgsql

Tom Lane-2
"David G. Johnston" <[hidden email]> writes:
> On Sat, Oct 17, 2020 at 4:05 AM Tijs van Dam <[hidden email]> wrote:
>> If no change is made to the parser, then I'd propose at least a big fat
>> warning in section 8.5.1.4 that 'now', 'yesterday', 'today', and 'tomorrow'
>> should only be used with the greatest caution, as these values will be
>> converted to constants and then cached in unexpected places.

> IMO, there really isn't anything surprising that these literal inputs end
> up converted to constants, which are indeed cached in parts of the system
> that utilize a cache, or are stored as the resultant literal instead of an
> expression.  That's how literal input values work.  If I need something to
> be dynamic I have to use a volatile function.

Indeed, but I concur with the OP that 8.5.1.4 doesn't really expend enough
words on this point.  Perhaps append something like

  <caution>
   While the values now, today, tomorrow, yesterday are fine to use in
   interactive SQL commands, they can have surprising behavior when used
   in prepared statements, views, or function definitions.  In such cases,
   plan caching can result in a converted specific time value continuing
   to be used long after it becomes stale.  Use one of the SQL functions
   instead in such contexts.
  </caution>

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: date/time special values incorrectly cached as constant in plpgsql

Tom Lane-2
I wrote:
> Indeed, but I concur with the OP that 8.5.1.4 doesn't really expend enough
> words on this point.

After a bit more thought, I committed

https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=540849814cdc22ea025777d374ff6705b4d64a0f

                        regards, tom lane