Re: [HACKERS] Why DEFAULT text 'now' does not work for TIMESTAMP columns

Thomas Lockhart <lockhart@alumni.caltech.edu> writes:
> Yes. We've had lots of small "improvements" in the code (there is
> blood on my hands :) which danced around a fundamental problem: it
> would be nice to pre-evaluate functions on constants, but there are a
> few functions/constants (e.g. "random" or 'now') which shouldn't be
> done this way. Functions and types really should have an "is cachable"
> attribute so that they can be pre-evaluated when possible.

Yes.  There is a proiscachable column in pg_proc, but it doesn't look
like it currently contains useful data, nor do I find any code looking
at it.  We need to put trustworthy data into that column and then we
can start using it to tell whether functions are safe to pre-evaluate
on constants.  In the case at hand, timestamp_in() would have to be
marked unsafe.   I'd like to add a generalized constant-subexpression-
collapser to the optimizer, and it would need something like that to
tell it whether to collapse functions whose inputs are constants.
(Once we did that, the parser would no longer need to worry about
pre-evaluating type conversion functions on constants, which is
effectively what it does now in selected cases...)

> I've been putting off relabeling "datetime" and "timespan" as
> "timestamp" and "interval", thinking that it should wait for the major
> rev bump to 7.0. But it really shouldn't wait.

As long as both sets of names are accepted, I think it probably wouldn't
matter if the implementation of one of them changes.  I wouldn't like to
have my tables containing "datetime" suddenly stop working though...

> Yeah, well, originally it was just passed through as a string, but the
> parser couldn't validate the syntax under those circumstances. So I
> had the parser tokenize it, and then reassemble the string. But
> apparently I didn't try very hard to reassemble it correctly.

I was thinking about letting the parser output the same parsetree as
it does for everything else (thereby saving a chunk of grammar code)
and then building a little subroutine that could deparse a parsetree
to text.  It wouldn't be that much bigger than the part of the grammar
that's doing the task ... for that matter, I think Jan may already have
such a thing somewhere in the rules support.

>> So, one possible
>> approach is to remove the coercion code from StoreAttrDefault
>> altogether.  That would mean that
>> field1 datetime 'now'
>> would start acting the same as
>> field1 datetime text 'now'
>> currently does: both of them would be coerced to datetime at runtime,
>> not when the constraint expression is created.  Given the frequency
>> with which newbies complain about the current behavior, I think that
>> that might be a Good Thing.  But it would be a change in behavior,
>> and I suppose there are scenarios where you'd like to be able to get
>> the old behavior.

> Sorry, how does that change behavior for the worse? I can see it
> taking a performance hit, but under which circumstances would runtime
> evaluation be counter-intuitive or wrong?

I'm not sure it would ever be counter-intuitive, but I can just see
someone coming along and saying "Hey, I *wanted* to store the time
of creation of the table as the default!".  There might be more
plausible examples with other non-cacheable functions, but I haven't
thought of any offhand.

In any case, you could get that result by evaluating the function in
a separate command and pasting its result into the CREATE TABLE, so
there's no serious loss of functionality.

> And while you are being annoyed by code, how about looking at problems
> with trying to use indices on constants and on functions calls?

"Indices on constants"?  I'm confused...
        regards, tom lane