cockroachdb / cockroachdb/cockroach

opt/exec: add implicit locking to upsert with distinct

Open
#147,806 2 comments 0 reactions 0 assignees View on GitHub
A-sql-optimizer C-performance E-quick-win O-25.2.1-scale-testing O-testcluster P-3 T-sql-queries
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

Some INSERT ... ON CONFLICT statements are planned with a distinct operator below the lookup join. These upserts don't pick up the usual implicit locking because [shouldApplyImplicitLockingToUpsertInput](https://github.com/cockroachdb/cockroach/blob/d18ede5b73c229d2eeeaf8a61f360460cb74fa8e/pkg/sql/opt/exec/execbuilder/mutation.go#L1277) does not check for distinct.

For example, compare the plans for these two statements:

```sql
CREATE TABLE ab (a INT PRIMARY KEY USING HASH, b INT);
EXPLAIN INSERT INTO ab VALUES (1, 2), (2, 3) ON CONFLICT (a) DO UPDATE SET b = excluded.b;
EXPLAIN INSERT INTO ab VALUES (1, 2), (2, 3) ON CONFLICT (crdb_internal_a_shard_16, a) DO UPDATE SET b = excluded.b;
```

In the first plan, the lookup join is locked. In the second plan it is unlocked:

```
demo@127.0.0.1:26257/demoapp/defaultdb> EXPLAIN INSERT INTO ab VALUES (1, 2), (2, 3) ON CONFLICT (a) DO UPDATE SET b = excluded.b;
info
----------------------------------------------------------------------------------------------
distribution: local
vectorized: true

• upsert
│ into: ab(crdb_internal_a_shard_16, a, b)
│ auto commit
│ arbiter constraints: ab_pkey

└── • render

└── • lookup join (left outer)
│ table: ab@ab_pkey
│ equality: (crdb_internal_a_shard_16_eq, column1) = (crdb_internal_a_shard_16, a)
│ equality cols are key
│ locking strength: for update

└── • render

└── • values
size: 2 columns, 2 rows
(20 rows)

Time: 5ms total (execution 4ms / network 1ms)

demo@127.0.0.1:26257/demoapp/defaultdb> EXPLAIN INSERT INTO ab VALUES (1, 2), (2, 3) ON CONFLICT (crdb_internal_a_shard_16, a) DO UPDATE SET b = excluded.b;
info
------------------------------------------------------------------------------------------------
distribution: local
vectorized: true

• upsert
│ into: ab(crdb_internal_a_shard_16, a, b)
│ auto commit
│ arbiter indexes: ab_pkey

└── • render

└── • lookup join (left outer)
│ estimated row count: 2
│ table: ab@ab_pkey
│ equality: (crdb_internal_a_shard_16_comp, column1) = (crdb_internal_a_shard_16, a)
│ equality cols are key

└── • distinct
│ estimated row count: 2
│ distinct on: crdb_internal_a_shard_16_comp, column1
│ nulls are distinct
│ error on duplicate

└── • render

└── • values
size: 2 columns, 2 rows
(26 rows)

Time: 3ms total (execution 2ms / network 1ms)
```

We should be able to change shouldApplyImplicitLockingToUpsertInput to handle the distinct.

Jira issue: CRDB-51257

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.