public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v1] Add an ALL option to EXPLAIN
9+ messages / 4 participants
[nested] [flat]

* [PATCH v1] Add an ALL option to EXPLAIN
@ 2019-05-07 07:26  David Fetter <[email protected]>
  0 siblings, 0 replies; 9+ messages in thread

From: David Fetter @ 2019-05-07 07:26 UTC (permalink / raw)


which starts all boolean options at once.

diff --git a/doc/src/sgml/ref/explain.sgml b/doc/src/sgml/ref/explain.sgml
index 385d10411f..ade3793c5f 100644
--- a/doc/src/sgml/ref/explain.sgml
+++ b/doc/src/sgml/ref/explain.sgml
@@ -43,6 +43,7 @@ EXPLAIN [ ANALYZE ] [ VERBOSE ] <replaceable class="parameter">statement</replac
     BUFFERS [ <replaceable class="parameter">boolean</replaceable> ]
     TIMING [ <replaceable class="parameter">boolean</replaceable> ]
     SUMMARY [ <replaceable class="parameter">boolean</replaceable> ]
+    ALL [ <replaceable class="parameter">boolean</replaceable> ]
     FORMAT { TEXT | XML | JSON | YAML }
 </synopsis>
  </refsynopsisdiv>
@@ -224,6 +225,16 @@ ROLLBACK;
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term><literal>ALL</literal></term>
+    <listitem>
+     <para>
+      Include (or exclude) all of the above settings before examining the rest
+      of the options. It defaults to <literal>FALSE</literal>.
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term><literal>FORMAT</literal></term>
     <listitem>
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index a6c6de78f1..966f4566c7 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -151,6 +151,26 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, const char *queryString,
 	bool		summary_set = false;
 
 	/* Parse options list. */
+	/* First, see whether ALL is set */
+	foreach(lc, stmt->options)
+	{
+		DefElem	   *opt = (DefElem *) lfirst(lc);
+
+		if (strcmp(opt->defname, "all") == 0)
+		{
+			es->analyze = defGetBoolean(opt);
+			es->verbose = defGetBoolean(opt);
+			es->costs = defGetBoolean(opt);
+			es->buffers = defGetBoolean(opt);
+			es->settings = defGetBoolean(opt);
+			timing_set = true;
+			es->timing = defGetBoolean(opt);
+			summary_set = true;
+			es->timing = defGetBoolean(opt);
+		}
+	}
+
+	/* If you add another boolean option here, remember to add it above, too */
 	foreach(lc, stmt->options)
 	{
 		DefElem    *opt = (DefElem *) lfirst(lc);


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

* [PATCH v2] Add an ALL option to EXPLAIN
@ 2019-05-07 07:26  David Fetter <[email protected]>
  0 siblings, 0 replies; 9+ messages in thread

From: David Fetter @ 2019-05-07 07:26 UTC (permalink / raw)


which starts all boolean options at once.

diff --git a/doc/src/sgml/ref/explain.sgml b/doc/src/sgml/ref/explain.sgml
index 385d10411f..ade3793c5f 100644
--- a/doc/src/sgml/ref/explain.sgml
+++ b/doc/src/sgml/ref/explain.sgml
@@ -43,6 +43,7 @@ EXPLAIN [ ANALYZE ] [ VERBOSE ] <replaceable class="parameter">statement</replac
     BUFFERS [ <replaceable class="parameter">boolean</replaceable> ]
     TIMING [ <replaceable class="parameter">boolean</replaceable> ]
     SUMMARY [ <replaceable class="parameter">boolean</replaceable> ]
+    ALL [ <replaceable class="parameter">boolean</replaceable> ]
     FORMAT { TEXT | XML | JSON | YAML }
 </synopsis>
  </refsynopsisdiv>
@@ -224,6 +225,16 @@ ROLLBACK;
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term><literal>ALL</literal></term>
+    <listitem>
+     <para>
+      Include (or exclude) all of the above settings before examining the rest
+      of the options. It defaults to <literal>FALSE</literal>.
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term><literal>FORMAT</literal></term>
     <listitem>
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index a6c6de78f1..f58b9d7290 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -151,6 +151,26 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, const char *queryString,
 	bool		summary_set = false;
 
 	/* Parse options list. */
+	/* First, see whether ALL is set */
+	foreach(lc, stmt->options)
+	{
+		DefElem	   *opt = (DefElem *) lfirst(lc);
+
+		if (strcmp(opt->defname, "all") == 0)
+		{
+			es->analyze = defGetBoolean(opt);
+			es->verbose = defGetBoolean(opt);
+			es->costs = defGetBoolean(opt);
+			es->buffers = defGetBoolean(opt);
+			es->settings = defGetBoolean(opt);
+			timing_set = true;
+			es->timing = defGetBoolean(opt);
+			summary_set = true;
+			es->summary_set = defGetBoolean(opt);
+		}
+	}
+
+	/* If you add another boolean option here, remember to add it above, too */
 	foreach(lc, stmt->options)
 	{
 		DefElem    *opt = (DefElem *) lfirst(lc);
@@ -194,6 +214,8 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, const char *queryString,
 								opt->defname, p),
 						 parser_errposition(pstate, opt->location)));
 		}
