Skip to content

Commit f88f3b1

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 f88f3b1

File tree

3 files changed

+128
-5
lines changed

3 files changed

+128
-5
lines changed

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

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -795,7 +795,22 @@ func (mb *mutationBuilder) addSynthesizedDefaultCols(
795795
}
796796

797797
// addSynthesizedComputedCols is a helper method for addSynthesizedColsForInsert
798-
// and addSynthesizedColsForUpdate that scans the list of table columns, looking
798+
// and addSynthesizedColsForUpdate that projects computed column expressions and
799+
// adds the corresponding target table columns to targetColList and
800+
// targetColSet. See projectSynthesizedComputedCols for more details.
801+
//
802+
// NOTE: colIDs is updated with the column IDs of any synthesized columns which
803+
// are added to mb.outScope. If restrict is true, only columns that depend on
804+
// columns that were already in the list (plus all write-only columns) are
805+
// updated.
806+
func (mb *mutationBuilder) addSynthesizedComputedCols(colIDs opt.OptionalColList, restrict bool) {
807+
mb.targetColSet = mb.projectSynthesizedComputedCols(colIDs, restrict)
808+
for col, ok := mb.targetColSet.Next(0); ok; col, ok = mb.targetColSet.Next(col + 1) {
809+
mb.targetColList = append(mb.targetColList, col)
810+
}
811+
}
812+
813+
// projectSynthesizedComputedCols scans the list of table columns, looking
799814
// for any that are computed and do not yet have values provided by the input
800815
// expression. New columns are synthesized for any missing columns using the
801816
// computed column expression.
@@ -804,7 +819,9 @@ func (mb *mutationBuilder) addSynthesizedDefaultCols(
804819
// are added to mb.outScope. If restrict is true, only columns that depend on
805820
// columns that were already in the list (plus all write-only columns) are
806821
// updated.
807-
func (mb *mutationBuilder) addSynthesizedComputedCols(colIDs opt.OptionalColList, restrict bool) {
822+
func (mb *mutationBuilder) projectSynthesizedComputedCols(
823+
colIDs opt.OptionalColList, restrict bool,
824+
) (targetColSet opt.ColSet) {
808825
// We will construct a new Project operator that will contain the newly
809826
// synthesized column(s).
810827
pb := makeProjectionBuilder(mb.b, mb.outScope)
@@ -876,11 +893,11 @@ func (mb *mutationBuilder) addSynthesizedComputedCols(colIDs opt.OptionalColList
876893
}
877894

878895
// Add corresponding target column.
879-
mb.targetColList = append(mb.targetColList, tabColID)
880-
mb.targetColSet.Add(tabColID)
896+
targetColSet.Add(tabColID)
881897
}
882898

883899
mb.outScope = pb.Finish()
900+
return targetColSet
884901
}
885902

886903
// maybeAddRegionColLookup adds a lookup join to the target table of a foreign

‎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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ func (mb *mutationBuilder) recomputeComputedColsForTrigger(eventType tree.Trigge
373373
colIDs[i] = 0
374374
}
375375
}
376-
mb.addSynthesizedComputedCols(colIDs, false /* restrict */)
376+
_ = mb.projectSynthesizedComputedCols(colIDs, false /* restrict */)
377377
}
378378

379379
// ============================================================================

0 commit comments

Comments
 (0)