Merge #60590

60590: sql: fix SHOW CREATE output for partitioned and interleaved partial indexes r=mgartner a=mgartner

#### sql: fix SHOW CREATE output for partitioned partial indexes

The accepted syntax for creating partitioned partial indexes requires
the `PARTITION BY` clause to precede the `WHERE` predicate clause.
Previously, `SHOW CREATE` formatted indexes with the `WHERE` clause
preceding the `PARTITION BY` clause. This commit fixes `SHOW CREATE` to
match the parser's accepted ordering of clauses.

Fixes #60019

Release note (bug fix): The `SHOW CREATE` output of a partitioned
partial index now lists the `PARTITION BY` and `WHERE` clauses in the
order accepted by the parser.

Release justification: Fixes a bug that makes `SHOW CREATE` not
round-trippable.

#### sql: fix SHOW CREATE output for partial interleaved indexes

The accepted syntax for creating partial interleaved indexes requires
the `INTERLEAVE` clause to precede the `WHERE` clause. This commit fixes
the output of partial interleaved indexes in `SHOW CREATE` to match this
syntax.

Fixes #60701

Release note (bug fix): The `SHOW CREATE` output of a partial
interleaved index now lists the `INTERLEAVED` and `WHERE` clauses in the
order accepted by the parser.

Release justification: Fixes a bug that makes `SHOW CREATE` not
round-trippable.


Co-authored-by: Marcus Gartner <[email protected]>

pkg/ccl/importccl/read_import_mysql_test.go

