Skip to content

Commit

Permalink
Merge #52819
Browse files Browse the repository at this point in the history
52819: sql: fix ALTER TABLE descriptorChanged bug r=solongordon a=solongordon

There was some bad accounting in the ALTER TABLE command for whether the
table descriptor had changed or not. This meant that for ALTER TABLE
statements with multiple actions, we might incorrectly treat the whole
statement as a no-op if the last action had no effect.

Fixes #52816

Release note (bug fix): Fixed a bug for ALTER TABLE statements with
multiple actions. In certain cases if the last action had no effect, the
entire statement would be treated as a no-op.

Co-authored-by: Solon Gordon <solon@cockroachlabs.com>
  • Loading branch information
craig[bot] and solongordon committed Aug 14, 2020
2 parents 96a7b74 + 6d065ef commit b289a79
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 4 deletions.
8 changes: 4 additions & 4 deletions pkg/sql/alter_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ func (n *alterTableNode) startExec(params runParams) error {
if err != nil {
return err
}
descriptorChanged = !proto.Equal(
descriptorChanged = descriptorChanged || !proto.Equal(
&n.tableDesc.PrimaryIndex.Partitioning,
&partitioning,
)
Expand All @@ -660,11 +660,11 @@ func (n *alterTableNode) startExec(params runParams) error {
n.tableDesc.PrimaryIndex.Partitioning = partitioning

case *tree.AlterTableSetAudit:
var err error
descriptorChanged, err = params.p.setAuditMode(params.ctx, n.tableDesc, t.Mode)
changed, err := params.p.setAuditMode(params.ctx, n.tableDesc, t.Mode)
if err != nil {
return err
}
descriptorChanged = descriptorChanged || changed

case *tree.AlterTableInjectStats:
sd, ok := n.statsData[i]
Expand All @@ -685,7 +685,7 @@ func (n *alterTableNode) startExec(params runParams) error {
if err != nil {
return err
}
descriptorChanged = descChanged
descriptorChanged = descriptorChanged || descChanged

case *tree.AlterTableRenameConstraint:
info, err := n.tableDesc.GetConstraintInfo(params.ctx, nil, params.ExecCfg().Codec)
Expand Down
6 changes: 6 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/alter_table
Original file line number Diff line number Diff line change
Expand Up @@ -1358,3 +1358,9 @@ child CREATE TABLE public.child (
CONSTRAINT "primary" PRIMARY KEY (c ASC),
FAMILY fam_0_c_p (c)
)

# Regression test for #52816.
statement ok
CREATE TABLE t52816 (x INT, y INT);
ALTER TABLE t52816 RENAME COLUMN x TO x2, RENAME COLUMN y TO y;
SELECT x2, y FROM t52816

0 comments on commit b289a79

Please sign in to comment.