Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: use a 3-valued type instead of 2 booleans for relocate statements #73803

Merged
merged 4 commits into from
Dec 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion docs/generated/sql/bnf/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ FILES = [
"alter_index_stmt",
"alter_partition_stmt",
"alter_primary_key",
"alter_range_relocate_lease_stmt",
"alter_range_relocate_stmt",
"alter_range_stmt",
"alter_rename_view_stmt",
Expand Down
3 changes: 0 additions & 3 deletions docs/generated/sql/bnf/alter_range_relocate_lease_stmt.bnf

This file was deleted.

8 changes: 4 additions & 4 deletions docs/generated/sql/bnf/alter_range_relocate_stmt.bnf
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
alter_range_relocate_stmt ::=
'ALTER' 'RANGE' relocate_kw voters_kw 'FROM' iconst64 'TO' iconst64 'FOR' select_stmt
| 'ALTER' 'RANGE' iconst64 relocate_kw voters_kw 'FROM' iconst64 'TO' iconst64
| 'ALTER' 'RANGE' relocate_kw 'NON_VOTERS' 'FROM' iconst64 'TO' iconst64 'FOR' select_stmt
| 'ALTER' 'RANGE' iconst64 relocate_kw 'NON_VOTERS' 'FROM' iconst64 'TO' iconst64
'ALTER' 'RANGE' relocate_kw 'LEASE' 'TO' iconst64 'FOR' select_stmt
| 'ALTER' 'RANGE' iconst64 relocate_kw 'LEASE' 'TO' iconst64
| 'ALTER' 'RANGE' relocate_kw relocate_subject_nonlease 'FROM' iconst64 'TO' iconst64 'FOR' select_stmt
| 'ALTER' 'RANGE' iconst64 relocate_kw relocate_subject_nonlease 'FROM' iconst64 'TO' iconst64
1 change: 0 additions & 1 deletion docs/generated/sql/bnf/alter_range_stmt.bnf
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
alter_range_stmt ::=
alter_zone_range_stmt
| alter_range_relocate_lease_stmt
| alter_range_relocate_stmt
16 changes: 6 additions & 10 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -1038,7 +1038,7 @@ unreserved_keyword ::=
| 'NOCONTROLJOB'
| 'NOLOGIN'
| 'NOMODIFYCLUSTERSETTING'
| 'NON_VOTERS'
| 'NONVOTERS'
| 'NOVIEWACTIVITY'
| 'NOWAIT'
| 'NULLS'
Expand Down Expand Up @@ -1354,7 +1354,6 @@ alter_database_stmt ::=

alter_range_stmt ::=
alter_zone_range_stmt
| alter_range_relocate_lease_stmt
| alter_range_relocate_stmt

alter_partition_stmt ::=
Expand Down Expand Up @@ -1846,15 +1845,11 @@ alter_database_primary_region_stmt ::=
alter_zone_range_stmt ::=
'ALTER' 'RANGE' zone_name set_zone_config

alter_range_relocate_lease_stmt ::=
alter_range_relocate_stmt ::=
'ALTER' 'RANGE' relocate_kw 'LEASE' 'TO' iconst64 'FOR' select_stmt
| 'ALTER' 'RANGE' iconst64 relocate_kw 'LEASE' 'TO' iconst64

alter_range_relocate_stmt ::=
'ALTER' 'RANGE' relocate_kw voters_kw 'FROM' iconst64 'TO' iconst64 'FOR' select_stmt
| 'ALTER' 'RANGE' iconst64 relocate_kw voters_kw 'FROM' iconst64 'TO' iconst64
| 'ALTER' 'RANGE' relocate_kw 'NON_VOTERS' 'FROM' iconst64 'TO' iconst64 'FOR' select_stmt
| 'ALTER' 'RANGE' iconst64 relocate_kw 'NON_VOTERS' 'FROM' iconst64 'TO' iconst64
| 'ALTER' 'RANGE' relocate_kw relocate_subject_nonlease 'FROM' iconst64 'TO' iconst64 'FOR' select_stmt
| 'ALTER' 'RANGE' iconst64 relocate_kw relocate_subject_nonlease 'FROM' iconst64 'TO' iconst64

alter_zone_partition_stmt ::=
'ALTER' 'PARTITION' partition_name 'OF' 'TABLE' table_name set_zone_config
Expand Down Expand Up @@ -2336,9 +2331,10 @@ relocate_kw ::=
| 'EXPERIMENTAL_RELOCATE'
| 'RELOCATE'

voters_kw ::=
relocate_subject_nonlease ::=
'VOTERS'
|
| 'NONVOTERS'

alter_default_privileges_target_object ::=
'TABLES'
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/kvccl/kvfollowerreadsccl/followerreads_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ func TestFollowerReadsWithStaleDescriptor(t *testing.T) {
// Remove the follower and add a new non-voter to n3. n2 will no longer have a
// replica.
n1.Exec(t, `ALTER TABLE test EXPERIMENTAL_RELOCATE VOTERS VALUES (ARRAY[1], 1)`)
n1.Exec(t, `ALTER TABLE test EXPERIMENTAL_RELOCATE NON_VOTERS VALUES (ARRAY[3], 1)`)
n1.Exec(t, `ALTER TABLE test EXPERIMENTAL_RELOCATE NONVOTERS VALUES (ARRAY[3], 1)`)

// Execute the query again and assert the cache is updated. This query will
// not be executed as a follower read since it attempts to use n2 which
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/distsql_spec_exec_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -1070,13 +1070,13 @@ func (e *distSQLSpecExecFactory) ConstructAlterTableUnsplitAll(index cat.Index)
}

func (e *distSQLSpecExecFactory) ConstructAlterTableRelocate(
index cat.Index, input exec.Node, relocateLease bool, relocateNonVoters bool,
index cat.Index, input exec.Node, relocateSubject tree.RelocateSubject,
) (exec.Node, error) {
return nil, unimplemented.NewWithIssue(47473, "experimental opt-driven distsql planning: alter table relocate")
}

func (e *distSQLSpecExecFactory) ConstructAlterRangeRelocate(
input exec.Node, relocateLease bool, relocateNonVoters bool, toStoreID int64, fromStoreID int64,
input exec.Node, relocateSubject tree.RelocateSubject, toStoreID int64, fromStoreID int64,
) (exec.Node, error) {
return nil, unimplemented.NewWithIssue(47473, "experimental opt-driven distsql planning: alter range relocate")
}
Expand Down
6 changes: 2 additions & 4 deletions pkg/sql/opt/exec/execbuilder/statement.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,7 @@ func (b *Builder) buildAlterTableRelocate(relocate *memo.AlterTableRelocateExpr)
node, err := b.factory.ConstructAlterTableRelocate(
table.Index(relocate.Index),
input.root,
relocate.RelocateLease,
relocate.RelocateNonVoters,
relocate.SubjectReplicas,
)
if err != nil {
return execPlan{}, err
Expand All @@ -238,8 +237,7 @@ func (b *Builder) buildAlterRangeRelocate(relocate *memo.AlterRangeRelocateExpr)
}
node, err := b.factory.ConstructAlterRangeRelocate(
input.root,
relocate.RelocateLease,
relocate.RelocateNonVoters,
relocate.SubjectReplicas,
relocate.ToStoreID,
relocate.FromStoreID,
)
Expand Down
6 changes: 2 additions & 4 deletions pkg/sql/opt/exec/factory.opt
Original file line number Diff line number Diff line change
Expand Up @@ -658,8 +658,7 @@ define AlterTableUnsplitAll {
define AlterTableRelocate {
Index cat.Index
input exec.Node
relocateLease bool
relocateNonVoters bool
subjectReplicas tree.RelocateSubject
}

# Buffer passes through the input rows but also saves them in a buffer, which
Expand Down Expand Up @@ -736,8 +735,7 @@ define Export {
# AlterTableRelocate implements ALTER RANGE RELOCATE.
define AlterRangeRelocate {
input exec.Node
relocateLease bool
relocateNonVoters bool
subjectReplicas tree.RelocateSubject
toStoreID int64
fromStoreID int64
}
7 changes: 4 additions & 3 deletions pkg/sql/opt/memo/expr_format.go
Original file line number Diff line number Diff line change
Expand Up @@ -1502,11 +1502,12 @@ func FormatPrivate(f *ExprFmtCtx, private interface{}, physProps *physical.Requi

case *AlterTableRelocatePrivate:
FormatPrivate(f, &t.AlterTableSplitPrivate, nil)
if t.RelocateLease {
switch t.SubjectReplicas {
case tree.RelocateLease:
f.Buffer.WriteString(" [lease]")
} else if t.RelocateNonVoters {
case tree.RelocateNonVoters:
f.Buffer.WriteString(" [non-voters]")
} else {
case tree.RelocateVoters:
f.Buffer.WriteString(" [voters]")
}

Expand Down
8 changes: 8 additions & 0 deletions pkg/sql/opt/memo/interner.go
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,10 @@ func (h *hasher) HashIndexOrdinal(val cat.IndexOrdinal) {
h.HashInt(val)
}

func (h *hasher) HashRelocateSubject(val tree.RelocateSubject) {
h.HashUint64(uint64(val))
}

func (h *hasher) HashIndexOrdinals(val cat.IndexOrdinals) {
hash := h.hash
for _, ord := range val {
Expand Down Expand Up @@ -978,6 +982,10 @@ func (h *hasher) IsIndexOrdinalEqual(l, r cat.IndexOrdinal) bool {
return l == r
}

func (h *hasher) IsRelocateSubjectEqual(l, r tree.RelocateSubject) bool {
return l == r
}

func (h *hasher) IsIndexOrdinalsEqual(l, r cat.IndexOrdinals) bool {
if len(l) != len(r) {
return false
Expand Down
6 changes: 2 additions & 4 deletions pkg/sql/opt/ops/statement.opt
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,7 @@ define AlterTableRelocate {

[Private]
define AlterTableRelocatePrivate {
RelocateLease bool
RelocateNonVoters bool
SubjectReplicas RelocateSubject
_ AlterTableSplitPrivate
}

Expand Down Expand Up @@ -303,8 +302,7 @@ define AlterRangeRelocate {

[Private]
define AlterRangeRelocatePrivate {
RelocateLease bool
RelocateNonVoters bool
SubjectReplicas RelocateSubject
ToStoreID int64
FromStoreID int64

Expand Down
16 changes: 6 additions & 10 deletions pkg/sql/opt/optbuilder/alter_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,7 @@ func (b *Builder) buildAlterRangeRelocate(
outScope = inScope.push()
b.synthesizeResultColumns(outScope, colinfo.AlterTableRelocateColumns)

cmdName := "RELOCATE"
if relocate.RelocateLease {
cmdName += " LEASE"
}
cmdName := "RELOCATE " + relocate.SubjectReplicas.String()
colNames := []string{"range ids"}
colTypes := []*types.T{types.Int}

Expand All @@ -51,12 +48,11 @@ func (b *Builder) buildAlterRangeRelocate(
outScope.expr = b.factory.ConstructAlterRangeRelocate(
inputScope.expr,
&memo.AlterRangeRelocatePrivate{
RelocateLease: relocate.RelocateLease,
RelocateNonVoters: relocate.RelocateNonVoters,
ToStoreID: relocate.ToStoreID,
FromStoreID: relocate.FromStoreID,
Columns: colsToColList(outScope.cols),
Props: physical.MinRequired,
SubjectReplicas: relocate.SubjectReplicas,
ToStoreID: relocate.ToStoreID,
FromStoreID: relocate.FromStoreID,
Columns: colsToColList(outScope.cols),
Props: physical.MinRequired,
},
)
return outScope
Expand Down
7 changes: 3 additions & 4 deletions pkg/sql/opt/optbuilder/alter_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ func (b *Builder) buildAlterTableRelocate(

// The first column is the target leaseholder or the relocation array,
// depending on variant.
cmdName := "EXPERIMENTAL_RELOCATE"
if relocate.RelocateLease {
cmdName := "RELOCATE"
if relocate.SubjectReplicas == tree.RelocateLease {
cmdName += " LEASE"
colNames = append([]string{"target leaseholder"}, colNames...)
colTypes = append([]*types.T{types.Int}, colTypes...)
Expand All @@ -181,8 +181,7 @@ func (b *Builder) buildAlterTableRelocate(
outScope.expr = b.factory.ConstructAlterTableRelocate(
inputScope.expr,
&memo.AlterTableRelocatePrivate{
RelocateLease: relocate.RelocateLease,
RelocateNonVoters: relocate.RelocateNonVoters,
SubjectReplicas: relocate.SubjectReplicas,
AlterTableSplitPrivate: memo.AlterTableSplitPrivate{
Table: b.factory.Metadata().AddTable(table, &tn),
Index: index.Ordinal(),
Expand Down
18 changes: 9 additions & 9 deletions pkg/sql/opt/optbuilder/testdata/alter_range
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ CREATE TABLE abc (a INT PRIMARY KEY, b INT, c STRING, INDEX b (b), UNIQUE INDEX
build
ALTER RANGE 1 RELOCATE FROM 1 TO 2
----
alter-range-relocate &{false false 2 1 [3 4 5] []}
alter-range-relocate &{VOTERS 2 1 [3 4 5] []}
├── columns: range_id:3 pretty:4 result:5
└── values
├── columns: column1:6!null
Expand All @@ -15,7 +15,7 @@ alter-range-relocate &{false false 2 1 [3 4 5] []}
build
ALTER RANGE RELOCATE FROM 1 TO 2 FOR SELECT a from abc
----
alter-range-relocate &{false false 2 1 [3 4 5] []}
alter-range-relocate &{VOTERS 2 1 [3 4 5] []}
├── columns: range_id:3 pretty:4 result:5
└── project
├── columns: a:6!null
Expand All @@ -30,21 +30,21 @@ error (42601): at or near "relocate": syntax error
build
ALTER RANGE RELOCATE FROM 1 TO 2 FOR SELECT c from abc
----
error (42601): RELOCATE data column 1 (range ids) must be of type int, not type string
error (42601): RELOCATE VOTERS data column 1 (range ids) must be of type int, not type string

build
ALTER RANGE 1 RELOCATE NON_VOTERS FROM 1 TO 2
ALTER RANGE 1 RELOCATE NONVOTERS FROM 1 TO 2
----
alter-range-relocate &{false true 2 1 [3 4 5] []}
alter-range-relocate &{NONVOTERS 2 1 [3 4 5] []}
├── columns: range_id:3 pretty:4 result:5
└── values
├── columns: column1:6!null
└── (1,)

build
ALTER RANGE RELOCATE NON_VOTERS FROM 1 TO 2 FOR SELECT a from abc
ALTER RANGE RELOCATE NONVOTERS FROM 1 TO 2 FOR SELECT a from abc
----
alter-range-relocate &{false true 2 1 [3 4 5] []}
alter-range-relocate &{NONVOTERS 2 1 [3 4 5] []}
├── columns: range_id:3 pretty:4 result:5
└── project
├── columns: a:6!null
Expand All @@ -55,7 +55,7 @@ alter-range-relocate &{false true 2 1 [3 4 5] []}
build
ALTER RANGE 1 RELOCATE LEASE TO 2
----
alter-range-relocate &{true false 2 0 [3 4 5] []}
alter-range-relocate &{LEASE 2 0 [3 4 5] []}
├── columns: range_id:3 pretty:4 result:5
└── values
├── columns: column1:6!null
Expand All @@ -64,7 +64,7 @@ alter-range-relocate &{true false 2 0 [3 4 5] []}
build
ALTER RANGE RELOCATE LEASE TO 2 FOR SELECT a from abc
----
alter-range-relocate &{true false 2 0 [3 4 5] []}
alter-range-relocate &{LEASE 2 0 [3 4 5] []}
├── columns: range_id:3 pretty:4 result:5
└── project
├── columns: a:6!null
Expand Down
16 changes: 8 additions & 8 deletions pkg/sql/opt/optbuilder/testdata/alter_table
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ alter-table-relocate abc [voters]
└── (ARRAY[4], 2)

build
ALTER TABLE abc EXPERIMENTAL_RELOCATE NON_VOTERS VALUES (ARRAY[1,2,3], 1), (ARRAY[4], 2)
ALTER TABLE abc EXPERIMENTAL_RELOCATE NONVOTERS VALUES (ARRAY[1,2,3], 1), (ARRAY[4], 2)
----
alter-table-relocate abc [non-voters]
├── columns: key:1 pretty:2
Expand All @@ -149,12 +149,12 @@ alter-table-relocate abc [non-voters]
build
ALTER TABLE abc EXPERIMENTAL_RELOCATE LEASE VALUES (10), (11)
----
error (42601): less than 2 columns in EXPERIMENTAL_RELOCATE LEASE data
error (42601): less than 2 columns in RELOCATE LEASE data

build
ALTER TABLE abc EXPERIMENTAL_RELOCATE LEASE VALUES (10, 1, 2), (11, 3, 4)
----
error (42601): too many columns in EXPERIMENTAL_RELOCATE LEASE data
error (42601): too many columns in RELOCATE LEASE data

build
ALTER INDEX abc@bc EXPERIMENTAL_RELOCATE VALUES (ARRAY[5], 1, 'foo'), (ARRAY[6,7,8], 2, 'bar')
Expand All @@ -167,7 +167,7 @@ alter-table-relocate abc@bc [voters]
└── (ARRAY[6,7,8], 2, 'bar')

build
ALTER INDEX abc@bc EXPERIMENTAL_RELOCATE NON_VOTERS VALUES (ARRAY[5], 1, 'foo'), (ARRAY[6,7,8], 2, 'bar')
ALTER INDEX abc@bc EXPERIMENTAL_RELOCATE NONVOTERS VALUES (ARRAY[5], 1, 'foo'), (ARRAY[6,7,8], 2, 'bar')
----
alter-table-relocate abc@bc [non-voters]
├── columns: key:1 pretty:2
Expand All @@ -179,22 +179,22 @@ alter-table-relocate abc@bc [non-voters]
build
ALTER INDEX abc@bc EXPERIMENTAL_RELOCATE VALUES (5, 1, 'foo'), (6, 2, 'bar')
----
error (42601): EXPERIMENTAL_RELOCATE data column 1 (relocation array) must be of type int[], not type int
error (42601): RELOCATE data column 1 (relocation array) must be of type int[], not type int

build
ALTER INDEX abc@bc EXPERIMENTAL_RELOCATE LEASE VALUES (ARRAY[5], 1, 'foo'), (ARRAY[6,7,8], 2, 'bar')
----
error (42601): EXPERIMENTAL_RELOCATE LEASE data column 1 (target leaseholder) must be of type int, not type int[]
error (42601): RELOCATE LEASE data column 1 (target leaseholder) must be of type int, not type int[]

build
ALTER INDEX abc@bc EXPERIMENTAL_RELOCATE VALUES (1, 2), (3, 4)
----
error (42601): EXPERIMENTAL_RELOCATE data column 1 (relocation array) must be of type int[], not type int
error (42601): RELOCATE data column 1 (relocation array) must be of type int[], not type int

build
ALTER INDEX abc@bc EXPERIMENTAL_RELOCATE VALUES (ARRAY[1,2], 1, 2), (ARRAY[3,4], 3, 4)
----
error (42601): EXPERIMENTAL_RELOCATE data column 3 (c) must be of type string, not type int
error (42601): RELOCATE data column 3 (c) must be of type string, not type int

build
ALTER INDEX abc@bc EXPERIMENTAL_RELOCATE LEASE VALUES (10, 1, 'foo'), (11, 3, 'bar')
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/opt/optgen/cmd/optgen/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ func newMetadata(compiled *lang.CompiledExpr, pkg string) *metadata {
"ScheduleCommand": {fullName: "tree.ScheduleCommand", passByVal: true},
"IndexOrdinal": {fullName: "cat.IndexOrdinal", passByVal: true},
"IndexOrdinals": {fullName: "cat.IndexOrdinals", passByVal: true},
"RelocateSubject": {fullName: "tree.RelocateSubject", passByVal: true},
"UniqueOrdinals": {fullName: "cat.UniqueOrdinals", passByVal: true},
"ViewDeps": {fullName: "opt.ViewDeps", passByVal: true},
"ViewTypeDeps": {fullName: "opt.ViewTypeDeps", passByVal: true},
Expand Down
Loading