+		else if (strcmp(opt->defname, "all") == 0)
+			; /* Do nothing */
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 3dc0e8a4fb..4a69f9711c 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -10704,6 +10704,7 @@ explain_option_elem:
 explain_option_name:
 			NonReservedWord			{ $$ = $1; }
 			| analyze_keyword		{ $$ = "analyze"; }
+			| ALL					{ $$ = "all"; }
 		;
 
 explain_option_arg:
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index bcddc7601e..8177f9c6da 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2862,9 +2862,9 @@ psql_completion(const char *text, int start, int end)
 		 * one word, so the above test is correct.
 		 */
 		if (ends_with(prev_wd, '(') || ends_with(prev_wd, ','))
-			COMPLETE_WITH("ANALYZE", "VERBOSE", "COSTS", "BUFFERS",
+			COMPLETE_WITH("ALL", "ANALYZE", "VERBOSE", "COSTS", "BUFFERS",
 						  "TIMING", "SUMMARY", "FORMAT");
-		else if (TailMatches("ANALYZE|VERBOSE|COSTS|BUFFERS|TIMING|SUMMARY"))
+		else if (TailMatches("ALL|ANALYZE|VERBOSE|COSTS|BUFFERS|TIMING|SUMMARY"))
 			COMPLETE_WITH("ON", "OFF");
 		else if (TailMatches("FORMAT"))
 			COMPLETE_WITH("TEXT", "XML", "JSON", "YAML");
@@ -2874,7 +2874,8 @@ psql_completion(const char *text, int start, int end)
 					  "VERBOSE");
 	else if (Matches("EXPLAIN", "(*)") ||
 			 Matches("EXPLAIN", "VERBOSE") ||
-			 Matches("EXPLAIN", "ANALYZE", "VERBOSE"))
+			 Matches("EXPLAIN", "ANALYZE", "VERBOSE") ||
+			 Matches("EXPLAIN", "ALL"))
 		COMPLETE_WITH("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE");
 
 /* FETCH && MOVE */


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

* [PATCH v2] Add an ALL option to EXPLAIN
@ 2019-05-07 07:26  David Fetter <[email protected]>
  0 siblings, 0 replies; 9+ messages in thread

From: David Fetter @ 2019-05-07 07:26 UTC (permalink / raw)


which starts all boolean options at once.

