Hi čt 2. 7. 2026 v 9:04 odesílatel Kyotaro Horiguchi <[email protected]> napsal:
> At Thu, 2 Jul 2026 06:50:21 +0200, Pavel Stehule <[email protected]> > wrote in > > Unfortunately EnableQueryId is not reversible. > > > > So when users force EnableQueryId by non-empty log_queryids, then can be > > confused > > when trying to set empty log_queryid, but the queryid will be computed > > until the end of session. > > > > If I can switch queryid computing to between on, off, then this can be > good > > idea, but becase > > I can only enable computing, then I don't like it too much. > > There is also the opposite way of looking at it. > > With the current behavior, simply loading auto_explain causes query ID > calculation to be requested, even if the session never intends to use > query IDs. Disabling that requires setting compute_query_id to off, > which also affects other modules. That seems less natural to me than > keeping query ID calculation enabled after log_queryids has been > enabled once in the session. > > To me, requesting query ID calculation only when auto_explain is > actually configured to use it better matches the user's expectation, > even if the request itself is not reversible. > > Of course, if others still think that enabling query ID calculation at > module load time is the better choice, then I don't have any strong > objection. > I understand - and the main problem is fact so query ID calculation doesn't allow more accurate management. I think so any way is legitime - just depends what is preferred - robustness or effectivity There is not any precedent in core - for pg_stat_statements and pg_stash_advice the queryid is required always. So I see a few possibilities: 1. EnableQueryId on start - it is the generic most simple solution - in my experience - it is only safe guard for cases, where auto_explain is used without pg_stat_statements (which is not too common). Negative impact - queryid will be calculated although it can be useless (until somebody explicitly disables it). But it is only in cases where queryid is not required by another module. 2. Delayed call EnableQueryId - called in GUC assignment - it should work too. I intuitively dislike it, because it is only a one way switch. 3. Don't call EnableQueryId in auto_explain - we can document - if log_queryids is used, then compute_queryid should be ON. 4. Introduction of a new API that allows accurate queryid computing management. Any module can signalize dynamically if a queryid is required. This needs a deeper discussion. Please, everybody, what do you prefer? > > > > My comment was about the ordering of the checks in the proposed patch. > > > If queryids_filter is empty, no query ID lookup is needed to decide > > > whether the query passes the filter. Likewise, if the duration check > > > fails, the filter result does not matter. So I was wondering whether > > > the decision logic could be arranged to avoid query ID lookups in > > > those cases. > > > > > > > do you think some like > > > > if (!queryid_filter || log_filter) && (msec >= > > auto_explain_log_min_duration)) > > > > ?? > > Not quite. My point was that if auto_explain_log_min_duration already > rejects the query, then any query ID lookup is unnecessary. > > So I was thinking more along these lines: > > if (msec >= auto_explain_log_min_duration) > { > if (queryids_filter_is_empty || queryid_matches_filter()) > log_plan(); > } > > The important part is that queryid_matches_filter() should only be > called after the duration check has passed and only when the filter is > not empty. > > The helper function above is just for illustration. I don't mean to > suggest introducing such a function; I'm perfectly fine with keeping the > decision logic open-coded if that fits the existing code better. > understand - changed Regards Pavel > > Regards, > > -- > Kyotaro Horiguchi > NTT Open Source Software Center >
From 222789991352d2983d1920e99970cc6e10c20c9b Mon Sep 17 00:00:00 2001 From: "[email protected]" <[email protected]> Date: Sun, 14 Jun 2026 19:43:04 +0200 Subject: [PATCH] auto_explain.log_queryids this patch implements new auto_explain configuration - auto_explain.log_queryids. It allow to specify list of queryid. Only plans of queries with queryid from specified list will be logged. This allows to use auto_explain in higher load environment for monitoring problematic or extra important queries with lower risk of high log bloating. --- contrib/auto_explain/auto_explain.c | 156 ++++++++++++++++++++- contrib/auto_explain/t/001_auto_explain.pl | 53 +++++++ doc/src/sgml/auto-explain.sgml | 20 +++ 3 files changed, 227 insertions(+), 2 deletions(-) diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c index 97ce498d0c1..6eb460b9050 100644 --- a/contrib/auto_explain/auto_explain.c +++ b/contrib/auto_explain/auto_explain.c @@ -22,8 +22,10 @@ #include "common/pg_prng.h" #include "executor/instrument.h" #include "nodes/makefuncs.h" +#include "nodes/queryjumble.h" #include "nodes/value.h" #include "parser/scansup.h" +#include "utils/builtins.h" #include "utils/guc.h" #include "utils/varlena.h" @@ -48,6 +50,7 @@ static int auto_explain_log_level = LOG; static bool auto_explain_log_nested_statements = false; static double auto_explain_sample_rate = 1; static char *auto_explain_log_extension_options = NULL; +static char *auto_explain_log_queryids = NULL; /* * Parsed form of one option from auto_explain.log_extension_options. @@ -81,6 +84,17 @@ static const struct config_enum_entry format_options[] = { {NULL, 0, false} }; +/* + * parsed form of auto_explain.log_queryids. + */ +typedef struct auto_explain_queryids +{ + int nqueryids; + int64 queryid[FLEXIBLE_ARRAY_MEMBER]; +} auto_explain_queryids; + +static auto_explain_queryids *queryid_filter = NULL; + static const struct config_enum_entry loglevel_options[] = { {"debug5", DEBUG5, false}, {"debug4", DEBUG4, false}, @@ -129,6 +143,9 @@ static int auto_explain_split_options(char *rawstring, auto_explain_option *options, int maxoptions, char **errmsg); +static bool check_log_queryids(char **newval, void **extra, GucSource source); +static void assign_log_queryids(const char *newval, void *extra); + /* * Module load callback */ @@ -307,6 +324,16 @@ _PG_init(void) NULL, NULL); + DefineCustomStringVariable("auto_explain.log_queryids", + "Only queries with queryid from list will be logged.", + NULL, + &auto_explain_log_queryids, + NULL, + PGC_SUSET, GUC_LIST_INPUT, + check_log_queryids, + assign_log_queryids, + NULL); + MarkGUCPrefixReserved("auto_explain"); /* Install hooks. */ @@ -318,6 +345,9 @@ _PG_init(void) ExecutorFinish_hook = explain_ExecutorFinish; prev_ExecutorEnd = ExecutorEnd_hook; ExecutorEnd_hook = explain_ExecutorEnd; + + /* If compute_query_id = 'auto', we would like query IDs. */ + EnableQueryId(); } /* @@ -414,6 +444,34 @@ explain_ExecutorFinish(QueryDesc *queryDesc) PG_END_TRY(); } +/* + * queryid_filter_check - returns true, when queryid is in watched query IDs + * or when list of query IDs is empty (and log_queryids is not active. + */ +static bool +queryid_filter_check(int64 queryid) +{ + if (queryid_filter) + { + if (queryid != INT64CONST(0)) + { + /* + * We expect only a few watched query IDs, so a linear search + * is sufficient. + */ + for (int i = 0; i < queryid_filter->nqueryids; i++) + { + if (queryid_filter->queryid[i] == queryid) + return true; + } + } + + return false; + } + else + return true; +} + /* * ExecutorEnd hook: log results if needed */ @@ -431,9 +489,10 @@ explain_ExecutorEnd(QueryDesc *queryDesc) */ oldcxt = MemoryContextSwitchTo(queryDesc->estate->es_query_cxt); - /* Log plan if duration is exceeded. */ + /* Log plan if all constraints are satisfied */ msec = INSTR_TIME_GET_MILLISEC(queryDesc->query_instr->total); - if (msec >= auto_explain_log_min_duration) + if (msec >= auto_explain_log_min_duration && + queryid_filter_check(queryDesc->plannedstmt->queryId)) { ExplainState *es = NewExplainState(); @@ -827,3 +886,96 @@ auto_explain_split_options(char *rawstring, auto_explain_option *options, return noptions; } + +/* + * log_queryids + */ +static bool +check_log_queryids(char **newval, void **extra, GucSource source) +{ + char *rawstring; + List *elemlist = NIL; + ListCell *l; + Size allocsize; + auto_explain_queryids *result; + int i = 0; + + if (*newval == NULL) + { + *extra = NULL; + return true; + } + + /* need a modifiable copy of string */ + rawstring = pstrdup(*newval); + + if (!SplitGUCList(rawstring, ',', &elemlist)) + { + pfree(rawstring); + list_free(elemlist); + + /* syntax error in list */ + GUC_check_errdetail("List syntax is invalid."); + return false; + } + + if (list_length(elemlist) == 0) + { + pfree(rawstring); + list_free(elemlist); + + *extra = NULL; + return true; + } + + /* Try to allocate an auto_explain_queryids object. */ + allocsize = offsetof(auto_explain_queryids, queryid) + + sizeof(int64) * list_length(elemlist); + + result = (auto_explain_queryids *) guc_malloc(LOG, allocsize); + if (result == NULL) + { + pfree(rawstring); + list_free(elemlist); + + return false; + } + + foreach(l, elemlist) + { + char *tok = (char *) lfirst(l); + int64 queryid; + ErrorSaveContext escontext = {T_ErrorSaveContext}; + + queryid = pg_strtoint64_safe(tok, (Node *) &escontext); + if (escontext.error_occurred || queryid == INT64CONST(0)) + { + GUC_check_errmsg("queryId \"%s\" is not valid", tok); + + pfree(rawstring); + list_free(elemlist); + guc_free(result); + + return false; + } + + result->queryid[i++] = queryid; + } + + result->nqueryids = i; + + pfree(rawstring); + list_free(elemlist); + + *extra = result; + + return true; +} + +static void +assign_log_queryids(const char *newval, void *extra) +{ + auto_explain_queryids *myextra = (auto_explain_queryids *) extra; + + queryid_filter = myextra; +} diff --git a/contrib/auto_explain/t/001_auto_explain.pl b/contrib/auto_explain/t/001_auto_explain.pl index f32a5e84f65..148e83f7545 100644 --- a/contrib/auto_explain/t/001_auto_explain.pl +++ b/contrib/auto_explain/t/001_auto_explain.pl @@ -7,6 +7,7 @@ use warnings FATAL => 'all'; use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use Test::More; +use JSON::PP; # Runs the specified query and returns the emitted server log. # params is an optional hash mapping GUC names to values; @@ -27,6 +28,15 @@ sub query_log return slurp_file($log, $offset); } +sub get_queryid +{ + my ($node, $sql) = @_; + + my $plan = $node->safe_psql('postgres', 'EXPLAIN (VERBOSE, FORMAT JSON) ' . $sql); + + return int(decode_json($plan)->[0]->{"Query Identifier"}); +} + my $node = PostgreSQL::Test::Cluster->new('main'); $node->init(auth_extra => [ '--create-role' => 'regress_user1' ]); $node->append_conf('postgresql.conf', @@ -239,4 +249,47 @@ WHERE module_name = 'auto_explain'; }); like($res, qr/^auto_explain\|t\|auto_explain$/, "pg_get_loaded_modules() ok"); +# test detection of broken format +$log_contents = query_log($node, "SELECT 1;", + { "auto_explain.log_queryids" => "x" }); + +like( + $log_contents, + qr/queryId "x" is not valid/, + "invalid queryid detected"); + +my $query1 = "SELECT * FROM pg_class LIMIT 1"; +my $query2 = "SELECT * FROM pg_proc LIMIT 1"; +my $query3 = "SELECT * FROM pg_namespace LIMIT 1"; + +my $queryid1 = get_queryid($node, $query1); +my $queryid2 = get_queryid($node, $query2); +my $queryid3 = get_queryid($node, $query3); + +my $qids = "$queryid1,$queryid2"; + +$log_contents = query_log($node, $query1, + { "auto_explain.log_queryids" => $qids }); + +like( + $log_contents, + qr/Query Text: SELECT \* FROM pg_class LIMIT 1/, + "plan for query specified by queryid found"); + +$log_contents = query_log($node, $query2, + { "auto_explain.log_queryids" => $qids }); + +like( + $log_contents, + qr/Query Text: SELECT \* FROM pg_proc LIMIT 1/, + "plan for query specified by queryid found"); + +$log_contents = query_log($node, $query3, + { "auto_explain.log_queryids" => $qids }); + +unlike( + $log_contents, + qr/Query Text: SELECT \* FROM pg_namespace LIMIT 1/, + "plan for query with disallowed queryid not found"); + done_testing(); diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgml index 06a8fcc6c5b..53cf3262698 100644 --- a/doc/src/sgml/auto-explain.sgml +++ b/doc/src/sgml/auto-explain.sgml @@ -63,6 +63,26 @@ LOAD 'auto_explain'; </listitem> </varlistentry> + <varlistentry id="auto-explain-configuration-parameters-log-queryids"> + <term> + <varname>auto_explain.log_queryids</varname> (<type>list of integers</type>) + <indexterm> + <primary><varname>auto_explain.log_queryids</varname> configuration parameter</primary> + </indexterm> + </term> + <listitem> + <para> + <varname>auto_explain.log_queryids</varname> allows to specify list of + query IDs. Only plans with listed IDs will be logged. When this parameter + is empty (default), the query ID is not used for filtering. This doesn't + disable <varname>auto_explain.log_min_duration</varname>. + <varname>auto_explain.log_queryids</varname> and + <varname>auto_explain.log_min_duration</varname> work together. Only + superusers can change this setting. + </para> + </listitem> + </varlistentry> + <varlistentry id="auto-explain-configuration-parameters-log-parameter-max-length"> <term> <varname>auto_explain.log_parameter_max_length</varname> (<type>integer</type>) -- 2.54.0
