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); List *querytree_sublist; - querytree_sublist = pg_analyze_and_rewrite_params(parsetree, + querytree_sublist = pg_analyze_and_rewrite_withcb(parsetree, prosrc, (ParserSetupHook) sql_fn_parser_setup, pinfo, diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index b6eacd5baa..378f2b232d 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -439,7 +439,7 @@ BeginCopyTo(ParseState *pstate, * Run parse analysis and rewrite. Note this also acquires sufficient * locks on the source table(s). */ - rewritten = pg_analyze_and_rewrite(raw_query, + rewritten = pg_analyze_and_rewrite_fixedparams(raw_query, pstate->p_sourcetext, NULL, 0, NULL); diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c index eaa76af47b..46b8ca9a00 100644 --- a/src/backend/commands/extension.c +++ b/src/backend/commands/extension.c @@ -746,7 +746,7 @@ execute_sql_string(const char *sql) /* Be sure parser can see any DDL done so far */ CommandCounterIncrement(); - stmt_list = pg_analyze_and_rewrite(parsetree, + stmt_list = pg_analyze_and_rewrite_fixedparams(parsetree, sql, NULL, 0, diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c index 6c6ab9ee34..0c376e070d 100644 --- a/src/backend/commands/schemacmds.c +++ b/src/backend/commands/schemacmds.c @@ -172,7 +172,7 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString, /* * Execute each command contained in the CREATE SCHEMA. Since the grammar * allows only utility commands in CREATE SCHEMA, there is no need to pass - * them through parse_analyze() or the rewriter; we can just hand them + * them through parse_analyze_*() or the rewriter; we can just hand them * straight to ProcessUtility. */ foreach(parsetree_item, parsetree_list) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 45e59e3d5c..41b0eac97a 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -12920,7 +12920,7 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, /* * We expect that we will get only ALTER TABLE and CREATE INDEX * statements. Hence, there is no need to pass them through - * parse_analyze() or the rewriter, but instead we need to pass them + * parse_analyze_*() or the rewriter, but instead we need to pass them * through parse_utilcmd.c to make them ready for execution. */ raw_parsetree_list = raw_parser(cmd, RAW_PARSE_DEFAULT); diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c index 4df05a0b33..8b6254cfce 100644 --- a/src/backend/commands/view.c +++ b/src/backend/commands/view.c @@ -423,7 +423,7 @@ DefineView(ViewStmt *stmt, const char *queryString, rawstmt->stmt_location = stmt_location; rawstmt->stmt_len = stmt_len; - viewParse = parse_analyze(rawstmt, queryString, NULL, 0, NULL); + viewParse = parse_analyze_fixedparams(rawstmt, queryString, NULL, 0, NULL); /* * The grammar should ensure that the result is a single SELECT Query. diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index 296e54e60a..fd878555ee 100644 --- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -718,7 +718,7 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK) RawStmt *parsetree = lfirst_node(RawStmt, lc); List *queryTree_sublist; - queryTree_sublist = pg_analyze_and_rewrite_params(parsetree, + queryTree_sublist = pg_analyze_and_rewrite_withcb(parsetree, fcache->src, (ParserSetupHook) sql_fn_parser_setup, fcache->pinfo, diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 0568ae123f..95989f237d 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -2165,7 +2165,7 @@ _SPI_prepare_plan(const char *src, SPIPlanPtr plan) if (plan->parserSetup != NULL) { Assert(plan->nargs == 0); - stmt_list = pg_analyze_and_rewrite_params(parsetree, + stmt_list = pg_analyze_and_rewrite_withcb(parsetree, src, plan->parserSetup, plan->parserSetupArg, @@ -2173,7 +2173,7 @@ _SPI_prepare_plan(const char *src, SPIPlanPtr plan) } else { - stmt_list = pg_analyze_and_rewrite(parsetree, + stmt_list = pg_analyze_and_rewrite_fixedparams(parsetree, src, plan->argtypes, plan->nargs, @@ -2402,7 +2402,7 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options, else if (plan->parserSetup != NULL) { Assert(plan->nargs == 0); - stmt_list = pg_analyze_and_rewrite_params(parsetree, + stmt_list = pg_analyze_and_rewrite_withcb(parsetree, src, plan->parserSetup, plan->parserSetupArg, @@ -2410,7 +2410,7 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options, } else { - stmt_list = pg_analyze_and_rewrite(parsetree, + stmt_list = pg_analyze_and_rewrite_fixedparams(parsetree, src, plan->argtypes, plan->nargs, diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 873e43bfe6..94735cf0e8 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -5057,7 +5057,7 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte) if (list_length(raw_parsetree_list) != 1) goto fail; - querytree_list = pg_analyze_and_rewrite_params(linitial(raw_parsetree_list), + querytree_list = pg_analyze_and_rewrite_withcb(linitial(raw_parsetree_list), src, (ParserSetupHook) sql_fn_parser_setup, pinfo, NULL); diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 146ee8dd1e..56c58d263d 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -96,7 +96,7 @@ static bool test_raw_expression_coverage(Node *node, void *context); /* - * parse_analyze + * parse_analyze_fixedparams * Analyze a raw parse tree and transform it to Query form. * * Optionally, information about $n parameter types can be supplied. @@ -107,8 +107,8 @@ static bool test_raw_expression_coverage(Node *node, void *context); * a dummy CMD_UTILITY Query node. */ Query * -parse_analyze(RawStmt *parseTree, const char *sourceText, - Oid *paramTypes, int numParams, +parse_analyze_fixedparams(RawStmt *parseTree, const char *sourceText, + const Oid *paramTypes, int numParams, QueryEnvironment *queryEnv) { ParseState *pstate = make_parsestate(NULL); @@ -120,7 +120,7 @@ parse_analyze(RawStmt *parseTree, const char *sourceText, pstate->p_sourcetext = sourceText; if (numParams > 0) - parse_fixed_parameters(pstate, paramTypes, numParams); + setup_parse_fixed_parameters(pstate, paramTypes, numParams); pstate->p_queryEnv = queryEnv; @@ -158,7 +158,7 @@ parse_analyze_varparams(RawStmt *parseTree, const char *sourceText, pstate->p_sourcetext = sourceText; - parse_variable_parameters(pstate, paramTypes, numParams); + setup_parse_variable_parameters(pstate, paramTypes, numParams); query = transformTopLevelStmt(pstate, parseTree); diff --git a/src/backend/parser/parse_param.c b/src/backend/parser/parse_param.c index 68a5534393..b9726e0757 100644 --- a/src/backend/parser/parse_param.c +++ b/src/backend/parser/parse_param.c @@ -35,7 +35,7 @@ typedef struct FixedParamState { - Oid *paramTypes; /* array of parameter type OIDs */ + const Oid *paramTypes; /* array of parameter type OIDs */ int numParams; /* number of array entries */ } FixedParamState; @@ -64,8 +64,8 @@ static bool query_contains_extern_params_walker(Node *node, void *context); * Set up to process a query containing references to fixed parameters. */ void -parse_fixed_parameters(ParseState *pstate, - Oid *paramTypes, int numParams) +setup_parse_fixed_parameters(ParseState *pstate, + const Oid *paramTypes, int numParams) { FixedParamState *parstate = palloc(sizeof(FixedParamState)); @@ -80,7 +80,7 @@ parse_fixed_parameters(ParseState *pstate, * Set up to process a query containing references to variable parameters. */ void -parse_variable_parameters(ParseState *pstate, +setup_parse_variable_parameters(ParseState *pstate, Oid **paramTypes, int *numParams) { VarParamState *parstate = palloc(sizeof(VarParamState)); diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 2d857a301b..464dc6c31b 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -3,7 +3,7 @@ * parse_utilcmd.c * Perform parse analysis work for various utility commands * - * Formerly we did this work during parse_analyze() in analyze.c. However + * Formerly we did this work during parse_analyze_*() in analyze.c. However * that is fairly unsafe in the presence of querytree caching, since any * database state that we depend on in making the transformations might be * obsolete by the time the utility command is executed; and utility commands diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 82de01cdc6..e86275c781 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -640,8 +640,8 @@ pg_parse_query(const char *query_string) * NOTE: for reasons mentioned above, this must be separate from raw parsing. */ List * -pg_analyze_and_rewrite(RawStmt *parsetree, const char *query_string, - Oid *paramTypes, int numParams, +pg_analyze_and_rewrite_fixedparams(RawStmt *parsetree, const char *query_string, + const Oid *paramTypes, int numParams, QueryEnvironment *queryEnv) { Query *query; @@ -655,7 +655,7 @@ pg_analyze_and_rewrite(RawStmt *parsetree, const char *query_string, if (log_parser_stats) ResetUsage(); - query = parse_analyze(parsetree, query_string, paramTypes, numParams, + query = parse_analyze_fixedparams(parsetree, query_string, paramTypes, numParams, queryEnv); if (log_parser_stats) @@ -672,12 +672,13 @@ pg_analyze_and_rewrite(RawStmt *parsetree, const char *query_string, } /* - * Do parse analysis and rewriting. This is the same as pg_analyze_and_rewrite - * except that external-parameter resolution is determined by parser callback - * hooks instead of a fixed list of parameter datatypes. + * Do parse analysis and rewriting. This is the same as + * pg_analyze_and_rewrite_* except that, instead of a fixed list of parameter + * datatypes, a parser callback is supplied that can do external-parameter + * resolution and possibly other things. */ List * -pg_analyze_and_rewrite_params(RawStmt *parsetree, +pg_analyze_and_rewrite_withcb(RawStmt *parsetree, const char *query_string, ParserSetupHook parserSetup, void *parserSetupArg, @@ -1128,7 +1129,7 @@ exec_simple_query(const char *query_string) else oldcontext = MemoryContextSwitchTo(MessageContext); - querytree_list = pg_analyze_and_rewrite(parsetree, query_string, + querytree_list = pg_analyze_and_rewrite_fixedparams(parsetree, query_string, NULL, 0, NULL); plantree_list = pg_plan_queries(querytree_list, query_string, diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c index 6767eae8f2..28871f0176 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -682,13 +682,13 @@ RevalidateCachedQuery(CachedPlanSource *plansource, if (rawtree == NULL) tlist = NIL; else if (plansource->parserSetup != NULL) - tlist = pg_analyze_and_rewrite_params(rawtree, + tlist = pg_analyze_and_rewrite_withcb(rawtree, plansource->query_string, plansource->parserSetup, plansource->parserSetupArg, queryEnv); else - tlist = pg_analyze_and_rewrite(rawtree, + tlist = pg_analyze_and_rewrite_fixedparams(rawtree, plansource->query_string, plansource->param_types, plansource->num_params, diff --git a/src/include/parser/analyze.h b/src/include/parser/analyze.h index a0f0bd38d7..e019bc9b1e 100644 --- a/src/include/parser/analyze.h +++ b/src/include/parser/analyze.h @@ -24,8 +24,8 @@ typedef void (*post_parse_analyze_hook_type) (ParseState *pstate, extern PGDLLIMPORT post_parse_analyze_hook_type post_parse_analyze_hook; -extern Query *parse_analyze(RawStmt *parseTree, const char *sourceText, - Oid *paramTypes, int numParams, QueryEnvironment *queryEnv); +extern Query *parse_analyze_fixedparams(RawStmt *parseTree, const char *sourceText, + const Oid *paramTypes, int numParams, QueryEnvironment *queryEnv); extern Query *parse_analyze_varparams(RawStmt *parseTree, const char *sourceText, Oid **paramTypes, int *numParams); diff --git a/src/include/parser/parse_param.h b/src/include/parser/parse_param.h index b42fff296c..e7069df6ac 100644 --- a/src/include/parser/parse_param.h +++ b/src/include/parser/parse_param.h @@ -15,9 +15,9 @@ #include "parser/parse_node.h" -extern void parse_fixed_parameters(ParseState *pstate, - Oid *paramTypes, int numParams); -extern void parse_variable_parameters(ParseState *pstate, +extern void setup_parse_fixed_parameters(ParseState *pstate, + const Oid *paramTypes, int numParams); +extern void setup_parse_variable_parameters(ParseState *pstate, Oid **paramTypes, int *numParams); extern void check_variable_parameters(ParseState *pstate, Query *query); extern bool query_contains_extern_params(Query *query); diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h index 5c77075aed..af056fe43a 100644 --- a/src/include/tcop/tcopprot.h +++ b/src/include/tcop/tcopprot.h @@ -45,11 +45,11 @@ extern PGDLLIMPORT int log_statement; extern List *pg_parse_query(const char *query_string); extern List *pg_rewrite_query(Query *query); -extern List *pg_analyze_and_rewrite(RawStmt *parsetree, +extern List *pg_analyze_and_rewrite_fixedparams(RawStmt *parsetree, const char *query_string, - Oid *paramTypes, int numParams, + const Oid *paramTypes, int numParams, QueryEnvironment *queryEnv); -extern List *pg_analyze_and_rewrite_params(RawStmt *parsetree, +extern List *pg_analyze_and_rewrite_withcb(RawStmt *parsetree, const char *query_string, ParserSetupHook parserSetup, void *parserSetupArg, base-commit: cab5b9ab2c066ba904f13de2681872dcda31e207 -- 2.34.1