diff --git a/pkg/sql/create_table.go b/pkg/sql/create_table.go index 364bb6d5f6a1..ebe716c43b57 100644 --- a/pkg/sql/create_table.go +++ b/pkg/sql/create_table.go @@ -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, ) } } diff --git a/pkg/sql/logictest/testdata/logic_test/fk b/pkg/sql/logictest/testdata/logic_test/fk index a4d39cfac7a8..e7110c35eb3a 100644 --- a/pkg/sql/logictest/testdata/logic_test/fk +++ b/pkg/sql/logictest/testdata/logic_test/fk @@ -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 ); @@ -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; @@ -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 @@ -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 @@ -1285,8 +1287,32 @@ 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 @@ -1294,8 +1320,26 @@ CREATE TABLE no_default_table ( ,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 @@ -1303,9 +1347,8 @@ 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 +CREATE TABLE no_default_table_ref2_default_on_update ( id INT PRIMARY KEY ,ref1 INT ,ref2 INT DEFAULT 1 @@ -1313,8 +1356,8 @@ CREATE TABLE no_default_table ( ,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 @@ -1322,8 +1365,8 @@ 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.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 @@ -1331,9 +1374,21 @@ CREATE TABLE no_default_table ( ,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 diff --git a/pkg/sql/logictest/testdata/logic_test/table b/pkg/sql/logictest/testdata/logic_test/table index 3c2de995b852..6004e9a80a21 100644 --- a/pkg/sql/logictest/testdata/logic_test/table +++ b/pkg/sql/logictest/testdata/logic_test/table @@ -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) ) diff --git a/pkg/sql/row/cascader.go b/pkg/sql/row/cascader.go index 55ddcf09f788..47f6ed0a5516 100644 --- a/pkg/sql/row/cascader.go +++ b/pkg/sql/row/cascader.go @@ -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.HasNoDefault() { + referencingIndexValuesByColIDs[columnID] = tree.DNull + continue + } parsedExpr, err := parser.ParseExpr(*column.DefaultExpr) if err != nil { return nil, nil, nil, 0, err diff --git a/pkg/sql/sqlbase/structured.go b/pkg/sql/sqlbase/structured.go index 3e528f2e3372..9caeb949fdfb 100644 --- a/pkg/sql/sqlbase/structured.go +++ b/pkg/sql/sqlbase/structured.go @@ -2867,17 +2867,7 @@ 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 - } - } - return desc.DefaultExpr != nil || !desc.Nullable || desc.IsComputed() + return !desc.HasNoDefault() || desc.HasNullDefault() || !desc.Nullable || desc.IsComputed() } // HasColumnBackfillMutation returns whether the table has any queued column @@ -3247,6 +3237,24 @@ func (desc *ColumnDescriptor) IsNullable() bool { return desc.Nullable } +// Checks that the column descriptor has no default. +func (desc *ColumnDescriptor) HasNoDefault() bool { + return desc.DefaultExpr == nil +} + +// Checks that the column descriptor has a default of NULL. +func (desc *ColumnDescriptor) HasNullDefault() bool { + if desc.HasNoDefault() { + 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) diff --git a/pkg/sql/sqlbase/structured_test.go b/pkg/sql/sqlbase/structured_test.go index 9411fed27b1f..e5544221bb17 100644 --- a/pkg/sql/sqlbase/structured_test.go +++ b/pkg/sql/sqlbase/structured_test.go @@ -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 @@ -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) + } + } + }) +} diff --git a/pkg/sql/sqlbase/table.go b/pkg/sql/sqlbase/table.go index 8964e529d0a8..b9ec76ee98fc 100644 --- a/pkg/sql/sqlbase/table.go +++ b/pkg/sql/sqlbase/table.go @@ -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() {