@@ -220,11 +220,15 @@ func compareTables(t *testing.T, expected, got *descpb.TableDescriptor) {
 		tableName := &descpb.AnonymousTable
 		expectedDesc := tabledesc.NewImmutable(*expected)
 		gotDesc := tabledesc.NewImmutable(*got)
-		e, err := catformat.IndexForDisplay(ctx, expectedDesc, tableName, &expected.Indexes[i], &semaCtx)
+		e, err := catformat.IndexForDisplay(
+			ctx, expectedDesc, tableName, &expected.Indexes[i], "" /* partition */, "" /* interleave */, &semaCtx,
+		)
 		if err != nil {
 			t.Fatalf("unexpected error: %s", err)
 		}
-		g, err := catformat.IndexForDisplay(ctx, gotDesc, tableName, &got.Indexes[i], &semaCtx)
+		g, err := catformat.IndexForDisplay(
+			ctx, gotDesc, tableName, &got.Indexes[i], "" /* partition */, "" /* interleave */, &semaCtx,
+		)
 		if err != nil {
 			t.Fatalf("unexpected error: %s", err)
 		}

pkg/ccl/logictestccl/testdata/logic_test/partitioning_index

@@ -184,3 +184,31 @@ inverted  CREATE TABLE public.inverted (
           FAMILY "primary" (a, b, j)
 )
 -- Warning: Partitioned table with no zone configurations.
+
+# Regression test for #60019. The index predicate should be formatted after the
+# PARTITION BY clause to match the syntax that is accepted.
+statement ok
+CREATE TABLE t60019 (
+  pk INT PRIMARY KEY,
+  a INT,
+  b INT,
+  INDEX (a, b) PARTITION BY LIST (a) (
+    PARTITION c_implicit VALUES IN (3)
+  ) WHERE b > 0,
+  FAMILY (pk, a, b)
+)
+
+query T
+SELECT create_statement FROM [SHOW CREATE TABLE t60019]
+----
+CREATE TABLE public.t60019 (
+  pk INT8 NOT NULL,
+  a INT8 NULL,
+  b INT8 NULL,
+  CONSTRAINT "primary" PRIMARY KEY (pk ASC),
+  INDEX t60019_a_b_idx (a ASC, b ASC) PARTITION BY LIST (a) (
+    PARTITION c_implicit VALUES IN ((3))
+  ) WHERE b > 0:::INT8,
+  FAMILY fam_0_pk_a_b (pk, a, b)
+)
+-- Warning: Partitioned table with no zone configurations.

pkg/ccl/multiregionccl/show_test.go

@@ -90,6 +90,22 @@ func TestShowCreateTable(t *testing.T) {
 	CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
 	INDEX a_idx (a ASC),
 	FAMILY "primary" (a, crdb_region_col, rowid)
+) LOCALITY REGIONAL BY ROW AS crdb_region_col`,
+			Database: "mrdb",
+		},
+		{
+			CreateStatement: `SET experimental_enable_implicit_column_partitioning = true; CREATE TABLE %s (
+				a INT,
+				crdb_region_col crdb_internal_region,
+				INDEX a_idx (a) WHERE a > 0
+			) LOCALITY REGIONAL BY ROW AS crdb_region_col`,
+			Expect: `CREATE TABLE public.%[1]s (
+	a INT8 NULL,
+	crdb_region_col public.crdb_internal_region NOT NULL,
+	rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
+	CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
+	INDEX a_idx (a ASC) WHERE a > 0:::INT8,
+	FAMILY "primary" (a, crdb_region_col, rowid)
 ) LOCALITY REGIONAL BY ROW AS crdb_region_col`,
 			Database: "mrdb",
 		},

pkg/sql/catalog/catformat/index.go

@@ -41,6 +41,8 @@ func IndexForDisplay(
 	table catalog.TableDescriptor,
 	tableName *tree.TableName,
 	index *descpb.IndexDescriptor,
+	partition string,
+	interleave string,
 	semaCtx *tree.SemaContext,
 ) (string, error) {
 	f := tree.NewFmtCtx(tree.FmtSimple)
@@ -76,6 +78,9 @@ func IndexForDisplay(
 		f.WriteByte(')')
 	}
 
+	f.WriteString(interleave)
+	f.WriteString(partition)
+
 	if index.GeoConfig.S2Geometry != nil || index.GeoConfig.S2Geography != nil {
 		var s2Config *geoindex.S2Config
 

pkg/sql/catalog/catformat/index_test.go

@@ -57,21 +57,46 @@ func TestIndexForDisplay(t *testing.T) {
 	partialIndex.Predicate = "a > 1:::INT8"
 
 	testData := []struct {
-		index     descpb.IndexDescriptor
-		tableName tree.TableName
-		expected  string
+		index      descpb.IndexDescriptor
+		tableName  tree.TableName
+		partition  string
+		interleave string
+		expected   string
 	}{
-		{baseIndex, descpb.AnonymousTable, "INDEX baz (a ASC, b DESC)"},
-		{baseIndex, tableName, "INDEX baz ON foo.public.bar (a ASC, b DESC)"},
-		{uniqueIndex, descpb.AnonymousTable, "UNIQUE INDEX baz (a ASC, b DESC)"},
-		{invertedIndex, descpb.AnonymousTable, "INVERTED INDEX baz (a)"},
-		{storingIndex, descpb.AnonymousTable, "INDEX baz (a ASC, b DESC) STORING (c)"},
-		{partialIndex, descpb.AnonymousTable, "INDEX baz (a ASC, b DESC) WHERE a > 1:::INT8"},
+		{baseIndex, descpb.AnonymousTable, "", "", "INDEX baz (a ASC, b DESC)"},
+		{baseIndex, tableName, "", "", "INDEX baz ON foo.public.bar (a ASC, b DESC)"},
+		{uniqueIndex, descpb.AnonymousTable, "", "", "UNIQUE INDEX baz (a ASC, b DESC)"},
+		{invertedIndex, descpb.AnonymousTable, "", "", "INVERTED INDEX baz (a)"},
+		{storingIndex, descpb.AnonymousTable, "", "", "INDEX baz (a ASC, b DESC) STORING (c)"},
+		{partialIndex, descpb.AnonymousTable, "", "", "INDEX baz (a ASC, b DESC) WHERE a > 1:::INT8"},
+		{
+			partialIndex,
+			descpb.AnonymousTable,
+			" PARTITION BY LIST (a) (PARTITION p VALUES IN (2))",
+			"",
+			"INDEX baz (a ASC, b DESC) PARTITION BY LIST (a) (PARTITION p VALUES IN (2)) WHERE a > 1:::INT8",
+		},
+		{
+			partialIndex,
+			descpb.AnonymousTable,
+			"",
+			" INTERLEAVE IN PARENT par (a)",
+			"INDEX baz (a ASC, b DESC) INTERLEAVE IN PARENT par (a) WHERE a > 1:::INT8",
+		},
+		{
+			partialIndex,
+			descpb.AnonymousTable,
+			" PARTITION BY LIST (a) (PARTITION p VALUES IN (2))",
+			" INTERLEAVE IN PARENT par (a)",
+			"INDEX baz (a ASC, b DESC) INTERLEAVE IN PARENT par (a) PARTITION BY LIST (a) (PARTITION p VALUES IN (2)) WHERE a > 1:::INT8",
+		},
 	}
 
 	for testIdx, tc := range testData {
 		t.Run(strconv.Itoa(testIdx), func(t *testing.T) {
-			got, err := IndexForDisplay(ctx, tableDesc, &tc.tableName, &tc.index, &semaCtx)
+			got, err := IndexForDisplay(
+				ctx, tableDesc, &tc.tableName, &tc.index, tc.partition, tc.interleave, &semaCtx,
+			)
 			if err != nil {
 				t.Fatalf("unexpected error: %s", err)
 			}

pkg/sql/crdb_internal.go

@@ -2033,7 +2033,9 @@ func showCreateIndexWithInterleave(
 	semaCtx *tree.SemaContext,
 ) error {
 	f.WriteString("CREATE ")
-	idxStr, err := catformat.IndexForDisplay(ctx, table, &tableName, idx, semaCtx)
+	idxStr, err := catformat.IndexForDisplay(
+		ctx, table, &tableName, idx, "" /* partition */, "" /* interleave */, semaCtx,
+	)
 	if err != nil {
 		return err
 	}

pkg/sql/logictest/testdata/logic_test/show_create

@@ -106,3 +106,24 @@ c  CREATE TABLE public.c (
 COMMENT ON TABLE public.c IS 'table';
 COMMENT ON COLUMN public.c.a IS 'column';
 COMMENT ON INDEX public.c@c_a_b_idx IS 'index'
+
+# Regression test for #60701. In the SHOW CREATE output of a table with a
+# partial interleaved index, the INTERLEAVE clause should precede the WHERE
+# clause. This matches the accepted syntax for creating partial interleaved
+# indexes.
+statement ok
+CREATE TABLE t60701_a (a INT PRIMARY KEY, FAMILY (a));
+CREATE TABLE t60701_b (b INT PRIMARY KEY, a INT REFERENCES t60701_a (a), FAMILY (b, a));
+CREATE INDEX i ON t60701_b (a) INTERLEAVE IN PARENT t60701_a (a) WHERE b > 0;
+
+query T
+SELECT create_statement FROM [SHOW CREATE TABLE t60701_b]
+----
+CREATE TABLE public.t60701_b (
+   b INT8 NOT NULL,
+   a INT8 NULL,
+   CONSTRAINT "primary" PRIMARY KEY (b ASC),
+   CONSTRAINT fk_a_ref_t60701_a FOREIGN KEY (a) REFERENCES public.t60701_a(a),
+   INDEX i (a ASC) INTERLEAVE IN PARENT public.t60701_a (a) WHERE b > 0:::INT8,
+   FAMILY fam_0_b_a (b, a)
+)

pkg/sql/show_create.go

@@ -151,27 +151,41 @@ func ShowCreateTable(
 		}
 		if !idx.Primary() && includeInterleaveClause {
 			// Showing the primary index is handled above.
-			f.WriteString(",\n\t")
-			idxStr, err := catformat.IndexForDisplay(ctx, desc, &descpb.AnonymousTable, idx.IndexDesc(), &p.RunParams(ctx).p.semaCtx)
-			if err != nil {
+
+			// Build the PARTITION BY clause.
+			var partitionBuf bytes.Buffer
+			if err := ShowCreatePartitioning(
+				a, p.ExecCfg().Codec, desc, idx.IndexDesc(), &idx.IndexDesc().Partitioning, &partitionBuf, 1 /* indent */, 0, /* colOffset */
+			); err != nil {
 				return "", err
 			}
-			f.WriteString(idxStr)
-			// Showing the INTERLEAVE and PARTITION BY for the primary index are
-			// handled last.
 
 			// Add interleave or Foreign Key indexes only to the create_table columns,
 			// and not the create_nofks column.
+			var interleaveBuf bytes.Buffer
 			if includeInterleaveClause {
-				if err := showCreateInterleave(idx.IndexDesc(), &f.Buffer, dbPrefix, lCtx); err != nil {
+				// TODO(mgartner): The logic in showCreateInterleave can be
+				// moved to catformat.IndexForDisplay.
+				if err := showCreateInterleave(idx.IndexDesc(), &interleaveBuf, dbPrefix, lCtx); err != nil {
 					return "", err
 				}
 			}
-			if err := ShowCreatePartitioning(
-				a, p.ExecCfg().Codec, desc, idx.IndexDesc(), &idx.IndexDesc().Partitioning, &f.Buffer, 1 /* indent */, 0, /* colOffset */
-			); err != nil {
+
+			f.WriteString(",\n\t")
+			idxStr, err := catformat.IndexForDisplay(
+				ctx,
+				desc,
+				&descpb.AnonymousTable,
+				idx.IndexDesc(),
+				partitionBuf.String(),
+				interleaveBuf.String(),
+				p.RunParams(ctx).p.SemaCtx(),
+			)
+			if err != nil {
 				return "", err
 			}
+			f.WriteString(idxStr)
+
 		}
 	}