public inbox for [email protected]
help / color / mirror / Atom feedFrom: Robert Haas <[email protected]>
To: Matheus Alcantara <[email protected]>
Cc: [email protected]
Cc: Lukas Fittl <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: Re: Add custom EXPLAIN options support to auto_explain
Date: Tue, 31 Mar 2026 12:18:48 -0400
Message-ID: <CA+TgmoYUdeCdGfk8H6Ni2obXVixLvYaDkRGtxKLEmaCVNffsVA@mail.gmail.com> (raw)
In-Reply-To: <CA+Tgmob-0W8306mvrJX5Urtqt1AAasu8pi4yLrZ1XfwZU-Uj1w@mail.gmail.com>
References: <[email protected]>
<CA+Tgmob-0W8306mvrJX5Urtqt1AAasu8pi4yLrZ1XfwZU-Uj1w@mail.gmail.com>
On Mon, Mar 30, 2026 at 5:49 PM Robert Haas <[email protected]> wrote:
> I'm currently poking at some ideas for fixing this... more soon.
Here are some patches. I got started poking at this in earnest
because, on the pg_plan_advice thread, Lukas was saying that instead
of adopting pg_collect_advice, we should just add an option to send
advice strings for each executed query to the server log. I went to
implement that and then felt like it should really be part of
auto_explain rather than its own thing, which took me down a bit of a
rathole. But I eventually found my way back out of it, so here's a
patch set implementing auto_explain.log_extension_options.
There were two main complications. One is that I didn't want to assume
all options are Boolean. Right now, at least in core, they are: both
pg_overexplain and pg_plan_advice only use Booleans. But, that might
change in the future, or already be true in other extensions that
other people have written. Also, we might at some point in the future
want to consider folding the built-in options back into the same
mechanism -- i.e. get rid of log_extension_options and a pile of the
other parameters and just have log_options -- and then we'd need to
handle FORMAT, at least. The other big complication is that right now
there is no way to sanity check an EXPLAIN extension option except try
to set it and see if things go boom. That would mean that if you were
to configure auto_explain.log_extension_options='KANGAROO', the server
would basically just go down. I mean, technically it would be up, but
every query would ERROR when auto_explain tried to kangaroo-ify the
EXPLAIN output. That seems not so nice.
So I ended up with this:
- 0001 does a bit of refactoring to expose some of the logic in
SplitIdentifierString and similar functions in a way that allows for
reuse.
- 0002 modifies the "explain extension" option mechanism so that each
explain extension option has an associated GUC check handler.
- 0003 then uses those things to implement the feature
One thing that I suspect Tom in particular may be unhappy about (and
thus I'm copying him...) is that the validation framework in 0002
means that you have to load EXPLAIN-option providers before you can
use their options in the value of the new GUC. We try pretty hard to
make sure that the legal values for a GUC don't depend on the values
of other GUCs, and this is technically not a violation of that
principle because it's making the legal values for a GUC depend on
module loading, not other GUCs, so maybe it's fine. But on the other
hand, maybe it isn't. I'm not quite sure what the reasonable
alternative is: I guess we could instead add a "soft apply" hook for
EXPLAIN options where the contract is that the EXPLAIN option provider
is to do nothing if the option is not valid rather than complaining,
but then setting an invalid option wouldn't ever give any kind of
error -- it would just not work. That seems pretty unpleasant.
Anyway, if you apply all these patches it does solve the problem that
pg_collect_advice was targeting, modulo the need for some log parsing.
You can do this:
pg_plan_advice.always_store_advice_details = on
auto_explain.log_min_duration = 0
auto_explain.log_extension_options = 'plan_advice'
And then you get log output like this:
2026-03-31 12:16:18.784 EDT [75224] LOG: duration: 0.013 ms plan:
Query Text: select 1;
Result (cost=0.00..0.01 rows=1 width=4)
Generated Plan Advice:
NO_GATHER("*RESULT*")
Thoughts?
--
Robert Haas
EDB: http://www.enterprisedb.com
Attachments:
[application/octet-stream] v1-0003-auto_explain-Add-new-GUC-auto_explain.log_extensi.patch (15.5K, 2-v1-0003-auto_explain-Add-new-GUC-auto_explain.log_extensi.patch)
download | inline diff:
From 2a4e75d661f612400226ca21114ba18a308a688b Mon Sep 17 00:00:00 2001
From: Robert Haas <[email protected]>
Date: Tue, 31 Mar 2026 11:48:48 -0400
Subject: [PATCH v1 3/3] auto_explain: Add new GUC,
auto_explain.log_extension_options.
The associated value should look like something that could be
part of an EXPLAIN options list, but restricted to EXPLAIN options
added by extensions.
For example, if pg_overexplain is loaded, you could set
auto_explain.log_extension_options = 'DEBUG, RANGE_TABLE'.
You can also specify arguments to these options in the same manner
as normal e.g. 'DEBUG 1, RANGE_TABLE false'.
---
contrib/auto_explain/Makefile | 2 +-
contrib/auto_explain/auto_explain.c | 378 ++++++++++++++++++++++++++++
contrib/auto_explain/meson.build | 1 +
doc/src/sgml/auto-explain.sgml | 23 ++
src/tools/pgindent/typedefs.list | 2 +
5 files changed, 405 insertions(+), 1 deletion(-)
diff --git a/contrib/auto_explain/Makefile b/contrib/auto_explain/Makefile
index 94ab28e7c06..8adaba0c3ab 100644
--- a/contrib/auto_explain/Makefile
+++ b/contrib/auto_explain/Makefile
@@ -6,7 +6,7 @@ OBJS = \
auto_explain.o
PGFILEDESC = "auto_explain - logging facility for execution plans"
-REGRESS = alter_reset
+REGRESS = alter_reset extension_options
TAP_TESTS = 1
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index e856cd35a6f..132c3025f33 100644
--- a/contrib/auto_explain/auto_explain.c
+++ b/contrib/auto_explain/auto_explain.c
@@ -15,12 +15,17 @@
#include <limits.h>
#include "access/parallel.h"
+#include "commands/defrem.h"
#include "commands/explain.h"
#include "commands/explain_format.h"
#include "commands/explain_state.h"
#include "common/pg_prng.h"
#include "executor/instrument.h"
+#include "nodes/makefuncs.h"
+#include "nodes/value.h"
+#include "parser/scansup.h"
#include "utils/guc.h"
+#include "utils/varlena.h"
PG_MODULE_MAGIC_EXT(
.name = "auto_explain",
@@ -41,6 +46,31 @@ static int auto_explain_log_format = EXPLAIN_FORMAT_TEXT;
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;
+
+/*
+ * Parsed form of one option from auto_explain.log_extension_options.
+ */
+typedef struct auto_explain_option
+{
+ char *name;
+ char *value;
+ NodeTag type;
+} auto_explain_option;
+
+/*
+ * Parsed form of the entirety of auto_explain.log_extension_options, stored
+ * as GUC extra. The options[] array will have pointers into the string
+ * following the array.
+ */
+typedef struct auto_explain_extension_options
+{
+ int noptions;
+ auto_explain_option options[FLEXIBLE_ARRAY_MEMBER];
+ /* a null-terminated copy of the GUC string follows the array */
+} auto_explain_extension_options;
+
+static auto_explain_extension_options *extension_options = NULL;
static const struct config_enum_entry format_options[] = {
{"text", EXPLAIN_FORMAT_TEXT, false},
@@ -88,6 +118,15 @@ static void explain_ExecutorRun(QueryDesc *queryDesc,
static void explain_ExecutorFinish(QueryDesc *queryDesc);
static void explain_ExecutorEnd(QueryDesc *queryDesc);
+static bool check_log_extension_options(char **newval, void **extra,
+ GucSource source);
+static void assign_log_extension_options(const char *newval, void *extra);
+static void apply_extension_options(ExplainState *es,
+ auto_explain_extension_options *ext);
+static char *auto_explain_scan_literal(char **endp, char **nextp);
+static int auto_explain_split_options(char *rawstring,
+ auto_explain_option *options,
+ int maxoptions, char **errmsg);
/*
* Module load callback
@@ -232,6 +271,17 @@ _PG_init(void)
NULL,
NULL);
+ DefineCustomStringVariable("auto_explain.log_extension_options",
+ "Extension EXPLAIN options to be added.",
+ NULL,
+ &auto_explain_log_extension_options,
+ NULL,
+ PGC_SUSET,
+ 0,
+ check_log_extension_options,
+ assign_log_extension_options,
+ NULL);
+
DefineCustomRealVariable("auto_explain.sample_rate",
"Fraction of queries to process.",
NULL,
@@ -404,6 +454,8 @@ explain_ExecutorEnd(QueryDesc *queryDesc)
es->format = auto_explain_log_format;
es->settings = auto_explain_log_settings;
+ apply_extension_options(es, extension_options);
+
ExplainBeginOutput(es);
ExplainQueryText(es, queryDesc);
ExplainQueryParameters(es, queryDesc->params, auto_explain_log_parameter_max_length);
@@ -412,6 +464,12 @@ explain_ExecutorEnd(QueryDesc *queryDesc)
ExplainPrintTriggers(es, queryDesc);
if (es->costs)
ExplainPrintJITSummary(es, queryDesc);
+ if (explain_per_plan_hook)
+ (*explain_per_plan_hook) (queryDesc->plannedstmt,
+ NULL, es,
+ queryDesc->sourceText,
+ queryDesc->params,
+ queryDesc->estate->es_queryEnv);
ExplainEndOutput(es);
/* Remove last line break */
@@ -445,3 +503,323 @@ explain_ExecutorEnd(QueryDesc *queryDesc)
else
standard_ExecutorEnd(queryDesc);
}
+
+/*
+ * GUC check hook for auto_explain.log_extension_options.
+ */
+static bool
+check_log_extension_options(char **newval, void **extra, GucSource source)
+{
+ char *rawstring;
+ auto_explain_extension_options *result;
+ auto_explain_option *options;
+ int maxoptions = 8;
+ Size rawstring_len;
+ Size allocsize;
+ char *errmsg;
+
+ /* NULL or empty string means no options. */
+ if (*newval == NULL || (*newval)[0] == '\0')
+ {
+ *extra = NULL;
+ return true;
+ }
+
+ rawstring_len = strlen(*newval) + 1;
+
+retry:
+ /* Try to allocate an auto_explain_extension_options object. */
+ allocsize = offsetof(auto_explain_extension_options, options) +
+ sizeof(auto_explain_option) * maxoptions +
+ rawstring_len;
+ result = (auto_explain_extension_options *) guc_malloc(LOG, allocsize);
+ if (result == NULL)
+ return false;
+
+ /* Copy the string after the options array. */
+ rawstring = (char *) &result->options[maxoptions];
+ memcpy(rawstring, *newval, rawstring_len);
+
+ /* Parse. */
+ options = result->options;
+ result->noptions = auto_explain_split_options(rawstring, options,
+ maxoptions, &errmsg);
+ if (result->noptions < 0)
+ {
+ GUC_check_errdetail("%s", errmsg);
+ guc_free(result);
+ return false;
+ }
+
+ /*
+ * Retry with a larger array if needed.
+ *
+ * It should be impossible for this to loop more than once, because
+ * auto_explain_split_options tells us how many entries are needed.
+ */
+ if (result->noptions > maxoptions)
+ {
+ maxoptions = result->noptions;
+ guc_free(result);
+ goto retry;
+ }
+
+ /* Validate each option against its registered check handler. */
+ for (int i = 0; i < result->noptions; i++)
+ {
+ if (!GUCCheckExplainExtensionOption(options[i].name, options[i].value,
+ options[i].type))
+ {
+ guc_free(result);
+ return false;
+ }
+ }
+
+ *extra = result;
+ return true;
+}
+
+/*
+ * GUC assign hook for auto_explain.log_extension_options.
+ */
+static void
+assign_log_extension_options(const char *newval, void *extra)
+{
+ extension_options = (auto_explain_extension_options *) extra;
+}
+
+/*
+ * Apply parsed extension options to an ExplainState.
+ */
+static void
+apply_extension_options(ExplainState *es, auto_explain_extension_options *ext)
+{
+ if (ext == NULL)
+ return;
+
+ for (int i = 0; i < ext->noptions; i++)
+ {
+ auto_explain_option *opt = &ext->options[i];
+ DefElem *def;
+ Node *arg;
+
+ if (opt->value == NULL)
+ arg = NULL;
+ else if (opt->type == T_Integer)
+ arg = (Node *) makeInteger(strtol(opt->value, NULL, 0));
+ else if (opt->type == T_Float)
+ arg = (Node *) makeFloat(opt->value);
+ else
+ arg = (Node *) makeString(opt->value);
+
+ def = makeDefElem(opt->name, arg, -1);
+ ApplyExtensionExplainOption(es, def, NULL);
+ }
+}
+
+/*
+ * auto_explain_scan_literal - In-place scanner for single-quoted string
+ * literals.
+ *
+ * This is the single-quote analog of scan_quoted_identifier from varlena.c.
+ */
+static char *
+auto_explain_scan_literal(char **endp, char **nextp)
+{
+ char *token = *nextp + 1;
+
+ for (;;)
+ {
+ *endp = strchr(*nextp + 1, '\'');
+ if (*endp == NULL)
+ return NULL; /* mismatched quotes */
+ if ((*endp)[1] != '\'')
+ break; /* found end of literal */
+ /* Collapse adjacent quotes into one quote, and look again */
+ memmove(*endp, *endp + 1, strlen(*endp));
+ *nextp = *endp;
+ }
+ /* *endp now points at the terminating quote */
+ *nextp = *endp + 1;
+
+ return token;
+}
+
+/*
+ * auto_explain_split_options - Parse an option string into an array of
+ * auto_explain_option structs.
+ *
+ * Much of this logic is similar to SplitIdentifierString and friends, but our
+ * needs are different enough that we roll our own parsing logic. The goal here
+ * is to accept the same syntax that the main parser would accepts inside of
+ * an EXPLAIN option list. While we can't do that perfectly without adding a
+ * lot more code, the goal of this implementation is to be close enough that
+ * users don't really notice the differences.
+ *
+ * The input string is modified in place (null-terminated, downcased, quotes
+ * collapsed). All name and value pointers in the output array refer into
+ * this string, so the caller must ensure the string outlives the array.
+ *
+ * Returns the full number of options in the input string, but stores no
+ * more than maxoptions into the caller-provided array. If a syntax error
+ * occurs, returns -1 and sets *errmsg.
+ */
+static int
+auto_explain_split_options(char *rawstring, auto_explain_option *options,
+ int maxoptions, char **errmsg)
+{
+ char *nextp = rawstring;
+ int noptions = 0;
+ bool done = false;
+
+ *errmsg = NULL;
+
+ while (scanner_isspace(*nextp))
+ nextp++; /* skip leading whitespace */
+
+ if (*nextp == '\0')
+ return 0; /* empty string is fine */
+
+ while (!done)
+ {
+ char *name;
+ char *name_endp;
+ char *value = NULL;
+ char *value_endp = NULL;
+ NodeTag type = T_Invalid;
+
+ /* Parse the option name. */
+ name = scan_identifier(&name_endp, &nextp, ',', true);
+ if (name == NULL || name_endp == name)
+ {
+ *errmsg = "option name missing or empty";
+ return -1;
+ }
+
+ /* Skip whitespace after the option name. */
+ while (scanner_isspace(*nextp))
+ nextp++;
+
+ /*
+ * Determine whether we have an option value. A comma or end of
+ * string means no value; otherwise we have one.
+ */
+ if (*nextp != '\0' && *nextp != ',')
+ {
+ if (*nextp == '\'')
+ {
+ /* Single-quoted string literal. */
+ type = T_String;
+ value = auto_explain_scan_literal(&value_endp, &nextp);
+ if (value == NULL)
+ {
+ *errmsg = "unterminated single-quoted string";
+ return -1;
+ }
+ }
+ else if (isdigit((unsigned char) *nextp) ||
+ ((*nextp == '+' || *nextp == '-') &&
+ isdigit((unsigned char) nextp[1])))
+ {
+ char *endptr;
+ long intval;
+ char saved;
+
+ /* Remember the start of the next token, and find the end. */
+ value = nextp;
+ while (*nextp && *nextp != ',' && !scanner_isspace(*nextp))
+ nextp++;
+ value_endp = nextp;
+
+ /* Temporarily '\0'-terminate so we can use strtol/strtod. */
+ saved = *value_endp;
+ *value_endp = '\0';
+
+ /* Integer, float, or neither? */
+ errno = 0;
+ intval = strtol(value, &endptr, 0);
+ if (errno == 0 && *endptr == '\0' && endptr != value &&
+ intval == (int) intval)
+ type = T_Integer;
+ else
+ {
+ type = T_Float;
+ (void) strtod(value, &endptr);
+ if (*endptr != '\0')
+ {
+ *value_endp = saved;
+ *errmsg = "invalid numeric value";
+ return -1;
+ }
+ }
+
+ /* Remove temporary terminator. */
+ *value_endp = saved;
+ }
+ else
+ {
+ /* Identifier, possibly double-quoted. */
+ type = T_String;
+ value = scan_identifier(&value_endp, &nextp, ',', true);
+ if (value == NULL)
+ {
+ /*
+ * scan_identifier will return NULL if it finds an
+ * unterminated double-quoted identifier or it finds no
+ * identifier at all because the next character is
+ * whitespace or the separator character, here a comma.
+ * But the latter case is impossible here because the code
+ * above has skipped whitespace and checked for commas.
+ */
+ *errmsg = "unterminated double-quoted string";
+ return -1;
+ }
+ }
+ }
+
+ /* Skip trailing whitespace. */
+ while (scanner_isspace(*nextp))
+ nextp++;
+
+ /* Expect comma or end of string. */
+ if (*nextp == ',')
+ {
+ nextp++;
+ while (scanner_isspace(*nextp))
+ nextp++;
+ if (*nextp == '\0')
+ {
+ *errmsg = "trailing comma in option list";
+ return -1;
+ }
+ }
+ else if (*nextp == '\0')
+ done = true;
+ else
+ {
+ *errmsg = "expected comma or end of option list";
+ return -1;
+ }
+
+ /*
+ * Now safe to null-terminate the name and value. We couldn't do this
+ * earlier because in the unquoted case, the null terminator position
+ * may coincide with a character that the scanning logic above still
+ * needed to read.
+ */
+ *name_endp = '\0';
+ if (value_endp != NULL)
+ *value_endp = '\0';
+
+ /* Always count this option, and store the details if there is room. */
+ if (noptions < maxoptions)
+ {
+ options[noptions].name = name;
+ options[noptions].type = type;
+ options[noptions].value = value;
+ }
+ noptions++;
+ }
+
+ return noptions;
+}
diff --git a/contrib/auto_explain/meson.build b/contrib/auto_explain/meson.build
index 6f9d22bf5d8..d2b0650af1c 100644
--- a/contrib/auto_explain/meson.build
+++ b/contrib/auto_explain/meson.build
@@ -23,6 +23,7 @@ tests += {
'regress': {
'sql': [
'alter_reset',
+ 'extension_options',
],
},
'tap': {
diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgml
index 15c868021e6..ee85a67eb2e 100644
--- a/doc/src/sgml/auto-explain.sgml
+++ b/doc/src/sgml/auto-explain.sgml
@@ -245,6 +245,29 @@ LOAD 'auto_explain';
</listitem>
</varlistentry>
+ <varlistentry id="auto-explain-configuration-parameters-log-extension-options">
+ <term>
+ <varname>auto_explain.log_extension_options</varname> (<type>string</type>)
+ <indexterm>
+ <primary><varname>auto_explain.log_extension_options</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Loadable modules can extend the <literal>EXPLAIN</literal> command with
+ additional options that affect the output format. Such options can also
+ be specified here. The value of this parameter is a comma-separated
+ list of options, each of which is an option name followed optionally by
+ an associated value. The module that provides the
+ <literal>EXPLAIN</literal> option, such as
+ <link linkend="pgplanadvice"><literal>pg_plan_advice</literal></link> or
+ <link linkend="pgoverexplain"><literal>pg_overexplain</literal></link>,
+ should be loaded before this parameter is set.
+ Only superusers can change this setting.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="auto-explain-configuration-parameters-log-level">
<term>
<varname>auto_explain.log_level</varname> (<type>enum</type>)
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 2e4e913331b..c602ae2d690 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3561,6 +3561,8 @@ astreamer_verify
astreamer_waldump
astreamer_zstd_frame
auth_password_hook_typ
+auto_explain_extension_options
+auto_explain_option
autovac_table
av_relation
avc_cache
--
2.51.0
[application/octet-stream] v1-0001-Expose-helper-functions-scan_quoted_identifier-an.patch (8.3K, 3-v1-0001-Expose-helper-functions-scan_quoted_identifier-an.patch)
download | inline diff:
From d33f6ab7bf00465383c414e4f0ed56ff062ef7af Mon Sep 17 00:00:00 2001
From: Robert Haas <[email protected]>
Date: Mon, 30 Mar 2026 18:49:56 -0400
Subject: [PATCH v1 1/3] Expose helper functions scan_quoted_identifier and
scan_identifier.
Previously, this logic was embedded within SplitIdentifierString,
SplitDirectoriesString, and SplitGUCList. Factoring it out saves
a bit of duplicated code, and also makes it available to extensions
that might want to do similar things without necessarily wanting to
do exactly the same thing.
---
src/backend/utils/adt/varlena.c | 189 +++++++++++++++++---------------
src/include/utils/varlena.h | 3 +
2 files changed, 102 insertions(+), 90 deletions(-)
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 7b1ee61bde6..5816df6587a 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -2750,6 +2750,96 @@ textToQualifiedNameList(text *textval)
return result;
}
+/*
+ * scan_quoted_identifier - In-place scanner for quoted identifiers.
+ *
+ * *nextp should point to the opening double-quote character, and will be
+ * updated to point just past the end. *endp is set to the position of
+ * the closing quote. The return value is the identifier, or NULL if the
+ * matching close-quote cannot be found.
+ *
+ * If we find two consecutive double quote characters, that doesn't end the
+ * identifier: instead, we collapse them into a double quote and include them
+ * in the resulting token. Note that this requires overwriting the rest of the
+ * string in place, including the portion beyond the final value of *nextp.
+ */
+char *
+scan_quoted_identifier(char **endp, char **nextp)
+{
+ char *token = *nextp + 1;
+
+ for (;;)
+ {
+ *endp = strchr(*nextp + 1, '"');
+ if (*endp == NULL)
+ return NULL; /* mismatched quotes */
+ if ((*endp)[1] != '"')
+ break; /* found end of quoted identifier */
+ /* Collapse adjacent quotes into one quote, and look again */
+ memmove(*endp, *endp + 1, strlen(*endp));
+ *nextp = *endp;
+ }
+ /* *endp now points at the terminating quote */
+ *nextp = *endp + 1;
+
+ return token;
+}
+
+/*
+ * scan_identifier - In-place scanner for quoted or unquoted identifiers.
+ *
+ * On success, *endp is set to the position where the caller should write '\0'
+ * to null-terminate the token, and *nextp is advanced past the token (and past
+ * the closing quote, if any). The return value is the token content, or NULL
+ * if there is a syntax error (mismatched quotes or empty unquoted token).
+ *
+ * Unquoted identifiers are terminated by whitespace or the first occurrence
+ * of the separator character. Additionally, if downcase_unquoted = true,
+ * unquoted identifiers are downcased in place. See scan_quoted_identifier for
+ * an additional way in which we modify the string in place.
+ */
+char *
+scan_identifier(char **endp, char **nextp, char separator, bool downcase_unquoted)
+{
+ char *token;
+
+ if (**nextp == '"')
+ return scan_quoted_identifier(endp, nextp);
+
+ /* Unquoted identifier --- extends to separator or whitespace */
+ token = *nextp;
+
+ while (**nextp && **nextp != separator && !scanner_isspace(**nextp))
+ (*nextp)++;
+
+ if (*nextp == token)
+ return NULL; /* empty token */
+
+ *endp = *nextp;
+
+ if (downcase_unquoted)
+ {
+ /*
+ * Downcase the identifier, using same code as main lexer does.
+ *
+ * XXX because we want to overwrite the input in-place, we cannot
+ * support a downcasing transformation that increases the string
+ * length. This is not a problem given the current implementation of
+ * downcase_truncate_identifier, but we'll probably have to do
+ * something about this someday.
+ */
+ int len = *endp - token;
+ char *downname = downcase_truncate_identifier(token, len, false);
+
+ Assert(strlen(downname) <= len);
+ strncpy(token, downname, len); /* strncpy is required here */
+ pfree(downname);
+ }
+
+ return token;
+}
+
+
/*
* SplitIdentifierString --- parse a string containing identifiers
*
@@ -2794,53 +2884,9 @@ SplitIdentifierString(char *rawstring, char separator,
char *curname;
char *endp;
- if (*nextp == '"')
- {
- /* Quoted name --- collapse quote-quote pairs, no downcasing */
- curname = nextp + 1;
- for (;;)
- {
- endp = strchr(nextp + 1, '"');
- if (endp == NULL)
- return false; /* mismatched quotes */
- if (endp[1] != '"')
- break; /* found end of quoted name */
- /* Collapse adjacent quotes into one quote, and look again */
- memmove(endp, endp + 1, strlen(endp));
- nextp = endp;
- }
- /* endp now points at the terminating quote */
- nextp = endp + 1;
- }
- else
- {
- /* Unquoted name --- extends to separator or whitespace */
- char *downname;
- int len;
-
- curname = nextp;
- while (*nextp && *nextp != separator &&
- !scanner_isspace(*nextp))
- nextp++;
- endp = nextp;
- if (curname == nextp)
- return false; /* empty unquoted name not allowed */
-
- /*
- * Downcase the identifier, using same code as main lexer does.
- *
- * XXX because we want to overwrite the input in-place, we cannot
- * support a downcasing transformation that increases the string
- * length. This is not a problem given the current implementation
- * of downcase_truncate_identifier, but we'll probably have to do
- * something about this someday.
- */
- len = endp - curname;
- downname = downcase_truncate_identifier(curname, len, false);
- Assert(strlen(downname) <= len);
- strncpy(curname, downname, len); /* strncpy is required here */
- pfree(downname);
- }
+ curname = scan_identifier(&endp, &nextp, separator, true);
+ if (curname == NULL)
+ return false; /* mismatched quotes or empty name */
while (scanner_isspace(*nextp))
nextp++; /* skip trailing whitespace */
@@ -2924,20 +2970,9 @@ SplitDirectoriesString(char *rawstring, char separator,
if (*nextp == '"')
{
/* Quoted name --- collapse quote-quote pairs */
- curname = nextp + 1;
- for (;;)
- {
- endp = strchr(nextp + 1, '"');
- if (endp == NULL)
- return false; /* mismatched quotes */
- if (endp[1] != '"')
- break; /* found end of quoted name */
- /* Collapse adjacent quotes into one quote, and look again */
- memmove(endp, endp + 1, strlen(endp));
- nextp = endp;
- }
- /* endp now points at the terminating quote */
- nextp = endp + 1;
+ curname = scan_quoted_identifier(&endp, &nextp);
+ if (curname == NULL)
+ return false; /* mismatched quotes */
}
else
{
@@ -3042,35 +3077,9 @@ SplitGUCList(char *rawstring, char separator,
char *curname;
char *endp;
- if (*nextp == '"')
- {
- /* Quoted name --- collapse quote-quote pairs */
- curname = nextp + 1;
- for (;;)
- {
- endp = strchr(nextp + 1, '"');
- if (endp == NULL)
- return false; /* mismatched quotes */
- if (endp[1] != '"')
- break; /* found end of quoted name */
- /* Collapse adjacent quotes into one quote, and look again */
- memmove(endp, endp + 1, strlen(endp));
- nextp = endp;
- }
- /* endp now points at the terminating quote */
- nextp = endp + 1;
- }
- else
- {
- /* Unquoted name --- extends to separator or whitespace */
- curname = nextp;
- while (*nextp && *nextp != separator &&
- !scanner_isspace(*nextp))
- nextp++;
- endp = nextp;
- if (curname == nextp)
- return false; /* empty unquoted name not allowed */
- }
+ curname = scan_identifier(&endp, &nextp, separator, false);
+ if (curname == NULL)
+ return false; /* mismatched quotes or empty name */
while (scanner_isspace(*nextp))
nextp++; /* skip trailing whitespace */
diff --git a/src/include/utils/varlena.h b/src/include/utils/varlena.h
index 4b32574a075..fe8d8a58952 100644
--- a/src/include/utils/varlena.h
+++ b/src/include/utils/varlena.h
@@ -27,6 +27,9 @@ extern int varstr_levenshtein_less_equal(const char *source, int slen,
int ins_c, int del_c, int sub_c,
int max_d, bool trusted);
extern List *textToQualifiedNameList(text *textval);
+extern char *scan_quoted_identifier(char **endp, char **nextp);
+extern char *scan_identifier(char **endp, char **nextp, char separator,
+ bool downcase_unquoted);
extern bool SplitIdentifierString(char *rawstring, char separator,
List **namelist);
extern bool SplitDirectoriesString(char *rawstring, char separator,
--
2.51.0
[application/octet-stream] v1-0002-Add-a-guc_check_handler-to-the-EXPLAIN-extension-.patch (9.6K, 4-v1-0002-Add-a-guc_check_handler-to-the-EXPLAIN-extension-.patch)
download | inline diff:
From 7f671c41039183907e39b5302b6cd5751553a837 Mon Sep 17 00:00:00 2001
From: Robert Haas <[email protected]>
Date: Mon, 30 Mar 2026 19:10:32 -0400
Subject: [PATCH v1 2/3] Add a guc_check_handler to the EXPLAIN extension
mechanism.
It would be useful to be able to tell auto_explain to set a custom
EXPLAIN option, but it would be bad if it tried to do so and the
option name or value wasn't valid, because thent every query would fail
with a complaint about the EXPLAIN option. So add a guc_check_handler
that auto_explain will be able to use to only try to set option
name/value/type combinations that have been determined to be legal,
and to emit useful messages about ones that aren't.
---
contrib/pg_overexplain/pg_overexplain.c | 6 +-
contrib/pg_plan_advice/pg_plan_advice.c | 3 +-
src/backend/commands/explain_state.c | 121 +++++++++++++++++++++++-
src/include/commands/explain_state.h | 13 ++-
4 files changed, 135 insertions(+), 8 deletions(-)
diff --git a/contrib/pg_overexplain/pg_overexplain.c b/contrib/pg_overexplain/pg_overexplain.c
index b4e90909289..715eda8dc56 100644
--- a/contrib/pg_overexplain/pg_overexplain.c
+++ b/contrib/pg_overexplain/pg_overexplain.c
@@ -73,9 +73,11 @@ _PG_init(void)
es_extension_id = GetExplainExtensionId("pg_overexplain");
/* Register the new EXPLAIN options implemented by this module. */
- RegisterExtensionExplainOption("debug", overexplain_debug_handler);
+ RegisterExtensionExplainOption("debug", overexplain_debug_handler,
+ GUCCheckBooleanExplainOption);
RegisterExtensionExplainOption("range_table",
- overexplain_range_table_handler);
+ overexplain_range_table_handler,
+ GUCCheckBooleanExplainOption);
/* Use the per-node and per-plan hooks to make our options do something. */
prev_explain_per_node_hook = explain_per_node_hook;
diff --git a/contrib/pg_plan_advice/pg_plan_advice.c b/contrib/pg_plan_advice/pg_plan_advice.c
index 2393ed55518..5629070faf4 100644
--- a/contrib/pg_plan_advice/pg_plan_advice.c
+++ b/contrib/pg_plan_advice/pg_plan_advice.c
@@ -126,7 +126,8 @@ _PG_init(void)
/* Register the new EXPLAIN options implemented by this module. */
RegisterExtensionExplainOption("plan_advice",
- pg_plan_advice_explain_option_handler);
+ pg_plan_advice_explain_option_handler,
+ GUCCheckBooleanExplainOption);
/* Install hooks */
pgpa_planner_install_hooks();
diff --git a/src/backend/commands/explain_state.c b/src/backend/commands/explain_state.c
index 77f59b8e500..65dd4111459 100644
--- a/src/backend/commands/explain_state.c
+++ b/src/backend/commands/explain_state.c
@@ -36,6 +36,8 @@
#include "commands/defrem.h"
#include "commands/explain.h"
#include "commands/explain_state.h"
+#include "utils/builtins.h"
+#include "utils/guc.h"
/* Hook to perform additional EXPLAIN options validation */
explain_validate_options_hook_type explain_validate_options_hook = NULL;
@@ -44,6 +46,7 @@ typedef struct
{
const char *option_name;
ExplainOptionHandler option_handler;
+ ExplainOptionGUCCheckHandler guc_check_handler;
} ExplainExtensionOption;
static const char **ExplainExtensionNameArray = NULL;
@@ -304,26 +307,39 @@ SetExplainExtensionState(ExplainState *es, int extension_id, void *opaque)
/*
* Register a new EXPLAIN option.
*
+ * option_name is assumed to be a constant string or allocated in storage
+ * that will never be freed.
+ *
* When option_name is used as an EXPLAIN option, handler will be called and
* should update the ExplainState passed to it. See comments at top of file
* for a more detailed explanation.
*
- * option_name is assumed to be a constant string or allocated in storage
- * that will never be freed.
+ * guc_check_handler is a function that can be safely called from a
+ * GUC check hook to validate a proposed value for a custom EXPLAIN option.
+ * Boolean-valued options can pass GUCCheckBooleanExplainOption. See the
+ * comments for GUCCheckBooleanExplainOption for further information on
+ * how a guc_check_handler should behave.
*/
void
RegisterExtensionExplainOption(const char *option_name,
- ExplainOptionHandler handler)
+ ExplainOptionHandler handler,
+ ExplainOptionGUCCheckHandler guc_check_handler)
{
ExplainExtensionOption *exopt;
+ Assert(handler != NULL);
+ Assert(guc_check_handler != NULL);
+
/* Search for an existing option by this name; if found, update handler. */
for (int i = 0; i < ExplainExtensionOptionsAssigned; ++i)
{
if (strcmp(ExplainExtensionOptionArray[i].option_name,
option_name) == 0)
{
- ExplainExtensionOptionArray[i].option_handler = handler;
+ exopt = &ExplainExtensionOptionArray[i];
+
+ exopt->option_handler = handler;
+ exopt->guc_check_handler = guc_check_handler;
return;
}
}
@@ -352,6 +368,7 @@ RegisterExtensionExplainOption(const char *option_name,
exopt = &ExplainExtensionOptionArray[ExplainExtensionOptionsAssigned++];
exopt->option_name = option_name;
exopt->option_handler = handler;
+ exopt->guc_check_handler = guc_check_handler;
}
/*
@@ -375,3 +392,99 @@ ApplyExtensionExplainOption(ExplainState *es, DefElem *opt, ParseState *pstate)
return false;
}
+
+/*
+ * Determine whether an EXPLAIN extension option will be accepted without
+ * error. Returns true if so, and false if not. See the comments for
+ * GUCCheckBooleanExplainOption for more details.
+ *
+ * The caller need not know that the option_name is valid; this function
+ * will indicate that the option is unrecognized if that is the case.
+ */
+bool
+GUCCheckExplainExtensionOption(const char *option_name,
+ const char *option_value,
+ NodeTag option_type)
+{
+ for (int i = 0; i < ExplainExtensionOptionsAssigned; i++)
+ {
+ ExplainExtensionOption *exopt = &ExplainExtensionOptionArray[i];
+
+ if (strcmp(exopt->option_name, option_name) == 0)
+ return exopt->guc_check_handler(option_name, option_value,
+ option_type);
+ }
+
+ /* Unrecognized option name. */
+ GUC_check_errmsg("unrecognized EXPLAIN option \"%s\"", option_name);
+ return false;
+}
+
+/*
+ * guc_check_handler for Boolean-valued EXPLAIN extension options.
+ *
+ * After receiving a "true" value from this or any other GUC check handler
+ * for an EXPLAIN extension option, the caller is entitled to assume that
+ * a suitably constructed DefElem passed to the main option handler will
+ * not cause an error. To construct this DefElem, the caller should set
+ * the DefElem's defname to option_name. If option_values is NULL, arg
+ * should be NULL. Otherwise, arg should be of the type given by
+ * option_type, with option_value as the associated value. The only option
+ * types that should be passed are T_String, T_Float, and T_Integer; in
+ * the last case, the caller will need to perform a string-to-integer
+ * conversion.
+ *
+ * A guc_check_handler should not throw an error, and should not allocate
+ * memory. If it returns false to indicate that the option_value is not
+ * acceptable, it may use GUC_check_errmsg(), GUC_check_errdetail(), etc.
+ * to clarify the nature of the problem.
+ *
+ * Since we're concerned with Boolean options here, the logic below must
+ * exactly match the semantics of defGetBoolean.
+ */
+bool
+GUCCheckBooleanExplainOption(const char *option_name,
+ const char *option_value,
+ NodeTag option_type)
+{
+ bool valid = false;
+
+ if (option_value == NULL)
+ {
+ /* defGetBoolean treats no argument as valid */
+ valid = true;
+ }
+ else if (option_type == T_String)
+ {
+ /* defGetBoolean accepts exactly these string values */
+ if (pg_strcasecmp(option_value, "true") == 0 ||
+ pg_strcasecmp(option_value, "false") == 0 ||
+ pg_strcasecmp(option_value, "on") == 0 ||
+ pg_strcasecmp(option_value, "off") == 0)
+ valid = true;
+ }
+ else if (option_type == T_Integer)
+ {
+ long value;
+ char *end;
+
+ /*
+ * defGetBoolean accepts only 0 and 1, but those can be spelled in
+ * various ways (e.g. 01, 0x01).
+ */
+ errno = 0;
+ value = strtol(option_value, &end, 0);
+ if (errno == 0 && *end == '\0' && end != option_value &&
+ value == (int) value && (value == 0 || value == 1))
+ valid = true;
+ }
+
+ if (!valid)
+ {
+ GUC_check_errmsg("EXPLAIN option \"%s\" requires a Boolean value",
+ option_name);
+ return false;
+ }
+
+ return true;
+}
diff --git a/src/include/commands/explain_state.h b/src/include/commands/explain_state.h
index 5a48bc6fbb1..6252fe11f15 100644
--- a/src/include/commands/explain_state.h
+++ b/src/include/commands/explain_state.h
@@ -78,6 +78,9 @@ typedef struct ExplainState
} ExplainState;
typedef void (*ExplainOptionHandler) (ExplainState *, DefElem *, ParseState *);
+typedef bool (*ExplainOptionGUCCheckHandler) (const char *option_name,
+ const char *option_value,
+ NodeTag option_type);
/* Hook to perform additional EXPLAIN options validation */
typedef void (*explain_validate_options_hook_type) (ExplainState *es, List *options,
@@ -94,8 +97,16 @@ extern void SetExplainExtensionState(ExplainState *es, int extension_id,
void *opaque);
extern void RegisterExtensionExplainOption(const char *option_name,
- ExplainOptionHandler handler);
+ ExplainOptionHandler handler,
+ ExplainOptionGUCCheckHandler guc_check_handler);
extern bool ApplyExtensionExplainOption(ExplainState *es, DefElem *opt,
ParseState *pstate);
+extern bool GUCCheckExplainExtensionOption(const char *option_name,
+ const char *option_value,
+ NodeTag option_type);
+
+extern bool GUCCheckBooleanExplainOption(const char *option_name,
+ const char *option_value,
+ NodeTag option_type);
#endif /* EXPLAIN_STATE_H */
--
2.51.0
view thread (16+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Add custom EXPLAIN options support to auto_explain
In-Reply-To: <CA+TgmoYUdeCdGfk8H6Ni2obXVixLvYaDkRGtxKLEmaCVNffsVA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox