Merge #95272

95272: sql: fix the pretty-printing of the PARTITION BY clauses r=otan a=knz

Fixes #95238.

The AST was incorrectly using tree.UnrestrictedName, whereas the PARTITION BY clauses could use special identifiers.

Release note: None

Co-authored-by: Raphael 'kena' Poss <[email protected]>

pkg/ccl/logictestccl/testdata/logic_test/partitioning

@@ -1226,3 +1226,36 @@ CREATE INDEX ON partition_array (a) PARTITION BY LIST (a) (PARTITION blah VALUES
 
 statement error unimplemented: partitioning by array column
 CREATE INDEX ON partition_array (a) PARTITION BY RANGE (a) (PARTITION blah VALUES FROM (ARRAY[1]) TO (ARRAY[2]))
+
+subtest regression_95238
+
+statement ok
+CREATE TABLE "a.b" ("c.d" INT PRIMARY KEY)
+PARTITION BY LIST ("c.d") (PARTITION "e.f" VALUES IN (0));
+CREATE TABLE "g.h" ("i.j" INT PRIMARY KEY)
+PARTITION BY RANGE ("i.j") (PARTITION "k.l" VALUES FROM (0) TO (1));
+---
+
+# Check that the result of pretty-printing quotes the identifiers
+# in the partition list and the partition names properly.
+query TT
+SHOW CREATE TABLE "a.b"
+----
+"a.b"  CREATE TABLE public."a.b" (
+         "c.d" INT8 NOT NULL,
+         CONSTRAINT "a.b_pkey" PRIMARY KEY ("c.d" ASC)
+       ) PARTITION BY LIST ("c.d") (
+         PARTITION "e.f" VALUES IN ((0))
+       )
+       -- Warning: Partitioned table with no zone configurations.
+
+query TT
+SHOW CREATE TABLE "g.h"
+----
+"g.h"  CREATE TABLE public."g.h" (
+         "i.j" INT8 NOT NULL,
+         CONSTRAINT "g.h_pkey" PRIMARY KEY ("i.j" ASC)
+       ) PARTITION BY RANGE ("i.j") (
+         PARTITION "k.l" VALUES FROM (0) TO (1)
+       )
+       -- Warning: Partitioned table with no zone configurations.

pkg/sql/catalog/multiregion/region_util.go

@@ -23,7 +23,7 @@ func PartitionByForRegionalByRow(regionConfig RegionConfig, col tree.Name) *tree
 	listPartition := make([]tree.ListPartition, len(regionConfig.Regions()))
 	for i, region := range regionConfig.Regions() {
 		listPartition[i] = tree.ListPartition{
-			Name:  tree.UnrestrictedName(region),
+			Name:  tree.Name(region),
 			Exprs: tree.Exprs{tree.NewStrVal(string(region))},
 		}
 	}

pkg/sql/parser/sql.y

@@ -8948,7 +8948,7 @@ list_partition:
   partition VALUES IN '(' expr_list ')' opt_partition_by
   {
     $$.val = tree.ListPartition{
-      Name: tree.UnrestrictedName($1),
+      Name: tree.Name($1),
       Exprs: $5.exprs(),
       Subpartition: $7.partitionBy(),
     }
@@ -8968,7 +8968,7 @@ range_partition:
   partition VALUES FROM '(' expr_list ')' TO '(' expr_list ')' opt_partition_by
   {
     $$.val = tree.RangePartition{
-      Name: tree.UnrestrictedName($1),
+      Name: tree.Name($1),
       From: $5.exprs(),
       To: $9.exprs(),
       Subpartition: $11.partitionBy(),

pkg/sql/parser/testdata/create_table

@@ -550,6 +550,15 @@ CREATE TABLE a (UNIQUE (b) PARTITION BY LIST (c) (PARTITION d VALUES IN ((1))))
 CREATE TABLE a (UNIQUE (b) PARTITION BY LIST (c) (PARTITION d VALUES IN (_))) -- literals removed
 CREATE TABLE _ (UNIQUE (_) PARTITION BY LIST (_) (PARTITION _ VALUES IN (1))) -- identifiers removed
 
+# Regression test for #95238
+parse
+CREATE TABLE a (UNIQUE INDEX (b) PARTITION BY LIST ("c d") (PARTITION "e f" VALUES IN (1)))
+----
+CREATE TABLE a (UNIQUE (b) PARTITION BY LIST ("c d") (PARTITION "e f" VALUES IN (1))) -- normalized!
+CREATE TABLE a (UNIQUE (b) PARTITION BY LIST ("c d") (PARTITION "e f" VALUES IN ((1)))) -- fully parenthesized
+CREATE TABLE a (UNIQUE (b) PARTITION BY LIST ("c d") (PARTITION "e f" VALUES IN (_))) -- literals removed
+CREATE TABLE _ (UNIQUE (_) PARTITION BY LIST (_) (PARTITION _ VALUES IN (1))) -- identifiers removed
+
 parse
 CREATE TABLE a (b INT8 UNIQUE WITHOUT INDEX)
 ----
@@ -2468,3 +2477,12 @@ DETAIL: source SQL:
 CREATE TABLE a (b INT8, c STRING, CONSTRAINT d UNIQUE WITHOUT INDEX (b, c) NOT VISIBLE)
                                                                                ^
 HINT: try \h CREATE TABLE
+
+# Regression test for #95238
+parse
+ALTER TABLE a PARTITION ALL BY LIST ("a b", "c.d") (PARTITION "e.f" VALUES IN (1))
+----
+ALTER TABLE a PARTITION ALL BY LIST ("a b", "c.d") (PARTITION "e.f" VALUES IN (1))
+ALTER TABLE a PARTITION ALL BY LIST ("a b", "c.d") (PARTITION "e.f" VALUES IN ((1))) -- fully parenthesized
+ALTER TABLE a PARTITION ALL BY LIST ("a b", "c.d") (PARTITION "e.f" VALUES IN (_)) -- literals removed
+ALTER TABLE _ PARTITION ALL BY LIST (_, _) (PARTITION _ VALUES IN (1)) -- identifiers removed

pkg/sql/partition.go

@@ -61,7 +61,7 @@ func partitionByFromTableDescImpl(
 	a := &tree.DatumAlloc{}
 	err := part.ForEachList(func(name string, values [][]byte, subPartitioning catalog.Partitioning) (err error) {
 		lp := tree.ListPartition{
-			Name:  tree.UnrestrictedName(name),
+			Name:  tree.Name(name),
 			Exprs: make(tree.Exprs, len(values)),
 		}
 		for j, values := range values {
@@ -101,7 +101,7 @@ func partitionByFromTableDescImpl(
 
 	// Copy the RANGE of the PARTITION BY clause.
 	err = part.ForEachRange(func(name string, from, to []byte) error {
-		rp := tree.RangePartition{Name: tree.UnrestrictedName(name)}
+		rp := tree.RangePartition{Name: tree.Name(name)}
 		fromTuple, _, err := rowenc.DecodePartitionTuple(
 			a, codec, tableDesc, idx, part, from, fakePrefixDatums)
 		if err != nil {

pkg/sql/randgen/schema.go

@@ -584,7 +584,7 @@ func randIndexTableDefFromCols(
 		numExpressions := rng.Intn(10) + 1
 		for i := 0; i < numPartitions; i++ {
 			var partition tree.ListPartition
-			partition.Name = tree.UnrestrictedName(fmt.Sprintf("%s_part_%d", tableName, i))
+			partition.Name = tree.Name(fmt.Sprintf("%s_part_%d", tableName, i))
 			// Add up to 10 expressions in each partition.
 			for j := 0; j < numExpressions; j++ {
 				// Use a tuple to contain the expressions in case there are multiple

pkg/sql/sem/tree/create.go

@@ -1362,7 +1362,7 @@ func (node *PartitionBy) formatListOrRange(ctx *FmtCtx) {
 
 // ListPartition represents a PARTITION definition within a PARTITION BY LIST.
 type ListPartition struct {
-	Name         UnrestrictedName
+	Name         Name
 	Exprs        Exprs
 	Subpartition *PartitionBy
 }
@@ -1381,7 +1381,7 @@ func (node *ListPartition) Format(ctx *FmtCtx) {
 
 // RangePartition represents a PARTITION definition within a PARTITION BY RANGE.
 type RangePartition struct {
-	Name         UnrestrictedName
+	Name         Name
 	From         Exprs
 	To           Exprs
 	Subpartition *PartitionBy

pkg/sql/sem/tree/pretty.go

@@ -1010,8 +1010,8 @@ func (node *Insert) doc(p *PrettyCfg) pretty.Doc {
 
 func (node *NameList) doc(p *PrettyCfg) pretty.Doc {
 	d := make([]pretty.Doc, len(*node))
-	for i, n := range *node {
-		d[i] = p.Doc(&n)
+	for i := range *node {
+		d[i] = p.Doc(&(*node)[i])
 	}
 	return p.commaSeparated(d...)
 }

pkg/sql/sem/tree/testdata/pretty/create_index_partition.align-deindent.golden.short

@@ -11,7 +11,7 @@ CREATE INDEX foo_b_idx ON foo (b, c, d)
 								VALUES IN (
 									'baz'
 								),
-							PARTITION default
+							PARTITION "default"
 								VALUES IN (
 									DEFAULT
 								)

pkg/sql/sem/tree/testdata/pretty/create_index_partition.align-only.golden.short

@@ -11,7 +11,7 @@ CREATE INDEX foo_b_idx ON foo (b, c, d)
 								VALUES IN (
 									'baz'
 								),
-							PARTITION default
+							PARTITION "default"
 								VALUES IN (
 									DEFAULT
 								)

pkg/sql/sem/tree/testdata/pretty/create_index_partition.ref.golden.short

@@ -8,7 +8,7 @@ CREATE INDEX foo_b_idx
 		(
 			PARTITION baz
 				VALUES IN ('baz'),
-			PARTITION default
+			PARTITION "default"
 				VALUES IN (DEFAULT)
 		)
 

pkg/sql/sem/tree/testdata/pretty/create_partition_list.align-deindent.golden.short

@@ -20,7 +20,7 @@ CREATE TABLE students_by_list (
 										'AU',
 										'NZ'
 									),
-								PARTITION default
+								PARTITION "default"
 									VALUES IN (
 										DEFAULT
 									)

pkg/sql/sem/tree/testdata/pretty/create_partition_list.align-only.golden.short

@@ -20,7 +20,7 @@ CREATE TABLE students_by_list (
 										'AU',
 										'NZ'
 									),
-								PARTITION default
+								PARTITION "default"
 									VALUES IN (
 										DEFAULT
 									)

pkg/sql/sem/tree/testdata/pretty/create_partition_list.ref.golden.short

@@ -21,7 +21,7 @@ CREATE TABLE students_by_list (
 				VALUES IN ('CA', 'US'),
 			PARTITION australia
 				VALUES IN ('AU', 'NZ'),
-			PARTITION default
+			PARTITION "default"
 				VALUES IN (DEFAULT)
 		)
 

pkg/sql/show_create_clauses.go

@@ -617,7 +617,7 @@ func ShowCreatePartitioning(
 		if i != 0 {
 			buf.WriteString(", ")
 		}
-		buf.WriteString(idx.GetKeyColumnName(colOffset + i))
+		buf.WriteString(tree.NameString(idx.GetKeyColumnName(colOffset + i)))
 	}
 	buf.WriteString(`) (`)
 	fmtCtx := tree.NewFmtCtx(tree.FmtSimple)
@@ -630,7 +630,7 @@ func ShowCreatePartitioning(
 		buf.WriteString("\n")
 		buf.WriteString(indentStr)
 		buf.WriteString("\tPARTITION ")
-		fmtCtx.FormatNameP(&name)
+		fmtCtx.FormatName(name)
 		_, _ = fmtCtx.Buffer.WriteTo(buf)
 		buf.WriteString(` VALUES IN (`)
 		for j, values := range values {
@@ -661,7 +661,7 @@ func ShowCreatePartitioning(
 		buf.WriteString("\n")
 		buf.WriteString(indentStr)
 		buf.WriteString("\tPARTITION ")
-		buf.WriteString(name)
+		buf.WriteString(tree.NameString(name))
 		buf.WriteString(" VALUES FROM ")
 		fromTuple, _, err := rowenc.DecodePartitionTuple(
 			a, codec, tableDesc, idx, part, from, fakePrefixDatums)