public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v48 3/7] Make archiver process an auxiliary process 3+ messages / 3 participants [nested] [flat]
* [PATCH v48 3/7] Make archiver process an auxiliary process @ 2020-03-13 07:59 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Kyotaro Horiguchi @ 2020-03-13 07:59 UTC (permalink / raw) This is a preliminary patch for shared-memory based stats collector. Archiver process must be a auxiliary process since it uses shared memory after stats data was moved into shared-memory. Make the process an auxiliary process in order to make it work. --- src/backend/access/transam/xlogarchive.c | 6 +- src/backend/bootstrap/bootstrap.c | 22 ++-- src/backend/postmaster/pgarch.c | 130 +++-------------------- src/backend/postmaster/postmaster.c | 50 +++++---- src/backend/storage/lmgr/proc.c | 1 + src/include/access/xlog.h | 3 + src/include/access/xlogarchive.h | 1 + src/include/miscadmin.h | 2 + src/include/postmaster/pgarch.h | 4 +- src/include/storage/pmsignal.h | 1 - src/include/storage/proc.h | 3 + 11 files changed, 69 insertions(+), 154 deletions(-) diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index 1c5a4f8b5a..d01859bde5 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -29,7 +29,9 @@ #include "storage/fd.h" #include "storage/ipc.h" #include "storage/lwlock.h" +#include "storage/latch.h" #include "storage/pmsignal.h" +#include "storage/proc.h" /* * Attempt to retrieve the specified file from off-line archival storage. @@ -490,8 +492,8 @@ XLogArchiveNotify(const char *xlog) } /* Notify archiver that it's got something to do */ - if (IsUnderPostmaster) - SendPostmasterSignal(PMSIGNAL_WAKEN_ARCHIVER); + if (IsUnderPostmaster && ProcGlobal->archiverLatch) + SetLatch(ProcGlobal->archiverLatch); } /* diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index 6f615e6622..41da0c5059 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -317,6 +317,9 @@ AuxiliaryProcessMain(int argc, char *argv[]) case StartupProcess: MyBackendType = B_STARTUP; break; + case ArchiverProcess: + MyBackendType = B_ARCHIVER; + break; case BgWriterProcess: MyBackendType = B_BG_WRITER; break; @@ -437,30 +440,29 @@ AuxiliaryProcessMain(int argc, char *argv[]) proc_exit(1); /* should never return */ case StartupProcess: - /* don't set signals, startup process has its own agenda */ StartupProcessMain(); - proc_exit(1); /* should never return */ + proc_exit(1); + + case ArchiverProcess: + PgArchiverMain(); + proc_exit(1); case BgWriterProcess: - /* don't set signals, bgwriter has its own agenda */ BackgroundWriterMain(); - proc_exit(1); /* should never return */ + proc_exit(1); case CheckpointerProcess: - /* don't set signals, checkpointer has its own agenda */ CheckpointerMain(); - proc_exit(1); /* should never return */ + proc_exit(1); case WalWriterProcess: - /* don't set signals, walwriter has its own agenda */ InitXLOGAccess(); WalWriterMain(); - proc_exit(1); /* should never return */ + proc_exit(1); case WalReceiverProcess: - /* don't set signals, walreceiver has its own agenda */ WalReceiverMain(); - proc_exit(1); /* should never return */ + proc_exit(1); default: elog(PANIC, "unrecognized process type: %d", (int) MyAuxProcType); diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index edec311f12..9a2e21bf86 100644 --- a/src/backend/postmaster/pgarch.c +++ b/src/backend/postmaster/pgarch.c @@ -48,6 +48,7 @@ #include "storage/latch.h" #include "storage/pg_shmem.h" #include "storage/pmsignal.h" +#include "storage/procsignal.h" #include "utils/guc.h" #include "utils/ps_status.h" @@ -78,13 +79,11 @@ * Local data * ---------- */ -static time_t last_pgarch_start_time; static time_t last_sigterm_time = 0; /* * Flags set by interrupt handlers for later service in the main loop. */ -static volatile sig_atomic_t wakened = false; static volatile sig_atomic_t ready_to_stop = false; /* ---------- @@ -95,8 +94,6 @@ static volatile sig_atomic_t ready_to_stop = false; static pid_t pgarch_forkexec(void); #endif -NON_EXEC_STATIC void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn(); -static void pgarch_waken(SIGNAL_ARGS); static void pgarch_waken_stop(SIGNAL_ARGS); static void pgarch_MainLoop(void); static void pgarch_ArchiverCopyLoop(void); @@ -110,75 +107,6 @@ static void pgarch_archiveDone(char *xlog); * ------------------------------------------------------------ */ -/* - * pgarch_start - * - * Called from postmaster at startup or after an existing archiver - * died. Attempt to fire up a fresh archiver process. - * - * Returns PID of child process, or 0 if fail. - * - * Note: if fail, we will be called again from the postmaster main loop. - */ -int -pgarch_start(void) -{ - time_t curtime; - pid_t pgArchPid; - - /* - * Do nothing if no archiver needed - */ - if (!XLogArchivingActive()) - return 0; - - /* - * Do nothing if too soon since last archiver start. This is a safety - * valve to protect against continuous respawn attempts if the archiver is - * dying immediately at launch. Note that since we will be re-called from - * the postmaster main loop, we will get another chance later. - */ - curtime = time(NULL); - if ((unsigned int) (curtime - last_pgarch_start_time) < - (unsigned int) PGARCH_RESTART_INTERVAL) - return 0; - last_pgarch_start_time = curtime; - -#ifdef EXEC_BACKEND - switch ((pgArchPid = pgarch_forkexec())) -#else - switch ((pgArchPid = fork_process())) -#endif - { - case -1: - ereport(LOG, - (errmsg("could not fork archiver: %m"))); - return 0; - -#ifndef EXEC_BACKEND - case 0: - /* in postmaster child ... */ - InitPostmasterChild(); - - /* Close the postmaster's sockets */ - ClosePostmasterPorts(false); - - /* Drop our connection to postmaster's shared memory, as well */ - dsm_detach_all(); - PGSharedMemoryDetach(); - - PgArchiverMain(0, NULL); - break; -#endif - - default: - return (int) pgArchPid; - } - - /* shouldn't get here */ - return 0; -} - /* ------------------------------------------------------------ * Local functions called by archiver follow * ------------------------------------------------------------ @@ -212,14 +140,9 @@ pgarch_forkexec(void) #endif /* EXEC_BACKEND */ -/* - * PgArchiverMain - * - * The argc/argv parameters are valid only in EXEC_BACKEND case. However, - * since we don't use 'em, it hardly matters... - */ -NON_EXEC_STATIC void -PgArchiverMain(int argc, char *argv[]) +/* Main entry point for archiver process */ +void +PgArchiverMain(void) { /* * Ignore all signals usually bound to some action in the postmaster, @@ -231,33 +154,26 @@ PgArchiverMain(int argc, char *argv[]) /* SIGQUIT handler was already set up by InitPostmasterChild */ pqsignal(SIGALRM, SIG_IGN); pqsignal(SIGPIPE, SIG_IGN); - pqsignal(SIGUSR1, pgarch_waken); + pqsignal(SIGUSR1, procsignal_sigusr1_handler); pqsignal(SIGUSR2, pgarch_waken_stop); + /* Reset some signals that are accepted by postmaster but not here */ pqsignal(SIGCHLD, SIG_DFL); + + /* Unblock signals (they were blocked when the postmaster forked us) */ PG_SETMASK(&UnBlockSig); - MyBackendType = B_ARCHIVER; - init_ps_display(NULL); + /* + * Advertise our latch that backends can use to wake us up while we're + * sleeping. + */ + ProcGlobal->archiverLatch = &MyProc->procLatch; pgarch_MainLoop(); exit(0); } -/* SIGUSR1 signal handler for archiver process */ -static void -pgarch_waken(SIGNAL_ARGS) -{ - int save_errno = errno; - - /* set flag that there is work to be done */ - wakened = true; - SetLatch(MyLatch); - - errno = save_errno; -} - /* SIGUSR2 signal handler for archiver process */ static void pgarch_waken_stop(SIGNAL_ARGS) @@ -282,14 +198,6 @@ pgarch_MainLoop(void) pg_time_t last_copy_time = 0; bool time_to_stop; - /* - * We run the copy loop immediately upon entry, in case there are - * unarchived files left over from a previous database run (or maybe the - * archiver died unexpectedly). After that we wait for a signal or - * timeout before doing more. - */ - wakened = true; - /* * There shouldn't be anything for the archiver to do except to wait for a * signal ... however, the archiver exists to protect our data, so she @@ -328,12 +236,8 @@ pgarch_MainLoop(void) } /* Do what we're here for */ - if (wakened || time_to_stop) - { - wakened = false; - pgarch_ArchiverCopyLoop(); - last_copy_time = time(NULL); - } + pgarch_ArchiverCopyLoop(); + last_copy_time = time(NULL); /* * Sleep until a signal is received, or until a poll is forced by @@ -354,13 +258,9 @@ pgarch_MainLoop(void) WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH, timeout * 1000L, WAIT_EVENT_ARCHIVER_MAIN); - if (rc & WL_TIMEOUT) - wakened = true; if (rc & WL_POSTMASTER_DEATH) time_to_stop = true; } - else - wakened = true; } /* diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index edab95a19e..22e1cd244c 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -548,6 +548,7 @@ static void ShmemBackendArrayRemove(Backend *bn); #endif /* EXEC_BACKEND */ #define StartupDataBase() StartChildProcess(StartupProcess) +#define StartArchiver() StartChildProcess(ArchiverProcess) #define StartBackgroundWriter() StartChildProcess(BgWriterProcess) #define StartCheckpointer() StartChildProcess(CheckpointerProcess) #define StartWalWriter() StartChildProcess(WalWriterProcess) @@ -1792,7 +1793,7 @@ ServerLoop(void) /* If we have lost the archiver, try to start a new one. */ if (PgArchPID == 0 && PgArchStartupAllowed()) - PgArchPID = pgarch_start(); + PgArchPID = StartArchiver(); /* If we need to signal the autovacuum launcher, do so now */ if (avlauncher_needs_signal) @@ -3007,7 +3008,7 @@ reaper(SIGNAL_ARGS) if (!IsBinaryUpgrade && AutoVacuumingActive() && AutoVacPID == 0) AutoVacPID = StartAutoVacLauncher(); if (PgArchStartupAllowed() && PgArchPID == 0) - PgArchPID = pgarch_start(); + PgArchPID = StartArchiver(); if (PgStatPID == 0) PgStatPID = pgstat_start(); @@ -3142,20 +3143,16 @@ reaper(SIGNAL_ARGS) } /* - * Was it the archiver? If so, just try to start a new one; no need - * to force reset of the rest of the system. (If fail, we'll try - * again in future cycles of the main loop.). Unless we were waiting - * for it to shut down; don't restart it in that case, and - * PostmasterStateMachine() will advance to the next shutdown step. + * Was it the archiver? Normal exit can be ignored; we'll start a new + * one at the next iteration of the postmaster's main loop, if + * necessary. Any other exit condition is treated as a crash. */ if (pid == PgArchPID) { PgArchPID = 0; if (!EXIT_STATUS_0(exitstatus)) - LogChildExit(LOG, _("archiver process"), - pid, exitstatus); - if (PgArchStartupAllowed()) - PgArchPID = pgarch_start(); + HandleChildCrash(pid, exitstatus, + _("archiver process")); continue; } @@ -3403,7 +3400,7 @@ CleanupBackend(int pid, /* * HandleChildCrash -- cleanup after failed backend, bgwriter, checkpointer, - * walwriter, autovacuum, or background worker. + * walwriter, autovacuum, archiver or background worker. * * The objectives here are to clean up our local state about the child * process, and to signal all other remaining children to quickdie. @@ -3609,6 +3606,18 @@ HandleChildCrash(int pid, int exitstatus, const char *procname) signal_child(AutoVacPID, (SendStop ? SIGSTOP : SIGQUIT)); } + /* Take care of the archiver too */ + if (pid == PgArchPID) + PgArchPID = 0; + else if (PgArchPID != 0 && take_action) + { + ereport(DEBUG2, + (errmsg_internal("sending %s to process %d", + (SendStop ? "SIGSTOP" : "SIGQUIT"), + (int) PgArchPID))); + signal_child(PgArchPID, (SendStop ? SIGSTOP : SIGQUIT)); + } + /* * Force a power-cycle of the pgarch process too. (This isn't absolutely * necessary, but it seems like a good idea for robustness, and it @@ -3912,6 +3921,7 @@ PostmasterStateMachine(void) Assert(CheckpointerPID == 0); Assert(WalWriterPID == 0); Assert(AutoVacPID == 0); + Assert(PgArchPID == 0); /* syslogger is not considered here */ pmState = PM_NO_CHILDREN; } @@ -5140,7 +5150,7 @@ sigusr1_handler(SIGNAL_ARGS) */ Assert(PgArchPID == 0); if (XLogArchivingAlways()) - PgArchPID = pgarch_start(); + PgArchPID = StartArchiver(); /* * If we aren't planning to enter hot standby mode later, treat @@ -5194,16 +5204,6 @@ sigusr1_handler(SIGNAL_ARGS) if (StartWorkerNeeded || HaveCrashedWorker) maybe_start_bgworkers(); - if (CheckPostmasterSignal(PMSIGNAL_WAKEN_ARCHIVER) && - PgArchPID != 0) - { - /* - * Send SIGUSR1 to archiver process, to wake it up and begin archiving - * next WAL file. - */ - signal_child(PgArchPID, SIGUSR1); - } - /* Tell syslogger to rotate logfile if requested */ if (SysLoggerPID != 0) { @@ -5445,6 +5445,10 @@ StartChildProcess(AuxProcType type) ereport(LOG, (errmsg("could not fork startup process: %m"))); break; + case ArchiverProcess: + ereport(LOG, + (errmsg("could not fork archiver process: %m"))); + break; case BgWriterProcess: ereport(LOG, (errmsg("could not fork background writer process: %m"))); diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 897045ee27..bf5d6da89b 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -182,6 +182,7 @@ InitProcGlobal(void) ProcGlobal->startupBufferPinWaitBufId = -1; ProcGlobal->walwriterLatch = NULL; ProcGlobal->checkpointerLatch = NULL; + ProcGlobal->archiverLatch = NULL; pg_atomic_init_u32(&ProcGlobal->procArrayGroupFirst, INVALID_PGPROCNO); pg_atomic_init_u32(&ProcGlobal->clogGroupFirst, INVALID_PGPROCNO); diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 75ec1073bd..551f518cc2 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -339,6 +339,9 @@ extern XLogRecPtr GetRedoRecPtr(void); extern XLogRecPtr GetInsertRecPtr(void); extern XLogRecPtr GetFlushRecPtr(void); extern XLogRecPtr GetLastImportantRecPtr(void); +extern void XLogArchiveWakeupStart(void); +extern void XLogArchiveWakeupEnd(void); +extern void XLogArchiveWakeup(void); extern void RemovePromoteSignalFiles(void); extern bool PromoteIsTriggered(void); diff --git a/src/include/access/xlogarchive.h b/src/include/access/xlogarchive.h index 3edd1a976c..1a59181cf9 100644 --- a/src/include/access/xlogarchive.h +++ b/src/include/access/xlogarchive.h @@ -25,6 +25,7 @@ extern void ExecuteRecoveryCommand(const char *command, const char *commandName, extern void KeepFileRestoredFromArchive(const char *path, const char *xlogfname); extern void XLogArchiveNotify(const char *xlog); extern void XLogArchiveNotifySeg(XLogSegNo segno); +extern void XLogArchiveWakeup(void); extern void XLogArchiveForceDone(const char *xlog); extern bool XLogArchiveCheckDone(const char *xlog); extern bool XLogArchiveIsBusy(const char *xlog); diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index 1bdc97e308..adb9f819bb 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -419,6 +419,7 @@ typedef enum BootstrapProcess, StartupProcess, BgWriterProcess, + ArchiverProcess, CheckpointerProcess, WalWriterProcess, WalReceiverProcess, @@ -431,6 +432,7 @@ extern AuxProcType MyAuxProcType; #define AmBootstrapProcess() (MyAuxProcType == BootstrapProcess) #define AmStartupProcess() (MyAuxProcType == StartupProcess) #define AmBackgroundWriterProcess() (MyAuxProcType == BgWriterProcess) +#define AmArchiverProcess() (MyAuxProcType == ArchiverProcess) #define AmCheckpointerProcess() (MyAuxProcType == CheckpointerProcess) #define AmWalWriterProcess() (MyAuxProcType == WalWriterProcess) #define AmWalReceiverProcess() (MyAuxProcType == WalReceiverProcess) diff --git a/src/include/postmaster/pgarch.h b/src/include/postmaster/pgarch.h index d102a21ab7..385b002dfe 100644 --- a/src/include/postmaster/pgarch.h +++ b/src/include/postmaster/pgarch.h @@ -32,8 +32,6 @@ */ extern int pgarch_start(void); -#ifdef EXEC_BACKEND -extern void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn(); -#endif +extern void PgArchiverMain(void) pg_attribute_noreturn(); #endif /* _PGARCH_H */ diff --git a/src/include/storage/pmsignal.h b/src/include/storage/pmsignal.h index dbbed18f61..8ed4d87ae6 100644 --- a/src/include/storage/pmsignal.h +++ b/src/include/storage/pmsignal.h @@ -34,7 +34,6 @@ typedef enum { PMSIGNAL_RECOVERY_STARTED, /* recovery has started */ PMSIGNAL_BEGIN_HOT_STANDBY, /* begin Hot Standby */ - PMSIGNAL_WAKEN_ARCHIVER, /* send a NOTIFY signal to xlog archiver */ PMSIGNAL_ROTATE_LOGFILE, /* send SIGUSR1 to syslogger to rotate logfile */ PMSIGNAL_START_AUTOVAC_LAUNCHER, /* start an autovacuum launcher */ PMSIGNAL_START_AUTOVAC_WORKER, /* start an autovacuum worker */ diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index a777cb64a1..491747c113 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -357,6 +357,9 @@ typedef struct PROC_HDR int startupProcPid; /* Buffer id of the buffer that Startup process waits for pin on, or -1 */ int startupBufferPinWaitBufId; + /* Archiver process's latch */ + Latch *archiverLatch; + /* Current shared estimate of appropriate spins_per_delay value */ } PROC_HDR; extern PGDLLIMPORT PROC_HDR *ProcGlobal; -- 2.27.0 ----Next_Part(Fri_Mar__5_17_18_56_2021_497)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v48-0004-Shared-memory-based-stats-collector.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Set query_id for query contained in utility statement @ 2024-08-26 03:26 jian he <[email protected]> 2024-08-26 08:54 ` Re: Set query_id for query contained in utility statement Anthonin Bonnefoy <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: jian he @ 2024-08-26 03:26 UTC (permalink / raw) To: Anthonin Bonnefoy <[email protected]>; +Cc: pgsql-hackers On Mon, Aug 5, 2024 at 3:19 PM Anthonin Bonnefoy <[email protected]> wrote: > > I've realised my initial approach was wrong, calling the post parse > for all nested queries in analyze.c prevents extension like pgss to > correctly track the query's nesting level. > > I've changed the approach to replicate what's done in ExplainQuery to > both CreateTableAs and DeclareCursor: Jumble the query contained by > the utility statement and call the post parse hook before it is > planned or executed. Additionally, explain's nested query can itself > be a CreateTableAs or DeclareCursor which also needs to be jumbled. > The issue is visible when explaining a CreateTableAs or DeclareCursor > Query, the queryId is missing despite the verbose. > > EXPLAIN (verbose) create table test_t as select 1; > QUERY PLAN > ------------------------------------------ > Result (cost=0.00..0.01 rows=1 width=4) > Output: 1 > > Post patch, the query id is correctly displayed. > > EXPLAIN (verbose) create table test_t as select 1; > QUERY PLAN > ------------------------------------------ > Result (cost=0.00..0.01 rows=1 width=4) > Output: 1 > Query Identifier: 2800308901962295548 > play with pg_stat_statements. settings: name | setting -----------------------------------+--------- pg_stat_statements.max | 5000 pg_stat_statements.save | on pg_stat_statements.track | all pg_stat_statements.track_planning | on pg_stat_statements.track_utility | on SELECT pg_stat_statements_reset(); select 1; select 2; SELECT queryid, left(query, 60),plans, calls, rows FROM pg_stat_statements ORDER BY calls DESC LIMIT 5; returns: queryid | left | plans | calls | rows ----------------------+--------------------------------------------------------------+-------+-------+------ 2800308901962295548 | select $1 | 2 | 2 | 2 The output is what we expect. now after applying your patch. SELECT pg_stat_statements_reset(); EXPLAIN (verbose) create table test_t as select 1; EXPLAIN (verbose) create table test_t as select 2; SELECT queryid, left(query, 60),plans, calls, rows FROM pg_stat_statements ORDER BY calls DESC LIMIT 5; the output is: queryid | left | plans | calls | rows ----------------------+--------------------------------------------------------------+-------+-------+------ 2800308901962295548 | EXPLAIN (verbose) create table test_t as select 1; | 2 | 2 | 0 2093602470903273926 | EXPLAIN (verbose) create table test_t as select $1 | 0 | 2 | 0 -2694081619397734273 | SELECT pg_stat_statements_reset() | 0 | 1 | 1 2643570658797872815 | SELECT queryid, left(query, $1),plans, calls, rows FROM pg_s | 1 | 0 | 0 "EXPLAIN (verbose) create table test_t as select 1;" called twice, is that what we expect? should first row, the normalized query becomes EXPLAIN (verbose) create table test_t as select $1; ? ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Set query_id for query contained in utility statement 2024-08-26 03:26 Re: Set query_id for query contained in utility statement jian he <[email protected]> @ 2024-08-26 08:54 ` Anthonin Bonnefoy <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Anthonin Bonnefoy @ 2024-08-26 08:54 UTC (permalink / raw) To: jian he <[email protected]>; +Cc: pgsql-hackers On Mon, Aug 26, 2024 at 5:26 AM jian he <[email protected]> wrote: > queryid | left > | plans | calls | rows > ----------------------+--------------------------------------------------------------+-------+-------+------ > 2800308901962295548 | EXPLAIN (verbose) create table test_t as > select 1; | 2 | 2 | 0 > 2093602470903273926 | EXPLAIN (verbose) create table test_t as > select $1 | 0 | 2 | 0 > > "EXPLAIN (verbose) create table test_t as select 1;" called twice, > is that what we expect? pg_stat_statements reports nested queries and toplevel queries separately. You can check with the toplevel column. 2800308901962295548 is the nested ctas part while 2093602470903273926 is the top explain utility statement (which explain why there's 0 plans since it's an utility statement). Since the explain ctas query was called twice, it will be reported as 2 toplevel queries and 2 nested queries. > should first row, the normalized query becomes > EXPLAIN (verbose) create table test_t as select $1; Good point, the issue in this case was the nested query was stored by pg_stat_statements during the ExecutorEnd hook. This hook doesn't have the jstate and parseState at that point so pg_stat_statements can't normalize the query. I've modified the patch to fix this. Instead of just jumbling the query in ExplainQuery, I've moved jumbling in ExplainOneUtility which already has specific code to handle ctas and dcs. Calling the post parse hook here allows pg_stat_statements to store the normalized version of the query for this queryid and nesting level. Attachments: [application/octet-stream] v3-0001-Set-query_id-for-queries-contained-in-utility-sta.patch (28.3K, ../../CAO6_Xqo5J-DP4aRGUc5J6pu5sSTgjFdNHQ4sWz+NbZx5VGFPOg@mail.gmail.com/2-v3-0001-Set-query_id-for-queries-contained-in-utility-sta.patch) download | inline diff: From 8719d2184affb9ecd69b4c1e7c204108b76d1b76 Mon Sep 17 00:00:00 2001 From: Anthonin Bonnefoy <[email protected]> Date: Tue, 23 Jul 2024 08:26:49 +0200 Subject: Set query_id for queries contained in utility statement Some utility statements like Explain, CreateTableAs and DeclareCursor contain a query which will be planned and executed. During post parse, only the top utility statement is jumbled, leaving the contained query without a set query_id. ExplainQuery does jumble the contained query but CreateTableAs and DeclareCursor do not. This led to extensions relying on query_id like pg_stat_statements to not be able to track those nested queries as the query_id was 0. This patch fixes this by jumbling the nested query of CreateTableAs and DeclareCursor before it is executed. Additionally, explain's nested query can itself be a CreateTableAs or DeclareCursor which also needs to be jumbled which is now done in ExplainOneUtility. --- .../expected/level_tracking.out | 255 ++++++++++++++++++ .../pg_stat_statements/sql/level_tracking.sql | 83 ++++++ src/backend/commands/createas.c | 10 + src/backend/commands/explain.c | 45 ++-- src/backend/commands/portalcmds.c | 10 + src/backend/commands/prepare.c | 18 +- src/include/commands/explain.h | 4 +- src/include/commands/prepare.h | 4 +- src/test/regress/expected/explain.out | 17 ++ src/test/regress/sql/explain.sql | 4 + 10 files changed, 415 insertions(+), 35 deletions(-) diff --git a/contrib/pg_stat_statements/expected/level_tracking.out b/contrib/pg_stat_statements/expected/level_tracking.out index d8dd8a2deea..6291f06d711 100644 --- a/contrib/pg_stat_statements/expected/level_tracking.out +++ b/contrib/pg_stat_statements/expected/level_tracking.out @@ -112,6 +112,261 @@ SELECT toplevel, calls, query FROM pg_stat_statements t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t (2 rows) +-- Create Table As, all-level tracking. +SET pg_stat_statements.track = 'all'; +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +CREATE TEMPORARY TABLE pgss_test AS SELECT 1; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + toplevel | calls | query +----------+-------+---------------------------------------------------- + t | 1 | CREATE TEMPORARY TABLE pgss_test AS SELECT $1 + f | 1 | CREATE TEMPORARY TABLE pgss_test AS SELECT $1; + t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t +(3 rows) + +-- Create Table As using prepared stmt, all-level tracking. +PREPARE test_prepare_pgss AS select generate_series(1, 10); +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +CREATE TEMPORARY TABLE pgss_test2 AS EXECUTE test_prepare_pgss; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + toplevel | calls | query +----------+-------+---------------------------------------------------------------- + t | 1 | CREATE TEMPORARY TABLE pgss_test2 AS EXECUTE test_prepare_pgss + f | 1 | PREPARE test_prepare_pgss AS select generate_series(1, 10) + t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t +(3 rows) + +-- Declare cursor, all-level tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +BEGIN; +DECLARE FOOCUR CURSOR FOR SELECT * from stats_track_tab; +FETCH FORWARD 1 FROM foocur; + x +--- +(0 rows) + +CLOSE foocur; +COMMIT; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + toplevel | calls | query +----------+-------+---------------------------------------------------------- + t | 1 | BEGIN + t | 1 | CLOSE foocur + t | 1 | COMMIT + t | 1 | DECLARE FOOCUR CURSOR FOR SELECT * from stats_track_tab + f | 1 | DECLARE FOOCUR CURSOR FOR SELECT * from stats_track_tab; + t | 1 | FETCH FORWARD 1 FROM foocur + t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t +(7 rows) + +-- Explain analyze, all-level tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT 100; + QUERY PLAN +-------------------------------- + Result (actual rows=1 loops=1) +(1 row) + +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + toplevel | calls | query +----------+-------+------------------------------------------------------------------ + t | 1 | EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT $1 + f | 1 | EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT $1; + t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t +(3 rows) + +-- Explain analyze with declare cursor, all-level tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) DECLARE foocur CURSOR FOR SELECT * FROM stats_track_tab; + QUERY PLAN +----------------------------------------------------- + Seq Scan on stats_track_tab (actual rows=0 loops=1) +(1 row) + +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + toplevel | calls | query +----------+-------+---------------------------------------------------------------------------------------------------------------- + t | 1 | EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) DECLARE foocur CURSOR FOR SELECT * FROM stats_track_tab + f | 1 | EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) DECLARE foocur CURSOR FOR SELECT * FROM stats_track_tab; + t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t +(3 rows) + +-- Explain with ctas, all-level tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +EXPLAIN (COSTS OFF, SUMMARY OFF, TIMING OFF) CREATE TABLE pgss_test_3 AS SELECT 1; + QUERY PLAN +------------ + Result +(1 row) + +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + toplevel | calls | query +----------+-------+------------------------------------------------------------------------------------- + t | 1 | EXPLAIN (COSTS OFF, SUMMARY OFF, TIMING OFF) CREATE TABLE pgss_test_3 AS SELECT $1 + f | 1 | EXPLAIN (COSTS OFF, SUMMARY OFF, TIMING OFF) CREATE TABLE pgss_test_3 AS SELECT $1; + t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t +(3 rows) + +-- Create Table As, top-level tracking. +SET pg_stat_statements.track = 'top'; +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +CREATE TABLE pgss_test_4 AS SELECT 1; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + toplevel | calls | query +----------+-------+---------------------------------------------------- + t | 1 | CREATE TABLE pgss_test_4 AS SELECT $1 + t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t +(2 rows) + +-- Create Table As using prepared stmt, top tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +CREATE TEMPORARY TABLE pgss_test5 AS EXECUTE test_prepare_pgss; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + toplevel | calls | query +----------+-------+---------------------------------------------------------------- + t | 1 | CREATE TEMPORARY TABLE pgss_test5 AS EXECUTE test_prepare_pgss + t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t +(2 rows) + +-- Declare cursor, top tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +BEGIN; +DECLARE FOOCUR CURSOR FOR SELECT * from stats_track_tab; +FETCH FORWARD 1 FROM foocur; + x +--- +(0 rows) + +CLOSE foocur; +COMMIT; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + toplevel | calls | query +----------+-------+--------------------------------------------------------- + t | 1 | BEGIN + t | 1 | CLOSE foocur + t | 1 | COMMIT + t | 1 | DECLARE FOOCUR CURSOR FOR SELECT * from stats_track_tab + t | 1 | FETCH FORWARD 1 FROM foocur + t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t +(6 rows) + +-- Explain analyze, top tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT 100; + QUERY PLAN +-------------------------------- + Result (actual rows=1 loops=1) +(1 row) + +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + toplevel | calls | query +----------+-------+----------------------------------------------------------------- + t | 1 | EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT $1 + t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t +(2 rows) + +-- Explain analyze with declare cursor, top tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) DECLARE foocur CURSOR FOR SELECT * FROM stats_track_tab; + QUERY PLAN +----------------------------------------------------- + Seq Scan on stats_track_tab (actual rows=0 loops=1) +(1 row) + +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + toplevel | calls | query +----------+-------+--------------------------------------------------------------------------------------------------------------- + t | 1 | EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) DECLARE foocur CURSOR FOR SELECT * FROM stats_track_tab + t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t +(2 rows) + +-- Explain with ctas, top-level tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +EXPLAIN (COSTS OFF, SUMMARY OFF, TIMING OFF) CREATE TABLE pgss_test_3 AS SELECT 1; + QUERY PLAN +------------ + Result +(1 row) + +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + toplevel | calls | query +----------+-------+------------------------------------------------------------------------------------ + t | 1 | EXPLAIN (COSTS OFF, SUMMARY OFF, TIMING OFF) CREATE TABLE pgss_test_3 AS SELECT $1 + t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t +(2 rows) + -- DO block - top-level tracking without utility. SET pg_stat_statements.track = 'top'; SET pg_stat_statements.track_utility = FALSE; diff --git a/contrib/pg_stat_statements/sql/level_tracking.sql b/contrib/pg_stat_statements/sql/level_tracking.sql index 65a17147a5a..2fc7054a9c6 100644 --- a/contrib/pg_stat_statements/sql/level_tracking.sql +++ b/contrib/pg_stat_statements/sql/level_tracking.sql @@ -55,6 +55,89 @@ CALL proc_with_utility_stmt(); SELECT toplevel, calls, query FROM pg_stat_statements ORDER BY query COLLATE "C", toplevel; +-- Create Table As, all-level tracking. +SET pg_stat_statements.track = 'all'; +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +CREATE TEMPORARY TABLE pgss_test AS SELECT 1; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + +-- Create Table As using prepared stmt, all-level tracking. +PREPARE test_prepare_pgss AS select generate_series(1, 10); +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +CREATE TEMPORARY TABLE pgss_test2 AS EXECUTE test_prepare_pgss; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + +-- Declare cursor, all-level tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +BEGIN; +DECLARE FOOCUR CURSOR FOR SELECT * from stats_track_tab; +FETCH FORWARD 1 FROM foocur; +CLOSE foocur; +COMMIT; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + +-- Explain analyze, all-level tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT 100; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + +-- Explain analyze with declare cursor, all-level tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) DECLARE foocur CURSOR FOR SELECT * FROM stats_track_tab; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + +-- Explain with ctas, all-level tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +EXPLAIN (COSTS OFF, SUMMARY OFF, TIMING OFF) CREATE TABLE pgss_test_3 AS SELECT 1; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + +-- Create Table As, top-level tracking. +SET pg_stat_statements.track = 'top'; +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +CREATE TABLE pgss_test_4 AS SELECT 1; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + +-- Create Table As using prepared stmt, top tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +CREATE TEMPORARY TABLE pgss_test5 AS EXECUTE test_prepare_pgss; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + +-- Declare cursor, top tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +BEGIN; +DECLARE FOOCUR CURSOR FOR SELECT * from stats_track_tab; +FETCH FORWARD 1 FROM foocur; +CLOSE foocur; +COMMIT; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + +-- Explain analyze, top tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT 100; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + +-- Explain analyze with declare cursor, top tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) DECLARE foocur CURSOR FOR SELECT * FROM stats_track_tab; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + +-- Explain with ctas, top-level tracking. +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +EXPLAIN (COSTS OFF, SUMMARY OFF, TIMING OFF) CREATE TABLE pgss_test_3 AS SELECT 1; +SELECT toplevel, calls, query FROM pg_stat_statements + ORDER BY query COLLATE "C", toplevel; + -- DO block - top-level tracking without utility. SET pg_stat_statements.track = 'top'; SET pg_stat_statements.track_utility = FALSE; diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c index 0b629b1f79c..90bb964cd89 100644 --- a/src/backend/commands/createas.c +++ b/src/backend/commands/createas.c @@ -38,6 +38,8 @@ #include "miscadmin.h" #include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" +#include "nodes/queryjumble.h" +#include "parser/analyze.h" #include "rewrite/rewriteHandler.h" #include "tcop/tcopprot.h" #include "utils/builtins.h" @@ -224,6 +226,7 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt, { Query *query = castNode(Query, stmt->query); IntoClause *into = stmt->into; + JumbleState *jstate = NULL; bool is_matview = (into->viewQuery != NULL); bool do_refresh = false; DestReceiver *dest; @@ -238,6 +241,13 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt, */ dest = CreateIntoRelDestReceiver(into); + /* Query contained by CTAS needs to be jumbled if requested */ + if (IsQueryIdEnabled()) + jstate = JumbleQuery(query); + + if (post_parse_analyze_hook) + (*post_parse_analyze_hook) (pstate, query, jstate); + /* * The contained Query could be a SELECT, or an EXECUTE utility command. * If the latter, we just pass it off to ExecuteQuery. diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 11df4a04d43..cd42fb1b5a0 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -71,8 +71,7 @@ typedef struct SerializeMetrics static void ExplainOneQuery(Query *query, int cursorOptions, IntoClause *into, ExplainState *es, - const char *queryString, ParamListInfo params, - QueryEnvironment *queryEnv); + ParseState *pstate, ParamListInfo params); static void ExplainPrintJIT(ExplainState *es, int jit_flags, JitInstrumentation *ji); static void ExplainPrintSerialize(ExplainState *es, @@ -319,7 +318,7 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, * came straight from the parser, or suitable locks were acquired by * plancache.c. */ - rewritten = QueryRewrite(castNode(Query, stmt->query)); + rewritten = QueryRewrite(query); /* emit opening boilerplate */ ExplainBeginOutput(es); @@ -342,7 +341,7 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, { ExplainOneQuery(lfirst_node(Query, l), CURSOR_OPT_PARALLEL_OK, NULL, es, - pstate->p_sourcetext, params, pstate->p_queryEnv); + pstate, params); /* Separate plans with an appropriate separator */ if (lnext(rewritten, l) != NULL) @@ -428,24 +427,22 @@ ExplainResultDesc(ExplainStmt *stmt) static void ExplainOneQuery(Query *query, int cursorOptions, IntoClause *into, ExplainState *es, - const char *queryString, ParamListInfo params, - QueryEnvironment *queryEnv) + ParseState *pstate, ParamListInfo params) { /* planner will not cope with utility statements */ if (query->commandType == CMD_UTILITY) { - ExplainOneUtility(query->utilityStmt, into, es, queryString, params, - queryEnv); + ExplainOneUtility(query->utilityStmt, into, es, pstate, params); return; } /* if an advisor plugin is present, let it manage things */ if (ExplainOneQuery_hook) (*ExplainOneQuery_hook) (query, cursorOptions, into, es, - queryString, params, queryEnv); + pstate->p_sourcetext, params, pstate->p_queryEnv); else standard_ExplainOneQuery(query, cursorOptions, into, es, - queryString, params, queryEnv); + pstate->p_sourcetext, params, pstate->p_queryEnv); } /* @@ -526,9 +523,10 @@ standard_ExplainOneQuery(Query *query, int cursorOptions, */ void ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es, - const char *queryString, ParamListInfo params, - QueryEnvironment *queryEnv) + ParseState *pstate, ParamListInfo params) { + JumbleState *jstate = NULL; + if (utilityStmt == NULL) return; @@ -539,6 +537,7 @@ ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es, * ExplainOneQuery. Copy to be safe in the EXPLAIN EXECUTE case. */ CreateTableAsStmt *ctas = (CreateTableAsStmt *) utilityStmt; + Query *ctas_query; List *rewritten; /* @@ -557,11 +556,16 @@ ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es, return; } - rewritten = QueryRewrite(castNode(Query, copyObject(ctas->query))); + ctas_query = castNode(Query, copyObject(ctas->query)); + if (IsQueryIdEnabled()) + jstate = JumbleQuery(ctas_query); + if (post_parse_analyze_hook) + (*post_parse_analyze_hook) (pstate, ctas_query, jstate); + rewritten = QueryRewrite(ctas_query); Assert(list_length(rewritten) == 1); ExplainOneQuery(linitial_node(Query, rewritten), CURSOR_OPT_PARALLEL_OK, ctas->into, es, - queryString, params, queryEnv); + pstate, params); } else if (IsA(utilityStmt, DeclareCursorStmt)) { @@ -574,17 +578,24 @@ ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es, * be created, however. */ DeclareCursorStmt *dcs = (DeclareCursorStmt *) utilityStmt; + Query *dcs_query; List *rewritten; - rewritten = QueryRewrite(castNode(Query, copyObject(dcs->query))); + dcs_query = castNode(Query, copyObject(dcs->query)); + if (IsQueryIdEnabled()) + jstate = JumbleQuery(dcs_query); + if (post_parse_analyze_hook) + (*post_parse_analyze_hook) (pstate, dcs_query, jstate); + + rewritten = QueryRewrite(dcs_query); Assert(list_length(rewritten) == 1); ExplainOneQuery(linitial_node(Query, rewritten), dcs->options, NULL, es, - queryString, params, queryEnv); + pstate, params); } else if (IsA(utilityStmt, ExecuteStmt)) ExplainExecuteQuery((ExecuteStmt *) utilityStmt, into, es, - queryString, params, queryEnv); + pstate, params); else if (IsA(utilityStmt, NotifyStmt)) { if (es->format == EXPLAIN_FORMAT_TEXT) diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c index 4f6acf67198..ac52ca25e99 100644 --- a/src/backend/commands/portalcmds.c +++ b/src/backend/commands/portalcmds.c @@ -28,6 +28,8 @@ #include "executor/executor.h" #include "executor/tstoreReceiver.h" #include "miscadmin.h" +#include "nodes/queryjumble.h" +#include "parser/analyze.h" #include "rewrite/rewriteHandler.h" #include "tcop/pquery.h" #include "tcop/tcopprot.h" @@ -44,6 +46,7 @@ PerformCursorOpen(ParseState *pstate, DeclareCursorStmt *cstmt, ParamListInfo pa bool isTopLevel) { Query *query = castNode(Query, cstmt->query); + JumbleState *jstate = NULL; List *rewritten; PlannedStmt *plan; Portal portal; @@ -71,6 +74,13 @@ PerformCursorOpen(ParseState *pstate, DeclareCursorStmt *cstmt, ParamListInfo pa (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("cannot create a cursor WITH HOLD within security-restricted operation"))); + /* Query contained by DeclareCursor needs to be jumbled if requested */ + if (IsQueryIdEnabled()) + jstate = JumbleQuery(query); + + if (post_parse_analyze_hook) + (*post_parse_analyze_hook) (pstate, query, jstate); + /* * Parse analysis was done already, but we still have to run the rule * rewriter. We do not do AcquireRewriteLocks: we assume the query either diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c index 07257d4db94..e141ecf9577 100644 --- a/src/backend/commands/prepare.c +++ b/src/backend/commands/prepare.c @@ -566,11 +566,9 @@ DropAllPreparedStatements(void) */ void ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es, - const char *queryString, ParamListInfo params, - QueryEnvironment *queryEnv) + ParseState *pstate, ParamListInfo params) { PreparedStatement *entry; - const char *query_string; CachedPlan *cplan; List *plan_list; ListCell *p; @@ -605,16 +603,9 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es, if (!entry->plansource->fixed_result) elog(ERROR, "EXPLAIN EXECUTE does not support variable-result cached plans"); - query_string = entry->plansource->query_string; - /* Evaluate parameters, if any */ if (entry->plansource->num_params) { - ParseState *pstate; - - pstate = make_parsestate(NULL); - pstate->p_sourcetext = queryString; - /* * Need an EState to evaluate parameters; must not delete it till end * of query, in case parameters are pass-by-reference. Note that the @@ -629,7 +620,7 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es, /* Replan if needed, and acquire a transient refcount */ cplan = GetCachedPlan(entry->plansource, paramLI, - CurrentResourceOwner, queryEnv); + CurrentResourceOwner, pstate->p_queryEnv); INSTR_TIME_SET_CURRENT(planduration); INSTR_TIME_SUBTRACT(planduration, planstart); @@ -655,12 +646,11 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es, PlannedStmt *pstmt = lfirst_node(PlannedStmt, p); if (pstmt->commandType != CMD_UTILITY) - ExplainOnePlan(pstmt, into, es, query_string, paramLI, queryEnv, + ExplainOnePlan(pstmt, into, es, pstate->p_sourcetext, paramLI, pstate->p_queryEnv, &planduration, (es->buffers ? &bufusage : NULL), es->memory ? &mem_counters : NULL); else - ExplainOneUtility(pstmt->utilityStmt, into, es, query_string, - paramLI, queryEnv); + ExplainOneUtility(pstmt->utilityStmt, into, es, pstate, paramLI); /* No need for CommandCounterIncrement, as ExplainOnePlan did it */ diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h index 9b8b351d9a2..0fd3933a17a 100644 --- a/src/include/commands/explain.h +++ b/src/include/commands/explain.h @@ -98,8 +98,8 @@ extern ExplainState *NewExplainState(void); extern TupleDesc ExplainResultDesc(ExplainStmt *stmt); extern void ExplainOneUtility(Node *utilityStmt, IntoClause *into, - ExplainState *es, const char *queryString, - ParamListInfo params, QueryEnvironment *queryEnv); + ExplainState *es, ParseState *pstate, + ParamListInfo params); extern void ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es, const char *queryString, diff --git a/src/include/commands/prepare.h b/src/include/commands/prepare.h index 61472c111d6..e6fd400e027 100644 --- a/src/include/commands/prepare.h +++ b/src/include/commands/prepare.h @@ -43,8 +43,8 @@ extern void ExecuteQuery(ParseState *pstate, DestReceiver *dest, QueryCompletion *qc); extern void DeallocateQuery(DeallocateStmt *stmt); extern void ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, - ExplainState *es, const char *queryString, - ParamListInfo params, QueryEnvironment *queryEnv); + ExplainState *es, ParseState *pstate, + ParamListInfo params); /* Low-level access to stored prepared statements */ extern void StorePreparedStatement(const char *stmt_name, diff --git a/src/test/regress/expected/explain.out b/src/test/regress/expected/explain.out index 6585c6a69ef..e01ed569989 100644 --- a/src/test/regress/expected/explain.out +++ b/src/test/regress/expected/explain.out @@ -653,6 +653,23 @@ select explain_filter('explain (verbose) select * from int8_tbl i8'); Query Identifier: N (3 rows) +-- Test compute_query_id with utility statements containing plannable query +select explain_filter('explain (verbose) declare test_cur cursor for select * from int8_tbl'); + explain_filter +------------------------------------------------------------- + Seq Scan on public.int8_tbl (cost=N.N..N.N rows=N width=N) + Output: q1, q2 + Query Identifier: N +(3 rows) + +select explain_filter('explain (verbose) create table test_ctas as select 1'); + explain_filter +---------------------------------------- + Result (cost=N.N..N.N rows=N width=N) + Output: N + Query Identifier: N +(3 rows) + -- Test SERIALIZE option select explain_filter('explain (analyze,serialize) select * from int8_tbl i8'); explain_filter diff --git a/src/test/regress/sql/explain.sql b/src/test/regress/sql/explain.sql index c7055f850c5..5796137c7ff 100644 --- a/src/test/regress/sql/explain.sql +++ b/src/test/regress/sql/explain.sql @@ -163,6 +163,10 @@ select explain_filter('explain (verbose) select * from t1 where pg_temp.mysin(f1 set compute_query_id = on; select explain_filter('explain (verbose) select * from int8_tbl i8'); +-- Test compute_query_id with utility statements containing plannable query +select explain_filter('explain (verbose) declare test_cur cursor for select * from int8_tbl'); +select explain_filter('explain (verbose) create table test_ctas as select 1'); + -- Test SERIALIZE option select explain_filter('explain (analyze,serialize) select * from int8_tbl i8'); select explain_filter('explain (analyze,serialize text,buffers,timing off) select * from int8_tbl i8'); -- 2.39.3 (Apple Git-146) ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2024-08-26 08:54 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-03-13 07:59 [PATCH v48 3/7] Make archiver process an auxiliary process Kyotaro Horiguchi <[email protected]> 2024-08-26 03:26 Re: Set query_id for query contained in utility statement jian he <[email protected]> 2024-08-26 08:54 ` Re: Set query_id for query contained in utility statement Anthonin Bonnefoy <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox