parse/analyze API refactoring

From: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: parse/analyze API refactoring
Date: 2021-12-28 16:22:04
Message-ID: c67ce276-52b4-0239-dc0e-39875bf81840@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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().

Attachment Content-Type Size
v1-0001-Parse-analyze-function-renaming.patch text/plain 17.0 KB
v1-0002-Add-pg_analyze_and_rewrite_varparams.patch text/plain 9.2 KB
v1-0003-Add-parse_analyze_withcb.patch text/plain 4.1 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2021-12-28 16:46:53 Re: Extend compatibility of PostgreSQL::Test::Cluster
Previous Message Tom Lane 2021-12-28 15:51:36 Re: Add Boolean node