agora inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH] Log query parameters in auto_explain
7+ messages / 2 participants
[nested] [flat]

* [PATCH] Log query parameters in auto_explain
@ 2022-05-31 20:12 Dagfinn Ilmari Mannsåker <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Dagfinn Ilmari Mannsåker @ 2022-05-31 20:12 UTC (permalink / raw)

Add an auto_explain.log_parameter_max_length config setting, similar
to the corresponding core setting, that controls the inclusion of
query parameters in the logged explain output.
---
 contrib/auto_explain/auto_explain.c        | 15 +++++++++++++
 contrib/auto_explain/t/001_auto_explain.pl | 25 ++++++++++++++++++++--
 doc/src/sgml/auto-explain.sgml             | 19 ++++++++++++++++
 src/backend/commands/explain.c             | 22 +++++++++++++++++++
 src/include/commands/explain.h             |  1 +
 5 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index c9a0d947c8..1ba7536879 100644
--- a/contrib/auto_explain/auto_explain.c
+++ b/contrib/auto_explain/auto_explain.c
@@ -19,12 +19,14 @@
 #include "common/pg_prng.h"
 #include "executor/instrument.h"
 #include "jit/jit.h"
+#include "nodes/params.h"
 #include "utils/guc.h"
 
 PG_MODULE_MAGIC;
 
 /* GUC variables */
 static int	auto_explain_log_min_duration = -1; /* msec or -1 */
+static int	auto_explain_log_parameter_max_length = -1; /* bytes or -1 */
 static bool auto_explain_log_analyze = false;
 static bool auto_explain_log_verbose = false;
 static bool auto_explain_log_buffers = false;
@@ -105,6 +107,18 @@ _PG_init(void)
 							NULL,
 							NULL);
 
+	DefineCustomIntVariable("auto_explain.log_parameter_max_length",
+							"Sets the maximum length of query parameters to log.",
+							"Zero logs no query parameters, -1 logs them in full.",
+							&auto_explain_log_parameter_max_length,
+							-1,
+							-1, INT_MAX,
+							PGC_SUSET,
+							GUC_UNIT_BYTE,
+							NULL,
+							NULL,
+							NULL);
+
 	DefineCustomBoolVariable("auto_explain.log_analyze",
 							 "Use EXPLAIN ANALYZE for plan logging.",
 							 NULL,
@@ -389,6 +403,7 @@ explain_ExecutorEnd(QueryDesc *queryDesc)
 
 			ExplainBeginOutput(es);
 			ExplainQueryText(es, queryDesc);
+			ExplainQueryParameters(es, queryDesc->params, auto_explain_log_parameter_max_length);
 			ExplainPrintPlan(es, queryDesc);
 			if (es->analyze && auto_explain_log_triggers)
 				ExplainPrintTriggers(es, queryDesc);
diff --git a/contrib/auto_explain/t/001_auto_explain.pl b/contrib/auto_explain/t/001_auto_explain.pl
index 82e4d9d15c..ac57c8d6bf 100644
--- a/contrib/auto_explain/t/001_auto_explain.pl
+++ b/contrib/auto_explain/t/001_auto_explain.pl
@@ -13,20 +13,21 @@
 $node->append_conf('postgresql.conf',
 	"shared_preload_libraries = 'auto_explain'");
 $node->append_conf('postgresql.conf', "auto_explain.log_min_duration = 0");
+$node->append_conf('postgresql.conf', "auto_explain.log_parameter_max_length = -1");
 $node->append_conf('postgresql.conf', "auto_explain.log_analyze = on");
 $node->start;
 
 # run a couple of queries
 $node->safe_psql("postgres", "SELECT * FROM pg_class;");
 $node->safe_psql("postgres",
-	"SELECT * FROM pg_proc WHERE proname = 'int4pl';");
+	q{PREPARE get_proc(name) AS SELECT * FROM pg_proc WHERE proname = $1; EXECUTE get_proc('int4pl');});
 
 # emit some json too
 $node->append_conf('postgresql.conf', "auto_explain.log_format = json");
 $node->reload;
 $node->safe_psql("postgres", "SELECT * FROM pg_proc;");
 $node->safe_psql("postgres",
-	"SELECT * FROM pg_class WHERE relname = 'pg_class';");
+	q{PREPARE get_class(name) AS SELECT * FROM pg_class WHERE relname = $1; EXECUTE get_class('pg_class');});
 
 $node->stop('fast');
 
@@ -34,6 +35,16 @@
 
 my $log_contents = slurp_file($log);
 
