From f8db76e875099e5e49f5cd729a673e84c0b0471b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 28 Apr 2013 00:18:45 -0400 Subject: [PATCH] Editorialize a bit on new ProcessUtility() API. Choose a saner ordering of parameters (adding a new input param after the output params seemed a bit random), update the function's header comment to match reality (cmon folks, is this really that hard?), get rid of useless and sloppily-defined distinction between PROCESS_UTILITY_SUBCOMMAND and PROCESS_UTILITY_GENERATED. --- .../pg_stat_statements/pg_stat_statements.c | 31 +++++----- contrib/sepgsql/hooks.c | 14 +++-- src/backend/commands/extension.c | 4 +- src/backend/commands/schemacmds.c | 4 +- src/backend/commands/trigger.c | 3 +- src/backend/executor/functions.c | 4 +- src/backend/executor/spi.c | 4 +- src/backend/tcop/pquery.c | 5 +- src/backend/tcop/utility.c | 62 +++++++++++-------- src/include/tcop/utility.h | 17 +++-- 10 files changed, 80 insertions(+), 68 deletions(-) diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 8b6f88baf7..a6ceaf4f38 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -239,10 +239,9 @@ static void pgss_ExecutorRun(QueryDesc *queryDesc, long count); static void pgss_ExecutorFinish(QueryDesc *queryDesc); static void pgss_ExecutorEnd(QueryDesc *queryDesc); -static void pgss_ProcessUtility(Node *parsetree, - const char *queryString, ParamListInfo params, - DestReceiver *dest, char *completionTag, - ProcessUtilityContext context); +static void pgss_ProcessUtility(Node *parsetree, const char *queryString, + ProcessUtilityContext context, ParamListInfo params, + DestReceiver *dest, char *completionTag); static uint32 pgss_hash_fn(const void *key, Size keysize); static int pgss_match_fn(const void *key1, const void *key2, Size keysize); static uint32 pgss_hash_string(const char *str); @@ -786,8 +785,8 @@ pgss_ExecutorEnd(QueryDesc *queryDesc) */ static void pgss_ProcessUtility(Node *parsetree, const char *queryString, - ParamListInfo params, DestReceiver *dest, - char *completionTag, ProcessUtilityContext context) + ProcessUtilityContext context, ParamListInfo params, + DestReceiver *dest, char *completionTag) { /* * If it's an EXECUTE statement, we don't track it and don't increment the @@ -819,11 +818,13 @@ pgss_ProcessUtility(Node *parsetree, const char *queryString, PG_TRY(); { if (prev_ProcessUtility) - prev_ProcessUtility(parsetree, queryString, params, - dest, completionTag, context); + prev_ProcessUtility(parsetree, queryString, + context, params, + dest, completionTag); else - standard_ProcessUtility(parsetree, queryString, params, - dest, completionTag, context); + standard_ProcessUtility(parsetree, queryString, + context, params, + dest, completionTag); nested_level--; } PG_CATCH(); @@ -880,11 +881,13 @@ pgss_ProcessUtility(Node *parsetree, const char *queryString, else { if (prev_ProcessUtility) - prev_ProcessUtility(parsetree, queryString, params, - dest, completionTag, context); + prev_ProcessUtility(parsetree, queryString, + context, params, + dest, completionTag); else - standard_ProcessUtility(parsetree, queryString, params, - dest, completionTag, context); + standard_ProcessUtility(parsetree, queryString, + context, params, + dest, completionTag); } } diff --git a/contrib/sepgsql/hooks.c b/contrib/sepgsql/hooks.c index 04c5120890..a1320e8638 100644 --- a/contrib/sepgsql/hooks.c +++ b/contrib/sepgsql/hooks.c @@ -299,10 +299,10 @@ sepgsql_exec_check_perms(List *rangeTabls, bool abort) static void sepgsql_utility_command(Node *parsetree, const char *queryString, + ProcessUtilityContext context, ParamListInfo params, DestReceiver *dest, - char *completionTag, - ProcessUtilityContext context) + char *completionTag) { sepgsql_context_info_t saved_context_info = sepgsql_context_info; ListCell *cell; @@ -362,11 +362,13 @@ sepgsql_utility_command(Node *parsetree, } if (next_ProcessUtility_hook) - (*next_ProcessUtility_hook) (parsetree, queryString, params, - dest, completionTag, context); + (*next_ProcessUtility_hook) (parsetree, queryString, + context, params, + dest, completionTag); else - standard_ProcessUtility(parsetree, queryString, params, - dest, completionTag, context); + standard_ProcessUtility(parsetree, queryString, + context, params, + dest, completionTag); } PG_CATCH(); { diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c index 36df8bddca..2d84ac8620 100644 --- a/src/backend/commands/extension.c +++ b/src/backend/commands/extension.c @@ -750,10 +750,10 @@ execute_sql_string(const char *sql, const char *filename) { ProcessUtility(stmt, sql, + PROCESS_UTILITY_QUERY, NULL, dest, - NULL, - PROCESS_UTILITY_QUERY); + NULL); } PopActiveSnapshot(); diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c index 21d7e608df..1d13ba0d30 100644 --- a/src/backend/commands/schemacmds.c +++ b/src/backend/commands/schemacmds.c @@ -151,10 +151,10 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString) /* do this step */ ProcessUtility(stmt, queryString, + PROCESS_UTILITY_SUBCOMMAND, NULL, None_Receiver, - NULL, - PROCESS_UTILITY_SUBCOMMAND); + NULL); /* make sure later steps can see the object created here */ CommandCounterIncrement(); } diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index bebb551f7f..a0473498bd 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -1014,7 +1014,8 @@ ConvertTriggerToFK(CreateTrigStmt *stmt, Oid funcoid) /* ... and execute it */ ProcessUtility((Node *) atstmt, "(generated ALTER TABLE ADD FOREIGN KEY command)", - NULL, None_Receiver, NULL, PROCESS_UTILITY_GENERATED); + PROCESS_UTILITY_SUBCOMMAND, NULL, + None_Receiver, NULL); /* Remove the matched item from the list */ info_list = list_delete_ptr(info_list, info); diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index c908f34cfe..dbb4805ae2 100644 --- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -831,10 +831,10 @@ postquel_getnext(execution_state *es, SQLFunctionCachePtr fcache) (Node *) es->qd->plannedstmt : es->qd->utilitystmt), fcache->src, + PROCESS_UTILITY_QUERY, es->qd->params, es->qd->dest, - NULL, - PROCESS_UTILITY_QUERY); + NULL); result = true; /* never stops early */ } else diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index de8d59a8cd..ca0d05d2cc 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -2093,10 +2093,10 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, ProcessUtility(stmt, plansource->query_string, + PROCESS_UTILITY_QUERY, paramLI, dest, - completionTag, - PROCESS_UTILITY_QUERY); + completionTag); /* Update "processed" if stmt returned tuples */ if (_SPI_current->tuptable) diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c index 107d8aedab..2c3156a2e9 100644 --- a/src/backend/tcop/pquery.c +++ b/src/backend/tcop/pquery.c @@ -1184,11 +1184,10 @@ PortalRunUtility(Portal portal, Node *utilityStmt, bool isTopLevel, ProcessUtility(utilityStmt, portal->sourceText, + isTopLevel ? PROCESS_UTILITY_TOPLEVEL : PROCESS_UTILITY_QUERY, portal->portalParams, dest, - completionTag, - isTopLevel ? - PROCESS_UTILITY_TOPLEVEL : PROCESS_UTILITY_QUERY); + completionTag); /* Some utility statements may change context on us */ MemoryContextSwitchTo(PortalGetHeapMemory(portal)); diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 2710db1232..c9408970b1 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -71,10 +71,10 @@ ProcessUtility_hook_type ProcessUtility_hook = NULL; /* local function declarations */ static void ProcessUtilitySlow(Node *parsetree, const char *queryString, + ProcessUtilityContext context, ParamListInfo params, DestReceiver *dest, - char *completionTag, - ProcessUtilityContext context); + char *completionTag); static void ExecDropStmt(DropStmt *stmt, bool isTopLevel); @@ -314,8 +314,9 @@ CheckRestrictedOperation(const char *cmdname) * * parsetree: the parse tree for the utility statement * queryString: original source text of command + * context: identifies source of statement (toplevel client command, + * non-toplevel client command, subcommand of a larger utility command) * params: parameters to use during execution - * isTopLevel: true if executing a "top level" (interactively issued) command * dest: where to send results * completionTag: points to a buffer of size COMPLETION_TAG_BUFSIZE * in which to store a command completion status string. @@ -331,10 +332,10 @@ CheckRestrictedOperation(const char *cmdname) void ProcessUtility(Node *parsetree, const char *queryString, + ProcessUtilityContext context, ParamListInfo params, DestReceiver *dest, - char *completionTag, - ProcessUtilityContext context) + char *completionTag) { Assert(queryString != NULL); /* required as of 8.4 */ @@ -344,11 +345,13 @@ ProcessUtility(Node *parsetree, * call standard_ProcessUtility(). */ if (ProcessUtility_hook) - (*ProcessUtility_hook) (parsetree, queryString, params, - dest, completionTag, context); + (*ProcessUtility_hook) (parsetree, queryString, + context, params, + dest, completionTag); else - standard_ProcessUtility(parsetree, queryString, params, - dest, completionTag, context); + standard_ProcessUtility(parsetree, queryString, + context, params, + dest, completionTag); } /* @@ -365,10 +368,10 @@ ProcessUtility(Node *parsetree, void standard_ProcessUtility(Node *parsetree, const char *queryString, + ProcessUtilityContext context, ParamListInfo params, DestReceiver *dest, - char *completionTag, - ProcessUtilityContext context) + char *completionTag) { bool isTopLevel = (context == PROCESS_UTILITY_TOPLEVEL); @@ -817,8 +820,9 @@ standard_ProcessUtility(Node *parsetree, DropStmt *stmt = (DropStmt *) parsetree; if (EventTriggerSupportsObjectType(stmt->removeType)) - ProcessUtilitySlow(parsetree, queryString, params, - dest, completionTag, context); + ProcessUtilitySlow(parsetree, queryString, + context, params, + dest, completionTag); else ExecDropStmt(stmt, isTopLevel); } @@ -829,8 +833,9 @@ standard_ProcessUtility(Node *parsetree, RenameStmt *stmt = (RenameStmt *) parsetree; if (EventTriggerSupportsObjectType(stmt->renameType)) - ProcessUtilitySlow(parsetree, queryString, params, - dest, completionTag, context); + ProcessUtilitySlow(parsetree, queryString, + context, params, + dest, completionTag); else ExecRenameStmt(stmt); } @@ -841,8 +846,9 @@ standard_ProcessUtility(Node *parsetree, AlterObjectSchemaStmt *stmt = (AlterObjectSchemaStmt *) parsetree; if (EventTriggerSupportsObjectType(stmt->objectType)) - ProcessUtilitySlow(parsetree, queryString, params, - dest, completionTag, context); + ProcessUtilitySlow(parsetree, queryString, + context, params, + dest, completionTag); else ExecAlterObjectSchemaStmt(stmt); } @@ -853,8 +859,9 @@ standard_ProcessUtility(Node *parsetree, AlterOwnerStmt *stmt = (AlterOwnerStmt *) parsetree; if (EventTriggerSupportsObjectType(stmt->objectType)) - ProcessUtilitySlow(parsetree, queryString, params, - dest, completionTag, context); + ProcessUtilitySlow(parsetree, queryString, + context, params, + dest, completionTag); else ExecAlterOwnerStmt(stmt); } @@ -862,8 +869,9 @@ standard_ProcessUtility(Node *parsetree, default: /* All other statement types have event trigger support */ - ProcessUtilitySlow(parsetree, queryString, params, - dest, completionTag, context); + ProcessUtilitySlow(parsetree, queryString, + context, params, + dest, completionTag); break; } } @@ -876,10 +884,10 @@ standard_ProcessUtility(Node *parsetree, static void ProcessUtilitySlow(Node *parsetree, const char *queryString, + ProcessUtilityContext context, ParamListInfo params, DestReceiver *dest, - char *completionTag, - ProcessUtilityContext context) + char *completionTag) { bool isTopLevel = (context == PROCESS_UTILITY_TOPLEVEL); bool isCompleteQuery = (context <= PROCESS_UTILITY_QUERY); @@ -966,10 +974,10 @@ ProcessUtilitySlow(Node *parsetree, /* Recurse for anything else */ ProcessUtility(stmt, queryString, + PROCESS_UTILITY_SUBCOMMAND, params, None_Receiver, - NULL, - PROCESS_UTILITY_GENERATED); + NULL); } /* Need CCI between commands */ @@ -1017,10 +1025,10 @@ ProcessUtilitySlow(Node *parsetree, /* Recurse for anything else */ ProcessUtility(stmt, queryString, + PROCESS_UTILITY_SUBCOMMAND, params, None_Receiver, - NULL, - PROCESS_UTILITY_GENERATED); + NULL); } /* Need CCI between commands */ diff --git a/src/include/tcop/utility.h b/src/include/tcop/utility.h index 0697678979..9864447b72 100644 --- a/src/include/tcop/utility.h +++ b/src/include/tcop/utility.h @@ -20,23 +20,22 @@ typedef enum { PROCESS_UTILITY_TOPLEVEL, /* toplevel interactive command */ PROCESS_UTILITY_QUERY, /* a complete query, but not toplevel */ - PROCESS_UTILITY_SUBCOMMAND, /* a piece of a query */ - PROCESS_UTILITY_GENERATED /* internally generated node query node */ + PROCESS_UTILITY_SUBCOMMAND /* a portion of a query */ } ProcessUtilityContext; /* Hook for plugins to get control in ProcessUtility() */ typedef void (*ProcessUtility_hook_type) (Node *parsetree, - const char *queryString, ParamListInfo params, - DestReceiver *dest, char *completionTag, - ProcessUtilityContext context); + const char *queryString, ProcessUtilityContext context, + ParamListInfo params, + DestReceiver *dest, char *completionTag); extern PGDLLIMPORT ProcessUtility_hook_type ProcessUtility_hook; extern void ProcessUtility(Node *parsetree, const char *queryString, - ParamListInfo params, DestReceiver *dest, char *completionTag, - ProcessUtilityContext context); + ProcessUtilityContext context, ParamListInfo params, + DestReceiver *dest, char *completionTag); extern void standard_ProcessUtility(Node *parsetree, const char *queryString, - ParamListInfo params, DestReceiver *dest, - char *completionTag, ProcessUtilityContext context); + ProcessUtilityContext context, ParamListInfo params, + DestReceiver *dest, char *completionTag); extern bool UtilityReturnsTuples(Node *parsetree);