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: allow cascading action when default is set implicitly to null #39136

Merged
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
11 changes: 7 additions & 4 deletions pkg/sql/create_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -599,14 +599,17 @@ func ResolveFK(
}
}

// Don't add a SET DEFAULT action on an index that has any column that does
// not have a DEFAULT expression.
// Don't add a SET DEFAULT action on an index that has any column that has
// a DEFAULT expression of NULL and a NOT NULL constraint.
if d.Actions.Delete == tree.SetDefault || d.Actions.Update == tree.SetDefault {
for _, sourceColumn := range srcCols {
if sourceColumn.DefaultExpr == nil {
// Having a default expression of NULL, and a constraint of NOT NULL is a
// contradiction and should never be allowed.
if sourceColumn.DefaultExpr == nil && !sourceColumn.Nullable {
col := qualifyFKColErrorWithDB(ctx, txn, tbl.TableDesc(), sourceColumn.Name)
return pgerror.Newf(pgcode.InvalidForeignKey,
"cannot add a SET DEFAULT cascading action on column %q which has no DEFAULT expression", col,
"cannot add a SET DEFAULT cascading action on column %q which has a "+
"NOT NULL constraint and a NULL default expression", col,
)
}
}
Expand Down
105 changes: 80 additions & 25 deletions pkg/sql/logictest/testdata/logic_test/fk
Original file line number Diff line number Diff line change
Expand Up @@ -1201,25 +1201,26 @@ CREATE TABLE a (
);

# Create a table with no DEFAULT expressions column and a SET DEFAULT action.
statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.no_default_table.delete_no_default" which has no DEFAULT expression
CREATE TABLE no_default_table (
statement ok
CREATE TABLE delete_no_default_table (
id INT PRIMARY KEY
,delete_no_default INT REFERENCES a ON DELETE SET DEFAULT
);

statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.no_default_table.update_no_default" which has no DEFAULT expression
CREATE TABLE no_default_table (
statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.update_no_default_table.update_no_default" which has a NOT NULL constraint and a NULL default expression
CREATE TABLE update_no_default_table (
id INT PRIMARY KEY
,update_no_default INT NOT NULL REFERENCES a ON UPDATE SET DEFAULT
);

# Create a table where the primary key has a SET DEFAULT action.
statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.primary_key_table.id" which has no DEFAULT expression
CREATE TABLE primary_key_table (
# Primary keys are not allowed to be NULL
statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.primary_key_table_set_default.id" which has a NOT NULL constraint and a NULL default expression
CREATE TABLE primary_key_table_set_default (
id INT PRIMARY KEY REFERENCES a ON DELETE SET DEFAULT
);

statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.primary_key_table.id" which has no DEFAULT expression
statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.primary_key_table.id" which has a NOT NULL constraint and a NULL default expression
CREATE TABLE primary_key_table (
id INT PRIMARY KEY REFERENCES a ON UPDATE SET DEFAULT
);
Expand All @@ -1232,12 +1233,12 @@ CREATE TABLE no_default_table (
,update_no_default INT
);

statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.no_default_table.delete_no_default" which has no DEFAULT expression
statement ok
ALTER TABLE no_default_table ADD CONSTRAINT no_default_delete_set_default
FOREIGN KEY (delete_no_default) REFERENCES a (id)
ON DELETE SET DEFAULT;

statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.no_default_table.update_no_default" which has no DEFAULT expression
statement ok
ALTER TABLE no_default_table ADD CONSTRAINT no_default_update_set_default
FOREIGN KEY (update_no_default) REFERENCES a (id)
ON UPDATE SET DEFAULT;
Expand All @@ -1253,19 +1254,20 @@ CREATE TABLE primary_key_table (
id INT PRIMARY KEY
);

statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.primary_key_table.id" which has no DEFAULT expression
# id is a primary key and thus cannot be NULL
statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.primary_key_table.id" which has a NOT NULL constraint and a NULL default expression
ALTER TABLE primary_key_table ADD CONSTRAINT no_default_delete_set_default
FOREIGN KEY (id) REFERENCES a (id)
ON DELETE SET DEFAULT;

statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.primary_key_table.id" which has no DEFAULT expression
statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.primary_key_table.id" which has a NOT NULL constraint and a NULL default expression
ALTER TABLE primary_key_table ADD CONSTRAINT no_default_update_set_default
FOREIGN KEY (id) REFERENCES a (id)
ON UPDATE SET DEFAULT;

# Clean up the tables used so far.
statement ok
DROP TABLE primary_key_table, a;
DROP TABLE primary_key_table, delete_no_default_table, a;

# Now test composite foreign keys
statement ok
Expand All @@ -1276,7 +1278,7 @@ CREATE TABLE a (
);

# Create a table with a column without a DEFAULT expression and a SET DEFAULT action.
statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.no_default_table.ref1" which has no DEFAULT expression
statement ok
CREATE TABLE no_default_table (
id INT PRIMARY KEY
,ref1 INT
Expand All @@ -1285,55 +1287,108 @@ CREATE TABLE no_default_table (
,FOREIGN KEY (ref1, ref2) REFERENCES a (id2, id1) ON DELETE SET DEFAULT
);

statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.no_default_table.ref1" which has no DEFAULT expression
CREATE TABLE no_default_table (
statement ok
INSERT INTO a VALUES (1, 2)

statement ok
INSERT INTO a VALUES (3, 4)

statement ok
INSERT INTO no_default_table VALUES (6, 2, 1)

query III colnames
SELECT * FROM no_default_table
----
id ref1 ref2
6 2 1

statement ok
DELETE FROM a WHERE id1=1

query III colnames
SELECT * FROM no_default_table
----
id ref1 ref2
6 NULL NULL

statement ok
CREATE TABLE no_default_table_on_update (
id INT PRIMARY KEY
,ref1 INT
,ref2 INT
,INDEX (ref1, ref2)
,FOREIGN KEY (ref1, ref2) REFERENCES a (id2, id1) ON UPDATE SET DEFAULT
);

statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.no_default_table.ref1" which has no DEFAULT expression
CREATE TABLE no_default_table (
statement ok
INSERT INTO no_default_table_on_update VALUES (0, 4, 3)

query III colnames
SELECT * FROM no_default_table_on_update
----
id ref1 ref2
0 4 3

statement ok
UPDATE a SET id1=33, id2=44 WHERE id1=3;

query III colnames
SELECT * FROM no_default_table_on_update
----
id ref1 ref2
0 NULL NULL

statement ok
CREATE TABLE no_default_table_ref2_default_on_delete (
id INT PRIMARY KEY
,ref1 INT
,ref2 INT DEFAULT 1
,INDEX (ref1, ref2)
,FOREIGN KEY (ref1, ref2) REFERENCES a (id2, id1) ON DELETE SET DEFAULT
);


statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.no_default_table.ref1" which has no DEFAULT expression
CREATE TABLE no_default_table (
statement ok
CREATE TABLE no_default_table_ref2_default_on_update (
id INT PRIMARY KEY
,ref1 INT
,ref2 INT DEFAULT 1
,INDEX (ref1, ref2)
,FOREIGN KEY (ref1, ref2) REFERENCES a (id2, id1) ON UPDATE SET DEFAULT
);

statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.no_default_table.ref2" which has no DEFAULT expression
CREATE TABLE no_default_table (
statement ok
CREATE TABLE no_default_table_ref1_default_on_delete (
id INT PRIMARY KEY
,ref1 INT DEFAULT 1
,ref2 INT
,INDEX (ref1, ref2)
,FOREIGN KEY (ref1, ref2) REFERENCES a (id2, id1) ON DELETE SET DEFAULT
);

statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.no_default_table.ref2" which has no DEFAULT expression
CREATE TABLE no_default_table (
statement ok
CREATE TABLE no_default_table_ref1_default_on_update (
id INT PRIMARY KEY
,ref1 INT DEFAULT 1
,ref2 INT
,INDEX (ref1, ref2)
,FOREIGN KEY (ref1, ref2) REFERENCES a (id2, id1) ON UPDATE SET DEFAULT
);

# Create a table with a NOT NULL column and a SET NULL action.
statement error pq: cannot add a SET DEFAULT cascading action on column "test.public.not_null_table.ref1" which has a NOT NULL constraint and a NULL default expression
CREATE TABLE not_null_table (
id INT PRIMARY KEY
,ref1 INT NOT NULL
,ref2 INT NOT NULL
,INDEX (ref1, ref2)
,FOREIGN KEY (ref1, ref2) REFERENCES a (id2, id1) ON DELETE SET DEFAULT
);

# Clean up after the test.
statement ok
DROP TABLE a;
DROP TABLE a, no_default_table, no_default_table_on_update, no_default_table_ref2_default_on_delete,
no_default_table_ref2_default_on_update, no_default_table_ref1_default_on_delete,
no_default_table_ref1_default_on_update

# Test that no column can have more than 1 FK constraint
subtest column_uniqueness
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/table
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ query TT
SHOW CREATE TABLE test.null_default
----
test.public.null_default CREATE TABLE null_default (
ts TIMESTAMP NULL DEFAULT NULL,
ts TIMESTAMP NULL,
FAMILY "primary" (ts, rowid)
)

Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/row/cascader.go
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,11 @@ func (c *cascader) updateRows(
if err != nil {
return nil, nil, nil, 0, err
}
// If the default expression is nil, treat it as a SET NULL case.
if !column.HasDefault() {
referencingIndexValuesByColIDs[columnID] = tree.DNull
continue
}
parsedExpr, err := parser.ParseExpr(*column.DefaultExpr)
if err != nil {
return nil, nil, nil, 0, err
Expand Down
26 changes: 16 additions & 10 deletions pkg/sql/sqlbase/structured.go
Original file line number Diff line number Diff line change
Expand Up @@ -2867,17 +2867,10 @@ func (desc *ImmutableTableDescriptor) MakeFirstMutationPublic(
// ColumnNeedsBackfill returns true if adding the given column requires a
// backfill (dropping a column always requires a backfill).
func ColumnNeedsBackfill(desc *ColumnDescriptor) bool {
// Consider the case where the user explicitly states the default value of a
// new column to be NULL. desc.DefaultExpr is not nil, but the string "NULL"
if desc.DefaultExpr != nil {
if defaultExpr, err := parser.ParseExpr(*desc.DefaultExpr); err != nil {
panic(errors.NewAssertionErrorWithWrappedErrf(err,
"failed to parse default expression %s", *desc.DefaultExpr))
} else if defaultExpr == tree.DNull {
return false
}
if desc.HasNullDefault() {
return false
}
return desc.DefaultExpr != nil || !desc.Nullable || desc.IsComputed()
return desc.HasDefault() || !desc.Nullable || desc.IsComputed()
}

// HasColumnBackfillMutation returns whether the table has any queued column
Expand Down Expand Up @@ -3247,6 +3240,19 @@ func (desc *ColumnDescriptor) IsNullable() bool {
return desc.Nullable
}

// HasNullDefault checks that the column descriptor has a default of NULL.
func (desc *ColumnDescriptor) HasNullDefault() bool {
if !desc.HasDefault() {
return false
}
defaultExpr, err := parser.ParseExpr(*desc.DefaultExpr)
if err != nil {
panic(errors.NewAssertionErrorWithWrappedErrf(err,
"failed to parse default expression %s", *desc.DefaultExpr))
}
return defaultExpr == tree.DNull
}

// ColID is part of the cat.Column interface.
func (desc *ColumnDescriptor) ColID() cat.StableID {
return cat.StableID(desc.ID)
Expand Down
37 changes: 36 additions & 1 deletion pkg/sql/sqlbase/structured_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1310,7 +1310,7 @@ func TestKeysPerRow(t *testing.T) {
}

func TestColumnNeedsBackfill(t *testing.T) {
// Define variable strings here such that we can pass their address below
// Define variable strings here such that we can pass their address below.
null := "NULL"
four := "4:::INT8"
// Create Column Descriptors that reflect the definition of a column with a
Expand Down Expand Up @@ -1757,3 +1757,38 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) {
}
}
}

func TestDefaultExprNil(t *testing.T) {
s, conn, _ := serverutils.StartServer(t, base.TestServerArgs{})
defer s.Stopper().Stop(context.TODO())
if _, err := conn.Exec(`CREATE DATABASE t`); err != nil {
t.Fatalf("%+v", err)
}
t.Run(fmt.Sprintf("%s - %d", "(a INT PRIMARY KEY)", 1), func(t *testing.T) {
sqlDB := sqlutils.MakeSQLRunner(conn)
// Execute SQL commands with both implicit and explicit setting of the
// default expression.
sqlDB.Exec(t, `CREATE TABLE t (a INT PRIMARY KEY)`)
sqlDB.Exec(t, `INSERT INTO t (a) VALUES (1), (2)`)
sqlDB.Exec(t, `ALTER TABLE t ADD COLUMN b INT NULL`)
sqlDB.Exec(t, `INSERT INTO t (a) VALUES (3)`)
sqlDB.Exec(t, `ALTER TABLE t ADD COLUMN c INT DEFAULT NULL`)

var descBytes []byte
// Grab the most recently created descriptor.
row := sqlDB.QueryRow(t,
`SELECT descriptor FROM system.descriptor ORDER BY id DESC LIMIT 1`)
row.Scan(&descBytes)
var desc Descriptor
if err := protoutil.Unmarshal(descBytes, &desc); err != nil {
t.Fatalf("%+v", err)
}
// Test and verify that the default expressions of the column descriptors
// are all nil.
for _, col := range desc.GetTable().Columns {
if col.DefaultExpr != nil {
t.Errorf("expected Column Default Expression to be 'nil', got %s instead.", *col.DefaultExpr)
}
}
})
}
13 changes: 8 additions & 5 deletions pkg/sql/sqlbase/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,15 @@ func MakeColumnDefDescs(
); err != nil {
return nil, nil, nil, err
}
// We keep the type checked expression so that the type annotation
// gets properly stored.
d.DefaultExpr.Expr = typedExpr

s := tree.Serialize(d.DefaultExpr.Expr)
col.DefaultExpr = &s
// Keep the type checked expression so that the type annotation gets
// properly stored, only if the default expression is not NULL.
// Otherwise we want to keep the default expression nil.
if typedExpr != tree.DNull {
d.DefaultExpr.Expr = typedExpr
s := tree.Serialize(d.DefaultExpr.Expr)
col.DefaultExpr = &s
}
}

if d.IsComputed() {
Expand Down