Re: Since '2001-09-09 01:46:40'::timestamp microseconds are lost when extracting epoch

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Cc: Vik Fearing <vik(at)postgresfriends(dot)org>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Petr Fedorov <petr(dot)fedorov(at)phystech(dot)edu>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Since '2001-09-09 01:46:40'::timestamp microseconds are lost when extracting epoch
Date: 2020-10-31 23:57:23
Message-ID: 298489.1604188643@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

I wrote:
> However: suppose that we continue to translate these things into FuncExpr
> nodes, the same as always, but we add a new CoercionForm variant, say
> COERCE_SQL_SYNTAX. 99% of the system ignores FuncExpr.funcformat,
> and would continue to do so, but ruleutils.c would take it to mean
> that (1) the call should be reverse-listed as some special SQL syntax
> and (2) the funcid is one of a small set of built-in functions for
> which ruleutils.c knows what to emit. (If it doesn't recognize the
> funcid, it could either throw an error, or fall back to normal display
> of the node.) For cases such as EXTRACT, this would also represent
> a promise that specific arguments are Const nodes from which the
> desired keyword can be extracted.

Attached is a draft patch that does this. I'm fairly pleased with it,
but there are some loose ends as described below. As the patch stands,
it reverse-lists all our special-format function call syntaxes
*except* EXTRACT. I left that out since I think we want to apply the
reverse-listing change when we add the numeric-output extraction
functions, as I said upthread.

The main thing that's incomplete here is that the switch on function
OID fails to cover some cases that ought to be covered, as a result
of limitations of Gen_fmgrtab.pl:

* Some C functions such as text_substr have multiple pg_proc entries,
and Gen_fmgrtab.pl chooses the wrong one for our purpose. We could
either invent new Gen_fmgrtab.pl behavior to allow having macros for
all the pg_proc entries, or we could add duplicate C functions so that
the pg_proc entries can point to different C symbols.

* Some of the functions we need to reference aren't C functions at
all, but SQL functions, for instance OID 1305 is defined as
select ($1, ($1 + $2)) overlaps ($3, ($3 + $4))
I think our best bet here is to replace these SQL definitions with
C equivalents, because really this implementation is pretty sucky.
Even if we manage to inline the SQL definition, that's expensive
to do; and evaluating some of the arguments twice is not nice either.

> This is kind of an abuse of "CoercionForm", since that typedef name
> implies that it only talks about how to handle cast cases, but
> semantically it's always been a how-to-display-function-calls thing.
> We could either hold our noses about that or rename the typedef.

I did nothing about that here, since it'd bloat the patch without
making anything but cosmetic changes. I'm tempted to propose though
that we rename "CoercionForm" to "DisplayForm" and rename its
COERCE_XXX values to DISPLAY_XXX, to make this less confusing.

Another bit of follow-up work we could contemplate is to get rid of
the SQLValueFunction node type, since there's nothing it does that
we couldn't do with regular FuncExpr nodes and COERCE_SQL_SYNTAX.
But that's just cleanup, and I don't think it would save a very
large amount of code.

Thoughts?

regards, tom lane

Attachment Content-Type Size
reverse-list-special-sql-syntaxes-1.patch text/x-diff 30.5 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Max Vikharev 2020-11-01 06:18:43 Re: BUG #16691: Autovacuum stops processing certain databases until postgresql rebooted
Previous Message Jeff Janes 2020-10-31 18:55:28 Re: BUG #16691: Autovacuum stops processing certain databases until postgresql rebooted

Browse pgsql-hackers by date

  From Date Subject
Next Message Euler Taveira 2020-11-01 00:01:15 Re: cleanup temporary files after crash
Previous Message Fabrízio de Royes Mello 2020-10-31 22:56:33 Re: Add important info about ANALYZE after create Functional Index