On Sun, Apr 07, 2019 at 08:15:06AM -0400, Alvaro Herrera wrote:
> So how about the attached version?

+1

I found a few issues.

\dP+ didn't work.  Fix attached.

+static const SchemaQuery Query_for_list_of_partitioned_relations = {           
                                                                  
+       .catname = "pg_catalog.pg_class c",                                     
                                                                  
+       .selcondition = "c.relkind = " CppAsString2(RELKIND_PARTITIONED_TABLE), 
                                                                  

=> Should it be called Query_for_list_of_partitioned_tables ?  Or should
c.relkind match indices, too ?

On Sat, Apr 06, 2019 at 01:36:23AM -0300, Alvaro Herrera wrote:
> Maybe the only behavior change I'd do to the submitted patch is to have
> \dP show both tables and indexes, while \dPt shows only tables and \dPi
> shows only indexes.  Maybe have \dPti show both tables and indexes? (
> identical to \dP)  That would be consistent with \d itself.

I think there's an issue with showing indices.  You said that \dP should be
same as \dPti, no?  Right now, indices are not shown in \dP, unless a pattern
is given.  I see you add that behavior in the regression tests; is that really
what's intended ?  Also, right now adding a pattern affects how sizes are
computed, I don't see why that's desirable or, if so, how to resolve that
inconsistency, or how to document it.

Justin
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 89f08fc..8254d61 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -789,6 +789,7 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd)
 					switch (cmd[2])
 					{
 						case '\0':
+						case '+':
 						case 't':
 						case 'i':
 						case 'n':
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 40f7531..936439e 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3804,7 +3804,6 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
 	printQueryOpt myopt = pset.popt;
 	bool translate_columns[] = {false, false, false, false, false, false, false, false, false};
 	const char *size_function;
-	const char *relkind_str;
 	const char *tabletitle;
 	bool		mixed_output = false;
 
@@ -3813,7 +3812,7 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
 		showTables = showIndexes = true;
 
 	/*
-	 * Note: Declarative table partitions are only supported as of Pg 10.0.
+	 * Note: Declarative table partitioning is only supported as of Pg 10.0.
 	 */
 	if (pset.sversion < 100000)
 	{
@@ -3829,14 +3828,12 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
 	{
 		/* \dPi */
 		size_function = "pg_table_size";
-		relkind_str = CppAsString2(RELKIND_PARTITIONED_INDEX);
 		tabletitle = gettext_noop("List of partitioned indexes");
 	}
 	else if (showTables && !showIndexes)
 	{
 		/* \dPt */
 		size_function = "pg_table_size";
-		relkind_str = CppAsString2(RELKIND_PARTITIONED_TABLE);
 		tabletitle = gettext_noop("List of partitioned tables");
 	}
 	else
@@ -3844,17 +3841,14 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
 		/* show all kinds */
 		tabletitle = gettext_noop("List of partitioned tables and indexes");
 		mixed_output = true;
+		size_function = "pg_table_size";
 		if (!pattern)
 		{
-			size_function = "pg_total_relation_size";
-			relkind_str = CppAsString2(RELKIND_PARTITIONED_TABLE);
+			// why ??? size_function = "pg_total_relation_size";
+			// why this too ??? relkind_str = CppAsString2(RELKIND_PARTITIONED_TABLE);
 		}
 		else
-		{
 			size_function = "pg_table_size";
-			relkind_str = CppAsString2(RELKIND_PARTITIONED_TABLE)
-				", " CppAsString2(RELKIND_PARTITIONED_INDEX);
-		}
 	}
 
 	initPQExpBuffer(&buf);
@@ -3963,7 +3957,14 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
 		}
 	}
 
-	appendPQExpBuffer(&buf, "\nWHERE c.relkind IN (%s)", relkind_str);
+	appendPQExpBufferStr(&buf, "\nWHERE c.relkind IN (");
+	if (showTables)
+		appendPQExpBufferStr(&buf, CppAsString2(RELKIND_PARTITIONED_TABLE) ",");
+	if (showIndexes)
+		appendPQExpBufferStr(&buf, CppAsString2(RELKIND_PARTITIONED_INDEX) ",");
+	appendPQExpBufferStr(&buf, "''");	/* dummy */
+	appendPQExpBufferStr(&buf, ")\n");
+
 	appendPQExpBufferStr(&buf, !showNested ? " AND NOT c.relispartition\n" : "\n");
 
 	if (!pattern)

Reply via email to