Getting rid of SQLValueFunction
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-09-30T06:04:12Z
Lists: pgsql-hackers
Attachments
- 0001-Remove-from-SQLValueFunctionOp-all-name-based-functi.patch (text/x-diff) patch 0001
- 0002-Replace-SQLValueFunction-by-direct-function-calls.patch (text/x-diff) patch 0002
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
Commits
-
Fix precision handling for some COERCE_SQL_SYNTAX functions
- 7aa81c61ec8f 16.0 landed
-
Replace SQLValueFunction by COERCE_SQL_SYNTAX
- f193883fc9ce 16.0 landed
-
Switch SQLValueFunction on "name" to use COERCE_SQL_SYNTAX
- fb32748e32e2 16.0 landed
-
Improve coverage of ruleutils.c for SQLValueFunctions
- 2e0d80c5bb72 16.0 landed