Getting rid of SQLValueFunction

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Getting rid of SQLValueFunction
Date: 2022-09-30 06:04:12
Message-ID: YzaG3MoryCguUOym@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all,

I have bumped a few days ago on the fact that COERCE_SQL_SYNTAX
(introduced by 40c24bf) and SQLValueFunction are around to do the
exact same thing, as known as enforcing single-function calls with
dedicated SQL keywords. For example, keywords like SESSION_USER,
CURRENT_DATE, etc. go through SQLValueFunction and rely on the parser
to set a state that gets then used in execExprInterp.c. And it is
rather easy to implement incorrect SQLValueFunctions, as these rely on
more hardcoded assumptions in the parser and executor than the
equivalent FuncCalls (like collation to assign when using a text-like
SQLValueFunctions).

There are two categories of single-value functions:
- The ones returning names, where we enforce a C collation in two
places of the code (current_role, role, current_catalog,
current_schema, current_database, current_user), even if
get_typcollation() should do that for name types.
- The ones working on time, date and timestamps (localtime[stamp],
current_date, current_time[stamp]), for 9 patterns as these accept an
optional typmod.

I have dug into the possibility to unify all that with a single
interface, and finish with the attached patch set which is a reduction
of code, where all the SQLValueFunctions are replaced by a set of
FuncCalls:
25 files changed, 338 insertions(+), 477 deletions(-)

0001 is the move done for the name-related functions, cleaning up two
places in the executor when a C collation is assigned to those
function expressions. 0002 is the remaining cleanup for the
time-related ones, moving a set of parser-side checks to the execution
path within each function, so as all this knowledge is now local to
each file holding the date and timestamp types. Most of the gain is
in 0002, obviously.

The pg_proc entries introduced for the sake of the move use the same
name as the SQL keywords. These should perhaps be prefixed with a
"pg_" at least. There would be an exception with pg_localtime[stamp],
though, where we could use a pg_localtime[stamp]_sql for the function
name for prosrc. I am open to suggestions for these names.

Thoughts?
--
Michael

Attachment Content-Type Size
0001-Remove-from-SQLValueFunctionOp-all-name-based-functi.patch text/x-diff 12.0 KB
0002-Replace-SQLValueFunction-by-direct-function-calls.patch text/x-diff 40.9 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Benjamin Coutu 2022-09-30 06:05:31 Re: disfavoring unparameterized nested loops
Previous Message John Naylor 2022-09-30 04:53:39 Re: [RFC] building postgres with meson - v13