+like(
+	$log_contents,
+	qr/Query Text: SELECT \* FROM pg_class;/,
+	"query text logged, text mode");
+
+like(
+	$log_contents,
+	qr/Query Parameters: \$1 = 'int4pl'/,
+	"query parameters logged, text mode");
+
 like(
 	$log_contents,
 	qr/Seq Scan on pg_class/,
@@ -44,6 +55,16 @@
 	qr/Index Scan using pg_proc_proname_args_nsp_index on pg_proc/,
 	"index scan logged, text mode");
 
+like(
+	$log_contents,
+	qr/"Query Text": "SELECT \* FROM pg_proc;"/,
+	"query text logged, json mode");
+
+like(
+	$log_contents,
+	qr/"Query Parameters": "\$1 = 'pg_class'"/,
+	"query parameters logged, json mode");
+
 like(
 	$log_contents,
 	qr/"Node Type": "Seq Scan"[^}]*"Relation Name": "pg_proc"/s,
diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgml
index 30e35a714a..a033c9c685 100644
--- a/doc/src/sgml/auto-explain.sgml
+++ b/doc/src/sgml/auto-explain.sgml
@@ -63,6 +63,25 @@
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term>
+     <varname>auto_explain.log_parameter_max_length</varname> (<type>integer</type>)
+     <indexterm>
+      <primary><varname>auto_explain.log_parameter_max_length</varname> configuration parameter</primary>
+     </indexterm>
+    </term>
+    <listitem>
+     <para>
+      <varname>auto_explain.log_parameter_max_length</varname> controls the
+      logging of query parameter values.  A value of<literal>-1</literal> (the
+      default) logs the parameter values in full. <literal>0</literal> disables
+      logging of parameter values.  A value greater than zero truncates each
+      parameter value to that many bytes. Only superusers can change this
+      setting.
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term>
      <varname>auto_explain.log_analyze</varname> (<type>boolean</type>)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 5d1f7089da..8584e3585d 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -972,6 +972,28 @@ ExplainQueryText(ExplainState *es, QueryDesc *queryDesc)
 		ExplainPropertyText("Query Text", queryDesc->sourceText, es);
 }
 
+/*
+ * ExplainQueryParameters -
+ *	  add a "Query Parameters" node that describes the parameters of the query,
+ *	  if any
+ *
+ * The caller should have set up the options fields of *es, as well as
+ * initializing the output buffer es->str.
+ *
+ */
+void
+ExplainQueryParameters(ExplainState *es, ParamListInfo params, int maxlen)
+{
+	char	   *str;
+
+	if (params == NULL || params->numParams == 0 || maxlen == 0)
+		return;
+
+	str = BuildParamLogString(params, NULL, maxlen);
+	if (str && str[0] != '\0')
+		ExplainPropertyText("Query Parameters", str, es);
+}
+
 /*
  * report_triggers -
  *		report execution stats for a single relation's triggers
diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index 666977fb1f..9ebde089ae 100644
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -99,6 +99,7 @@ extern void ExplainPrintTriggers(ExplainState *es, QueryDesc *queryDesc);
 extern void ExplainPrintJITSummary(ExplainState *es, QueryDesc *queryDesc);
 
 extern void ExplainQueryText(ExplainState *es, QueryDesc *queryDesc);
+extern void ExplainQueryParameters(ExplainState *es, ParamListInfo params, int maxlen);
 
 extern void ExplainBeginOutput(ExplainState *es);
 extern void ExplainEndOutput(ExplainState *es);
-- 
2.30.2


--=-=-=--





^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH v4 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches()
@ 2025-06-05 00:38 Yugo Nagata <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Yugo Nagata @ 2025-06-05 00:38 UTC (permalink / raw)

Most tab completions in match_previous_words() use COMPLETE_WITH* macros,
which wrap rl_completion_matches(). However, some direct calls to
rl_completion_matches() still remained.

This commit replaces the remaining direct calls with the new macro,
COMPLETE_WITH_FILES or COMPLETE_WITH_GENERATOR, for improved consistency and
readability.
---
 src/bin/psql/tab-complete.in.c | 38 ++++++++++++++++------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 37524364290..1b4855edab6 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -443,6 +443,16 @@ do { \
 	matches = rl_completion_matches(text, complete_from_schema_query); \
 } while (0)
 
+#define COMPLETE_WITH_FILES(escape, force_quote) \
+do { \
+	completion_charp = escape; \
+	completion_force_quote = force_quote; \
+	matches = rl_completion_matches(text, complete_from_files); \
+} while (0)
+
+#define COMPLETE_WITH_GENERATOR(function) \
+	matches = rl_completion_matches(text, function)
+
 /*
  * Assembly instructions for schema queries
  *
@@ -2179,7 +2189,7 @@ match_previous_words(int pattern_id,
 			/* for INDEX and TABLE/SEQUENCE, respectively */
 						  "UNIQUE", "UNLOGGED");
 		else
