From 99a11f7788aec3ed11dccc51753758a264a7ed9e Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Thu, 4 Mar 2021 17:18:25 -0800 Subject: [PATCH] opt: fix fetch scope in UPDATE..FROM statements Previously, the fetch scope incorrectly included columns in the FROM clause of an UPDATE..FROM statement. As a result, column names shared by the FROM clause and the mutating table lead to ambiguity when resolving partial index DEL column expressions. This commit ensures that the fetch scope does not include columns in the FROM clause. Fixes #61284 Release justification: This is a low-risk bug fix to existing functionality. Release note (bug fix): An UPDATE..FROM statement where the FROM clause contained column names that match table column names erred if the table had a partial index predicate referencing those columns. This bug, present since partial indexes were released in version 20.2.0, has been fixed. --- .../testdata/logic_test/partial_index | 14 +++++ pkg/sql/opt/optbuilder/mutation_builder.go | 14 +++-- .../opt/optbuilder/testdata/partial-indexes | 53 +++++++++++++++++++ 3 files changed, 77 insertions(+), 4 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/partial_index b/pkg/sql/logictest/testdata/logic_test/partial_index index 0f72d2d63d7b..234b0c5ebccf 100644 --- a/pkg/sql/logictest/testdata/logic_test/partial_index +++ b/pkg/sql/logictest/testdata/logic_test/partial_index @@ -1795,3 +1795,17 @@ CREATE TABLE t61414_c ( statement ok UPSERT INTO t61414_c (k, a, b, d) VALUES (1, 2, 3, 4) + +# Regression test for #61284. When building partial index DEL column +# expressions, there should not be ambiguous column errors if there exists +# columns in an UPDATE FROM clause that match column names in the partial index +# predicate. + +statement ok +CREATE TABLE t61284 ( + a INT, + INDEX (a) WHERE a > 0 +) + +statement ok +UPDATE t61284 SET a = v.a FROM (VALUES (1), (2)) AS v(a) WHERE t61284.a = v.a diff --git a/pkg/sql/opt/optbuilder/mutation_builder.go b/pkg/sql/opt/optbuilder/mutation_builder.go index 109e84646dc7..6424bed8c4f8 100644 --- a/pkg/sql/opt/optbuilder/mutation_builder.go +++ b/pkg/sql/opt/optbuilder/mutation_builder.go @@ -287,10 +287,9 @@ func (mb *mutationBuilder) buildInputForUpdate( noRowLocking, inScope, ) - mb.outScope = mb.fetchScope // Set list of columns that will be fetched by the input expression. - mb.setFetchColIDs(mb.outScope.cols) + mb.setFetchColIDs(mb.fetchScope.cols) // If there is a FROM clause present, we must join all the tables // together with the table being updated. @@ -299,18 +298,25 @@ func (mb *mutationBuilder) buildInputForUpdate( fromScope := mb.b.buildFromTables(from, noRowLocking, inScope) // Check that the same table name is not used multiple times. - mb.b.validateJoinTableNames(mb.outScope, fromScope) + mb.b.validateJoinTableNames(mb.fetchScope, fromScope) // The FROM table columns can be accessed by the RETURNING clause of the // query and so we have to make them accessible. mb.extraAccessibleCols = fromScope.cols // Add the columns in the FROM scope. + // We create a new scope so that fetchScope is not modified. It will be + // used later to build partial index predicate expressions, and we do + // not want ambiguities with column names in the FROM clause. + mb.outScope = mb.fetchScope.replace() + mb.outScope.appendColumnsFromScope(mb.fetchScope) mb.outScope.appendColumnsFromScope(fromScope) - left := mb.outScope.expr.(memo.RelExpr) + left := mb.fetchScope.expr.(memo.RelExpr) right := fromScope.expr.(memo.RelExpr) mb.outScope.expr = mb.b.factory.ConstructInnerJoin(left, right, memo.TrueFilter, memo.EmptyJoinPrivate) + } else { + mb.outScope = mb.fetchScope } // WHERE diff --git a/pkg/sql/opt/optbuilder/testdata/partial-indexes b/pkg/sql/opt/optbuilder/testdata/partial-indexes index ea840602a2fd..fde352cd0d56 100644 --- a/pkg/sql/opt/optbuilder/testdata/partial-indexes +++ b/pkg/sql/opt/optbuilder/testdata/partial-indexes @@ -339,6 +339,59 @@ update partial_indexes ├── c:7 = 'delete-only' [as=partial_index_put3:13] └── c:7 = 'write-only' [as=partial_index_put4:14] +build +UPDATE partial_indexes SET a = v.a FROM (VALUES (1), (2)) AS v(a) WHERE partial_indexes.a = v.a +---- +update partial_indexes + ├── columns: + ├── fetch columns: a:5 b:6 c:7 + ├── update-mapping: + │ └── column1:9 => a:1 + ├── partial index put columns: partial_index_put1:10 partial_index_put2:11 partial_index_put3:13 partial_index_put4:14 + ├── partial index del columns: partial_index_put1:10 partial_index_del2:12 partial_index_put3:13 partial_index_put4:14 + └── project + ├── columns: partial_index_put1:10 partial_index_put2:11 partial_index_del2:12 partial_index_put3:13 partial_index_put4:14 a:5!null b:6 c:7 crdb_internal_mvcc_timestamp:8 column1:9!null + ├── distinct-on + │ ├── columns: a:5!null b:6 c:7 crdb_internal_mvcc_timestamp:8 column1:9!null + │ ├── grouping columns: a:5!null + │ ├── select + │ │ ├── columns: a:5!null b:6 c:7 crdb_internal_mvcc_timestamp:8 column1:9!null + │ │ ├── inner-join (cross) + │ │ │ ├── columns: a:5!null b:6 c:7 crdb_internal_mvcc_timestamp:8 column1:9!null + │ │ │ ├── scan partial_indexes + │ │ │ │ ├── columns: a:5!null b:6 c:7 crdb_internal_mvcc_timestamp:8 + │ │ │ │ └── partial index predicates + │ │ │ │ ├── secondary: filters + │ │ │ │ │ └── c:7 = 'foo' + │ │ │ │ ├── secondary: filters + │ │ │ │ │ └── (a:5 > b:6) AND (c:7 = 'bar') + │ │ │ │ ├── b: filters + │ │ │ │ │ └── c:7 = 'delete-only' + │ │ │ │ └── b: filters + │ │ │ │ └── c:7 = 'write-only' + │ │ │ ├── values + │ │ │ │ ├── columns: column1:9!null + │ │ │ │ ├── (1,) + │ │ │ │ └── (2,) + │ │ │ └── filters (true) + │ │ └── filters + │ │ └── a:5 = column1:9 + │ └── aggregations + │ ├── first-agg [as=b:6] + │ │ └── b:6 + │ ├── first-agg [as=c:7] + │ │ └── c:7 + │ ├── first-agg [as=crdb_internal_mvcc_timestamp:8] + │ │ └── crdb_internal_mvcc_timestamp:8 + │ └── first-agg [as=column1:9] + │ └── column1:9 + └── projections + ├── c:7 = 'foo' [as=partial_index_put1:10] + ├── (column1:9 > b:6) AND (c:7 = 'bar') [as=partial_index_put2:11] + ├── (a:5 > b:6) AND (c:7 = 'bar') [as=partial_index_del2:12] + ├── c:7 = 'delete-only' [as=partial_index_put3:13] + └── c:7 = 'write-only' [as=partial_index_put4:14] + # Do not error with "column reference is ambiguous" when table column names # match synthesized column names. build