From 67b0933ea5b805377e0fde843772cb79daaf84af Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Sun, 15 Jan 2023 21:41:19 +0100 Subject: [PATCH] sql: fix the pretty-printing of the PARTITION BY clauses The AST was incorrectly using tree.UnrestrictedName, whereas the PARTITION BY clauses could use special identifiers. Release note: None --- .../testdata/logic_test/partitioning | 33 +++++++++++++++++++ pkg/sql/catalog/multiregion/region_util.go | 2 +- pkg/sql/parser/sql.y | 4 +-- pkg/sql/parser/testdata/create_table | 18 ++++++++++ pkg/sql/partition.go | 4 +-- pkg/sql/randgen/schema.go | 2 +- pkg/sql/sem/tree/create.go | 4 +-- pkg/sql/sem/tree/pretty.go | 4 +-- ...ndex_partition.align-deindent.golden.short | 2 +- ...te_index_partition.align-only.golden.short | 2 +- .../create_index_partition.ref.golden.short | 2 +- ...partition_list.align-deindent.golden.short | 2 +- ...ate_partition_list.align-only.golden.short | 2 +- .../create_partition_list.ref.golden.short | 2 +- pkg/sql/show_create_clauses.go | 6 ++-- 15 files changed, 70 insertions(+), 19 deletions(-) diff --git a/pkg/ccl/logictestccl/testdata/logic_test/partitioning b/pkg/ccl/logictestccl/testdata/logic_test/partitioning index 3cc7223e66c6..63a5ccd8d592 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/partitioning +++ b/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. diff --git a/pkg/sql/catalog/multiregion/region_util.go b/pkg/sql/catalog/multiregion/region_util.go index 26012745c96b..534b9434b3c3 100644 --- a/pkg/sql/catalog/multiregion/region_util.go +++ b/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))}, } } diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y index d04bc4d84e92..fecbab060881 100644 --- a/pkg/sql/parser/sql.y +++ b/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(), diff --git a/pkg/sql/parser/testdata/create_table b/pkg/sql/parser/testdata/create_table index da542c9cecdf..41c772f43321 100644 --- a/pkg/sql/parser/testdata/create_table +++ b/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 diff --git a/pkg/sql/partition.go b/pkg/sql/partition.go index 26e824e3a3db..22a8ab2b5718 100644 --- a/pkg/sql/partition.go +++ b/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 { diff --git a/pkg/sql/randgen/schema.go b/pkg/sql/randgen/schema.go index e5eee95c8702..3d1543fbaeb1 100644 --- a/pkg/sql/randgen/schema.go +++ b/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 diff --git a/pkg/sql/sem/tree/create.go b/pkg/sql/sem/tree/create.go index 264b4f63f93e..6211ce627729 100644 --- a/pkg/sql/sem/tree/create.go +++ b/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 diff --git a/pkg/sql/sem/tree/pretty.go b/pkg/sql/sem/tree/pretty.go index 1e6c7a6017b5..e713c28b08a0 100644 --- a/pkg/sql/sem/tree/pretty.go +++ b/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...) } diff --git a/pkg/sql/sem/tree/testdata/pretty/create_index_partition.align-deindent.golden.short b/pkg/sql/sem/tree/testdata/pretty/create_index_partition.align-deindent.golden.short index dfde1731f4f3..bd83aaf85ba5 100644 --- a/pkg/sql/sem/tree/testdata/pretty/create_index_partition.align-deindent.golden.short +++ b/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 ) diff --git a/pkg/sql/sem/tree/testdata/pretty/create_index_partition.align-only.golden.short b/pkg/sql/sem/tree/testdata/pretty/create_index_partition.align-only.golden.short index dfde1731f4f3..bd83aaf85ba5 100644 --- a/pkg/sql/sem/tree/testdata/pretty/create_index_partition.align-only.golden.short +++ b/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 ) diff --git a/pkg/sql/sem/tree/testdata/pretty/create_index_partition.ref.golden.short b/pkg/sql/sem/tree/testdata/pretty/create_index_partition.ref.golden.short index 19e6100a694a..252c902f1e88 100644 --- a/pkg/sql/sem/tree/testdata/pretty/create_index_partition.ref.golden.short +++ b/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) ) diff --git a/pkg/sql/sem/tree/testdata/pretty/create_partition_list.align-deindent.golden.short b/pkg/sql/sem/tree/testdata/pretty/create_partition_list.align-deindent.golden.short index 2651ad3212c6..b74982807f54 100644 --- a/pkg/sql/sem/tree/testdata/pretty/create_partition_list.align-deindent.golden.short +++ b/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 ) diff --git a/pkg/sql/sem/tree/testdata/pretty/create_partition_list.align-only.golden.short b/pkg/sql/sem/tree/testdata/pretty/create_partition_list.align-only.golden.short index 2651ad3212c6..b74982807f54 100644 --- a/pkg/sql/sem/tree/testdata/pretty/create_partition_list.align-only.golden.short +++ b/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 ) diff --git a/pkg/sql/sem/tree/testdata/pretty/create_partition_list.ref.golden.short b/pkg/sql/sem/tree/testdata/pretty/create_partition_list.ref.golden.short index 4d03182a0834..e33e989d40b2 100644 --- a/pkg/sql/sem/tree/testdata/pretty/create_partition_list.ref.golden.short +++ b/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) ) diff --git a/pkg/sql/show_create_clauses.go b/pkg/sql/show_create_clauses.go index d66ae0327e45..52d809f186f1 100644 --- a/pkg/sql/show_create_clauses.go +++ b/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)