-			matches = rl_completion_matches(text, create_command_generator);
+			COMPLETE_WITH_GENERATOR(create_command_generator);
 	}
 	/* complete with something you can create or replace */
 	else if (TailMatches("CREATE", "OR", "REPLACE"))
@@ -2189,7 +2199,7 @@ match_previous_words(int pattern_id,
 /* DROP, but not DROP embedded in other commands */
 	/* complete with something you can drop */
 	else if (Matches("DROP"))
-		matches = rl_completion_matches(text, drop_command_generator);
+		COMPLETE_WITH_GENERATOR(drop_command_generator);
 
 /* ALTER */
 
@@ -2200,7 +2210,7 @@ match_previous_words(int pattern_id,
 
 	/* ALTER something */
 	else if (Matches("ALTER"))
-		matches = rl_completion_matches(text, alter_command_generator);
+		COMPLETE_WITH_GENERATOR(alter_command_generator);
 	/* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx */
 	else if (TailMatches("ALL", "IN", "TABLESPACE", MatchAny))
 		COMPLETE_WITH("SET TABLESPACE", "OWNED BY");
@@ -3308,17 +3318,9 @@ match_previous_words(int pattern_id,
 		COMPLETE_WITH("FROM", "TO");
 	/* Complete COPY <sth> FROM|TO with filename */
 	else if (Matches("COPY", MatchAny, "FROM|TO"))
-	{
-		completion_charp = "";
-		completion_force_quote = true;	/* COPY requires quoted filename */
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("", true);	/* COPY requires quoted filename */
 	else if (Matches("\\copy", MatchAny, "FROM|TO"))
-	{
-		completion_charp = "";
-		completion_force_quote = false;
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("", false);
 
 	/* Complete COPY <sth> TO <sth> */
 	else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny))
@@ -5417,9 +5419,9 @@ match_previous_words(int pattern_id,
 	else if (TailMatchesCS("\\h|\\help", MatchAny))
 	{
 		if (TailMatches("DROP"))
-			matches = rl_completion_matches(text, drop_command_generator);
+			COMPLETE_WITH_GENERATOR(drop_command_generator);
 		else if (TailMatches("ALTER"))
-			matches = rl_completion_matches(text, alter_command_generator);
+			COMPLETE_WITH_GENERATOR(alter_command_generator);
 
 		/*
 		 * CREATE is recognized by tail match elsewhere, so doesn't need to be
@@ -5519,11 +5521,7 @@ match_previous_words(int pattern_id,
 	else if (TailMatchesCS("\\cd|\\e|\\edit|\\g|\\gx|\\i|\\include|"
 						   "\\ir|\\include_relative|\\o|\\out|"
 						   "\\s|\\w|\\write|\\lo_import"))
-	{
-		completion_charp = "\\";
-		completion_force_quote = false;
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("\\", false);
 
 	/* gen_tabcomplete.pl ends special processing here */
 	/* END GEN_TABCOMPLETE */
-- 
2.43.0


--Multipart=_Thu__17_Jul_2025_10_57_36_+0900_dz7Bjsed2AEN=CJ+--





^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH v5 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches()
@ 2025-06-05 00:38 Yugo Nagata <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Yugo Nagata @ 2025-06-05 00:38 UTC (permalink / raw)

Most tab completions in match_previous_words() use COMPLETE_WITH* macros,
which wrap rl_completion_matches(). However, some direct calls to
rl_completion_matches() still remained.

This commit replaces the remaining direct calls with the new macro,
COMPLETE_WITH_FILES or COMPLETE_WITH_GENERATOR, for improved consistency and
readability.
---
 src/bin/psql/tab-complete.in.c | 38 ++++++++++++++++------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 37524364290..1b4855edab6 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -443,6 +443,16 @@ do { \
 	matches = rl_completion_matches(text, complete_from_schema_query); \
 } while (0)
 
+#define COMPLETE_WITH_FILES(escape, force_quote) \
+do { \
+	completion_charp = escape; \
+	completion_force_quote = force_quote; \
+	matches = rl_completion_matches(text, complete_from_files); \
+} while (0)
+
+#define COMPLETE_WITH_GENERATOR(function) \
+	matches = rl_completion_matches(text, function)
+
 /*
  * Assembly instructions for schema queries
  *
@@ -2179,7 +2189,7 @@ match_previous_words(int pattern_id,
 			/* for INDEX and TABLE/SEQUENCE, respectively */
 						  "UNIQUE", "UNLOGGED");
 		else
-			matches = rl_completion_matches(text, create_command_generator);
+			COMPLETE_WITH_GENERATOR(create_command_generator);
 	}
 	/* complete with something you can create or replace */
 	else if (TailMatches("CREATE", "OR", "REPLACE"))
@@ -2189,7 +2199,7 @@ match_previous_words(int pattern_id,
 /* DROP, but not DROP embedded in other commands */
 	/* complete with something you can drop */
 	else if (Matches("DROP"))
-		matches = rl_completion_matches(text, drop_command_generator);
+		COMPLETE_WITH_GENERATOR(drop_command_generator);
 
 /* ALTER */
 
@@ -2200,7 +2210,7 @@ match_previous_words(int pattern_id,
 
 	/* ALTER something */
 	else if (Matches("ALTER"))
-		matches = rl_completion_matches(text, alter_command_generator);
+		COMPLETE_WITH_GENERATOR(alter_command_generator);
 	/* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx */
 	else if (TailMatches("ALL", "IN", "TABLESPACE", MatchAny))
 		COMPLETE_WITH("SET TABLESPACE", "OWNED BY");
@@ -3308,17 +3318,9 @@ match_previous_words(int pattern_id,
 		COMPLETE_WITH("FROM", "TO");
 	/* Complete COPY <sth> FROM|TO with filename */
 	else if (Matches("COPY", MatchAny, "FROM|TO"))
-	{
-		completion_charp = "";
-		completion_force_quote = true;	/* COPY requires quoted filename */
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("", true);	/* COPY requires quoted filename */
 	else if (Matches("\\copy", MatchAny, "FROM|TO"))
-	{
-		completion_charp = "";
-		completion_force_quote = false;
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("", false);
 
 	/* Complete COPY <sth> TO <sth> */
 	else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny))
@@ -5417,9 +5419,9 @@ match_previous_words(int pattern_id,
 	else if (TailMatchesCS("\\h|\\help", MatchAny))
 	{
 		if (TailMatches("DROP"))
-			matches = rl_completion_matches(text, drop_command_generator);
+			COMPLETE_WITH_GENERATOR(drop_command_generator);
 		else if (TailMatches("ALTER"))
-			matches = rl_completion_matches(text, alter_command_generator);
+			COMPLETE_WITH_GENERATOR(alter_command_generator);
 
 		/*
 		 * CREATE is recognized by tail match elsewhere, so doesn't need to be
@@ -5519,11 +5521,7 @@ match_previous_words(int pattern_id,
 	else if (TailMatchesCS("\\cd|\\e|\\edit|\\g|\\gx|\\i|\\include|"
 						   "\\ir|\\include_relative|\\o|\\out|"
 						   "\\s|\\w|\\write|\\lo_import"))
-	{
-		completion_charp = "\\";
-		completion_force_quote = false;
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("\\", false);
 
 	/* gen_tabcomplete.pl ends special processing here */
 	/* END GEN_TABCOMPLETE */
-- 
2.43.0


--Multipart=_Fri__18_Jul_2025_16_49_28_+0900_ThDewLMcz6Ys6Zwo--





^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH v6 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches()
@ 2025-06-05 00:38 Yugo Nagata <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Yugo Nagata @ 2025-06-05 00:38 UTC (permalink / raw)

Most tab completions in match_previous_words() use COMPLETE_WITH* macros,
which wrap rl_completion_matches(). However, some direct calls to
rl_completion_matches() still remained.

This commit replaces the remaining direct calls with the new macro,
COMPLETE_WITH_FILES or COMPLETE_WITH_GENERATOR, for improved consistency and
readability.
---
 src/bin/psql/tab-complete.in.c | 38 ++++++++++++++++------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 6b20a4404b2..6176741d20b 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -443,6 +443,16 @@ do { \
 	matches = rl_completion_matches(text, complete_from_schema_query); \
 } while (0)
 
+#define COMPLETE_WITH_FILES(escape, force_quote) \
+do { \
+	completion_charp = escape; \
+	completion_force_quote = force_quote; \
+	matches = rl_completion_matches(text, complete_from_files); \
+} while (0)
+
+#define COMPLETE_WITH_GENERATOR(generator) \
+	matches = rl_completion_matches(text, generator)
+
 /*
  * Assembly instructions for schema queries
  *
@@ -2182,7 +2192,7 @@ match_previous_words(int pattern_id,
 			/* for INDEX and TABLE/SEQUENCE, respectively */
 						  "UNIQUE", "UNLOGGED");
 		else
-			matches = rl_completion_matches(text, create_command_generator);
+			COMPLETE_WITH_GENERATOR(create_command_generator);
 	}
 	/* complete with something you can create or replace */
 	else if (TailMatches("CREATE", "OR", "REPLACE"))
@@ -2192,7 +2202,7 @@ match_previous_words(int pattern_id,
 /* DROP, but not DROP embedded in other commands */
 	/* complete with something you can drop */
 	else if (Matches("DROP"))
-		matches = rl_completion_matches(text, drop_command_generator);
+		COMPLETE_WITH_GENERATOR(drop_command_generator);
 
 /* ALTER */
 
@@ -2203,7 +2213,7 @@ match_previous_words(int pattern_id,
 
 	/* ALTER something */
 	else if (Matches("ALTER"))
-		matches = rl_completion_matches(text, alter_command_generator);
+		COMPLETE_WITH_GENERATOR(alter_command_generator);
 	/* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx */
 	else if (TailMatches("ALL", "IN", "TABLESPACE", MatchAny))
 		COMPLETE_WITH("SET TABLESPACE", "OWNED BY");
@@ -3316,17 +3326,9 @@ match_previous_words(int pattern_id,
 		COMPLETE_WITH("FROM", "TO");
 	/* Complete COPY <sth> FROM|TO with filename */
 	else if (Matches("COPY", MatchAny, "FROM|TO"))
-	{
-		completion_charp = "";
-		completion_force_quote = true;	/* COPY requires quoted filename */
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("", true);	/* COPY requires quoted filename */
 	else if (Matches("\\copy", MatchAny, "FROM|TO"))
-	{
-		completion_charp = "";
-		completion_force_quote = false;
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("", false);
 
 	/* Complete COPY <sth> TO <sth> */
 	else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny))
@@ -5427,9 +5429,9 @@ match_previous_words(int pattern_id,
 	else if (TailMatchesCS("\\h|\\help", MatchAny))
 	{
 		if (TailMatches("DROP"))
-			matches = rl_completion_matches(text, drop_command_generator);
+			COMPLETE_WITH_GENERATOR(drop_command_generator);
 		else if (TailMatches("ALTER"))
-			matches = rl_completion_matches(text, alter_command_generator);
+			COMPLETE_WITH_GENERATOR(alter_command_generator);
 
 		/*
 		 * CREATE is recognized by tail match elsewhere, so doesn't need to be
@@ -5529,11 +5531,7 @@ match_previous_words(int pattern_id,
 	else if (TailMatchesCS("\\cd|\\e|\\edit|\\g|\\gx|\\i|\\include|"
 						   "\\ir|\\include_relative|\\o|\\out|"
 						   "\\s|\\w|\\write|\\lo_import"))
-	{
-		completion_charp = "\\";
-		completion_force_quote = false;
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("\\", false);
 
 	/* gen_tabcomplete.pl ends special processing here */
 	/* END GEN_TABCOMPLETE */
-- 
2.43.0


--Multipart=_Thu__18_Sep_2025_12_05_06_+0900_I2R/usu+NR/K2VUG--





^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches()
@ 2025-06-05 00:38 Yugo Nagata <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Yugo Nagata @ 2025-06-05 00:38 UTC (permalink / raw)

Most tab completions in match_previous_words() use COMPLETE_WITH* macros,
which wrap rl_completion_matches(). However, some direct calls to
rl_completion_matches() still remained.

This commit replaces the remaining direct calls with the new macro,
COMPLETE_WITH_FILES or COMPLETE_WITH_GENERATOR, for improved consistency and
readability.
---
 src/bin/psql/tab-complete.in.c | 38 ++++++++++++++++------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index ec65ab79fec..8a85a285281 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -443,6 +443,16 @@ do { \
 	matches = rl_completion_matches(text, complete_from_schema_query); \
 } while (0)
 
+#define COMPLETE_WITH_FILES(escape, force_quote) \
+do { \
+	completion_charp = escape; \
+	completion_force_quote = force_quote; \
+	matches = rl_completion_matches(text, complete_from_files); \
+} while (0)
+
+#define COMPLETE_WITH_GENERATOR(function) \
+	matches = rl_completion_matches(text, function)
+
 /*
  * Assembly instructions for schema queries
  *
@@ -2158,7 +2168,7 @@ match_previous_words(int pattern_id,
 			/* for INDEX and TABLE/SEQUENCE, respectively */
 						  "UNIQUE", "UNLOGGED");
 		else
-			matches = rl_completion_matches(text, create_command_generator);
+			COMPLETE_WITH_GENERATOR(create_command_generator);
 	}
 	/* complete with something you can create or replace */
 	else if (TailMatches("CREATE", "OR", "REPLACE"))
@@ -2168,7 +2178,7 @@ match_previous_words(int pattern_id,
 /* DROP, but not DROP embedded in other commands */
 	/* complete with something you can drop */
 	else if (Matches("DROP"))
-		matches = rl_completion_matches(text, drop_command_generator);
+		COMPLETE_WITH_GENERATOR(drop_command_generator);
 
 /* ALTER */
 
@@ -2179,7 +2189,7 @@ match_previous_words(int pattern_id,
 
 	/* ALTER something */
 	else if (Matches("ALTER"))
-		matches = rl_completion_matches(text, alter_command_generator);
+		COMPLETE_WITH_GENERATOR(alter_command_generator);
 	/* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx */
 	else if (TailMatches("ALL", "IN", "TABLESPACE", MatchAny))
 		COMPLETE_WITH("SET TABLESPACE", "OWNED BY");
@@ -3264,17 +3274,9 @@ match_previous_words(int pattern_id,
 		COMPLETE_WITH("FROM", "TO");
 	/* Complete COPY <sth> FROM|TO with filename */
 	else if (Matches("COPY", MatchAny, "FROM|TO"))
-	{
-		completion_charp = "";
-		completion_force_quote = true;	/* COPY requires quoted filename */
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("", true);	/* COPY requires quoted filename */
 	else if (Matches("\\copy", MatchAny, "FROM|TO"))
-	{
-		completion_charp = "";
-		completion_force_quote = false;
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("", false);
 
 	/* Complete COPY <sth> TO <sth> */
 	else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny))
@@ -5347,9 +5349,9 @@ match_previous_words(int pattern_id,
 	else if (TailMatchesCS("\\h|\\help", MatchAny))
 	{
 		if (TailMatches("DROP"))
-			matches = rl_completion_matches(text, drop_command_generator);
+			COMPLETE_WITH_GENERATOR(drop_command_generator);
 		else if (TailMatches("ALTER"))
-			matches = rl_completion_matches(text, alter_command_generator);
+			COMPLETE_WITH_GENERATOR(alter_command_generator);
 
 		/*
 		 * CREATE is recognized by tail match elsewhere, so doesn't need to be
@@ -5449,11 +5451,7 @@ match_previous_words(int pattern_id,
 	else if (TailMatchesCS("\\cd|\\e|\\edit|\\g|\\gx|\\i|\\include|"
 						   "\\ir|\\include_relative|\\o|\\out|"
 						   "\\s|\\w|\\write|\\lo_import"))
-	{
-		completion_charp = "\\";
-		completion_force_quote = false;
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("\\", false);
 
 	/* gen_tabcomplete.pl ends special processing here */
 	/* END GEN_TABCOMPLETE */
-- 
2.43.0


--Multipart=_Thu__5_Jun_2025_10_08_35_+0900_P.h1uSLN4yp_7Ild--





^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH v2 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches()
@ 2025-06-05 00:38 Yugo Nagata <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Yugo Nagata @ 2025-06-05 00:38 UTC (permalink / raw)

Most tab completions in match_previous_words() use COMPLETE_WITH* macros,
which wrap rl_completion_matches(). However, some direct calls to
rl_completion_matches() still remained.

This commit replaces the remaining direct calls with the new macro,
COMPLETE_WITH_FILES or COMPLETE_WITH_GENERATOR, for improved consistency and
readability.
---
 src/bin/psql/tab-complete.in.c | 38 ++++++++++++++++------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index ec65ab79fec..8a85a285281 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -443,6 +443,16 @@ do { \
 	matches = rl_completion_matches(text, complete_from_schema_query); \
 } while (0)
 
+#define COMPLETE_WITH_FILES(escape, force_quote) \
+do { \
+	completion_charp = escape; \
+	completion_force_quote = force_quote; \
+	matches = rl_completion_matches(text, complete_from_files); \
+} while (0)
+
+#define COMPLETE_WITH_GENERATOR(function) \
+	matches = rl_completion_matches(text, function)
+
 /*
  * Assembly instructions for schema queries
  *
@@ -2158,7 +2168,7 @@ match_previous_words(int pattern_id,
 			/* for INDEX and TABLE/SEQUENCE, respectively */
 						  "UNIQUE", "UNLOGGED");
 		else
-			matches = rl_completion_matches(text, create_command_generator);
+			COMPLETE_WITH_GENERATOR(create_command_generator);
 	}
 	/* complete with something you can create or replace */
 	else if (TailMatches("CREATE", "OR", "REPLACE"))
@@ -2168,7 +2178,7 @@ match_previous_words(int pattern_id,
 /* DROP, but not DROP embedded in other commands */
 	/* complete with something you can drop */
 	else if (Matches("DROP"))
-		matches = rl_completion_matches(text, drop_command_generator);
+		COMPLETE_WITH_GENERATOR(drop_command_generator);
 
 /* ALTER */
 
@@ -2179,7 +2189,7 @@ match_previous_words(int pattern_id,
 
 	/* ALTER something */
 	else if (Matches("ALTER"))
-		matches = rl_completion_matches(text, alter_command_generator);
+		COMPLETE_WITH_GENERATOR(alter_command_generator);
 	/* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx */
 	else if (TailMatches("ALL", "IN", "TABLESPACE", MatchAny))
 		COMPLETE_WITH("SET TABLESPACE", "OWNED BY");
@@ -3264,17 +3274,9 @@ match_previous_words(int pattern_id,
 		COMPLETE_WITH("FROM", "TO");
 	/* Complete COPY <sth> FROM|TO with filename */
 	else if (Matches("COPY", MatchAny, "FROM|TO"))
-	{
-		completion_charp = "";
-		completion_force_quote = true;	/* COPY requires quoted filename */
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("", true);	/* COPY requires quoted filename */
 	else if (Matches("\\copy", MatchAny, "FROM|TO"))
-	{
-		completion_charp = "";
-		completion_force_quote = false;
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("", false);
 
 	/* Complete COPY <sth> TO <sth> */
 	else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny))
@@ -5347,9 +5349,9 @@ match_previous_words(int pattern_id,
 	else if (TailMatchesCS("\\h|\\help", MatchAny))
 	{
 		if (TailMatches("DROP"))
-			matches = rl_completion_matches(text, drop_command_generator);
+			COMPLETE_WITH_GENERATOR(drop_command_generator);
 		else if (TailMatches("ALTER"))
-			matches = rl_completion_matches(text, alter_command_generator);
+			COMPLETE_WITH_GENERATOR(alter_command_generator);
 
 		/*
 		 * CREATE is recognized by tail match elsewhere, so doesn't need to be
@@ -5449,11 +5451,7 @@ match_previous_words(int pattern_id,
 	else if (TailMatchesCS("\\cd|\\e|\\edit|\\g|\\gx|\\i|\\include|"
 						   "\\ir|\\include_relative|\\o|\\out|"
 						   "\\s|\\w|\\write|\\lo_import"))
-	{
-		completion_charp = "\\";
-		completion_force_quote = false;
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("\\", false);
 
 	/* gen_tabcomplete.pl ends special processing here */
 	/* END GEN_TABCOMPLETE */
-- 
2.43.0


--Multipart=_Thu__5_Jun_2025_16_52_00_+0900_d_5fKAN_xESbgyqx--





^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH v3 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches()
@ 2025-06-05 00:38 Yugo Nagata <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Yugo Nagata @ 2025-06-05 00:38 UTC (permalink / raw)

Most tab completions in match_previous_words() use COMPLETE_WITH* macros,
which wrap rl_completion_matches(). However, some direct calls to
rl_completion_matches() still remained.

This commit replaces the remaining direct calls with the new macro,
COMPLETE_WITH_FILES or COMPLETE_WITH_GENERATOR, for improved consistency and
readability.
---
 src/bin/psql/tab-complete.in.c | 38 ++++++++++++++++------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 2c0b4f28c14..2716554aa0b 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -443,6 +443,16 @@ do { \
 	matches = rl_completion_matches(text, complete_from_schema_query); \
 } while (0)
 
+#define COMPLETE_WITH_FILES(escape, force_quote) \
+do { \
+	completion_charp = escape; \
+	completion_force_quote = force_quote; \
+	matches = rl_completion_matches(text, complete_from_files); \
+} while (0)
+
+#define COMPLETE_WITH_GENERATOR(function) \
+	matches = rl_completion_matches(text, function)
+
 /*
  * Assembly instructions for schema queries
  *
@@ -2158,7 +2168,7 @@ match_previous_words(int pattern_id,
 			/* for INDEX and TABLE/SEQUENCE, respectively */
 						  "UNIQUE", "UNLOGGED");
 		else
-			matches = rl_completion_matches(text, create_command_generator);
+			COMPLETE_WITH_GENERATOR(create_command_generator);
 	}
 	/* complete with something you can create or replace */
 	else if (TailMatches("CREATE", "OR", "REPLACE"))
@@ -2168,7 +2178,7 @@ match_previous_words(int pattern_id,
 /* DROP, but not DROP embedded in other commands */
 	/* complete with something you can drop */
 	else if (Matches("DROP"))
-		matches = rl_completion_matches(text, drop_command_generator);
+		COMPLETE_WITH_GENERATOR(drop_command_generator);
 
 /* ALTER */
 
@@ -2179,7 +2189,7 @@ match_previous_words(int pattern_id,
 
 	/* ALTER something */
 	else if (Matches("ALTER"))
-		matches = rl_completion_matches(text, alter_command_generator);
+		COMPLETE_WITH_GENERATOR(alter_command_generator);
 	/* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx */
 	else if (TailMatches("ALL", "IN", "TABLESPACE", MatchAny))
 		COMPLETE_WITH("SET TABLESPACE", "OWNED BY");
@@ -3264,17 +3274,9 @@ match_previous_words(int pattern_id,
 		COMPLETE_WITH("FROM", "TO");
 	/* Complete COPY <sth> FROM|TO with filename */
 	else if (Matches("COPY", MatchAny, "FROM|TO"))
-	{
-		completion_charp = "";
-		completion_force_quote = true;	/* COPY requires quoted filename */
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("", true);	/* COPY requires quoted filename */
 	else if (Matches("\\copy", MatchAny, "FROM|TO"))
-	{
-		completion_charp = "";
-		completion_force_quote = false;
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("", false);
 
 	/* Complete COPY <sth> TO <sth> */
 	else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny))
@@ -5348,9 +5350,9 @@ match_previous_words(int pattern_id,
 	else if (TailMatchesCS("\\h|\\help", MatchAny))
 	{
 		if (TailMatches("DROP"))
-			matches = rl_completion_matches(text, drop_command_generator);
+			COMPLETE_WITH_GENERATOR(drop_command_generator);
 		else if (TailMatches("ALTER"))
-			matches = rl_completion_matches(text, alter_command_generator);
+			COMPLETE_WITH_GENERATOR(alter_command_generator);
 
 		/*
 		 * CREATE is recognized by tail match elsewhere, so doesn't need to be
@@ -5450,11 +5452,7 @@ match_previous_words(int pattern_id,
 	else if (TailMatchesCS("\\cd|\\e|\\edit|\\g|\\gx|\\i|\\include|"
 						   "\\ir|\\include_relative|\\o|\\out|"
 						   "\\s|\\w|\\write|\\lo_import"))
-	{
-		completion_charp = "\\";
-		completion_force_quote = false;
-		matches = rl_completion_matches(text, complete_from_files);
-	}
+		COMPLETE_WITH_FILES("\\", false);
 
 	/* gen_tabcomplete.pl ends special processing here */
 	/* END GEN_TABCOMPLETE */
-- 
2.43.0


--Multipart=_Tue__17_Jun_2025_00_08_32_+0900_g9awdn.2U/+M5mrO--





^ permalink  raw  reply  [nested|flat] 7+ messages in thread


end of thread, other threads:[~2025-06-05 00:38 UTC | newest]

Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-05-31 20:12 [PATCH] Log query parameters in auto_explain Dagfinn Ilmari Mannsåker <[email protected]>
2025-06-05 00:38 [PATCH v4 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() Yugo Nagata <[email protected]>
2025-06-05 00:38 [PATCH v5 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() Yugo Nagata <[email protected]>
2025-06-05 00:38 [PATCH v6 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() Yugo Nagata <[email protected]>
2025-06-05 00:38 [PATCH 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() Yugo Nagata <[email protected]>
2025-06-05 00:38 [PATCH v2 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() Yugo Nagata <[email protected]>
2025-06-05 00:38 [PATCH v3 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() Yugo Nagata <[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