From f88f3b19cfb2070c9e6f56d2f034601669529b95 Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Mon, 6 Oct 2025 19:07:52 -0400 Subject: [PATCH] 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. --- pkg/sql/opt/optbuilder/mutation_builder.go | 25 ++++- pkg/sql/opt/optbuilder/testdata/trigger | 106 +++++++++++++++++++++ pkg/sql/opt/optbuilder/trigger.go | 2 +- 3 files changed, 128 insertions(+), 5 deletions(-) diff --git a/pkg/sql/opt/optbuilder/mutation_builder.go b/pkg/sql/opt/optbuilder/mutation_builder.go index efc95b20f056..c79a88d418dc 100644 --- a/pkg/sql/opt/optbuilder/mutation_builder.go +++ b/pkg/sql/opt/optbuilder/mutation_builder.go @@ -795,7 +795,22 @@ func (mb *mutationBuilder) addSynthesizedDefaultCols( } // addSynthesizedComputedCols is a helper method for addSynthesizedColsForInsert -// and addSynthesizedColsForUpdate that scans the list of table columns, looking +// and addSynthesizedColsForUpdate that projects computed column expressions and +// adds the corresponding target table columns to targetColList and +// targetColSet. See projectSynthesizedComputedCols for more details. +// +// NOTE: colIDs is updated with the column IDs of any synthesized columns which +// are added to mb.outScope. If restrict is true, only columns that depend on +// columns that were already in the list (plus all write-only columns) are +// updated. +func (mb *mutationBuilder) addSynthesizedComputedCols(colIDs opt.OptionalColList, restrict bool) { + mb.targetColSet = mb.projectSynthesizedComputedCols(colIDs, restrict) + for col, ok := mb.targetColSet.Next(0); ok; col, ok = mb.targetColSet.Next(col + 1) { + mb.targetColList = append(mb.targetColList, col) + } +} + +// projectSynthesizedComputedCols scans the list of table columns, looking // for any that are computed and do not yet have values provided by the input // expression. New columns are synthesized for any missing columns using the // computed column expression. @@ -804,7 +819,9 @@ func (mb *mutationBuilder) addSynthesizedDefaultCols( // are added to mb.outScope. If restrict is true, only columns that depend on // columns that were already in the list (plus all write-only columns) are // updated. -func (mb *mutationBuilder) addSynthesizedComputedCols(colIDs opt.OptionalColList, restrict bool) { +func (mb *mutationBuilder) projectSynthesizedComputedCols( + colIDs opt.OptionalColList, restrict bool, +) (targetColSet opt.ColSet) { // We will construct a new Project operator that will contain the newly // synthesized column(s). pb := makeProjectionBuilder(mb.b, mb.outScope) @@ -876,11 +893,11 @@ func (mb *mutationBuilder) addSynthesizedComputedCols(colIDs opt.OptionalColList } // Add corresponding target column. - mb.targetColList = append(mb.targetColList, tabColID) - mb.targetColSet.Add(tabColID) + targetColSet.Add(tabColID) } mb.outScope = pb.Finish() + return targetColSet } // maybeAddRegionColLookup adds a lookup join to the target table of a foreign diff --git a/pkg/sql/opt/optbuilder/testdata/trigger b/pkg/sql/opt/optbuilder/testdata/trigger index 26945266e03d..f8b377808bbb 100644 --- a/pkg/sql/opt/optbuilder/testdata/trigger +++ b/pkg/sql/opt/optbuilder/testdata/trigger @@ -1253,3 +1253,109 @@ insert t133329 └── filters ├── a = t133329.a └── k != t133329.k + +# Regression test for #154672. Do not set target columns when re-projecting +# computed columns after a BEFORE trigger. +exec-ddl +CREATE TABLE t154672 ( + k INT PRIMARY KEY, + a INT, + b INT AS (a + 1) STORED +) +---- + +exec-ddl +CREATE FUNCTION f154672() RETURNS TRIGGER LANGUAGE PLpgSQL AS $$ + BEGIN + RETURN COALESCE(NEW, OLD); + END +$$ +---- + +exec-ddl +CREATE TRIGGER tr154672 BEFORE INSERT OR UPDATE OR DELETE ON t154672 FOR EACH ROW EXECUTE FUNCTION f154672() +---- + +norm +INSERT INTO t154672 (k, a) VALUES (1, 2) ON CONFLICT (k) DO UPDATE SET a = 3 +---- +upsert t154672 + ├── arbiter indexes: t154672_pkey + ├── columns: + ├── canary column: k:9 + ├── fetch columns: k:9 a:10 b:11 + ├── insert-mapping: + │ ├── k_new:29 => k:1 + │ ├── a_new:30 => a:2 + │ └── b_comp:31 => b:3 + ├── update-mapping: + │ ├── upsert_k:40 => k:1 + │ ├── upsert_a:41 => a:2 + │ └── upsert_b:42 => b:3 + ├── before-triggers + │ └── tr154672 + └── project + ├── 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 + ├── project + │ ├── columns: k_new:37 a_new:38 k:9 a:10 b:11 k_new:29 a_new:30 b_comp:31 + │ ├── barrier + │ │ ├── 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 + │ │ └── select + │ │ ├── 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 + │ │ ├── project + │ │ │ ├── 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 + │ │ │ ├── barrier + │ │ │ │ ├── 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 + │ │ │ │ └── project + │ │ │ │ ├── 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 + │ │ │ │ ├── barrier + │ │ │ │ │ ├── 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 + │ │ │ │ │ └── select + │ │ │ │ │ ├── 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 + │ │ │ │ │ ├── project + │ │ │ │ │ │ ├── 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 + │ │ │ │ │ │ ├── barrier + │ │ │ │ │ │ │ ├── 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 + │ │ │ │ │ │ │ └── project + │ │ │ │ │ │ │ ├── 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 + │ │ │ │ │ │ │ ├── left-join (cross) + │ │ │ │ │ │ │ │ ├── columns: column1:6!null column2:7!null b_comp:8!null k:9 a:10 b:11 crdb_internal_mvcc_timestamp:12 tableoid:13 + │ │ │ │ │ │ │ │ ├── values + │ │ │ │ │ │ │ │ │ ├── columns: column1:6!null column2:7!null b_comp:8!null + │ │ │ │ │ │ │ │ │ └── (1, 2, 3) + │ │ │ │ │ │ │ │ ├── select + │ │ │ │ │ │ │ │ │ ├── columns: k:9!null a:10 b:11 crdb_internal_mvcc_timestamp:12 tableoid:13 + │ │ │ │ │ │ │ │ │ ├── scan t154672 + │ │ │ │ │ │ │ │ │ │ ├── columns: k:9!null a:10 b:11 crdb_internal_mvcc_timestamp:12 tableoid:13 + │ │ │ │ │ │ │ │ │ │ ├── computed column expressions + │ │ │ │ │ │ │ │ │ │ │ └── b:11 + │ │ │ │ │ │ │ │ │ │ │ └── a:10 + 1 + │ │ │ │ │ │ │ │ │ │ └── flags: avoid-full-scan disabled not visible index feature + │ │ │ │ │ │ │ │ │ └── filters + │ │ │ │ │ │ │ │ │ └── k:9 = 1 + │ │ │ │ │ │ │ │ └── filters (true) + │ │ │ │ │ │ │ └── projections + │ │ │ │ │ │ │ └── ((column1:6, column2:7, CAST(NULL AS INT8)) AS k, a, b) [as=new:14] + │ │ │ │ │ │ └── projections + │ │ │ │ │ │ └── f154672(new:14, NULL, 'tr154672', 'BEFORE', 'ROW', 'INSERT', 61, 't154672', 't154672', 'public', 0, ARRAY[]) [as=f154672:28] + │ │ │ │ │ └── filters + │ │ │ │ │ └── f154672:28 IS DISTINCT FROM NULL + │ │ │ │ └── projections + │ │ │ │ ├── ((k:9, 3, CAST(NULL AS INT8)) AS k, a, b) [as=new:35] + │ │ │ │ ├── ((k:9, a:10, CAST(NULL AS INT8)) AS k, a, b) [as=old:34] + │ │ │ │ ├── 4 [as=b_comp:33] + │ │ │ │ ├── 3 [as=a_new:32] + │ │ │ │ ├── a:10 + 1 [as=b_comp:31] + │ │ │ │ ├── (f154672:28).k [as=k_new:29] + │ │ │ │ └── (f154672:28).a [as=a_new:30] + │ │ │ └── projections + │ │ │ └── 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] + │ │ └── filters + │ │ └── f154672:36 IS DISTINCT FROM NULL + │ └── projections + │ ├── (f154672:36).k [as=k_new:37] + │ └── (f154672:36).a [as=a_new:38] + └── projections + ├── CASE WHEN k:9 IS NULL THEN k_new:29 ELSE k_new:37 END [as=upsert_k:40] + ├── CASE WHEN k:9 IS NULL THEN a_new:30 ELSE a_new:38 END [as=upsert_a:41] + └── CASE WHEN k:9 IS NULL THEN b_comp:31 ELSE a_new:38 + 1 END [as=upsert_b:42] diff --git a/pkg/sql/opt/optbuilder/trigger.go b/pkg/sql/opt/optbuilder/trigger.go index 61aeef1e078b..85b7345f7efe 100644 --- a/pkg/sql/opt/optbuilder/trigger.go +++ b/pkg/sql/opt/optbuilder/trigger.go @@ -373,7 +373,7 @@ func (mb *mutationBuilder) recomputeComputedColsForTrigger(eventType tree.Trigge colIDs[i] = 0 } } - mb.addSynthesizedComputedCols(colIDs, false /* restrict */) + _ = mb.projectSynthesizedComputedCols(colIDs, false /* restrict */) } // ============================================================================