Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opt: fix fetch scope in UPDATE..FROM statements #61522

Merged
merged 1 commit into from
Mar 5, 2021
+77 −4
Merged

opt: fix fetch scope in UPDATE..FROM statements #61522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/partial_index
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
14 changes: 10 additions & 4 deletions pkg/sql/opt/optbuilder/mutation_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
53 changes: 53 additions & 0 deletions pkg/sql/opt/optbuilder/testdata/partial-indexes
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: <none>
├── 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
Expand Down