From 442dc4389fedd9547f4c8d934c13ad77da572c9f Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Thu, 4 Mar 2021 09:34:17 -0800 Subject: [PATCH] sql: fix UPSERT with columns after partial index PUT and DEL columns This commit fixes a bug in UPSERT execution that was caused by the incorrect assumption that there are never columns following the partial index PUT and DEL columns in the mutation's input. I was unable to reproduce this bug for INSERTs or DELETEs. However, I updated the execution code for both out of caution. The logic for both no longer makes the same assumption that partial index PUT and DEL columns will be the last columns in the input. Fixes #61414 Release justification: This is a low-risk fix for a bug causing UPSERT statements to err when executed on tables with partial indexes. Release note (bug fix): A bug which caused UPSERT and INSERT..ON CONFLICT..DO UPDATE statements to fail on tables with both partial indexes and foreign key references has been fixed. This bug has been present since version 20.2.0. --- pkg/sql/delete.go | 5 +- pkg/sql/insert.go | 5 +- .../testdata/logic_test/partial_index | 46 +++++++++++++++++++ pkg/sql/update.go | 4 +- pkg/sql/upsert.go | 14 +++--- 5 files changed, 61 insertions(+), 13 deletions(-) diff --git a/pkg/sql/delete.go b/pkg/sql/delete.go index e51d96794d83..b09ea39ad89d 100644 --- a/pkg/sql/delete.go +++ b/pkg/sql/delete.go @@ -155,8 +155,9 @@ func (d *deleteNode) processSourceRow(params runParams, sourceVals tree.Datums) // satisfy the predicate and therefore do not exist in the partial index. // This set is passed as a argument to tableDeleter.row below. var pm row.PartialIndexUpdateHelper - if len(d.run.td.tableDesc().PartialIndexes()) > 0 { - partialIndexDelVals := sourceVals[d.run.partialIndexDelValsOffset:] + if n := len(d.run.td.tableDesc().PartialIndexes()); n > 0 { + offset := d.run.partialIndexDelValsOffset + partialIndexDelVals := sourceVals[offset : offset+n] err := pm.Init(tree.Datums{}, partialIndexDelVals, d.run.td.tableDesc()) if err != nil { diff --git a/pkg/sql/insert.go b/pkg/sql/insert.go index 8fd546b5e055..7fa7b5a3e096 100644 --- a/pkg/sql/insert.go +++ b/pkg/sql/insert.go @@ -130,8 +130,9 @@ func (r *insertRun) processSourceRow(params runParams, rowVals tree.Datums) erro // written to when they are partial indexes and the row does not satisfy the // predicate. This set is passed as a parameter to tableInserter.row below. var pm row.PartialIndexUpdateHelper - if len(r.ti.tableDesc().PartialIndexes()) > 0 { - partialIndexPutVals := rowVals[len(r.insertCols)+r.checkOrds.Len():] + if n := len(r.ti.tableDesc().PartialIndexes()); n > 0 { + offset := len(r.insertCols) + r.checkOrds.Len() + partialIndexPutVals := rowVals[offset : offset+n] err := pm.Init(partialIndexPutVals, tree.Datums{}, r.ti.tableDesc()) if err != nil { diff --git a/pkg/sql/logictest/testdata/logic_test/partial_index b/pkg/sql/logictest/testdata/logic_test/partial_index index 1d2e7c06f0d5..0f72d2d63d7b 100644 --- a/pkg/sql/logictest/testdata/logic_test/partial_index +++ b/pkg/sql/logictest/testdata/logic_test/partial_index @@ -1749,3 +1749,49 @@ CREATE TABLE t58390 ( statement ok ALTER TABLE t58390 ALTER PRIMARY KEY USING COLUMNS (b, a) + +# Regression tests for #61414. Upsert execution should not error if partial +# index PUT and DEL columns are not the last columns in the input of the +# mutation. + +statement ok +create table t61414_a ( + k INT PRIMARY KEY +) + +statement ok +create table t61414_b ( + k INT PRIMARY KEY, + a STRING, + b INT REFERENCES t61414_a(k), + UNIQUE INDEX (a, b), + INDEX (a) WHERE a = 'x' +) + +statement ok +INSERT INTO t61414_a VALUES (2) + +statement ok +INSERT INTO t61414_b (k, a, b) +VALUES (1, 'a', 2) +ON CONFLICT (a, b) DO UPDATE SET a = excluded.a +WHERE t61414_b.a = 'x' +RETURNING k + +statement ok +SET experimental_enable_unique_without_index_constraints = true + +statement ok +CREATE TABLE t61414_c ( + k INT PRIMARY KEY, + a INT, + b INT, + c INT, + d INT, + INDEX (b) WHERE b > 0, + UNIQUE WITHOUT INDEX (b) WHERE b > 0, + FAMILY (k, a, c) +) + +statement ok +UPSERT INTO t61414_c (k, a, b, d) VALUES (1, 2, 3, 4) diff --git a/pkg/sql/update.go b/pkg/sql/update.go index 48ea5cf4a3ae..262e78a90520 100644 --- a/pkg/sql/update.go +++ b/pkg/sql/update.go @@ -297,8 +297,8 @@ func (u *updateNode) processSourceRow(params runParams, sourceVals tree.Datums) // from. var pm row.PartialIndexUpdateHelper if n := len(u.run.tu.tableDesc().PartialIndexes()); n > 0 { - partialIndexValOffset := len(u.run.tu.ru.FetchCols) + len(u.run.tu.ru.UpdateCols) + u.run.checkOrds.Len() + u.run.numPassthrough - partialIndexVals := sourceVals[partialIndexValOffset:] + offset := len(u.run.tu.ru.FetchCols) + len(u.run.tu.ru.UpdateCols) + u.run.checkOrds.Len() + u.run.numPassthrough + partialIndexVals := sourceVals[offset:] partialIndexPutVals := partialIndexVals[:n] partialIndexDelVals := partialIndexVals[n : n*2] diff --git a/pkg/sql/upsert.go b/pkg/sql/upsert.go index 7ebef9e6b3f3..8cafaf2212cf 100644 --- a/pkg/sql/upsert.go +++ b/pkg/sql/upsert.go @@ -143,14 +143,14 @@ func (n *upsertNode) processSourceRow(params runParams, rowVals tree.Datums) err // Create a set of partial index IDs to not add or remove entries from. var pm row.PartialIndexUpdateHelper - if len(n.run.tw.tableDesc().PartialIndexes()) > 0 { - partialIndexValOffset := len(n.run.insertCols) + len(n.run.tw.fetchCols) + len(n.run.tw.updateCols) + n.run.checkOrds.Len() + if numPartialIndexes := len(n.run.tw.tableDesc().PartialIndexes()); numPartialIndexes > 0 { + offset := len(n.run.insertCols) + len(n.run.tw.fetchCols) + len(n.run.tw.updateCols) + n.run.checkOrds.Len() if n.run.tw.canaryOrdinal != -1 { - partialIndexValOffset++ + offset++ } - partialIndexVals := rowVals[partialIndexValOffset:] - partialIndexPutVals := partialIndexVals[:len(partialIndexVals)/2] - partialIndexDelVals := partialIndexVals[len(partialIndexVals)/2:] + partialIndexVals := rowVals[offset:] + partialIndexPutVals := partialIndexVals[:numPartialIndexes] + partialIndexDelVals := partialIndexVals[numPartialIndexes : numPartialIndexes*2] err := pm.Init(partialIndexPutVals, partialIndexDelVals, n.run.tw.tableDesc()) if err != nil { @@ -159,7 +159,7 @@ func (n *upsertNode) processSourceRow(params runParams, rowVals tree.Datums) err // Truncate rowVals so that it no longer includes partial index predicate // values. - rowVals = rowVals[:partialIndexValOffset] + rowVals = rowVals[:offset] } // Verify the CHECK constraints by inspecting boolean columns from the input that