Skip to content

Commit

Permalink
workload/schemachange: update and disable ALTER TYPE DROP VALUE
Browse files Browse the repository at this point in the history
Previously, `ALTER TYPE DROP VALUE` was using the procedural generation
that is most common in this workload.

The desire to add support UDFs required revisiting the implement as
referenced enum values cannot be dropped.

Unfortunately, there is not a straightforward way to determine if a
specific enum is being referenced by other descriptors (namely default
expressions for columns and UDFs).

After the initial update and a few testing rounds with UDF support, a
bug between enums and UDFs was discovered. (See #114844 for details).

This commit upgrades the implementation of `ALTER TYPE DROP VALUE` to
utilize the `Generate` helper and dodge cases of referenced enum values.

Due to the aforementioned bug and another outstanding flake, this commit
also disabled `ALTER TYPE DROP VALUE` to prevent further flakes an
noise.

Epic: CRDB-19168
Informs: #113859, #114844
Release note: None
  • Loading branch information
chrisseto committed Nov 29, 2023
1 parent c943dde commit f2b181f
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 34 deletions.
70 changes: 43 additions & 27 deletions pkg/workload/schemachange/operation_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2021,37 +2021,53 @@ func (og *operationGenerator) dropView(ctx context.Context, tx pgx.Tx) (*opStmt,
return stmt, nil
}

func (og *operationGenerator) dropTypeValue(ctx context.Context, tx pgx.Tx) (*opStmt, error) {
enum, exists, err := og.randEnum(ctx, tx, og.pctExisting(true))
func (og *operationGenerator) alterTypeDropValue(ctx context.Context, tx pgx.Tx) (*opStmt, error) {
query := With([]CTE{
{"descriptors", descJSONQuery},
{"enums", enumDescsQuery},
{"enum_members", enumMemberDescsQuery},
}, `SELECT
quote_ident(schema_id::REGNAMESPACE::TEXT) || '.' || quote_ident(name) AS name,
quote_literal(member->>'logicalRepresentation') AS value,
COALESCE(member->>'direction' = 'REMOVE', false) AS dropping,
COALESCE(json_array_length(descriptor->'referencingDescriptorIds') > 0, false) AS has_references
FROM enum_members
`)

enumMembers, err := Collect(ctx, og, tx, pgx.RowToMap, query)
if err != nil {
return nil, err
}

// TODO(chrisseto): We're currently missing cases around enum members being
// referenced as it's quite difficult to tell if an individual member is
// referenced. Unreferenced members can be dropped but referenced members may
// not. For now, we skip over all enums where the type itself is being
// referenced.

stmt, code, err := Generate[*tree.AlterType](og.params.rng, og.produceError(), []GenerationCase{
// Fail to drop values from a type that doesn't exist.
{pgcode.UndefinedObject, `ALTER TYPE "EnumThatDoesntExist" DROP VALUE 'IrrelevantValue'`},
// Fail to drop a value that is in the process of being dropped.
{pgcode.ObjectNotInPrerequisiteState, `{ with (EnumValue true false) } ALTER TYPE { .name } DROP VALUE { .value } { end }`},
// Fail to drop types that don't exist.
{pgcode.UndefinedObject, `{ with (EnumValue false false) } ALTER TYPE { .name } DROP VALUE 'ValueThatDoesntExist' { end }`},
// Successful drop of an enum value.
{pgcode.SuccessfulCompletion, `{ with (EnumValue false false) } ALTER TYPE { .name } DROP VALUE { .value } { end }`},
}, template.FuncMap{
"EnumValue": func(dropping, referenced bool) (map[string]any, error) {
return PickOne(og.params.rng, util.Filter(enumMembers, func(enum map[string]any) bool {
return enum["has_references"].(bool) == referenced && enum["dropping"].(bool) == dropping
}))
},
})
if err != nil {
return nil, err
}

validValue := false
value := "IrrelevantEnumValue"

if exists {
value, validValue, err = og.randEnumValue(ctx, tx, og.pctExisting(true), enum)
if err != nil {
return nil, err
}
}

valueDropping := false
if validValue {
valueDropping, err = og.enumValueIsBeingRemoved(ctx, tx, enum.String(), value)
if err != nil {
return nil, err
}
}

stmt := makeOpStmt(OpStmtDDL)
stmt.expectedExecErrors.addAll(codesWithConditions{
{pgcode.UndefinedObject, !exists || !validValue},
{pgcode.ObjectNotInPrerequisiteState, valueDropping},
})
stmt.sql = fmt.Sprintf(`ALTER TYPE %s DROP VALUE '%s'`, enum, value)
return stmt, nil
return newOpStmt(stmt, codesWithConditions{
{code, true},
}), nil
}

func (og *operationGenerator) renameColumn(ctx context.Context, tx pgx.Tx) (*opStmt, error) {
Expand Down
10 changes: 5 additions & 5 deletions pkg/workload/schemachange/optype.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,10 @@ var opFuncs = []func(*operationGenerator, context.Context, pgx.Tx) (*opStmt, err

// DDL Operations
alterDatabaseAddRegion: (*operationGenerator).addRegion,
alterDatabasePrimaryRegion: (*operationGenerator).primaryRegion,
alterDatabaseSurvivalGoal: (*operationGenerator).survive,
alterDatabaseAddSuperRegion: (*operationGenerator).alterDatabaseAddSuperRegion,
alterDatabaseDropSuperRegion: (*operationGenerator).alterDatabaseDropSuperRegion,
alterDatabasePrimaryRegion: (*operationGenerator).primaryRegion,
alterDatabaseSurvivalGoal: (*operationGenerator).survive,
alterTableAddColumn: (*operationGenerator).addColumn,
alterTableAddConstraint: (*operationGenerator).addConstraint,
alterTableAddConstraintForeignKey: (*operationGenerator).addForeignKeyConstraint,
Expand All @@ -224,13 +224,13 @@ var opFuncs = []func(*operationGenerator, context.Context, pgx.Tx) (*opStmt, err
alterTableRenameColumn: (*operationGenerator).renameColumn,
alterTableSetColumnDefault: (*operationGenerator).setColumnDefault,
alterTableSetColumnNotNull: (*operationGenerator).setColumnNotNull,
alterTypeDropValue: (*operationGenerator).dropTypeValue,
createTypeEnum: (*operationGenerator).createEnum,
alterTypeDropValue: (*operationGenerator).alterTypeDropValue,
createIndex: (*operationGenerator).createIndex,
createSchema: (*operationGenerator).createSchema,
createSequence: (*operationGenerator).createSequence,
createTable: (*operationGenerator).createTable,
createTableAs: (*operationGenerator).createTableAs,
createTypeEnum: (*operationGenerator).createEnum,
createView: (*operationGenerator).createView,
dropIndex: (*operationGenerator).dropIndex,
dropSchema: (*operationGenerator).dropSchema,
Expand Down Expand Up @@ -275,7 +275,7 @@ var opWeights = []int{
dropSequence: 1,
dropTable: 1,
dropView: 1,
alterTypeDropValue: 1,
alterTypeDropValue: 0, // Disabled and tracked with #114844 and #113859
dropSchema: 1,
alterTableRenameColumn: 1,
renameIndex: 1,
Expand Down
33 changes: 31 additions & 2 deletions pkg/workload/schemachange/query_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,38 @@ import (
"github.com/jackc/pgx/v5"
)

var (
const (
// descJSONQuery returns the JSONified version of all descriptors in the
// current database joined with system.namespace.
//
// id::int | schema_id::int | name::text | descriptor::json
descJSONQuery = `SELECT
descriptor.id,
"parentSchemaID" AS schema_id,
namespace.name AS name,
crdb_internal.pb_to_json('desc', descriptor) AS descriptor
FROM system.descriptor
JOIN system.namespace ON namespace.id = descriptor.id
WHERE "parentID" = (SELECT id FROM system.namespace WHERE name = current_database() AND "parentID" = 0)
`

// enumDescsQuery returns the JSONified version of all enum descriptors in
// the current database.
//
// [descJSONQuery] must be bound to the name "descriptors".
//
// id::int | schema_id::int | name::text | descriptor::json
enumDescsQuery = `SELECT id, schema_id, name, descriptor->'type' AS descriptor FROM descriptors WHERE descriptor ? 'type'`

// enumDescsQuery returns the JSONified version of all enum members, along
// with their enum descriptors, in the current database.
//
// [enumDescsQuery] must be bound to the name "enums".
//
// id::int | schema_id::int | name::text | descriptor::json | member::json
enumMemberDescsQuery = `SELECT *, jsonb_array_elements(descriptor->'enumMembers') AS member FROM enums`

regionsFromClusterQuery = `SELECT * FROM [SHOW REGIONS FROM CLUSTER]`
descJSONQuery = `SELECT id, crdb_internal.pb_to_json('desc', descriptor) AS descriptor FROM system.descriptor`
)

func regionsFromDatabaseQuery(database string) string {
Expand Down

0 comments on commit f2b181f

Please sign in to comment.