diff --git a/pkg/sql/catalog/tabledesc/ttl.go b/pkg/sql/catalog/tabledesc/ttl.go index 6367885c4a67..f4678125dfcc 100644 --- a/pkg/sql/catalog/tabledesc/ttl.go +++ b/pkg/sql/catalog/tabledesc/ttl.go @@ -15,10 +15,12 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo" "github.com/cockroachdb/cockroach/pkg/sql/catalog/schemaexpr" "github.com/cockroachdb/cockroach/pkg/sql/parser" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" + "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" "github.com/cockroachdb/errors" "github.com/robfig/cron/v3" ) @@ -67,11 +69,13 @@ func ValidateRowLevelTTL(ttl *catpb.RowLevelTTL) error { return nil } -// ValidateTTLExpirationExpr validates that the ttl_expiration_expression -// only references existing columns. -func ValidateTTLExpirationExpr( - desc catalog.TableDescriptor, expirationExpr catpb.Expression, -) error { +// ValidateTTLExpirationExpr validates that the ttl_expiration_expression, if +// any, only references existing columns. +func ValidateTTLExpirationExpr(desc catalog.TableDescriptor) error { + if !desc.HasRowLevelTTL() { + return nil + } + expirationExpr := desc.GetRowLevelTTL().ExpirationExpr if expirationExpr == "" { return nil } @@ -91,6 +95,53 @@ func ValidateTTLExpirationExpr( return nil } +// ValidateTTLExpirationColumn validates that the ttl_expire_after setting, if +// any, is in a valid state. It requires that the TTLDefaultExpirationColumn +// exists and has DEFAULT/ON UPDATE clauses. +func ValidateTTLExpirationColumn(desc catalog.TableDescriptor) error { + if !desc.HasRowLevelTTL() { + return nil + } + if !desc.GetRowLevelTTL().HasDurationExpr() { + return nil + } + intervalExpr := desc.GetRowLevelTTL().DurationExpr + col, err := desc.FindColumnWithName(colinfo.TTLDefaultExpirationColumnName) + if err != nil { + return errors.Wrapf(err, "expected column %s", colinfo.TTLDefaultExpirationColumnName) + } + expectedStr := `current_timestamp():::TIMESTAMPTZ + ` + string(intervalExpr) + if col.GetDefaultExpr() != expectedStr { + return pgerror.Newf( + pgcode.InvalidTableDefinition, + "expected DEFAULT expression of %s to be %s", + colinfo.TTLDefaultExpirationColumnName, + expectedStr, + ) + } + if col.GetOnUpdateExpr() != expectedStr { + return pgerror.Newf( + pgcode.InvalidTableDefinition, + "expected ON UPDATE expression of %s to be %s", + colinfo.TTLDefaultExpirationColumnName, + expectedStr, + ) + } + + // For row-level TTL, only ascending PKs are permitted. + pk := desc.GetPrimaryIndex() + for i := 0; i < pk.NumKeyColumns(); i++ { + dir := pk.GetKeyColumnDirection(i) + if dir != catpb.IndexColumn_ASC { + return unimplemented.NewWithIssuef( + 76912, + `non-ascending ordering on PRIMARY KEYs are not supported with row-level TTL`, + ) + } + } + return nil +} + // ValidateTTLBatchSize validates the batch size of a TTL. func ValidateTTLBatchSize(key string, val int64) error { if val <= 0 { diff --git a/pkg/sql/catalog/tabledesc/validate.go b/pkg/sql/catalog/tabledesc/validate.go index 5dc91971fdf4..b6688d94919f 100644 --- a/pkg/sql/catalog/tabledesc/validate.go +++ b/pkg/sql/catalog/tabledesc/validate.go @@ -213,51 +213,8 @@ func (desc *wrapper) ValidateCrossReferences( } } - // For row-level TTL, only ascending PKs are permitted. + // For row-level TTL, no FKs are allowed. if desc.HasRowLevelTTL() { - rowLevelTTL := desc.RowLevelTTL - if rowLevelTTL.HasDurationExpr() { - if col, err := desc.FindColumnWithName(colinfo.TTLDefaultExpirationColumnName); err != nil { - vea.Report(errors.Wrapf(err, "expected column %s", colinfo.TTLDefaultExpirationColumnName)) - } else { - intervalExpr := desc.GetRowLevelTTL().DurationExpr - expectedStr := `current_timestamp():::TIMESTAMPTZ + ` + string(intervalExpr) - if col.GetDefaultExpr() != expectedStr { - vea.Report(pgerror.Newf( - pgcode.InvalidTableDefinition, - "expected DEFAULT expression of %s to be %s", - colinfo.TTLDefaultExpirationColumnName, - expectedStr, - )) - } - if col.GetOnUpdateExpr() != expectedStr { - vea.Report(pgerror.Newf( - pgcode.InvalidTableDefinition, - "expected ON UPDATE expression of %s to be %s", - colinfo.TTLDefaultExpirationColumnName, - expectedStr, - )) - } - } - } - - if rowLevelTTL.HasExpirationExpr() { - _, err := parser.ParseExpr(string(rowLevelTTL.ExpirationExpr)) - if err != nil { - vea.Report(errors.Wrapf(err, "value of 'ttl_expiration_expression' must be a valid expression")) - } - } - - pk := desc.GetPrimaryIndex() - for i := 0; i < pk.NumKeyColumns(); i++ { - dir := pk.GetKeyColumnDirection(i) - if dir != catpb.IndexColumn_ASC { - vea.Report(unimplemented.NewWithIssuef( - 76912, - `non-ascending ordering on PRIMARY KEYs are not supported`, - )) - } - } if len(desc.OutboundFKs) > 0 || len(desc.InboundFKs) > 0 { vea.Report(unimplemented.NewWithIssuef( 76407, @@ -784,15 +741,17 @@ func (desc *wrapper) ValidateSelf(vea catalog.ValidationErrorAccumulator) { }) vea.Report(ValidateRowLevelTTL(desc.GetRowLevelTTL())) - if desc.HasRowLevelTTL() { - // ValidateTTLExpirationExpr is called separately from ValidateRowLevelTTL - // because it can only be called on an initialized table descriptor. - // ValidateRowLevelTTL is also used before the table descriptor is fully - // initialized to validate the storage parameters. - if err := ValidateTTLExpirationExpr(desc, desc.GetRowLevelTTL().ExpirationExpr); err != nil { - vea.Report(err) - return - } + // The remaining validation is called separately from ValidateRowLevelTTL + // because it can only be called on an initialized table descriptor. + // ValidateRowLevelTTL is also used before the table descriptor is fully + // initialized to validate the storage parameters. + if err := ValidateTTLExpirationExpr(desc); err != nil { + vea.Report(err) + return + } + if err := ValidateTTLExpirationColumn(desc); err != nil { + vea.Report(err) + return } // Validate that there are no column with both a foreign key ON UPDATE and an diff --git a/pkg/sql/catalog/tabledesc/validate_test.go b/pkg/sql/catalog/tabledesc/validate_test.go index c63c7e5ce6d9..cab4fbe9d1aa 100644 --- a/pkg/sql/catalog/tabledesc/validate_test.go +++ b/pkg/sql/catalog/tabledesc/validate_test.go @@ -372,6 +372,9 @@ func TestValidateTableDesc(t *testing.T) { boolTrue := true negativeOne := int64(-1) negativeOneFloat := float64(-1) + pointer := func(s string) *string { + return &s + } testData := []struct { err string @@ -2039,6 +2042,243 @@ func TestValidateTableDesc(t *testing.T) { NextColumnID: 2, AutoStatsSettings: &catpb.AutoStatsSettings{FractionStaleRows: &negativeOneFloat}, }}, + {`row-level TTL expiration expression "missing_col" refers to unknown columns`, + descpb.TableDescriptor{ + ID: 2, + ParentID: 1, + Name: "foo", + FormatVersion: descpb.InterleavedFormatVersion, + Columns: []descpb.ColumnDescriptor{ + {ID: 1, Name: "a"}, + }, + Families: []descpb.ColumnFamilyDescriptor{ + {ID: 0, Name: "fam", ColumnIDs: []descpb.ColumnID{1}, ColumnNames: []string{"a"}}, + }, + PrimaryIndex: descpb.IndexDescriptor{ + ID: 1, + Name: "primary", + Unique: true, + KeyColumnIDs: []descpb.ColumnID{1}, + KeyColumnNames: []string{"a"}, + KeyColumnDirections: []catpb.IndexColumn_Direction{catpb.IndexColumn_ASC}, + Version: descpb.PrimaryIndexWithStoredColumnsVersion, + EncodingType: descpb.PrimaryIndexEncoding, + ConstraintID: 1, + }, + NextColumnID: 2, + NextFamilyID: 1, + NextIndexID: 2, + RowLevelTTL: &catpb.RowLevelTTL{ + ExpirationExpr: catpb.Expression("missing_col"), + }, + }}, + {`"ttl_expire_after" and/or "ttl_expiration_expression" must be set`, + descpb.TableDescriptor{ + ID: 2, + ParentID: 1, + Name: "foo", + FormatVersion: descpb.InterleavedFormatVersion, + Columns: []descpb.ColumnDescriptor{ + {ID: 1, Name: "a"}, + }, + Families: []descpb.ColumnFamilyDescriptor{ + {ID: 0, Name: "fam", ColumnIDs: []descpb.ColumnID{1}, ColumnNames: []string{"a"}}, + }, + PrimaryIndex: descpb.IndexDescriptor{ + ID: 1, + Name: "primary", + Unique: true, + KeyColumnIDs: []descpb.ColumnID{1}, + KeyColumnNames: []string{"a"}, + KeyColumnDirections: []catpb.IndexColumn_Direction{catpb.IndexColumn_ASC}, + Version: descpb.PrimaryIndexWithStoredColumnsVersion, + EncodingType: descpb.PrimaryIndexEncoding, + ConstraintID: 1, + }, + NextColumnID: 2, + NextFamilyID: 1, + NextIndexID: 2, + RowLevelTTL: &catpb.RowLevelTTL{ + SelectBatchSize: 5, + }, + }}, + {`expected column crdb_internal_expiration: column "crdb_internal_expiration" does not exist`, + descpb.TableDescriptor{ + ID: 2, + ParentID: 1, + Name: "foo", + FormatVersion: descpb.InterleavedFormatVersion, + Columns: []descpb.ColumnDescriptor{ + {ID: 1, Name: "a"}, + }, + Families: []descpb.ColumnFamilyDescriptor{ + {ID: 0, Name: "fam", ColumnIDs: []descpb.ColumnID{1}, ColumnNames: []string{"a"}}, + }, + PrimaryIndex: descpb.IndexDescriptor{ + ID: 1, + Name: "primary", + Unique: true, + KeyColumnIDs: []descpb.ColumnID{1}, + KeyColumnNames: []string{"a"}, + KeyColumnDirections: []catpb.IndexColumn_Direction{catpb.IndexColumn_ASC}, + Version: descpb.PrimaryIndexWithStoredColumnsVersion, + EncodingType: descpb.PrimaryIndexEncoding, + ConstraintID: 1, + }, + NextColumnID: 2, + NextFamilyID: 1, + NextIndexID: 2, + RowLevelTTL: &catpb.RowLevelTTL{ + DurationExpr: catpb.Expression("INTERVAL '2 minutes'"), + }, + }}, + {`expected DEFAULT expression of crdb_internal_expiration to be current_timestamp():::TIMESTAMPTZ + INTERVAL '2 minutes'`, + descpb.TableDescriptor{ + ID: 2, + ParentID: 1, + Name: "foo", + FormatVersion: descpb.InterleavedFormatVersion, + Columns: []descpb.ColumnDescriptor{ + {ID: 1, Name: "a"}, + { + ID: 2, + Name: "crdb_internal_expiration", + Hidden: true, + OnUpdateExpr: pointer("current_timestamp():::TIMESTAMPTZ + INTERVAL '2 minutes'"), + }, + }, + Families: []descpb.ColumnFamilyDescriptor{ + {ID: 0, Name: "fam", ColumnIDs: []descpb.ColumnID{1, 2}, ColumnNames: []string{"a", "crdb_internal_expiration"}}, + }, + PrimaryIndex: descpb.IndexDescriptor{ + ID: 1, + Name: "primary", + Unique: true, + KeyColumnIDs: []descpb.ColumnID{1}, + KeyColumnNames: []string{"a"}, + KeyColumnDirections: []catpb.IndexColumn_Direction{catpb.IndexColumn_ASC}, + Version: descpb.PrimaryIndexWithStoredColumnsVersion, + EncodingType: descpb.PrimaryIndexEncoding, + ConstraintID: 1, + }, + NextColumnID: 3, + NextFamilyID: 1, + NextIndexID: 2, + RowLevelTTL: &catpb.RowLevelTTL{ + DurationExpr: catpb.Expression("INTERVAL '2 minutes'"), + }, + }}, + {`expected ON UPDATE expression of crdb_internal_expiration to be current_timestamp():::TIMESTAMPTZ + INTERVAL '2 minutes'`, + descpb.TableDescriptor{ + ID: 2, + ParentID: 1, + Name: "foo", + FormatVersion: descpb.InterleavedFormatVersion, + Columns: []descpb.ColumnDescriptor{ + {ID: 1, Name: "a"}, + { + ID: 2, + Name: "crdb_internal_expiration", + Hidden: true, + DefaultExpr: pointer("current_timestamp():::TIMESTAMPTZ + INTERVAL '2 minutes'"), + }, + }, + Families: []descpb.ColumnFamilyDescriptor{ + {ID: 0, Name: "fam", ColumnIDs: []descpb.ColumnID{1, 2}, ColumnNames: []string{"a", "crdb_internal_expiration"}}, + }, + PrimaryIndex: descpb.IndexDescriptor{ + ID: 1, + Name: "primary", + Unique: true, + KeyColumnIDs: []descpb.ColumnID{1}, + KeyColumnNames: []string{"a"}, + KeyColumnDirections: []catpb.IndexColumn_Direction{catpb.IndexColumn_ASC}, + Version: descpb.PrimaryIndexWithStoredColumnsVersion, + EncodingType: descpb.PrimaryIndexEncoding, + ConstraintID: 1, + }, + NextColumnID: 3, + NextFamilyID: 1, + NextIndexID: 2, + RowLevelTTL: &catpb.RowLevelTTL{ + DurationExpr: catpb.Expression("INTERVAL '2 minutes'"), + }, + }}, + {`"ttl_select_batch_size" must be at least 1`, + descpb.TableDescriptor{ + ID: 2, + ParentID: 1, + Name: "foo", + FormatVersion: descpb.InterleavedFormatVersion, + Columns: []descpb.ColumnDescriptor{ + {ID: 1, Name: "a"}, + { + ID: 2, + Name: "crdb_internal_expiration", + Hidden: true, + OnUpdateExpr: pointer("current_timestamp():::TIMESTAMPTZ + INTERVAL '2 minutes'"), + DefaultExpr: pointer("current_timestamp():::TIMESTAMPTZ + INTERVAL '2 minutes'"), + }, + }, + Families: []descpb.ColumnFamilyDescriptor{ + {ID: 0, Name: "fam", ColumnIDs: []descpb.ColumnID{1, 2}, ColumnNames: []string{"a", "crdb_internal_expiration"}}, + }, + PrimaryIndex: descpb.IndexDescriptor{ + ID: 1, + Name: "primary", + Unique: true, + KeyColumnIDs: []descpb.ColumnID{1}, + KeyColumnNames: []string{"a"}, + KeyColumnDirections: []catpb.IndexColumn_Direction{catpb.IndexColumn_ASC}, + Version: descpb.PrimaryIndexWithStoredColumnsVersion, + EncodingType: descpb.PrimaryIndexEncoding, + ConstraintID: 1, + }, + NextColumnID: 3, + NextFamilyID: 1, + NextIndexID: 2, + RowLevelTTL: &catpb.RowLevelTTL{ + DurationExpr: catpb.Expression("INTERVAL '2 minutes'"), + SelectBatchSize: -2, + }, + }}, + {`unimplemented: non-ascending ordering on PRIMARY KEYs are not supported with row-level TTL`, + descpb.TableDescriptor{ + ID: 2, + ParentID: 1, + Name: "foo", + FormatVersion: descpb.InterleavedFormatVersion, + Columns: []descpb.ColumnDescriptor{ + {ID: 1, Name: "a"}, + { + ID: 2, + Name: "crdb_internal_expiration", + Hidden: true, + OnUpdateExpr: pointer("current_timestamp():::TIMESTAMPTZ + INTERVAL '2 minutes'"), + DefaultExpr: pointer("current_timestamp():::TIMESTAMPTZ + INTERVAL '2 minutes'"), + }, + }, + Families: []descpb.ColumnFamilyDescriptor{ + {ID: 0, Name: "fam", ColumnIDs: []descpb.ColumnID{1, 2}, ColumnNames: []string{"a", "crdb_internal_expiration"}}, + }, + PrimaryIndex: descpb.IndexDescriptor{ + ID: 1, + Name: "primary", + Unique: true, + KeyColumnIDs: []descpb.ColumnID{1}, + KeyColumnNames: []string{"a"}, + KeyColumnDirections: []catpb.IndexColumn_Direction{catpb.IndexColumn_DESC}, + Version: descpb.PrimaryIndexWithStoredColumnsVersion, + EncodingType: descpb.PrimaryIndexEncoding, + ConstraintID: 1, + }, + NextColumnID: 3, + NextFamilyID: 1, + NextIndexID: 2, + RowLevelTTL: &catpb.RowLevelTTL{ + DurationExpr: catpb.Expression("INTERVAL '2 minutes'"), + }, + }}, } for i, d := range testData { t.Run(d.err, func(t *testing.T) { @@ -2443,7 +2683,7 @@ func TestValidateCrossTableReferences(t *testing.T) { DependsOn: []descpb.ID{51}, }}, }, - { + { // 15 err: `depended-on-by function "f" (100) has no corresponding depends-on forward reference`, desc: descpb.TableDescriptor{ Name: "foo", @@ -2455,7 +2695,7 @@ func TestValidateCrossTableReferences(t *testing.T) { }, }, }, - { + { // 16 err: `depends-on function "f" (100) has no corresponding depended-on-by back reference`, desc: descpb.TableDescriptor{ Name: "foo",