Skip to content

Commit 6cc7569

Browse files
authored
Merge pull request #159252 from spilchen/backport24.3-159188
release-24.3: workload: remove FK dependency check in schemachanger drop column
2 parents cedc567 + 5f76a53 commit 6cc7569

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)