Keeping CURRENT_DATE and similar constructs in original format

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Keeping CURRENT_DATE and similar constructs in original format
Date: 2016-05-12 22:14:54
Message-ID: 30058.1463091294@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
value-function-1.patch.gz application/x-gzip 9.9 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David G. Johnston 2016-05-12 23:59:30 Keeping CURRENT_DATE and similar constructs in original format
Previous Message David E. Wheeler 2016-05-12 21:25:56 Re: Does Type Have = Operator?