Skip to content

Commit

Permalink
sql: block set default on computed col
Browse files Browse the repository at this point in the history
This patch blocks setting/dropping a default (even a
null default) on a computed column in the
LSC and DSC.

Fixes: #127522

Release note (bug fix): Setting or dropping a default value
on a computed column is now blocked -- even for null
defaults. Previously, setting or dropping a default value on a
computed column was a no-op.
  • Loading branch information
annrpom committed Jul 26, 2024
1 parent afb9fde commit 5eedebc
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 5 deletions.
19 changes: 19 additions & 0 deletions pkg/sql/alter_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -943,6 +943,25 @@ func applyColumnMutation(
return AlterColumnType(ctx, tableDesc, col, t, params, cmds, tn)

case *tree.AlterTableSetDefault:
// If our column is computed, block mixing defaults in entirely.
// This check exists here instead of later on during validation because
// adding a null default to a computed column should also be blocked, but
// is undetectable later on since SET DEFAULT NUL means a nil default
// expression.
if col.IsComputed() {
// Block dropping a computed column "default" as well.
if t.Default == nil {
return pgerror.Newf(
pgcode.Syntax,
"column %q of relation %q is a computed column",
col.GetName(),
tn.ObjectName)
}
return pgerror.Newf(
pgcode.Syntax,
"computed column %q cannot also have a DEFAULT expression",
col.GetName())
}
if err := updateNonComputedColExpr(
params,
tableDesc,
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/catalog/tabledesc/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -1143,7 +1143,7 @@ func (desc *wrapper) validateColumns() error {

if column.IsComputed() {
if column.HasDefault() {
return pgerror.Newf(pgcode.InvalidTableDefinition,
return pgerror.Newf(pgcode.Syntax,
"computed column %q cannot also have a DEFAULT expression",
column.GetName(),
)
Expand Down
25 changes: 24 additions & 1 deletion pkg/sql/logictest/testdata/logic_test/computed
Original file line number Diff line number Diff line change
Expand Up @@ -1137,7 +1137,7 @@ SELECT * FROM t69327
f f

# Regression test for #72881. Computed columns can't have a DEFAULT expr.
statement error pgcode 42P16 computed column "v" cannot also have a DEFAULT expression
statement error pgcode 42601 computed column "v" cannot also have a DEFAULT expression
ALTER TABLE t69327 ALTER COLUMN v SET DEFAULT 'foo'

# Regression test for #69665.Computed columns should be evaluated after
Expand Down Expand Up @@ -1184,3 +1184,26 @@ FROM t88128
----
b1 expected_b1 b2 expected_b2
true true true true

# Regression test for #127522 where we do not properly block adding a default
# value to a computed column.
subtest computed_column_with_default

statement ok
CREATE TABLE foooooo (
id INT PRIMARY KEY,
x INT NOT NULL,
y INT NOT NULL,
gen INT AS (x + y) STORED
);

statement error pgcode 42601 computed column "gen" cannot also have a DEFAULT expression
ALTER TABLE foooooo ALTER COLUMN gen SET DEFAULT 1;

statement error pgcode 42601 computed column "gen" cannot also have a DEFAULT expression
ALTER TABLE foooooo ALTER COLUMN gen SET DEFAULT NULL;

statement error pgcode 42601 column "gen" of relation "foooooo" is a computed column
ALTER TABLE foooooo ALTER COLUMN gen DROP DEFAULT;

subtest end
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ func alterTableSetDefault(
colID := getColumnIDFromColumnName(b, tbl.TableID, t.Column, true /* required */)
col := mustRetrieveColumnElem(b, tbl.TableID, colID)
oldDefaultExpr := retrieveColumnDefaultExpressionElem(b, tbl.TableID, colID)
colType := mustRetrieveColumnTypeElem(b, tbl.TableID, colID)

// Block alters on system columns.
panicIfSystemColumn(col, t.Column.String())

// Block disallowed operations on computed columns.
panicIfComputedColumn(tn.ObjectName, colType, t.Column.String(), t.Default)

// For DROP DEFAULT.
if t.Default == nil {
Expand All @@ -41,9 +48,6 @@ func alterTableSetDefault(
return
}

// Block alters on system columns.
panicIfSystemColumn(col, t.Column.String())

// If our target column already has a default expression, we want to drop it first.
if oldDefaultExpr != nil {
b.Drop(oldDefaultExpr)
Expand Down Expand Up @@ -142,3 +146,22 @@ func sanitizeColumnExpression(
s := tree.Serialize(typedExpr)
return typedExpr, s, nil
}

// panicIfComputedColumn blocks disallowed operations on computed columns.
func panicIfComputedColumn(tn tree.Name, col *scpb.ColumnType, colName string, def tree.Expr) {
// Block setting a column default if the column is computed.
if col.ComputeExpr != nil {
// Block dropping a computed col "default" as well.
if def == nil {
panic(pgerror.Newf(
pgcode.Syntax,
"column %q of relation %q is a computed column",
colName,
tn))
}
panic(pgerror.Newf(
pgcode.Syntax,
"computed column %q cannot also have a DEFAULT expression",
colName))
}
}

0 comments on commit 5eedebc

Please sign in to comment.