Skip to content

Commit 30a89a7

Browse files
mgartnermgartner
committed
opt: fix trigger+computed column internal error
This commit fixes a bug that was caused by incorrectly modifying the set and list of target columns when re-projecting computed columns after building a `BEFORE` trigger. Fixes #154672 Release note (bug fix): A bug has been fixed that caused internal errors for `INSERT .. ON CONFLICT .. DO UPDATE` statements when the target table had both a computed column and a `BEFORE` trigger. This bug has been present since triggers were introduced in v24.3.0.
1 parent 2a1771e commit 30a89a7

File tree

5 files changed

+124
-6
lines changed

5 files changed

+124
-6
lines changed

‎pkg/sql/opt/optbuilder/insert.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,8 @@ func (mb *mutationBuilder) addSynthesizedColsForInsert() {
749749
mb.addAssignmentCasts(mb.insertColIDs)
750750

751751
// Now add all computed columns.
752-
mb.addSynthesizedComputedCols(mb.insertColIDs, false /* restrict */)
752+
mb.addSynthesizedComputedCols(mb.insertColIDs,
753+
&mb.targetColList, &mb.targetColSet, false /* restrict */)
753754

754755
// Add assignment casts for computed column values.
755756
mb.addAssignmentCasts(mb.insertColIDs)

‎pkg/sql/opt/optbuilder/mutation_builder.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -800,11 +800,16 @@ func (mb *mutationBuilder) addSynthesizedDefaultCols(
800800
// expression. New columns are synthesized for any missing columns using the
801801
// computed column expression.
802802
//
803+
// The target table columns corresponding to each projected computed column are
804+
// added to targetColList and targetColSet, if they are not nil.
805+
//
803806
// NOTE: colIDs is updated with the column IDs of any synthesized columns which
804807
// are added to mb.outScope. If restrict is true, only columns that depend on
805808
// columns that were already in the list (plus all write-only columns) are
806809
// updated.
807-
func (mb *mutationBuilder) addSynthesizedComputedCols(colIDs opt.OptionalColList, restrict bool) {
810+
func (mb *mutationBuilder) addSynthesizedComputedCols(
811+
colIDs opt.OptionalColList, targetColList *opt.ColList, targetColSet *opt.ColSet, restrict bool,
812+
) {
808813
// We will construct a new Project operator that will contain the newly
809814
// synthesized column(s).
810815
pb := makeProjectionBuilder(mb.b, mb.outScope)
@@ -876,8 +881,12 @@ func (mb *mutationBuilder) addSynthesizedComputedCols(colIDs opt.OptionalColList
876881
}
877882

878883
// Add corresponding target column.
879-
mb.targetColList = append(mb.targetColList, tabColID)
880-
mb.targetColSet.Add(tabColID)
884+
if targetColList != nil {
885+
*targetColList = append(*targetColList, tabColID)
886+
}
887+
if targetColSet != nil {
888+
targetColSet.Add(tabColID)
889+
}
881890
}
882891

883892
mb.outScope = pb.Finish()

‎pkg/sql/opt/optbuilder/testdata/trigger‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,3 +1253,109 @@ insert t133329
12531253
└── filters
12541254
├── a = t133329.a
12551255
└── k != t133329.k
1256+
1257+
# Regression test for #154672. Do not set target columns when re-projecting
1258+
# computed columns after a BEFORE trigger.
1259+
exec-ddl
1260+
CREATE TABLE t154672 (
1261+
k INT PRIMARY KEY,
1262+
a INT,
1263+
b INT AS (a + 1) STORED
1264+
)
1265+
----
1266+
1267+
exec-ddl
1268+
CREATE FUNCTION f154672() RETURNS TRIGGER LANGUAGE PLpgSQL AS $$
1269+
BEGIN
1270+
RETURN COALESCE(NEW, OLD);
1271+
END
1272+
$$
1273+
----
1274+
1275+
exec-ddl
1276+
CREATE TRIGGER tr154672 BEFORE INSERT OR UPDATE OR DELETE ON t154672 FOR EACH ROW EXECUTE FUNCTION f154672()
1277+
----
1278+
1279+
norm
1280+
INSERT INTO t154672 (k, a) VALUES (1, 2) ON CONFLICT (k) DO UPDATE SET a = 3
1281+
----
1282+
upsert t154672
1283+
├── arbiter indexes: t154672_pkey
1284+
├── columns: <none>
1285+
├── canary column: k:9
1286+
├── fetch columns: k:9 a:10 b:11
1287+
├── insert-mapping:
1288+
│ ├── k_new:29 => k:1
1289+
│ ├── a_new:30 => a:2
1290+
│ └── b_comp:31 => b:3
1291+
├── update-mapping:
1292+
│ ├── upsert_k:40 => k:1
1293+
│ ├── upsert_a:41 => a:2
1294+
│ └── upsert_b:42 => b:3
1295+
├── before-triggers
1296+
│ └── tr154672
1297+
└── project
1298+
├── columns: upsert_k:40 upsert_a:41 upsert_b:42 k:9 a:10 b:11 k_new:29 a_new:30 b_comp:31
1299+
├── project
1300+
│ ├── columns: k_new:37 a_new:38 k:9 a:10 b:11 k_new:29 a_new:30 b_comp:31
1301+
│ ├── barrier
1302+
│ │ ├── columns: column1:6!null column2:7!null b_comp:8!null k:9 a:10 b:11 crdb_internal_mvcc_timestamp:12 tableoid:13 new:14 f154672:28!null k_new:29 a_new:30 b_comp:31 a_new:32!null b_comp:33!null old:34 new:35 f154672:36!null
1303+
│ │ └── select
1304+
│ │ ├── columns: column1:6!null column2:7!null b_comp:8!null k:9 a:10 b:11 crdb_internal_mvcc_timestamp:12 tableoid:13 new:14 f154672:28!null k_new:29 a_new:30 b_comp:31 a_new:32!null b_comp:33!null old:34 new:35 f154672:36!null
1305+
│ │ ├── project
1306+
│ │ │ ├── columns: f154672:36 column1:6!null column2:7!null b_comp:8!null k:9 a:10 b:11 crdb_internal_mvcc_timestamp:12 tableoid:13 new:14 f154672:28!null k_new:29 a_new:30 b_comp:31 a_new:32!null b_comp:33!null old:34 new:35
1307+
│ │ │ ├── barrier
1308+
│ │ │ │ ├── columns: column1:6!null column2:7!null b_comp:8!null k:9 a:10 b:11 crdb_internal_mvcc_timestamp:12 tableoid:13 new:14 f154672:28!null k_new:29 a_new:30 b_comp:31 a_new:32!null b_comp:33!null old:34 new:35
1309+
│ │ │ │ └── project
1310+
│ │ │ │ ├── columns: new:35 old:34 b_comp:33!null a_new:32!null b_comp:31 k_new:29 a_new:30 column1:6!null column2:7!null b_comp:8!null k:9 a:10 b:11 crdb_internal_mvcc_timestamp:12 tableoid:13 new:14 f154672:28!null
1311+
│ │ │ │ ├── barrier
1312+
│ │ │ │ │ ├── columns: column1:6!null column2:7!null b_comp:8!null k:9 a:10 b:11 crdb_internal_mvcc_timestamp:12 tableoid:13 new:14 f154672:28!null
1313+
│ │ │ │ │ └── select
1314+
│ │ │ │ │ ├── columns: column1:6!null column2:7!null b_comp:8!null k:9 a:10 b:11 crdb_internal_mvcc_timestamp:12 tableoid:13 new:14 f154672:28!null
1315+
│ │ │ │ │ ├── project
1316+
│ │ │ │ │ │ ├── columns: f154672:28 column1:6!null column2:7!null b_comp:8!null k:9 a:10 b:11 crdb_internal_mvcc_timestamp:12 tableoid:13 new:14
1317+
│ │ │ │ │ │ ├── barrier
1318+
│ │ │ │ │ │ │ ├── columns: column1:6!null column2:7!null b_comp:8!null k:9 a:10 b:11 crdb_internal_mvcc_timestamp:12 tableoid:13 new:14
1319+
│ │ │ │ │ │ │ └── project
1320+
│ │ │ │ │ │ │ ├── columns: new:14 column1:6!null column2:7!null b_comp:8!null k:9 a:10 b:11 crdb_internal_mvcc_timestamp:12 tableoid:13
1321+
│ │ │ │ │ │ │ ├── left-join (cross)
1322+
│ │ │ │ │ │ │ │ ├── columns: column1:6!null column2:7!null b_comp:8!null k:9 a:10 b:11 crdb_internal_mvcc_timestamp:12 tableoid:13
1323+
│ │ │ │ │ │ │ │ ├── values
1324+
│ │ │ │ │ │ │ │ │ ├── columns: column1:6!null column2:7!null b_comp:8!null
1325+
│ │ │ │ │ │ │ │ │ └── (1, 2, 3)
1326+
│ │ │ │ │ │ │ │ ├── select
1327+
│ │ │ │ │ │ │ │ │ ├── columns: k:9!null a:10 b:11 crdb_internal_mvcc_timestamp:12 tableoid:13
1328+
│ │ │ │ │ │ │ │ │ ├── scan t154672
1329+
│ │ │ │ │ │ │ │ │ │ ├── columns: k:9!null a:10 b:11 crdb_internal_mvcc_timestamp:12 tableoid:13
1330+
│ │ │ │ │ │ │ │ │ │ ├── computed column expressions
1331+
│ │ │ │ │ │ │ │ │ │ │ └── b:11
1332+
│ │ │ │ │ │ │ │ │ │ │ └── a:10 + 1
1333+
│ │ │ │ │ │ │ │ │ │ └── flags: avoid-full-scan disabled not visible index feature
1334+
│ │ │ │ │ │ │ │ │ └── filters
1335+
│ │ │ │ │ │ │ │ │ └── k:9 = 1
1336+
│ │ │ │ │ │ │ │ └── filters (true)
1337+
│ │ │ │ │ │ │ └── projections
1338+
│ │ │ │ │ │ │ └── ((column1:6, column2:7, CAST(NULL AS INT8)) AS k, a, b) [as=new:14]
1339+
│ │ │ │ │ │ └── projections
1340+
│ │ │ │ │ │ └── f154672(new:14, NULL, 'tr154672', 'BEFORE', 'ROW', 'INSERT', 61, 't154672', 't154672', 'public', 0, ARRAY[]) [as=f154672:28]
1341+
│ │ │ │ │ └── filters
1342+
│ │ │ │ │ └── f154672:28 IS DISTINCT FROM NULL
1343+
│ │ │ │ └── projections
1344+
│ │ │ │ ├── ((k:9, 3, CAST(NULL AS INT8)) AS k, a, b) [as=new:35]
1345+
│ │ │ │ ├── ((k:9, a:10, CAST(NULL AS INT8)) AS k, a, b) [as=old:34]
1346+
│ │ │ │ ├── 4 [as=b_comp:33]
1347+
│ │ │ │ ├── 3 [as=a_new:32]
1348+
│ │ │ │ ├── a:10 + 1 [as=b_comp:31]
1349+
│ │ │ │ ├── (f154672:28).k [as=k_new:29]
1350+
│ │ │ │ └── (f154672:28).a [as=a_new:30]
1351+
│ │ │ └── projections
1352+
│ │ │ └── CASE WHEN k:9 IS NOT NULL THEN f154672(new:35, old:34, 'tr154672', 'BEFORE', 'ROW', 'UPDATE', 61, 't154672', 't154672', 'public', 0, ARRAY[]) ELSE new:35 END [as=f154672:36]
1353+
│ │ └── filters
1354+
│ │ └── f154672:36 IS DISTINCT FROM NULL
1355+
│ └── projections
1356+
│ ├── (f154672:36).k [as=k_new:37]
1357+
│ └── (f154672:36).a [as=a_new:38]
1358+
└── projections
1359+
├── CASE WHEN k:9 IS NULL THEN k_new:29 ELSE k_new:37 END [as=upsert_k:40]
1360+
├── CASE WHEN k:9 IS NULL THEN a_new:30 ELSE a_new:38 END [as=upsert_a:41]
1361+
└── CASE WHEN k:9 IS NULL THEN b_comp:31 ELSE a_new:38 + 1 END [as=upsert_b:42]

‎pkg/sql/opt/optbuilder/trigger.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,8 @@ func (mb *mutationBuilder) recomputeComputedColsForTrigger(eventType tree.Trigge
373373
colIDs[i] = 0
374374
}
375375
}
376-
mb.addSynthesizedComputedCols(colIDs, false /* restrict */)
376+
mb.addSynthesizedComputedCols(colIDs,
377+
nil /* targetColList */, nil /* targetColSet */, false /* restrict */)
377378
}
378379

379380
// ============================================================================

‎pkg/sql/opt/optbuilder/update.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,8 @@ func (mb *mutationBuilder) addSynthesizedColsForUpdate() {
337337
mb.disambiguateColumns()
338338

339339
// Add all computed columns in case their values have changed.
340-
mb.addSynthesizedComputedCols(mb.updateColIDs, true /* restrict */)
340+
mb.addSynthesizedComputedCols(mb.updateColIDs,
341+
&mb.targetColList, &mb.targetColSet, true /* restrict */)
341342

342343
// Add assignment casts for computed column values.
343344
mb.addAssignmentCasts(mb.updateColIDs)

0 commit comments

Comments
 (0)