diff --git a/doc/src/sgml/ref/explain.sgml b/doc/src/sgml/ref/explain.sgml
index 385d10411f..ade3793c5f 100644
--- a/doc/src/sgml/ref/explain.sgml
+++ b/doc/src/sgml/ref/explain.sgml
@@ -43,6 +43,7 @@ EXPLAIN [ ANALYZE ] [ VERBOSE ] <replaceable class="parameter">statement</replac
     BUFFERS [ <replaceable class="parameter">boolean</replaceable> ]
     TIMING [ <replaceable class="parameter">boolean</replaceable> ]
     SUMMARY [ <replaceable class="parameter">boolean</replaceable> ]
+    ALL [ <replaceable class="parameter">boolean</replaceable> ]
     FORMAT { TEXT | XML | JSON | YAML }
 </synopsis>
  </refsynopsisdiv>
@@ -224,6 +225,16 @@ ROLLBACK;
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term><literal>ALL</literal></term>
+    <listitem>
+     <para>
+      Include (or exclude) all of the above settings before examining the rest
+      of the options. It defaults to <literal>FALSE</literal>.
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term><literal>FORMAT</literal></term>
     <listitem>
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index a6c6de78f1..7ad73e4482 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -151,6 +151,26 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, const char *queryString,
 	bool		summary_set = false;
 
 	/* Parse options list. */
+	/* First, see whether ALL is set */
+	foreach(lc, stmt->options)
+	{
+		DefElem	   *opt = (DefElem *) lfirst(lc);
+
+		if (strcmp(opt->defname, "all") == 0)
+		{
+			es->analyze = defGetBoolean(opt);
+			es->verbose = defGetBoolean(opt);
+			es->costs = defGetBoolean(opt);
+			es->buffers = defGetBoolean(opt);
+			es->settings = defGetBoolean(opt);
+			timing_set = true;
+			es->timing = defGetBoolean(opt);
+			summary_set = true;
+			es->timing = defGetBoolean(opt);
+		}
+	}
+
+	/* If you add another boolean option here, remember to add it above, too */
 	foreach(lc, stmt->options)
 	{
 		DefElem    *opt = (DefElem *) lfirst(lc);
@@ -194,6 +214,8 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, const char *queryString,
 								opt->defname, p),
 						 parser_errposition(pstate, opt->location)));
 		}
+		else if (strcmp(opt->defname, "all") == 0)
+			; /* Do nothing */
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 3dc0e8a4fb..4a69f9711c 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -10704,6 +10704,7 @@ explain_option_elem:
 explain_option_name:
 			NonReservedWord			{ $$ = $1; }
 			| analyze_keyword		{ $$ = "analyze"; }
+			| ALL					{ $$ = "all"; }
 		;
 
 explain_option_arg:
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index bcddc7601e..8177f9c6da 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2862,9 +2862,9 @@ psql_completion(const char *text, int start, int end)
 		 * one word, so the above test is correct.
 		 */
 		if (ends_with(prev_wd, '(') || ends_with(prev_wd, ','))
-			COMPLETE_WITH("ANALYZE", "VERBOSE", "COSTS", "BUFFERS",
+			COMPLETE_WITH("ALL", "ANALYZE", "VERBOSE", "COSTS", "BUFFERS",
 						  "TIMING", "SUMMARY", "FORMAT");
-		else if (TailMatches("ANALYZE|VERBOSE|COSTS|BUFFERS|TIMING|SUMMARY"))
+		else if (TailMatches("ALL|ANALYZE|VERBOSE|COSTS|BUFFERS|TIMING|SUMMARY"))
 			COMPLETE_WITH("ON", "OFF");
 		else if (TailMatches("FORMAT"))
 			COMPLETE_WITH("TEXT", "XML", "JSON", "YAML");
@@ -2874,7 +2874,8 @@ psql_completion(const char *text, int start, int end)
 					  "VERBOSE");
 	else if (Matches("EXPLAIN", "(*)") ||
 			 Matches("EXPLAIN", "VERBOSE") ||
-			 Matches("EXPLAIN", "ANALYZE", "VERBOSE"))
+			 Matches("EXPLAIN", "ANALYZE", "VERBOSE") ||
+			 Matches("EXPLAIN", "ALL"))
 		COMPLETE_WITH("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE");
 
 /* FETCH && MOVE */


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

