I have found some of the parse/analyze API calls confusing one too many
times, so here I'm proposing some renaming and refactoring.
Notionally, there are three parallel ways to call the parse/analyze
phase: with fixed parameters (for example, used by SPI), with variable
parameters (for example, used by PREPARE), and with a parser callback
(for example, used to parse the body of SQL functions). Some of the
involved functions were confusingly named and made this API structure
more confusing.
For example, at the top level there are pg_analyze_and_rewrite() and
pg_analyze_and_rewrite_params(). You'd think the first one doesn't take
parameters and the second one takes parameters. But the truth is, the
first one takes fixed parameters and the second one takes a parser
callback. The parser callback can be used to parse parameters, but also
other things. There isn't any variant that takes variable parameters;
that code is sprinkled around other places altogether.
One level below that, there is parse_analyze() (for fixed parameters)
and parse_analyze_varparams() (good name). But there is no analogous
function for the callback variant; that code is spread out in
pg_analyze_and_rewrite_params().
And then there are parse_fixed_parameters() and
parse_variable_parameters(). But they don't do any parsing at all.
They just set up callbacks for the parsing to follow.
This doesn't need to be so confusing. With the attached patch set, the
calls end up:
pg_analyze_and_rewrite_fixedparams()
-> parse_analyze_fixedparams()
-> setup_parse_fixed_parameters()
pg_analyze_and_rewrite_varparams() [new]
-> parse_analyze_varparams()
-> setup_parse_variable_parameters()
pg_analyze_and_rewrite_withcb()
-> parse_analyze_withcb() [new]
-> (nothing needed here)
(The "withcb" naming maybe isn't great; better ideas welcome.)
Not included in this patch set, but food for further thought: The
pg_analyze_and_rewrite_*() functions aren't all that useful (anymore).
One might as well write
pg_rewrite_query(parse_analyze_xxx(...))
The only things that pg_analyze_and_rewrite_*() do in addition to that
is handle log_parser_stats, which could be moved into parse_analyze_*(),
and TRACE_POSTGRESQL_QUERY_REWRITE_*(), which IMO doesn't make sense to
begin with and should be in pg_rewrite_query().From 9a72ceadebd6b9309fe0157c1cd181b29ee8bd52 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Tue, 28 Dec 2021 09:01:45 +0100
Subject: [PATCH v1 1/3] Parse/analyze function renaming
There are three parallel ways to call parse/analyze: with fixed
parameters, with variable parameters, and by supplying your own parser
callback. Some of the involved functions were confusingly named and
made this API structure more confusing. This patch renames some functions to
make this clearer:
parse_analyze() -> parse_analyze_fixedparams()
pg_analyze_and_rewrite() -> pg_analyze_and_rewrite_fixedparams()
(Otherwise one might think this variant doesn't accept parameters, but
in fact all three ways accept parameters.)
pg_analyze_and_rewrite_params() -> pg_analyze_and_rewrite_withcb()
(Before, and also when considering pg_analyze_and_rewrite(), one might
think this is the only way to pass parameters. Moreover, the parser
callback doesn't necessarily need to parse only parameters, it's just
one of the things it could do.)
parse_fixed_parameters() -> setup_parse_fixed_parameters()
parse_variable_parameters() -> setup_parse_variable_parameters()
(These functions don't actually do any parsing, they just set up
callbacks to use during parsing later.)
This patch also adds some const decorations to the fixed-parameters
API, so the distinction from the variable-parameters API is more
clear.
---
src/backend/catalog/pg_proc.c| 2 +-
src/backend/commands/copyto.c| 2 +-
src/backend/commands/extension.c | 2 +-
src/backend/commands/schemacmds.c| 2 +-
src/backend/commands/tablecmds.c | 2 +-
src/backend/commands/view.c | 2 +-
src/backend/executor/functions.c | 2 +-
src/backend/executor/spi.c | 8
src/backend/optimizer/util/clauses.c | 2 +-
src/backend/parser/analyze.c | 10 +-
src/backend/parser/parse_param.c | 8
src/backend/parser/parse_utilcmd.c | 2 +-
src/backend/tcop/postgres.c | 17 +
src/backend/utils/cache/plancache.c | 4 ++--
src/include/parser/analyze.h | 4 ++--
src/include/parser/parse_param.h | 6 +++---
src/include/tcop/tcopprot.h | 6 +++---
17 files changed, 41 insertions(+), 40 deletions(-)
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 25d35230d0..8dffcb3a19 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -947,7 +947,7 @@ fmgr_sql_validator(PG_FUNCTION_ARGS)
RawStmt*parsetree =
lfirst_node(RawStmt, lc);
