cockroachdb / cockroachdb/cockroach

sql: fast-path blind UPSERT does not have distinct check

Open
#146,759 2 comments 0 reactions 0 assignees View on GitHub
A-sql-optimizer branch-release-20.1 C-bug T-sql-queries
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

(Thanks to @fabiog1901 for finding this!)

https://github.com/cockroachdb/cockroach/pull/45372 added a distinct check to UPSERT and INSERT ON CONFLICT that prevents the same row from being modified twice by the statement. But it looks like the fast-path blind upsert does not have this same distinct check, leading to inconsistent behavior for UPSERT statements.

Here's a repro using current tip of master (59c35f8494b033c9827ff81ab36b1df1c6404846):

```sql
CREATE TABLE t (id INT PRIMARY KEY, v STRING);

-- INSERT ON CONFLICT has a distinct check
EXPLAIN INSERT INTO t VALUES (5, '1'), (5, '2') ON CONFLICT (id) DO UPDATE SET v = EXCLUDED.v;
-- • upsert
-- │ into: t(id, v)
-- │ auto commit
-- │ arbiter indexes: t_pkey
-- │
-- └── • lookup join (left outer)
-- │ estimated row count: 2
-- │ table: t@t_pkey
-- │ equality: (column1) = (id)
-- │ equality cols are key
-- │
-- └── • distinct
-- │ estimated row count: 2
-- │ distinct on: column1
-- │ nulls are distinct
-- │ error on duplicate
-- │
-- └── • values
-- size: 2 columns, 2 rows

INSERT INTO t VALUES (5, '1'), (5, '2') ON CONFLICT (id) DO UPDATE SET v = EXCLUDED.v;
-- ERROR: UPSERT or INSERT...ON CONFLICT command cannot affect row a second time
-- SQLSTATE: 21000

-- the fast-path blind upsert plan is missing this distinct check
EXPLAIN UPSERT INTO t VALUES (5, '1'), (5, '2');
-- • upsert
-- │ into: t(id, v)
-- │ auto commit
-- │
-- └── • values
-- size: 2 columns, 2 rows

UPSERT INTO t VALUES (5, '1'), (5, '2');
-- succeeds

SELECT * FROM t;
-- id | v
-- -----+----
-- 5 | 2

CREATE INDEX ON t (v);

-- after we create a secondary index, the fast-path no longer applies
EXPLAIN UPSERT INTO t VALUES (6, '1'), (6, '2');
-- • upsert
-- │ into: t(id, v)
-- │ auto commit
-- │ arbiter indexes: t_pkey
-- │
-- └── • lookup join (left outer)
-- │ estimated row count: 2
-- │ table: t@t_pkey
-- │ equality: (column1) = (id)
-- │ equality cols are key
-- │
-- └── • distinct
-- │ estimated row count: 2
-- │ distinct on: column1
-- │ nulls are distinct
-- │ error on duplicate
-- │
-- └── • values
-- size: 2 columns, 2 rows

-- and now this matches the behavior of INSERT ON CONFLICT
UPSERT INTO t VALUES (6, '1'), (6, '2');
-- ERROR: UPSERT or INSERT...ON CONFLICT command cannot affect row a second time
-- SQLSTATE: 21000
```

I don't think this fast-path UPSERT can cause corruption, as in #70731, so the only problem here is the confusing difference in behavior between plans for the same statement, depending on the indexes.

Jira issue: CRDB-50690

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.