Merge #139084

139084: sql: relax validation for IDENTITY columns r=rafiss a=rafiss

The validations added in 4f99f9c were
too strict. This patch makes it so we can continue having IDENTITY
columns that are not backed by a sequence. This can happen if a SERIAL
column is created with IDENTITY, and serial_normalization=rowid.

informs #138925
informs #138972

Release note (bug fix): Fixed a bug where columns created with GENERATED ... BY IDENTITY with the SERIAL type could incorrectly fail internal validations.

Co-authored-by: Rafi Shamim <[email protected]>

pkg/sql/catalog/tabledesc/table_desc_builder.go

@@ -1313,15 +1313,15 @@ func maybeFixUsesSequencesIDForIdentityColumns(
 		if len(column.UsesSequenceIds) == 1 {
 			continue
 		}
-		// The first ownership will point to the owner from CREATE TABLE.
-		tableDesc = builder.getOrInitModifiedDesc()
-		column = &tableDesc.Columns[idx]
 		if len(column.OwnsSequenceIds) == 0 {
-			return false, errors.AssertionFailedf("identity column %s (%d) on table %s must own a sequence",
-				column.Name,
-				column.ID,
-				tableDesc.Name)
+			// With serial_normalization=rowid, a table definition like
+			// `CREATE TABLE t (a SERIAL GENERATED ALWAYS AS IDENTITY)`
+			// creates an identity column without any backing sequence. If that's the
+			// case, there's no need to add a sequence reference.
+			continue
 		}
+		tableDesc = builder.getOrInitModifiedDesc()
+		column = &tableDesc.Columns[idx]
 		column.UsesSequenceIds = append(column.UsesSequenceIds, column.OwnsSequenceIds[0])
 		wasRepaired = true
 	}

pkg/sql/catalog/tabledesc/validate.go

@@ -1225,9 +1225,10 @@ func (desc *wrapper) validateColumns() error {
 				}
 			}
 
-			// For generated as identity columns ensure that the column uses sequences.
-			if column.NumUsesSequences() != 1 {
-				return errors.Newf("column is identity without sequence references %q", column.GetName())
+			// For generated as identity columns ensure that the column uses sequences
+			// if it is backed by a sequence.
+			if column.NumOwnsSequences() == 1 && column.NumUsesSequences() != 1 {
+				return errors.Newf("column %q is GENERATED BY IDENTITY without sequence references", column.GetName())
 			}
 		}
 

pkg/sql/catalog/tabledesc/validate_test.go

@@ -3069,14 +3069,14 @@ func TestValidateTableDesc(t *testing.T) {
 					},
 				}
 			})},
-		{err: `column is identity without sequence references "bar"`,
+		{err: `column "bar" is GENERATED BY IDENTITY without sequence references`,
 			desc: descpb.TableDescriptor{
 				ID:            2,
 				ParentID:      1,
 				Name:          "foo",
 				FormatVersion: descpb.InterleavedFormatVersion,
 				Columns: []descpb.ColumnDescriptor{
-					{ID: 1, Name: "bar", GeneratedAsIdentityType: catpb.GeneratedAsIdentityType_GENERATED_ALWAYS},
+					{ID: 1, Name: "bar", OwnsSequenceIds: []descpb.ID{5}, GeneratedAsIdentityType: catpb.GeneratedAsIdentityType_GENERATED_ALWAYS},
 				},
 				NextColumnID: 2,
 			}},

pkg/sql/logictest/testdata/logic_test/alter_table

@@ -3842,6 +3842,54 @@ CREATE TABLE t_identity_drop_dependency(id INT PRIMARY KEY DEFAULT nextval('publ
 statement error pgcode 2BP01 pq: cannot drop table t_identity_drop because other objects depend on it
 ALTER TABLE t_identity_drop ALTER COLUMN b DROP IDENTITY;
 
+let $old_serial_normalization
+SHOW serial_normalization
+
+statement ok
+SET serial_normalization = rowid
+
+# Allow creating an IDENTITY column that isn't backed by a sequence.
+statement ok
+CREATE TABLE t_serial_identity_no_sequence(a SERIAL PRIMARY KEY GENERATED ALWAYS AS IDENTITY)
+
+query T
+SELECT crdb_internal.pb_to_json('desc', descriptor)->'table'->'columns'->0->>'defaultExpr'
+FROM system.descriptor
+WHERE id = 't_serial_identity_no_sequence'::regclass::oid
+----
+unique_rowid()
+
+query T
+SELECT crdb_internal.pb_to_json('desc', descriptor)->'table'->'columns'->0->>'generatedAsIdentityType'
+FROM system.descriptor
+WHERE id = 't_serial_identity_no_sequence'::regclass::oid
+----
+GENERATED_ALWAYS
+
+statement ok
+SET serial_normalization = sql_sequence
+
+# Allow creating an IDENTITY column that is backed by a sequence.
+statement ok
+CREATE TABLE t_serial_identity_with_sequence(a SERIAL PRIMARY KEY GENERATED ALWAYS AS IDENTITY)
+
+query T
+SELECT substring(crdb_internal.pb_to_json('desc', descriptor)->'table'->'columns'->0->>'defaultExpr' FOR 7)
+FROM system.descriptor
+WHERE id = 't_serial_identity_with_sequence'::regclass::oid
+----
+nextval
+
+query T
+SELECT crdb_internal.pb_to_json('desc', descriptor)->'table'->'columns'->0->>'generatedAsIdentityType'
+FROM system.descriptor
+WHERE id = 't_serial_identity_with_sequence'::regclass::oid
+----
+GENERATED_ALWAYS
+
+statement ok
+SET serial_normalization = $old_serial_normalization
+
 subtest comma_syntax
 
 statement ok