Skip to content

Commit 5f76a53

Browse files
committed
workload: remove FK dependency check in schemachanger drop column
The schemachanger previously attempted to predict whether a DROP COLUMN operation would fail due to a foreign key (FK) dependency. This logic has proven to be unreliable and difficult to maintain correctly. Rather than investing more effort into fixing the latest edge cases, we’ve decided to remove the FK dependency detection logic altogether. This simplifies the codebase. Resolves: #159034 Release note: none
1 parent cedc567 commit 5f76a53

File tree

2 files changed

+25
-149
lines changed

2 files changed

+25
-149
lines changed

pkg/workload/schemachange/error_screening.go

Lines changed: 0 additions & 144 deletions
Original file line numberDiff line numberDiff line change
@@ -131,150 +131,6 @@ func (og *operationGenerator) tableHasDependencies(
131131
return og.scanBool(ctx, tx, q, tableName.Object(), tableName.Schema())
132132
}
133133

134-
// columnRemovalWillDropFKBackingIndexes determines if dropping this column
135-
// will lead to no indexes backing a foreign key.
136-
func (og *operationGenerator) columnRemovalWillDropFKBackingIndexes(
137-
ctx context.Context, tx pgx.Tx, tableName *tree.TableName, columName tree.Name,
138-
) (bool, error) {
139-
return og.scanBool(ctx, tx, fmt.Sprintf(`
140-
WITH
141-
fk
142-
AS (
143-
SELECT
144-
oid,
145-
(
146-
SELECT
147-
r.relname
148-
FROM
149-
pg_class AS r
150-
WHERE
151-
r.oid = c.confrelid
152-
)
153-
AS base_table,
154-
a.attname AS base_col,
155-
array_position(c.confkey, a.attnum)
156-
AS base_ordinal,
157-
(
158-
SELECT
159-
r.relname
160-
FROM
161-
pg_class AS r
162-
WHERE
163-
r.oid = c.conrelid
164-
)
165-
AS referencing_table,
166-
unnest(
167-
(
168-
SELECT
169-
array_agg(attname)
170-
FROM
171-
pg_attribute
172-
WHERE
173-
attrelid = c.conrelid
174-
AND ARRAY[attnum] <@ c.conkey
175-
AND array_position(
176-
c.confkey,
177-
a.attnum
178-
)
179-
= array_position(
180-
c.conkey,
181-
attnum
182-
)
183-
)
184-
)
185-
AS referencing_col
186-
FROM
187-
pg_constraint AS c
188-
JOIN pg_attribute AS a ON
189-
c.confrelid = a.attrelid
190-
AND ARRAY[attnum] <@ c.confkey
191-
WHERE
192-
c.confrelid = $1::REGCLASS::OID
193-
),
194-
valid_indexes
195-
AS (
196-
SELECT
197-
*
198-
FROM
199-
[SHOW INDEXES FROM %s]
200-
WHERE
201-
index_name
202-
NOT IN (
203-
SELECT
204-
DISTINCT index_name
205-
FROM
206-
[SHOW INDEXES FROM %s]
207-
WHERE
208-
column_name = $2
209-
AND index_name
210-
NOT LIKE '%%_pkey' -- renames would keep the old table name
211-
)
212-
),
213-
matching_indexes
214-
AS (
215-
SELECT
216-
oid,
217-
index_name,
218-
count(base_ordinal) AS count_base_ordinal,
219-
count(seq_in_index) AS count_seq_in_index
220-
FROM
221-
fk, valid_indexes
222-
WHERE
223-
storing = 'f'
224-
AND non_unique = 'f'
225-
AND base_col = column_name
226-
GROUP BY
227-
(oid, index_name)
228-
),
229-
valid_index_attrib_count
230-
AS (
231-
SELECT
232-
index_name,
233-
max(seq_in_index) AS max_seq_in_index
234-
FROM
235-
valid_indexes
236-
WHERE
237-
storing = 'f'
238-
GROUP BY
239-
index_name
240-
),
241-
valid_fk_count
242-
AS (
243-
SELECT
244-
oid, max(base_ordinal) AS max_base_ordinal
245-
FROM
246-
fk
247-
GROUP BY
248-
fk
249-
),
250-
matching_fks
251-
AS (
252-
SELECT
253-
DISTINCT f.oid
254-
FROM
255-
valid_index_attrib_count AS i,
256-
valid_fk_count AS f,
257-
matching_indexes AS m
258-
WHERE
259-
f.oid = m.oid
260-
AND i.index_name = m.index_name
261-
AND i.max_seq_in_index
262-
= m.count_seq_in_index
263-
AND f.max_base_ordinal
264-
= m.count_base_ordinal
265-
)
266-
SELECT
267-
EXISTS(
268-
SELECT
269-
*
270-
FROM
271-
fk
272-
WHERE
273-
oid NOT IN (SELECT oid FROM matching_fks)
274-
);
275-
`, tableName.String(), tableName.String()), tableName.String(), columName)
276-
}
277-
278134
func (og *operationGenerator) columnIsDependedOn(
279135
ctx context.Context, tx pgx.Tx, tableName *tree.TableName, columnName tree.Name,
280136
) (bool, error) {

pkg/workload/schemachange/operation_generator.go

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1652,6 +1652,14 @@ func (og *operationGenerator) dropColumn(ctx context.Context, tx pgx.Tx) (*opStm
16521652
return nil, err
16531653
}
16541654

1655+
// Check if the table has any foreign keys.
1656+
tableHasFK := false
1657+
if tableExists {
1658+
if tableHasFK, err = og.tableHasFK(ctx, tx, tableName); err != nil {
1659+
return nil, err
1660+
}
1661+
}
1662+
16551663
columnName, err := og.randColumn(ctx, tx, *tableName, og.pctExisting(true))
16561664
if err != nil {
16571665
return nil, err
@@ -1668,10 +1676,6 @@ func (og *operationGenerator) dropColumn(ctx context.Context, tx pgx.Tx) (*opStm
16681676
if err != nil {
16691677
return nil, err
16701678
}
1671-
columnRemovalWillDropFKBackingIndexes, err := og.columnRemovalWillDropFKBackingIndexes(ctx, tx, tableName, columnName)
1672-
if err != nil {
1673-
return nil, err
1674-
}
16751679
columnIsInAddingOrDroppingIndex, err := og.columnIsInAddingOrDroppingIndex(ctx, tx, tableName, columnName)
16761680
if err != nil {
16771681
return nil, err
@@ -1690,7 +1694,7 @@ func (og *operationGenerator) dropColumn(ctx context.Context, tx pgx.Tx) (*opStm
16901694
{code: pgcode.ObjectNotInPrerequisiteState, condition: columnIsInAddingOrDroppingIndex},
16911695
{code: pgcode.UndefinedColumn, condition: !columnExists},
16921696
{code: pgcode.InvalidColumnReference, condition: colIsPrimaryKey || colIsRefByComputed},
1693-
{code: pgcode.DependentObjectsStillExist, condition: columnIsDependedOn || columnRemovalWillDropFKBackingIndexes},
1697+
{code: pgcode.DependentObjectsStillExist, condition: columnIsDependedOn},
16941698
{code: pgcode.FeatureNotSupported, condition: hasAlterPKSchemaChange && !og.useDeclarativeSchemaChanger},
16951699
})
16961700
stmt.potentialExecErrors.addAll(codesWithConditions{
@@ -1700,11 +1704,27 @@ func (og *operationGenerator) dropColumn(ctx context.Context, tx pgx.Tx) (*opStm
17001704
// It is possible the column we are dropping is in the new primary key,
17011705
// so a potential error is an invalid reference in this case.
17021706
{code: pgcode.InvalidColumnReference, condition: og.useDeclarativeSchemaChanger && hasAlterPKSchemaChange},
1707+
// It is possible that we cannot drop column because
1708+
// it is depended on by a foreign key.
1709+
{code: pgcode.DependentObjectsStillExist, condition: tableHasFK},
17031710
})
17041711
stmt.sql = fmt.Sprintf(`ALTER TABLE %s DROP COLUMN %s`, tableName.String(), columnName.String())
17051712
return stmt, nil
17061713
}
17071714

1715+
// tableHasFK checks if a table participates in any foreign key constraints.
1716+
func (og *operationGenerator) tableHasFK(
1717+
ctx context.Context, tx pgx.Tx, tableName *tree.TableName,
1718+
) (bool, error) {
1719+
return og.scanBool(ctx, tx, `
1720+
SELECT EXISTS (
1721+
SELECT 1
1722+
FROM pg_constraint
1723+
WHERE contype = 'f'
1724+
AND (conrelid = $1::REGCLASS OR confrelid = $1::REGCLASS)
1725+
)`, tableName.String())
1726+
}
1727+
17081728
func (og *operationGenerator) dropColumnDefault(ctx context.Context, tx pgx.Tx) (*opStmt, error) {
17091729
tableName, err := og.randTable(ctx, tx, og.pctExisting(true), "")
17101730
if err != nil {

0 commit comments

Comments
 (0)