Обсуждение: Keeping CURRENT_DATE and similar constructs in original format

I got annoyed again about a minor issue I've complained about before,
and this time decided to do something about it.  The issue is that gram.y
translates a number of argument-less SQL constructs, such as CURRENT_DATE,
into very implementation-specific things such as 'now'::text::date.  There
are several reasons not to like that:

* It exposes what should be implementation details in reverse-listed views
and rules.  That's bad for us because it reduces our freedom to improve
the implementation, and it's bad for users because it looks ugly, and is
harder to understand (since it's not what you wrote to start with), and it
creates unnecessary lock-in to Postgres.

* Actually, we're exposing implementation details even without looking
at reverse listings:

regression=# select current_time;
       timetz
--------------------
 17:46:11.589945-04
(1 row)

Where did that column heading come from?  It appears because the topmost
node in what "current_time" expands to is a cast to timetz.  If you're
not aware of that, it doesn't exactly meet the POLA.

* Performance is fairly bad, because of the need to parse the 'now' string
each time.  A quick experiment puts the cost of now() at about 60ns on my
machine, while current_timestamp(6) takes over 500ns to deliver the same
result.  Admittedly, this is probably not a hot-button for most users,
but it's not good.

* Including a constant in the translated construct requires ugly hacks for
pg_stat_statements, cf commit 69c7a9838c82bbfd.


So what I've wanted to do for some time is invent a new expression node
type that represents any one of these functions and can be reverse-listed
in the same format that the input had.  The attached proposed patch does
that.  (I'm not particularly in love with the node type name
ValueFunction; anybody got a better idea?)

Obviously this is 9.7 material; I'm posting it now just so I can add
it to the next CF and thereby not forget about it.


By the by, a scan through gram.y reveals other stuff we aren't trying
to reverse-list in original form:

    a_expr AT TIME ZONE a_expr
    LIKE, ILIKE, SIMILAR TO
    OVERLAPS
    BETWEEN
    COLLATION FOR '(' a_expr ')'
    EXTRACT '(' extract_list ')'
    OVERLAY '(' overlay_list ')'
    POSITION '(' position_list ')'
    SUBSTRING '(' substr_list ')'
    TREAT '(' a_expr AS Typename ')'
    TRIM '(' BOTH trim_list ')'
    TRIM '(' LEADING trim_list ')'
    TRIM '(' TRAILING trim_list ')'
    TRIM '(' trim_list ')'

Each of these gets converted to some PG-specific function or operator
name, and then will get reverse-listed using that name and ordinary
function or operator syntax, rather than using the SQL-approved special
syntax.  I'm less excited about doing something about these cases,
because (1) they aren't exposing implementation details in any real way,
and (2) in most of these cases, the SQL-approved syntax is just randomly
inconsistent with anything else.  But perhaps somebody else would want
to think about changing that.  (Note that I do think we need to handle
BETWEEN better, in particular to avoid multiple-evaluation risks, but
that's a separate matter.)

            regards, tom lane

So what I've wanted to do for some time is invent a new expression node
type that represents any one of these functions and can be reverse-listed
in the same format that the input had.  The attached proposed patch does
that.  (I'm not particularly in love with the node type name
ValueFunction; anybody got a better idea?)

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Thursday, May 12, 2016, Tom Lane <tgl@sss.pgh.pa.us
> <javascript:_e(%7B%7D,'cvml','tgl@sss.pgh.pa.us');>> wrote:
>> (I'm not particularly in love with the node type name
>> ValueFunction; anybody got a better idea?)

> SQL99DateTimeFunction (or roughly whenever they were introduced)?

Some of them aren't datetime-related, though.  I thought about
NiladicFunction but it seemed maybe too technical.

> I agree with the premise.  I took notice of it recently in explain output
> on these lists using current_date.  That example read like
> ('now'::cstring)::date which was really odd since I was at least expecting
> text as the intermediate cast...

Yeah, that's another fun thing: the reverse listing currently differs
depending on whether you're looking at an expression tree that's been
through const-folding.  It didn't use to --- looks like the mention
of cstring started in 9.2.
        regards, tom lane

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Thursday, May 12, 2016, Tom Lane <tgl@sss.pgh.pa.us
> <javascript:_e(%7B%7D,'cvml','tgl@sss.pgh.pa.us');>> wrote:
>> (I'm not particularly in love with the node type name
>> ValueFunction; anybody got a better idea?)

> SQL99DateTimeFunction (or roughly whenever they were introduced)?

Some of them aren't datetime-related, though.  I thought about
NiladicFunction but it seemed maybe too technical.

On 5/12/16 6:14 PM, Tom Lane wrote:
> So what I've wanted to do for some time is invent a new expression node
> type that represents any one of these functions and can be reverse-listed
> in the same format that the input had.  The attached proposed patch does
> that.

I was experimenting with this as well when I found your patch, and I
think this is the right solution.  Your patch works fine for me.

Because of the refactoring in 2f153ddfdd318b211590dd5585f65f357d85c2de,
you will need to update your patch a bit.

> (I'm not particularly in love with the node type name
> ValueFunction; anybody got a better idea?)

I think this is fine.  The only other idea I have would be
SQLValueFunction, to emphasize that this is about SQL-mandated special
cases.

> By the by, a scan through gram.y reveals other stuff we aren't trying
> to reverse-list in original form:
> 
>     a_expr AT TIME ZONE a_expr
>     LIKE, ILIKE, SIMILAR TO
>     OVERLAPS
>     BETWEEN
>     COLLATION FOR '(' a_expr ')'
>     EXTRACT '(' extract_list ')'
>     OVERLAY '(' overlay_list ')'
>     POSITION '(' position_list ')'
>     SUBSTRING '(' substr_list ')'
>     TREAT '(' a_expr AS Typename ')'
>     TRIM '(' BOTH trim_list ')'
>     TRIM '(' LEADING trim_list ')'
>     TRIM '(' TRAILING trim_list ')'
>     TRIM '(' trim_list ')'
> 
> Each of these gets converted to some PG-specific function or operator
> name, and then will get reverse-listed using that name and ordinary
> function or operator syntax, rather than using the SQL-approved special
> syntax.

I think those could be addressed by having ruleutils.c *always* convert
matching function calls back to the special syntax.  Alternatively, tag
the function call node in the grammar with "this is special syntax" and
then look for that in ruleutils.c.  This is sort of what I was playing
with, except that the several levels of casting for the datetime
functions make that a mess.  If it's only one function call, it should
be easier.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 5/12/16 6:14 PM, Tom Lane wrote:
>> So what I've wanted to do for some time is invent a new expression node
>> type that represents any one of these functions and can be reverse-listed
>> in the same format that the input had.  The attached proposed patch does
>> that.

> I was experimenting with this as well when I found your patch, and I
> think this is the right solution.  Your patch works fine for me.

Thanks for reviewing this patch.  I've pushed it now.

>> (I'm not particularly in love with the node type name
>> ValueFunction; anybody got a better idea?)

> I think this is fine.  The only other idea I have would be
> SQLValueFunction, to emphasize that this is about SQL-mandated special
> cases.

I went with SQLValueFunction.
        regards, tom lane