* Re: Do away with a few backwards compatibility macros
@ 2023-11-16 15:46  Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Nathan Bossart @ 2023-11-16 15:46 UTC (permalink / raw)
  To: Bharath Rupireddy <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

On Thu, Nov 16, 2023 at 07:11:41PM +0530, Bharath Rupireddy wrote:
> After a recent commit 6a72c42f (a related discussion [1]) which
> removed MemoryContextResetAndDeleteChildren(), I think there are a
> couple of other backward compatibility macros out there that can be
> removed. These macros are tuplestore_donestoring() which was
> introduced by commit dd04e95 21 years ago and SPI_push() and friends
> which were made no-ops macros by commit 1833f1a 7 years ago. Debian
> code search shows very minimal usages of these macros. Here's a patch
> attached to remove them.

I'm fine with this because all of these macros are no-ops for all supported
versions of Postgres.  Even if an extension is using them today, you'll get
the same behavior as before if you remove the uses and rebuild against
v12-v16.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com






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

* Re: Do away with a few backwards compatibility macros
@ 2023-11-21 04:58  Nathan Bossart <[email protected]>
  parent: Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Nathan Bossart @ 2023-11-21 04:58 UTC (permalink / raw)
  To: Bharath Rupireddy <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

On Thu, Nov 16, 2023 at 09:46:22AM -0600, Nathan Bossart wrote:
> On Thu, Nov 16, 2023 at 07:11:41PM +0530, Bharath Rupireddy wrote:
>> After a recent commit 6a72c42f (a related discussion [1]) which
>> removed MemoryContextResetAndDeleteChildren(), I think there are a
>> couple of other backward compatibility macros out there that can be
>> removed. These macros are tuplestore_donestoring() which was
>> introduced by commit dd04e95 21 years ago and SPI_push() and friends
>> which were made no-ops macros by commit 1833f1a 7 years ago. Debian
>> code search shows very minimal usages of these macros. Here's a patch
>> attached to remove them.
> 
> I'm fine with this because all of these macros are no-ops for all supported
> versions of Postgres.  Even if an extension is using them today, you'll get
> the same behavior as before if you remove the uses and rebuild against
> v12-v16.

Barring objections, I'll plan on committing this in the next week or so.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com






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

* Re: Do away with a few backwards compatibility macros
@ 2023-11-21 05:05  Tom Lane <[email protected]>
  parent: Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Tom Lane @ 2023-11-21 05:05 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; +Cc: Bharath Rupireddy <[email protected]>; PostgreSQL Hackers <[email protected]>

Nathan Bossart <[email protected]> writes:
> On Thu, Nov 16, 2023 at 09:46:22AM -0600, Nathan Bossart wrote:
>> I'm fine with this because all of these macros are no-ops for all supported
>> versions of Postgres.  Even if an extension is using them today, you'll get
>> the same behavior as before if you remove the uses and rebuild against
>> v12-v16.

> Barring objections, I'll plan on committing this in the next week or so.

No objection here, but should we try to establish some sort of project
policy around this sort of change (ie, removal of backwards-compatibility
support)?  "Once it no longer matters for any supported version" sounds
about right to me, but maybe somebody has an argument for thinking about
it differently.

			regards, tom lane






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

* Re: Do away with a few backwards compatibility macros
@ 2023-11-21 15:52  Nathan Bossart <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Nathan Bossart @ 2023-11-21 15:52 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Bharath Rupireddy <[email protected]>; PostgreSQL Hackers <[email protected]>

