parse/analyze API refactoring - Mailing list pgsql-hackers

From Peter Eisentraut
Subject parse/analyze API refactoring
Date
Msg-id c67ce276-52b4-0239-dc0e-39875bf81840@enterprisedb.com
Whole thread Raw
Responses Re: parse/analyze API refactoring  ("Bossart, Nathan" <bossartn@amazon.com>)
List 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

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Add Boolean node
Next
From: Andrew Dunstan
Date:
Subject: Re: Extend compatibility of PostgreSQL::Test::Cluster