From 7956651200ef5d3698d454df7dd8e2cfcdb82a33 Mon Sep 17 00:00:00 2001 From: bplunkett-stripe Date: Sun, 29 Sep 2024 21:57:58 -0700 Subject: [PATCH] Update dependency system --- pkg/diff/plan.go | 4 +- pkg/diff/plan_test.go | 4 +- pkg/diff/policy_sql_generator.go | 22 ++-- pkg/diff/sql_generator.go | 172 +++++++++++++++---------------- pkg/diff/sql_graph.go | 116 ++++++++++++++------- pkg/diff/sql_vertex_generator.go | 55 ++++++---- 6 files changed, 211 insertions(+), 162 deletions(-) diff --git a/pkg/diff/plan.go b/pkg/diff/plan.go index ebc215a..7650f48 100644 --- a/pkg/diff/plan.go +++ b/pkg/diff/plan.go @@ -36,11 +36,11 @@ func (p MigrationHazard) String() string { type Statement struct { DDL string // Timeout is the statement_timeout to apply to this statement. If implementing your own plan executor, be sure to set - // the session-level statement_timeout to this value before executing the statement. A transaction-level statement_timeout + // the session-level statement_timeout to this value beforeSchemaObj executing the statement. A transaction-level statement_timeout // will not work since building indexes concurrently cannot be done in a transaction Timeout time.Duration // LockTimeout is the lock_timeout to apply to this statement. If implementing your own plan executor, be sure to set - // the session-level lock_timeout to this value before executing the statement. A transaction-level lock_timeout + // the session-level lock_timeout to this value beforeSchemaObj executing the statement. A transaction-level lock_timeout // will not work since building indexes concurrently cannot be done in a transaction LockTimeout time.Duration // The hazards this statement poses diff --git a/pkg/diff/plan_test.go b/pkg/diff/plan_test.go index c071f97..77bfa6e 100644 --- a/pkg/diff/plan_test.go +++ b/pkg/diff/plan_test.go @@ -281,7 +281,7 @@ func TestPlan_InsertStatement(t *testing.T) { }, }, { - name: "insert after first statement", + name: "insert afterSchemaObj first statement", plan: diff.Plan{ Statements: []diff.Statement{ {DDL: "statement 1", Timeout: time.Second}, @@ -304,7 +304,7 @@ func TestPlan_InsertStatement(t *testing.T) { }, }, { - name: "insert after last statement statement", + name: "insert afterSchemaObj last statement statement", plan: diff.Plan{ Statements: []diff.Statement{ {DDL: "statement 1", Timeout: time.Second}, diff --git a/pkg/diff/policy_sql_generator.go b/pkg/diff/policy_sql_generator.go index 8ef2a91..00fd5fe 100644 --- a/pkg/diff/policy_sql_generator.go +++ b/pkg/diff/policy_sql_generator.go @@ -58,9 +58,9 @@ var ( // // If not done in this order, we may create an outtage for a user's queries where RLS rejects their queries because // the policy allowing them hasn't been created yet. The same is true for disabling RLS, but in the reverse order. RLS -// must be disabled before policies are dropped. +// must be disabled beforeSchemaObj policies are dropped. // -// Another quirk of policies: Policies on partitions must be dropped before the base table is altered, otherwise +// Another quirk of policies: Policies on partitions must be dropped beforeSchemaObj the base table is altered, otherwise // the SQL could fail because, e.g., the policy references a column that no longer exists. func enableRLSForTable(t schema.Table) Statement { @@ -272,7 +272,7 @@ func buildPolicyVertexId(owningTable schema.SchemaQualifiedName, policyEscapedNa func (psg *policySQLVertexGenerator) GetAddAlterDependencies(newPolicy, oldPolicy schema.Policy) ([]dependency, error) { deps := []dependency{ - mustRun(psg.GetSQLVertexId(newPolicy), diffTypeDelete).before(psg.GetSQLVertexId(newPolicy), diffTypeAddAlter), + mustRun(psg.GetSQLVertexId(newPolicy), diffTypeDelete).beforeSchemaObj(psg.GetSQLVertexId(newPolicy), diffTypeAddAlter), } newTargetColumns, err := getTargetColumns(newPolicy.Columns, psg.newSchemaColumnsByName) @@ -280,21 +280,21 @@ func (psg *policySQLVertexGenerator) GetAddAlterDependencies(newPolicy, oldPolic return nil, fmt.Errorf("getting target columns: %w", err) } - // Run after the new columns are added/altered + // Run afterSchemaObj the new columns are added/altered for _, tc := range newTargetColumns { - deps = append(deps, mustRun(psg.GetSQLVertexId(newPolicy), diffTypeAddAlter).after(buildColumnVertexId(tc.Name), diffTypeAddAlter)) + deps = append(deps, mustRun(psg.GetSQLVertexId(newPolicy), diffTypeAddAlter).afterSchemaObj(buildColumnVertexId(tc.Name), diffTypeAddAlter)) } if !cmp.Equal(oldPolicy, schema.Policy{}) { - // Run before the old columns are deleted (if they are deleted) + // Run beforeSchemaObj the old columns are deleted (if they are deleted) oldTargetColumns, err := getTargetColumns(oldPolicy.Columns, psg.oldSchemaColumnsByName) if err != nil { return nil, fmt.Errorf("getting target columns: %w", err) } for _, tc := range oldTargetColumns { - // It only needs to run before the delete if the column is actually being deleted + // It only needs to run beforeSchemaObj the delete if the column is actually being deleted if _, stillExists := psg.newSchemaColumnsByName[tc.GetName()]; !stillExists { - deps = append(deps, mustRun(psg.GetSQLVertexId(newPolicy), diffTypeAddAlter).before(buildColumnVertexId(tc.Name), diffTypeDelete)) + deps = append(deps, mustRun(psg.GetSQLVertexId(newPolicy), diffTypeAddAlter).beforeSchemaObj(buildColumnVertexId(tc.Name), diffTypeDelete)) } } } @@ -309,10 +309,10 @@ func (psg *policySQLVertexGenerator) GetDeleteDependencies(pol schema.Policy) ([ if err != nil { return nil, fmt.Errorf("getting target columns: %w", err) } - // The policy needs to be deleted before all the columns it references are deleted or add/altered + // The policy needs to be deleted beforeSchemaObj all the columns it references are deleted or add/altered for _, c := range columns { - deps = append(deps, mustRun(psg.GetSQLVertexId(pol), diffTypeDelete).before(buildColumnVertexId(c.Name), diffTypeDelete)) - deps = append(deps, mustRun(psg.GetSQLVertexId(pol), diffTypeDelete).before(buildColumnVertexId(c.Name), diffTypeAddAlter)) + deps = append(deps, mustRun(psg.GetSQLVertexId(pol), diffTypeDelete).beforeSchemaObj(buildColumnVertexId(c.Name), diffTypeDelete)) + deps = append(deps, mustRun(psg.GetSQLVertexId(pol), diffTypeDelete).beforeSchemaObj(buildColumnVertexId(c.Name), diffTypeAddAlter)) } return deps, nil diff --git a/pkg/diff/sql_generator.go b/pkg/diff/sql_generator.go index 527a58b..e1547ae 100644 --- a/pkg/diff/sql_generator.go +++ b/pkg/diff/sql_generator.go @@ -45,7 +45,7 @@ var ( Message: "Dependencies, i.e. other functions used in the function body, of non-sql functions cannot be tracked. " + "As a result, we cannot guarantee that function dependencies are ordered properly relative to this " + "statement. For adds, this means you need to ensure that all functions this function depends on are " + - "created/altered before this statement.", + "created/altered beforeSchemaObj this statement.", } migrationHazardIndexDroppedQueryPerf = MigrationHazard{ Type: MigrationHazardTypeIndexDropped, @@ -825,7 +825,7 @@ func (t *tableSQLVertexGenerator) Alter(diff tableDiff) ([]Statement, error) { var stmts []Statement // Only handle disabling RLS if it was previously enabled. - // We want to disable RLS before we do any other operations on the table, e.g., delete policies, to avoid creating an + // We want to disable RLS beforeSchemaObj we do any other operations on the table, e.g., delete policies, to avoid creating an // outage while RLS is being disabled if !diff.new.RLSEnabled && diff.old.RLSEnabled { stmts = append(stmts, disableRLSForTable(diff.new)) @@ -856,7 +856,7 @@ func (t *tableSQLVertexGenerator) Alter(diff tableDiff) ([]Statement, error) { stmts = append(stmts, alterReplicaIdentityStmt) } - // We want to enable RLS after we do any other operations on the table, i.e., create policies, to avoid creating an + // We want to enable RLS afterSchemaObj we do any other operations on the table, i.e., create policies, to avoid creating an // outtage while RLS is being enabled if diff.new.RLSEnabled && !diff.old.RLSEnabled { stmts = append(stmts, enableRLSForTable(diff.new)) @@ -1045,12 +1045,12 @@ func (t *tableSQLVertexGenerator) GetSQLVertexId(table schema.Table) string { func (t *tableSQLVertexGenerator) GetAddAlterDependencies(table, _ schema.Table) ([]dependency, error) { deps := []dependency{ - mustRun(t.GetSQLVertexId(table), diffTypeAddAlter).after(t.GetSQLVertexId(table), diffTypeDelete), + mustRun(t.GetSQLVertexId(table), diffTypeAddAlter).afterSchemaObj(t.GetSQLVertexId(table), diffTypeDelete), } if table.ParentTable != nil { deps = append(deps, - mustRun(t.GetSQLVertexId(table), diffTypeAddAlter).after(buildTableVertexId(*table.ParentTable), diffTypeAddAlter), + mustRun(t.GetSQLVertexId(table), diffTypeAddAlter).afterSchemaObj(buildTableVertexId(*table.ParentTable), diffTypeAddAlter), ) } return deps, nil @@ -1118,7 +1118,7 @@ func (t *tableSQLVertexGenerator) GetDeleteDependencies(table schema.Table) ([]d var deps []dependency if table.ParentTable != nil { deps = append(deps, - mustRun(t.GetSQLVertexId(table), diffTypeDelete).after(buildTableVertexId(*table.ParentTable), diffTypeDelete), + mustRun(t.GetSQLVertexId(table), diffTypeDelete).afterSchemaObj(buildTableVertexId(*table.ParentTable), diffTypeDelete), ) } return deps, nil @@ -1166,7 +1166,7 @@ func (csg *columnSQLVertexGenerator) Alter(diff columnDiff) ([]Statement, error) var stmts []Statement alterColumnPrefix := fmt.Sprintf("%s ALTER COLUMN %s", alterTablePrefix(csg.tableName), schema.EscapeIdentifier(newColumn.Name)) - // Adding a "NOT NULL" constraint must come before updating a column to be an identity column, otherwise + // Adding a "NOT NULL" constraint must come beforeSchemaObj updating a column to be an identity column, otherwise // the add statement will fail because a column must be non-nullable to become an identity column. if oldColumn.IsNullable != newColumn.IsNullable && !newColumn.IsNullable { stmts = append(stmts, Statement{ @@ -1182,7 +1182,7 @@ func (csg *columnSQLVertexGenerator) Alter(diff columnDiff) ([]Statement, error) } stmts = append(stmts, updateIdentityStmts...) - // Removing a "NOT NULL" constraint must come after updating a column to no longer be an identity column, otherwise + // Removing a "NOT NULL" constraint must come afterSchemaObj updating a column to no longer be an identity column, otherwise // the "DROP NOT NULL" statement will fail because the column will still be an identity column. if oldColumn.IsNullable != newColumn.IsNullable && newColumn.IsNullable { stmts = append(stmts, Statement{ @@ -1193,7 +1193,7 @@ func (csg *columnSQLVertexGenerator) Alter(diff columnDiff) ([]Statement, error) } if len(oldColumn.Default) > 0 && len(newColumn.Default) == 0 { - // Drop the default before type conversion. This will allow type conversions + // Drop the default beforeSchemaObj type conversion. This will allow type conversions // between incompatible types if the previous column has a default and the new column is dropping its default stmts = append(stmts, Statement{ DDL: fmt.Sprintf("%s DROP DEFAULT", alterColumnPrefix), @@ -1214,7 +1214,7 @@ func (csg *columnSQLVertexGenerator) Alter(diff columnDiff) ([]Statement, error) ), // When "SET TYPE" is used to alter a column, that column's statistics are removed, which could // affect query plans. In order to mitigate the effect on queries, re-generate the statistics for the - // column before continuing with the migration. + // column beforeSchemaObj continuing with the migration. { DDL: fmt.Sprintf("ANALYZE %s (%s)", csg.tableName.GetFQEscapedName(), schema.EscapeIdentifier(newColumn.Name)), Timeout: statementTimeoutAnalyzeColumn, @@ -1232,7 +1232,7 @@ func (csg *columnSQLVertexGenerator) Alter(diff columnDiff) ([]Statement, error) } if oldColumn.Default != newColumn.Default && len(newColumn.Default) > 0 { - // Set the default after the type conversion. This will allow type conversions + // Set the default afterSchemaObj the type conversion. This will allow type conversions // between incompatible types if the previous column has no default and the new column has a default stmts = append(stmts, Statement{ DDL: fmt.Sprintf("%s SET DEFAULT %s", alterColumnPrefix, newColumn.Default), @@ -1380,7 +1380,7 @@ func buildColumnVertexId(columnName string) string { func (csg *columnSQLVertexGenerator) GetAddAlterDependencies(col, _ schema.Column) ([]dependency, error) { return []dependency{ - mustRun(csg.GetSQLVertexId(col), diffTypeDelete).before(csg.GetSQLVertexId(col), diffTypeAddAlter), + mustRun(csg.GetSQLVertexId(col), diffTypeDelete).beforeSchemaObj(csg.GetSQLVertexId(col), diffTypeAddAlter), }, nil } @@ -1412,7 +1412,7 @@ func (rsg *renameConflictingIndexSQLVertexGenerator) Add(index schema.Index) ([] if oldIndex, indexIsBeingRecreated := rsg.oldSchemaIndexesByName[index.GetName()]; !indexIsBeingRecreated { return nil, nil } else if oldIndex.IsPk() && index.IsPk() { - // Don't bother renaming if both are primary keys, since the new index will need to be created after the old + // Don't bother renaming if both are primary keys, since the new index will need to be created afterSchemaObj the old // index because we can't have two primary keys at the same time. // // To make changing primary keys index-gap free (mostly online), we could build the underlying new primary key index, @@ -1505,7 +1505,7 @@ type indexSQLVertexGenerator struct { // renameSQLVertexGenerator is used to find renames renameSQLVertexGenerator *renameConflictingIndexSQLVertexGenerator - // attachPartitionSQLVertexGenerator is used to find if a partition will be attached after an index builds + // attachPartitionSQLVertexGenerator is used to find if a partition will be attached afterSchemaObj an index builds attachPartitionSQLVertexGenerator *attachPartitionSQLVertexGenerator } @@ -1578,7 +1578,7 @@ func (isg *indexSQLVertexGenerator) addIdxStmtsWithHazards(index schema.Index) ( } if index.ParentIdx != nil && isg.attachPartitionSQLVertexGenerator.isPartitionAlreadyAttachedBeforeIndexBuilds(index.OwningTable) { - // Only attach the index if the index is built after the table is partitioned. If the partition + // Only attach the index if the index is built afterSchemaObj the table is partitioned. If the partition // hasn't already been attached, the index/constraint will be automatically attached when the table partition is // attached stmts = append(stmts, buildAttachIndex(index)) @@ -1744,15 +1744,15 @@ func (*indexSQLVertexGenerator) GetSQLVertexId(index schema.Index) string { func (isg *indexSQLVertexGenerator) GetAddAlterDependencies(index, _ schema.Index) ([]dependency, error) { dependencies := []dependency{ - mustRun(isg.GetSQLVertexId(index), diffTypeAddAlter).after(buildTableVertexId(index.OwningTable), diffTypeAddAlter), - // To allow for online changes to indexes, rename the older version of the index (if it exists) before the new version is added - mustRun(isg.GetSQLVertexId(index), diffTypeAddAlter).after(buildRenameConflictingIndexVertexId(index.GetSchemaQualifiedName()), diffTypeAddAlter), + mustRun(isg.GetSQLVertexId(index), diffTypeAddAlter).afterSchemaObj(buildTableVertexId(index.OwningTable), diffTypeAddAlter), + // To allow for online changes to indexes, rename the older version of the index (if it exists) beforeSchemaObj the new version is added + mustRun(isg.GetSQLVertexId(index), diffTypeAddAlter).afterSchemaObj(buildRenameConflictingIndexVertexId(index.GetSchemaQualifiedName()), diffTypeAddAlter), } if index.ParentIdx != nil { - // Partitions of indexes must be created after the parent index is created + // Partitions of indexes must be created afterSchemaObj the parent index is created dependencies = append(dependencies, - mustRun(isg.GetSQLVertexId(index), diffTypeAddAlter).after(buildIndexVertexId(*index.ParentIdx), diffTypeAddAlter)) + mustRun(isg.GetSQLVertexId(index), diffTypeAddAlter).afterSchemaObj(buildIndexVertexId(*index.ParentIdx), diffTypeAddAlter)) } return dependencies, nil @@ -1760,16 +1760,16 @@ func (isg *indexSQLVertexGenerator) GetAddAlterDependencies(index, _ schema.Inde func (isg *indexSQLVertexGenerator) GetDeleteDependencies(index schema.Index) ([]dependency, error) { dependencies := []dependency{ - mustRun(isg.GetSQLVertexId(index), diffTypeDelete).after(buildTableVertexId(index.OwningTable), diffTypeDelete), - // Drop the index after it has been potentially renamed - mustRun(isg.GetSQLVertexId(index), diffTypeDelete).after(buildRenameConflictingIndexVertexId(index.GetSchemaQualifiedName()), diffTypeAddAlter), + mustRun(isg.GetSQLVertexId(index), diffTypeDelete).afterSchemaObj(buildTableVertexId(index.OwningTable), diffTypeDelete), + // Drop the index afterSchemaObj it has been potentially renamed + mustRun(isg.GetSQLVertexId(index), diffTypeDelete).afterSchemaObj(buildRenameConflictingIndexVertexId(index.GetSchemaQualifiedName()), diffTypeAddAlter), } if index.ParentIdx != nil { // Since dropping the parent index will cause the partition of the index to drop, the parent drop should come - // before + // beforeSchemaObj dependencies = append(dependencies, - mustRun(isg.GetSQLVertexId(index), diffTypeDelete).after(buildIndexVertexId(*index.ParentIdx), diffTypeDelete)) + mustRun(isg.GetSQLVertexId(index), diffTypeDelete).afterSchemaObj(buildIndexVertexId(*index.ParentIdx), diffTypeDelete)) } dependencies = append(dependencies, isg.addDepsOnTableAddAlterIfNecessary(index)...) @@ -1781,24 +1781,24 @@ func (isg *indexSQLVertexGenerator) addDepsOnTableAddAlterIfNecessary(index sche parentTable, ok := isg.tablesInNewSchemaByName[index.OwningTable.GetName()] if !ok { // If the parent table is deleted, we don't need to worry about making the index statement come - // before any alters + // beforeSchemaObj any alters return nil } - // These dependencies will force the index deletion statement to come before the table AddAlter + // These dependencies will force the index deletion statement to come beforeSchemaObj the table AddAlter addAlterColumnDeps := []dependency{ - mustRun(isg.GetSQLVertexId(index), diffTypeDelete).before(buildTableVertexId(index.OwningTable), diffTypeAddAlter), + mustRun(isg.GetSQLVertexId(index), diffTypeDelete).beforeSchemaObj(buildTableVertexId(index.OwningTable), diffTypeAddAlter), } if parentTable.ParentTable != nil { // If the table is partitioned, columns modifications occur on the base table not the children. Thus, we // need the dependency to also be on the parent table add/alter statements addAlterColumnDeps = append( addAlterColumnDeps, - mustRun(isg.GetSQLVertexId(index), diffTypeDelete).before(buildTableVertexId(*parentTable.ParentTable), diffTypeAddAlter), + mustRun(isg.GetSQLVertexId(index), diffTypeDelete).beforeSchemaObj(buildTableVertexId(*parentTable.ParentTable), diffTypeAddAlter), ) } - // If the parent table still exists and the index is a primary key, we should drop the PK index before + // If the parent table still exists and the index is a primary key, we should drop the PK index beforeSchemaObj // any statements associated with altering the table run. This is important for changing the nullability of // columns if index.IsPk() { @@ -1807,7 +1807,7 @@ func (isg *indexSQLVertexGenerator) addDepsOnTableAddAlterIfNecessary(index sche parentTableColumnsByName := buildSchemaObjByNameMap(parentTable.Columns) for _, idxColumn := range index.Columns { - // We need to force the index drop to come before the statements to drop columns. Otherwise, the columns + // We need to force the index drop to come beforeSchemaObj the statements to drop columns. Otherwise, the columns // drops will force the index to drop non-concurrently if _, columnStillPresent := parentTableColumnsByName[idxColumn]; !columnStillPresent { return addAlterColumnDeps @@ -1920,7 +1920,7 @@ func (*checkConstraintSQLVertexGenerator) GetSQLVertexId(con schema.CheckConstra func (csg *checkConstraintSQLVertexGenerator) GetAddAlterDependencies(con, _ schema.CheckConstraint) ([]dependency, error) { deps := []dependency{ - mustRun(csg.GetSQLVertexId(con), diffTypeDelete).before(csg.GetSQLVertexId(con), diffTypeAddAlter), + mustRun(csg.GetSQLVertexId(con), diffTypeDelete).beforeSchemaObj(csg.GetSQLVertexId(con), diffTypeAddAlter), } targetColumns, err := getTargetColumns(con.KeyColumns, csg.newSchemaColumnsByName) @@ -1937,12 +1937,12 @@ func (csg *checkConstraintSQLVertexGenerator) GetAddAlterDependencies(con, _ sch } if isOnValidNotNullPreExistingColumn { - // If the NOT NULL check constraint is on a pre-existing column, then we should ensure it is added before + // If the NOT NULL check constraint is on a pre-existing column, then we should ensure it is added beforeSchemaObj // the column alter. - deps = append(deps, mustRun(csg.GetSQLVertexId(con), diffTypeAddAlter).before(buildColumnVertexId(targetColumns[0].Name), diffTypeAddAlter)) + deps = append(deps, mustRun(csg.GetSQLVertexId(con), diffTypeAddAlter).beforeSchemaObj(buildColumnVertexId(targetColumns[0].Name), diffTypeAddAlter)) } else { for _, tc := range targetColumns { - deps = append(deps, mustRun(csg.GetSQLVertexId(con), diffTypeAddAlter).after(buildColumnVertexId(tc.Name), diffTypeAddAlter)) + deps = append(deps, mustRun(csg.GetSQLVertexId(con), diffTypeAddAlter).afterSchemaObj(buildColumnVertexId(tc.Name), diffTypeAddAlter)) } } return deps, nil @@ -1956,28 +1956,28 @@ func (csg *checkConstraintSQLVertexGenerator) GetDeleteDependencies(con schema.C return nil, fmt.Errorf("getting target columns: %w", err) } - // If it's a not-null check constraint, we can drop the check constraint whenever convenient, i.e., after + // If it's a not-null check constraint, we can drop the check constraint whenever convenient, i.e., afterSchemaObj // the column has been altered because `NOT NULL` does not depend on the type of the column. It is important we // delete the NOT NULL check constraint AFTER the column is altered because we want to ensure any `SET NULL` alters // are backed with a check constraint. // // For all other check constraints, they can rely on the type of the column. Thus, we should drop these - // check constraint before any columns are altered because the new type might not be compatible with the old + // check constraint beforeSchemaObj any columns are altered because the new type might not be compatible with the old // check constraint. if isValidNotNullCC(con) { tc := targetColumns[0] if _, ok := csg.deletedColumnsByName[tc.Name]; ok { - // If the column is being deleted, we should drop the not null check constraint before the column is deleted. - deps = append(deps, mustRun(csg.GetSQLVertexId(con), diffTypeDelete).before(buildColumnVertexId(tc.Name), diffTypeDelete)) + // If the column is being deleted, we should drop the not null check constraint beforeSchemaObj the column is deleted. + deps = append(deps, mustRun(csg.GetSQLVertexId(con), diffTypeDelete).beforeSchemaObj(buildColumnVertexId(tc.Name), diffTypeDelete)) } else { - // Otherwise, we should drop the not null check constraint after the column is altered. This dependency + // Otherwise, we should drop the not null check constraint afterSchemaObj the column is altered. This dependency // doesn't need to be explicitly, since our topological sort prioritizes adds/alters over deletes. Nevertheless, - // we'll add it for clarity and to ensure that an error is returned if the delete is not placed after the alter. - deps = append(deps, mustRun(csg.GetSQLVertexId(con), diffTypeDelete).after(buildColumnVertexId(tc.Name), diffTypeAddAlter)) + // we'll add it for clarity and to ensure that an error is returned if the delete is not placed afterSchemaObj the alter. + deps = append(deps, mustRun(csg.GetSQLVertexId(con), diffTypeDelete).afterSchemaObj(buildColumnVertexId(tc.Name), diffTypeAddAlter)) } } else { for _, tc := range targetColumns { - deps = append(deps, mustRun(csg.GetSQLVertexId(con), diffTypeDelete).before(buildColumnVertexId(tc.Name), diffTypeAddAlter)) + deps = append(deps, mustRun(csg.GetSQLVertexId(con), diffTypeDelete).beforeSchemaObj(buildColumnVertexId(tc.Name), diffTypeAddAlter)) // This is a weird quirk of our graph system, where if a -> b and b -> c and b does-not-exist, b will be // implicitly created s.t. a -> b -> c (https://github.com/stripe/pg-schema-diff/issues/84) // @@ -1985,7 +1985,7 @@ func (csg *checkConstraintSQLVertexGenerator) GetDeleteDependencies(con schema.C // the column, and "c" is the alter/addition of the column. We do not want this behavior. We only want // a -> b -> c iff the column is being deleted. if _, ok := csg.deletedColumnsByName[tc.Name]; ok { - deps = append(deps, mustRun(csg.GetSQLVertexId(con), diffTypeDelete).before(buildColumnVertexId(tc.Name), diffTypeDelete)) + deps = append(deps, mustRun(csg.GetSQLVertexId(con), diffTypeDelete).beforeSchemaObj(buildColumnVertexId(tc.Name), diffTypeDelete)) } } } @@ -2010,7 +2010,7 @@ type attachPartitionSQLVertexGenerator struct { addedTablesByName map[string]schema.Table // isPartitionAttachedAfterIdxBuildsByTableName is a map of table name to whether or not the table partition will be - // attached after its indexes are built. This is useful for determining when indexes need to be attached + // attached afterSchemaObj its indexes are built. This is useful for determining when indexes need to be attached isPartitionAttachedAfterIdxBuildsByTableName map[string]bool sqlVertexGenerator[schema.Table, tableDiff] @@ -2059,23 +2059,23 @@ func (a *attachPartitionSQLVertexGenerator) GetAddAlterDependencies(table, old s } deps := []dependency{ - mustRun(a.GetSQLVertexId(table), diffTypeAddAlter).after(buildTableVertexId(table.SchemaQualifiedName), diffTypeAddAlter), + mustRun(a.GetSQLVertexId(table), diffTypeAddAlter).afterSchemaObj(buildTableVertexId(table.SchemaQualifiedName), diffTypeAddAlter), } if _, baseTableIsNew := a.addedTablesByName[table.ParentTable.GetName()]; baseTableIsNew { - // If the base table is new, we should force the partition to be attached before we build any non-local indexes. + // If the base table is new, we should force the partition to be attached beforeSchemaObj we build any non-local indexes. // This allows us to create fresh schemas where the base table has a PK but none of the children tables // have the PK (this is useful when creating the fresh database schema for migration validation) - // If we attach the partition after the index is built, the index will be automatically built by Postgres + // If we attach the partition afterSchemaObj the index is built, the index will be automatically built by Postgres for _, idx := range a.indexesInNewSchemaByTableName[table.ParentTable.GetName()] { - deps = append(deps, mustRun(a.GetSQLVertexId(table), diffTypeAddAlter).before(buildIndexVertexId(idx.GetSchemaQualifiedName()), diffTypeAddAlter)) + deps = append(deps, mustRun(a.GetSQLVertexId(table), diffTypeAddAlter).beforeSchemaObj(buildIndexVertexId(idx.GetSchemaQualifiedName()), diffTypeAddAlter)) } return deps, nil } a.isPartitionAttachedAfterIdxBuildsByTableName[table.GetName()] = true for _, idx := range a.indexesInNewSchemaByTableName[table.GetName()] { - deps = append(deps, mustRun(a.GetSQLVertexId(table), diffTypeAddAlter).after(buildIndexVertexId(idx.GetSchemaQualifiedName()), diffTypeAddAlter)) + deps = append(deps, mustRun(a.GetSQLVertexId(table), diffTypeAddAlter).afterSchemaObj(buildIndexVertexId(idx.GetSchemaQualifiedName()), diffTypeAddAlter)) } return deps, nil } @@ -2123,13 +2123,13 @@ type foreignKeyConstraintSQLVertexGenerator struct { // This is used to identify if hazards are necessary addedTablesByName map[string]schema.Table // indexInOldSchemaByTableName is a map of index name to the index in the old schema - // This is used to force the foreign key constraint to be dropped before the index it depends on is dropped + // This is used to force the foreign key constraint to be dropped beforeSchemaObj the index it depends on is dropped indexInOldSchemaByTableName map[string][]schema.Index // childrenInOldSchemaByPartitionedIndexName gives all child indexes (across all levels) for a given // partitioned index in the old schema. childrenInOldSchemaByPartitionedIndexName map[string][]schema.Index // indexesInNewSchemaByTableName is a map of index name to the index - // Same as above but for adds and after + // Same as above but for adds and afterSchemaObj indexesInNewSchemaByTableName map[string][]schema.Index // childrenInNewSchemaByPartitionedIndexName gives all child indexes (across all levels) for a given // partitioned index in the new schema. @@ -2227,19 +2227,19 @@ func (*foreignKeyConstraintSQLVertexGenerator) GetSQLVertexId(con schema.Foreign func (f *foreignKeyConstraintSQLVertexGenerator) GetAddAlterDependencies(con, _ schema.ForeignKeyConstraint) ([]dependency, error) { deps := []dependency{ - mustRun(f.GetSQLVertexId(con), diffTypeAddAlter).after(f.GetSQLVertexId(con), diffTypeDelete), - mustRun(f.GetSQLVertexId(con), diffTypeAddAlter).after(buildTableVertexId(con.OwningTable), diffTypeAddAlter), - mustRun(f.GetSQLVertexId(con), diffTypeAddAlter).after(buildTableVertexId(con.ForeignTable), diffTypeAddAlter), + mustRun(f.GetSQLVertexId(con), diffTypeAddAlter).afterSchemaObj(f.GetSQLVertexId(con), diffTypeDelete), + mustRun(f.GetSQLVertexId(con), diffTypeAddAlter).afterSchemaObj(buildTableVertexId(con.OwningTable), diffTypeAddAlter), + mustRun(f.GetSQLVertexId(con), diffTypeAddAlter).afterSchemaObj(buildTableVertexId(con.ForeignTable), diffTypeAddAlter), } - // This is the slightly lazy way of ensuring the foreign key constraint is added after the requisite index is + // This is the slightly lazy way of ensuring the foreign key constraint is added afterSchemaObj the requisite index is // built and marked as valid. // We __could__ do this just for the index the fk depends on, but that's slightly more wiring than we need right now // because of partitioned indexes, which are only valid when all child indexes have been built for _, i := range f.indexesInNewSchemaByTableName[con.ForeignTable.GetName()] { - deps = append(deps, mustRun(f.GetSQLVertexId(con), diffTypeAddAlter).after(buildIndexVertexId(i.GetSchemaQualifiedName()), diffTypeAddAlter)) + deps = append(deps, mustRun(f.GetSQLVertexId(con), diffTypeAddAlter).afterSchemaObj(buildIndexVertexId(i.GetSchemaQualifiedName()), diffTypeAddAlter)) // Build a dependency on any child index if the index is partitioned for _, c := range f.childrenInNewSchemaByPartitionedIndexName[i.GetName()] { - deps = append(deps, mustRun(f.GetSQLVertexId(con), diffTypeAddAlter).after(buildIndexVertexId(c.GetSchemaQualifiedName()), diffTypeAddAlter)) + deps = append(deps, mustRun(f.GetSQLVertexId(con), diffTypeAddAlter).afterSchemaObj(buildIndexVertexId(c.GetSchemaQualifiedName()), diffTypeAddAlter)) } } @@ -2248,17 +2248,17 @@ func (f *foreignKeyConstraintSQLVertexGenerator) GetAddAlterDependencies(con, _ func (f *foreignKeyConstraintSQLVertexGenerator) GetDeleteDependencies(con schema.ForeignKeyConstraint) ([]dependency, error) { deps := []dependency{ - mustRun(f.GetSQLVertexId(con), diffTypeDelete).before(buildTableVertexId(con.OwningTable), diffTypeDelete), - mustRun(f.GetSQLVertexId(con), diffTypeDelete).before(buildTableVertexId(con.ForeignTable), diffTypeDelete), + mustRun(f.GetSQLVertexId(con), diffTypeDelete).beforeSchemaObj(buildTableVertexId(con.OwningTable), diffTypeDelete), + mustRun(f.GetSQLVertexId(con), diffTypeDelete).beforeSchemaObj(buildTableVertexId(con.ForeignTable), diffTypeDelete), } - // This is the slightly lazy way of ensuring the foreign key constraint is deleted before the index it depends on is deleted + // This is the slightly lazy way of ensuring the foreign key constraint is deleted beforeSchemaObj the index it depends on is deleted // We __could__ do this just for the index the fk depends on, but that's slightly more wiring than we need right now // because of partitioned indexes, which are only valid when all child indexes have been built for _, i := range f.indexInOldSchemaByTableName[con.ForeignTable.GetName()] { - deps = append(deps, mustRun(f.GetSQLVertexId(con), diffTypeDelete).before(buildIndexVertexId(i.GetSchemaQualifiedName()), diffTypeDelete)) + deps = append(deps, mustRun(f.GetSQLVertexId(con), diffTypeDelete).beforeSchemaObj(buildIndexVertexId(i.GetSchemaQualifiedName()), diffTypeDelete)) // Build a dependency on any child index if the index is partitioned for _, c := range f.childrenInOldSchemaByPartitionedIndexName[i.GetName()] { - deps = append(deps, mustRun(f.GetSQLVertexId(con), diffTypeDelete).before(buildIndexVertexId(c.GetSchemaQualifiedName()), diffTypeDelete)) + deps = append(deps, mustRun(f.GetSQLVertexId(con), diffTypeDelete).beforeSchemaObj(buildIndexVertexId(c.GetSchemaQualifiedName()), diffTypeDelete)) } } return deps, nil @@ -2361,11 +2361,11 @@ func (s *sequenceSQLVertexGenerator) GetSQLVertexId(seq schema.Sequence) string func (s *sequenceSQLVertexGenerator) GetAddAlterDependencies(new schema.Sequence, _ schema.Sequence) ([]dependency, error) { deps := []dependency{ - mustRun(s.GetSQLVertexId(new), diffTypeAddAlter).after(s.GetSQLVertexId(new), diffTypeDelete), + mustRun(s.GetSQLVertexId(new), diffTypeAddAlter).afterSchemaObj(s.GetSQLVertexId(new), diffTypeDelete), } if new.Owner != nil { - // Sequences should be added/altered before the table they are owned by - deps = append(deps, mustRun(s.GetSQLVertexId(new), diffTypeAddAlter).before(buildTableVertexId(new.Owner.TableName), diffTypeAddAlter)) + // Sequences should be added/altered beforeSchemaObj the table they are owned by + deps = append(deps, mustRun(s.GetSQLVertexId(new), diffTypeAddAlter).beforeSchemaObj(buildTableVertexId(new.Owner.TableName), diffTypeAddAlter)) } return deps, nil } @@ -2373,13 +2373,13 @@ func (s *sequenceSQLVertexGenerator) GetAddAlterDependencies(new schema.Sequence func (s *sequenceSQLVertexGenerator) GetDeleteDependencies(seq schema.Sequence) ([]dependency, error) { var deps []dependency // This is an unfortunate hackaround. It would make sense to also have a dependency on the owner column, such that - // the sequence can only be considered deleted after the owning column is deleted. However, we currently don't separate + // the sequence can only be considered deleted afterSchemaObj the owning column is deleted. However, we currently don't separate // column deletes from table add/alters. We can't build this dependency without possibly creating a circular dependency: - // the sequence add/alter must occur before the new owner column add/alter, but the sequence delete must occur after the + // the sequence add/alter must occur beforeSchemaObj the new owner column add/alter, but the sequence delete must occur afterSchemaObj the // old owner column delete (equivalent to add/alter) and the sequence add/alter. We can get away with this because // we, so far, no columns are ever "re-created". If we ever do support that, we'll need to revisit this. if seq.Owner != nil { - deps = append(deps, mustRun(s.GetSQLVertexId(seq), diffTypeDelete).after(buildTableVertexId(seq.Owner.TableName), diffTypeDelete)) + deps = append(deps, mustRun(s.GetSQLVertexId(seq), diffTypeDelete).afterSchemaObj(buildTableVertexId(seq.Owner.TableName), diffTypeDelete)) } return deps, nil } @@ -2450,18 +2450,18 @@ func (s sequenceOwnershipSQLVertexGenerator) GetAddAlterDependencies(new schema. } deps := []dependency{ - // Always change ownership after the sequence has been added/altered - mustRun(s.GetSQLVertexId(new), diffTypeAddAlter).after(buildSequenceVertexId(new.SchemaQualifiedName), diffTypeAddAlter), + // Always change ownership afterSchemaObj the sequence has been added/altered + mustRun(s.GetSQLVertexId(new), diffTypeAddAlter).afterSchemaObj(buildSequenceVertexId(new.SchemaQualifiedName), diffTypeAddAlter), } if old.Owner != nil { - // Always update ownership before the old owner has been deleted - deps = append(deps, mustRun(s.GetSQLVertexId(new), diffTypeAddAlter).before(buildTableVertexId(old.Owner.TableName), diffTypeDelete)) + // Always update ownership beforeSchemaObj the old owner has been deleted + deps = append(deps, mustRun(s.GetSQLVertexId(new), diffTypeAddAlter).beforeSchemaObj(buildTableVertexId(old.Owner.TableName), diffTypeDelete)) } if new.Owner != nil { - // Always update ownership after the new owner has been created - deps = append(deps, mustRun(s.GetSQLVertexId(new), diffTypeAddAlter).after(buildTableVertexId(new.Owner.TableName), diffTypeAddAlter)) + // Always update ownership afterSchemaObj the new owner has been created + deps = append(deps, mustRun(s.GetSQLVertexId(new), diffTypeAddAlter).afterSchemaObj(buildTableVertexId(new.Owner.TableName), diffTypeAddAlter)) } return deps, nil @@ -2557,7 +2557,7 @@ func (f *functionSQLVertexGenerator) Delete(function schema.Function) ([]Stateme Message: "Dependencies, i.e. other functions used in the function body, of non-sql functions cannot be " + "tracked. As a result, we cannot guarantee that function dependencies are ordered properly relative to " + "this statement. For drops, this means you need to ensure that all functions this function depends on " + - "are dropped after this statement.", + "are dropped afterSchemaObj this statement.", }) } return []Statement{{ @@ -2601,15 +2601,15 @@ func (f *functionSQLVertexGenerator) GetAddAlterDependencies(newFunction, oldFun // because there won't be one if it is being added/altered var deps []dependency for _, depFunction := range newFunction.DependsOnFunctions { - deps = append(deps, mustRun(f.GetSQLVertexId(newFunction), diffTypeAddAlter).after(buildFunctionVertexId(depFunction), diffTypeAddAlter)) + deps = append(deps, mustRun(f.GetSQLVertexId(newFunction), diffTypeAddAlter).afterSchemaObj(buildFunctionVertexId(depFunction), diffTypeAddAlter)) } if !cmp.Equal(oldFunction, schema.Function{}) { // If the function is being altered: // If the old version of the function calls other functions that are being deleted come, those deletions - // must come after the function is altered, so it is no longer dependent on those dropped functions + // must come afterSchemaObj the function is altered, so it is no longer dependent on those dropped functions for _, depFunction := range oldFunction.DependsOnFunctions { - deps = append(deps, mustRun(f.GetSQLVertexId(newFunction), diffTypeAddAlter).before(buildFunctionVertexId(depFunction), diffTypeDelete)) + deps = append(deps, mustRun(f.GetSQLVertexId(newFunction), diffTypeAddAlter).beforeSchemaObj(buildFunctionVertexId(depFunction), diffTypeDelete)) } } @@ -2619,7 +2619,7 @@ func (f *functionSQLVertexGenerator) GetAddAlterDependencies(newFunction, oldFun func (f *functionSQLVertexGenerator) GetDeleteDependencies(function schema.Function) ([]dependency, error) { var deps []dependency for _, depFunction := range function.DependsOnFunctions { - deps = append(deps, mustRun(f.GetSQLVertexId(function), diffTypeDelete).before(buildFunctionVertexId(depFunction), diffTypeDelete)) + deps = append(deps, mustRun(f.GetSQLVertexId(function), diffTypeDelete).beforeSchemaObj(buildFunctionVertexId(depFunction), diffTypeDelete)) } return deps, nil } @@ -2675,16 +2675,16 @@ func (t *triggerSQLVertexGenerator) GetAddAlterDependencies(newTrigger, oldTrigg // added and dropped in the same migration. Thus, we don't need a dependency on the delete node of a function // because there won't be one if it is being added/altered deps := []dependency{ - mustRun(t.GetSQLVertexId(newTrigger), diffTypeAddAlter).after(buildFunctionVertexId(newTrigger.Function), diffTypeAddAlter), - mustRun(t.GetSQLVertexId(newTrigger), diffTypeAddAlter).after(buildTableVertexId(newTrigger.OwningTable), diffTypeAddAlter), + mustRun(t.GetSQLVertexId(newTrigger), diffTypeAddAlter).afterSchemaObj(buildFunctionVertexId(newTrigger.Function), diffTypeAddAlter), + mustRun(t.GetSQLVertexId(newTrigger), diffTypeAddAlter).afterSchemaObj(buildTableVertexId(newTrigger.OwningTable), diffTypeAddAlter), } if !cmp.Equal(oldTrigger, schema.Trigger{}) { // If the trigger is being altered: - // If the old version of the trigger called a function being deleted, the function deletion must come after the + // If the old version of the trigger called a function being deleted, the function deletion must come afterSchemaObj the // trigger is altered, so the trigger no longer has a dependency on the function deps = append(deps, - mustRun(t.GetSQLVertexId(newTrigger), diffTypeAddAlter).before(buildFunctionVertexId(oldTrigger.Function), diffTypeDelete), + mustRun(t.GetSQLVertexId(newTrigger), diffTypeAddAlter).beforeSchemaObj(buildFunctionVertexId(oldTrigger.Function), diffTypeDelete), ) } @@ -2693,8 +2693,8 @@ func (t *triggerSQLVertexGenerator) GetAddAlterDependencies(newTrigger, oldTrigg func (t *triggerSQLVertexGenerator) GetDeleteDependencies(trigger schema.Trigger) ([]dependency, error) { return []dependency{ - mustRun(t.GetSQLVertexId(trigger), diffTypeDelete).before(buildFunctionVertexId(trigger.Function), diffTypeDelete), - mustRun(t.GetSQLVertexId(trigger), diffTypeDelete).before(buildTableVertexId(trigger.OwningTable), diffTypeDelete), + mustRun(t.GetSQLVertexId(trigger), diffTypeDelete).beforeSchemaObj(buildFunctionVertexId(trigger.Function), diffTypeDelete), + mustRun(t.GetSQLVertexId(trigger), diffTypeDelete).beforeSchemaObj(buildTableVertexId(trigger.OwningTable), diffTypeDelete), }, nil } diff --git a/pkg/diff/sql_graph.go b/pkg/diff/sql_graph.go index 5c2e99d..7277282 100644 --- a/pkg/diff/sql_graph.go +++ b/pkg/diff/sql_graph.go @@ -7,17 +7,55 @@ import ( "github.com/stripe/pg-schema-diff/internal/schema" ) +// sqlVertexId is an interface for a vertex id in the SQL graph +type sqlVertexId interface { + fmt.Stringer +} + +// sqlPriority is an enum for the priority of a statement in the SQL graph, i.e., whether it should be run sooner +// or later in the topological sort of the graph +type sqlPriority int + +const ( + // Indicates a statement should run as soon as possible + sqlPrioritySooner sqlPriority = 1 + // sqlPriorityUnset is the default priority for a statement + sqlPriorityUnset sqlPriority = 0 + // Indicates a statement should run as late as possible + sqlPriorityLater sqlPriority = -1 +) + +// schemaObjSqlVertexId is a vertex id for a standard schema object node, i.e., indicating the creation/deletiion +// of a schema object. It slots into the legacySqlVertexGenerator system. +type schemaObjSqlVertexId struct { + schemaObjId string + diffType diffType +} + +func (s schemaObjSqlVertexId) String() string { + return fmt.Sprintf("%s_%s", s.schemaObjId, s.diffType) +} + type sqlVertex struct { - // todo(bplunkett) - rename to statement id? - // these ids will need to be globally unique - ObjId string - Statements []Statement - // todo(bplunkett) - this should not affect the id - DiffType diffType + // id is used to identify the sql vertex + id sqlVertexId + + // priority is used to determine if the sql vertex should be included sooner or later in the topological + // sort of the graph + priority sqlPriority + + // statements is the set of statements to run for this vertex + statements []Statement } func (s sqlVertex) GetId() string { - return fmt.Sprintf("%s_%s", s.DiffType, s.ObjId) + return s.id.String() +} + +func (s sqlVertex) GetPriority() int { + // Prioritize adds/alters over deletes. Weight by number of statements. A 0 statement delete should be + // prioritized over a 1 statement delete + return len(s.statements) * int(s.priority) } func buildTableVertexId(name schema.SchemaQualifiedName) string { @@ -35,45 +73,51 @@ func buildIndexVertexId(name schema.SchemaQualifiedName) string { // These nodes will almost always be present in the sqlGraph even if the schema object is not being deleted (or added/altered). // If a node is present for a schema object where the "diffType" is NOT occurring, it will just be a no-op (no SQl statements) type dependency struct { - sourceObjId string - sourceType diffType - - targetObjId string - targetType diffType + source sqlVertexId + target sqlVertexId } type dependencyBuilder struct { - valObjId string - valType diffType + base sqlVertexId } func mustRun(schemaObjId string, schemaDiffType diffType) dependencyBuilder { return dependencyBuilder{ - valObjId: schemaObjId, - valType: schemaDiffType, + base: schemaObjSqlVertexId{ + schemaObjId: schemaObjId, + diffType: schemaDiffType, + }, } } -func (d dependencyBuilder) before(valObjId string, valType diffType) dependency { +func (d dependencyBuilder) before(id sqlVertexId) dependency { return dependency{ - sourceType: d.valType, - sourceObjId: d.valObjId, - - targetType: valType, - targetObjId: valObjId, + target: id, + source: d.base, } } -func (d dependencyBuilder) after(valObjId string, valType diffType) dependency { +func (d dependencyBuilder) after(id sqlVertexId) dependency { return dependency{ - sourceObjId: valObjId, - sourceType: valType, - - targetObjId: d.valObjId, - targetType: d.valType, + source: id, + target: d.base, } } +func (d dependencyBuilder) beforeSchemaObj(valObjId string, valType diffType) dependency { + return d.before(schemaObjSqlVertexId{ + schemaObjId: valObjId, + diffType: valType, + }) +} + +func (d dependencyBuilder) afterSchemaObj(valObjId string, valType diffType) dependency { + return d.after(schemaObjSqlVertexId{ + schemaObjId: valObjId, + diffType: valType, + }) +} + // sqlGraph represents two dependency webs of SQL statements type sqlGraph struct { *graph.Graph[sqlVertex] @@ -86,23 +130,15 @@ func newSqlGraph() *sqlGraph { } func (s *sqlGraph) toOrderedStatements() ([]Statement, error) { - vertices, err := s.TopologicallySortWithPriority(graph.IsLowerPriorityFromGetPriority( - func(vertex sqlVertex) int { - multiplier := 1 - if vertex.DiffType == diffTypeDelete { - multiplier = -1 - } - // Prioritize adds/alters over deletes. Weight by number of statements. A 0 statement delete should be - // prioritized over a 1 statement delete - return len(vertex.Statements) * multiplier - }), - ) + vertices, err := s.TopologicallySortWithPriority(graph.IsLowerPriorityFromGetPriority(func(v sqlVertex) int { + return v.GetPriority() + })) if err != nil { return nil, fmt.Errorf("topologically sorting graph: %w", err) } var stmts []Statement for _, v := range vertices { - stmts = append(stmts, v.Statements...) + stmts = append(stmts, v.statements...) } return stmts, nil } diff --git a/pkg/diff/sql_vertex_generator.go b/pkg/diff/sql_vertex_generator.go index b453825..0c915ac 100644 --- a/pkg/diff/sql_vertex_generator.go +++ b/pkg/diff/sql_vertex_generator.go @@ -24,7 +24,7 @@ type partialSQLGraph struct { func (s *partialSQLGraph) statements() []Statement { var statements []Statement for _, vertex := range s.vertices { - statements = append(statements, vertex.Statements...) + statements = append(statements, vertex.statements...) } return statements } @@ -54,14 +54,14 @@ func graphFromPartials(parts partialSQLGraph) (*sqlGraph, error) { for _, dep := range parts.dependencies { sourceVertex := sqlVertex{ - ObjId: dep.sourceObjId, - DiffType: dep.sourceType, - Statements: nil, + id: dep.source, + priority: sqlPriorityUnset, + statements: nil, } targetVertex := sqlVertex{ - ObjId: dep.targetObjId, - DiffType: dep.targetType, - Statements: nil, + id: dep.target, + priority: sqlPriorityUnset, + statements: nil, } // To maintain the correctness of the graph, we will add a dummy vertex for the missing dependencies @@ -77,10 +77,14 @@ func graphFromPartials(parts partialSQLGraph) (*sqlGraph, error) { } func mergeVertices(old, new sqlVertex) sqlVertex { + priority := old.priority + if old.priority == sqlPriorityUnset { + priority = new.priority + } return sqlVertex{ - ObjId: old.ObjId, - DiffType: old.DiffType, - Statements: append(old.Statements, new.Statements...), + id: old.id, + priority: priority, + statements: append(old.statements, new.statements...), } } @@ -139,12 +143,12 @@ type legacySqlVertexGenerator[S schema.Object, Diff diff[S]] interface { // no statements. If the diff is just an add, then old will be the zero value // // These dependencies can also be built in reverse: the SQL returned by the sqlVertexGenerator to resolve the - // diff for the object must always be run before the SQL required to resolve another SQL vertex diff + // diff for the object must always be run beforeSchemaObj the SQL required to resolve another SQL vertex diff GetAddAlterDependencies(new S, old S) ([]dependency, error) // GetDeleteDependencies is the same as above but for deletes. // Invariant to maintain: - // - If an object X depends on the delete for an object Y (generated by the sqlVertexGenerator), immediately after the + // - If an object X depends on the delete for an object Y (generated by the sqlVertexGenerator), immediately afterSchemaObj the // the (Y, diffTypeDelete) sqlVertex's SQL is run, Y must no longer be present in the schema; either the // (Y, diffTypeDelete) statements deleted Y or something that vertex depended on deleted Y. In other words, if a // delete is cascaded by another delete (e.g., index dropped by table drop) and the index SQL is empty, @@ -176,9 +180,12 @@ func (s *wrappedLegacySqlVertexGenerator[S, Diff]) Add(o S) (partialSQLGraph, er return partialSQLGraph{ vertices: []sqlVertex{{ - DiffType: diffTypeAddAlter, - ObjId: s.generator.GetSQLVertexId(o), - Statements: statements, + id: schemaObjSqlVertexId{ + schemaObjId: s.generator.GetSQLVertexId(o), + diffType: diffTypeAddAlter, + }, + priority: sqlPrioritySooner, + statements: statements, }}, dependencies: deps, }, nil @@ -196,9 +203,12 @@ func (s *wrappedLegacySqlVertexGenerator[S, Diff]) Delete(o S) (partialSQLGraph, return partialSQLGraph{ vertices: []sqlVertex{{ - DiffType: diffTypeDelete, - ObjId: s.generator.GetSQLVertexId(o), - Statements: statements, + id: schemaObjSqlVertexId{ + schemaObjId: s.generator.GetSQLVertexId(o), + diffType: diffTypeDelete, + }, + priority: sqlPriorityLater, + statements: statements, }}, dependencies: deps, }, nil @@ -216,9 +226,12 @@ func (s *wrappedLegacySqlVertexGenerator[S, Diff]) Alter(d Diff) (partialSQLGrap return partialSQLGraph{ vertices: []sqlVertex{{ - DiffType: diffTypeAddAlter, - ObjId: s.generator.GetSQLVertexId(d.GetNew()), - Statements: statements, + id: schemaObjSqlVertexId{ + schemaObjId: s.generator.GetSQLVertexId(d.GetNew()), + diffType: diffTypeAddAlter, + }, + priority: sqlPrioritySooner, + statements: statements, }}, dependencies: deps, }, nil