On Tue, Nov 21, 2023 at 12:05:36AM -0500, Tom Lane wrote:
> No objection here, but should we try to establish some sort of project
> policy around this sort of change (ie, removal of backwards-compatibility
> support)?  "Once it no longer matters for any supported version" sounds
> about right to me, but maybe somebody has an argument for thinking about
> it differently.

That seems reasonable to me.  I don't think we need to mandate that
backwards-compatibility support be removed as soon as it is eligible, but
it can be considered fair game at that point.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com






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

* Re: Do away with a few backwards compatibility macros
@ 2023-11-27 10:59  Bharath Rupireddy <[email protected]>
  parent: Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Bharath Rupireddy @ 2023-11-27 10:59 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; +Cc: Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>

On Tue, Nov 21, 2023 at 9:22 PM Nathan Bossart <[email protected]> wrote:
>
> On Tue, Nov 21, 2023 at 12:05:36AM -0500, Tom Lane wrote:
> > No objection here, but should we try to establish some sort of project
> > policy around this sort of change (ie, removal of backwards-compatibility
> > support)?  "Once it no longer matters for any supported version" sounds
> > about right to me, but maybe somebody has an argument for thinking about
> > it differently.
>
> That seems reasonable to me.  I don't think we need to mandate that
> backwards-compatibility support be removed as soon as it is eligible, but
> it can be considered fair game at that point.

I think it's easy to miss/enforce a documented policy. IMV, moving
towards pg_attribute_deprecated as Alvaro Herrera said in the other
thread https://www.postgresql.org/message-id/202311141920.edtj56saukiv%40alvherre.pgsql
can help. Authors then can declare the variables and functions as
deprecated so that the code compilation with
-Wno-deprecated-declarations can help track all such deprecated code.

Having said that, I'm all +1 if the v1 patch proposed in this thread gets in.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com






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

* Re: Do away with a few backwards compatibility macros
@ 2023-11-27 19:14  Nathan Bossart <[email protected]>
  parent: Bharath Rupireddy <[email protected]>
  0 siblings, 0 replies; 9+ messages in thread

From: Nathan Bossart @ 2023-11-27 19:14 UTC (permalink / raw)
  To: Bharath Rupireddy <[email protected]>; +Cc: Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>

On Mon, Nov 27, 2023 at 04:29:18PM +0530, Bharath Rupireddy wrote:
> I think it's easy to miss/enforce a documented policy. IMV, moving
> towards pg_attribute_deprecated as Alvaro Herrera said in the other
> thread https://www.postgresql.org/message-id/202311141920.edtj56saukiv%40alvherre.pgsql
> can help. Authors then can declare the variables and functions as
> deprecated so that the code compilation with
> -Wno-deprecated-declarations can help track all such deprecated code.

I'm +1 for adding pg_attribute_deprecated once we have something to use it
for.

> Having said that, I'm all +1 if the v1 patch proposed in this thread gets in.

Committed.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com






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


end of thread, other threads:[~2023-11-27 19:14 UTC | newest]

Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-05-07 07:26 [PATCH v1] Add an ALL option to EXPLAIN David Fetter <[email protected]>
2019-05-07 07:26 [PATCH v2] Add an ALL option to EXPLAIN David Fetter <[email protected]>
2019-05-07 07:26 [PATCH v2] Add an ALL option to EXPLAIN David Fetter <[email protected]>
2023-11-16 15:46 Re: Do away with a few backwards compatibility macros Nathan Bossart <[email protected]>
2023-11-21 04:58 ` Re: Do away with a few backwards compatibility macros Nathan Bossart <[email protected]>
2023-11-21 05:05   ` Re: Do away with a few backwards compatibility macros Tom Lane <[email protected]>
2023-11-21 15:52     ` Re: Do away with a few backwards compatibility macros Nathan Bossart <[email protected]>
2023-11-27 10:59       ` Re: Do away with a few backwards compatibility macros Bharath Rupireddy <[email protected]>
2023-11-27 19:14         ` Re: Do away with a few backwards compatibility macros Nathan Bossart <[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