cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
29.89k stars 3.77k forks source link

sql, kv: support explicit unique checks under non-serializable isolations for INSERT & UPDATE #110873

Open michae2 opened 12 months ago

michae2 commented 12 months ago

In CockroachDB, most unique constraints are enforced by an accompanying unique index. The uniqueness check which maintains the constraint is simply the use of ConditionalPut instead of Put when writing a row to that index.

However, for unique constraints without an accompanying unique index, the uniqueness check is an explicit search in the target table for any potential conflicting rows. Under read committed isolation this search must use predicate locking, which is not yet implemented.

Consequently, any INSERT, UPDATE, or UPSERT that needs to perform an explicit unique check is not yet support under read committed isolation.

This affects inserts, updates, or upserts into:

The last case is the most common and the most complex, so here are some examples with and without explicit unique checks:

-- This table requires explicit unique checks during inserts and updates
-- because neither the primary key nor the unique constraint include region.
CREATE TABLE ab (
  a INT NOT NULL,
  b INT,
  PRIMARY KEY (a),
  UNIQUE (b)
) LOCALITY REGIONAL BY ROW;

-- Hence this insert is not yet supported under read committed isolation.
INSERT INTO ab VALUES (1, 2);

-- This table does not require an explicit unique check because both the
-- primary key and the unique constraint start with the region.
CREATE TABLE cd (
  c INT NOT NULL,
  d INT,
  region crdb_internal_region NOT NULL,
  PRIMARY KEY (region, c),
  UNIQUE (region, d)
) LOCALITY REGIONAL BY ROW AS region;

-- Hence this insert is supported under read committed isolation.
INSERT INTO cd VALUES (3, 4, 'us-east1');

-- This table does not require an explicit unique check because the region can
-- be computed from the columns in both the primary key and the unique
-- constraint.
CREATE TABLE ef (
  e INT NOT NULL,
  f INT,
  region crdb_internal_region NOT NULL AS (
    CASE WHEN e % 2 = 0 THEN 'us-east1' ELSE 'us-west1' END
  ) STORED,
  PRIMARY KEY (e),
  UNIQUE (f, e)
) LOCALITY REGIONAL BY ROW AS region;

-- Hence this insert is supported under read committed isolation.
INSERT INTO ef VALUES (5, 6);

-- This table requires an explicit unique check because though the region can
-- be computed from the primary key, it cannot be computed from the column in
-- the unique constraint, and the unique constraint does not start with the
-- region.
CREATE TABLE gh (
  g INT NOT NULL,
  h INT,
  region crdb_internal_region NOT NULL AS (
    CASE WHEN g % 2 = 0 THEN 'us-east1' ELSE 'us-west1' END
  ) STORED,
  PRIMARY KEY (g),
  UNIQUE (h)  
) LOCALITY REGIONAL BY ROW AS region;

-- Hence this insert is not yet supported under read committed isolation.
INSERT INTO gh VALUES (7, 8);

-- This table does not require an explicit unique check because even though
-- the primary key does not include region, we have special behavior to elide the
-- unique check when using gen_random_uuid().
CREATE TABLE ij (
  i UUID NOT NULL DEFAULT gen_random_uuid(),
  j INT,
  PRIMARY KEY (i)
) LOCALITY REGIONAL BY ROW;

-- Hence these inserts are supported under read committed isolation.
INSERT INTO ij (j) VALUES (9);
INSERT INTO ij VALUES (gen_random_uuid(), 10);

-- But these inserts are not yet supported under read committed isolation.
INSERT INTO ij VALUES ('1a29408d-7530-44ad-aa12-917a2ae1c96d', 11);
SET CLUSTER SETTING sql.optimizer.uniqueness_checks_for_gen_random_uuid.enabled = true;
INSERT INTO ij (j) VALUES (12);

Jira issue: CRDB-31651

Epic CRDB-38938

michae2 commented 10 months ago

Added docs-known-limitation because we do not support certain RBR tables under RC in 23.2, due to lack of support for explicit unique checks.

michae2 commented 9 months ago

Added this to KV because it depends on predicate locking.

michae2 commented 9 months ago

Also added to Queries because there's some optbuilder work needed to use predicate locking once available.

michae2 commented 4 months ago

[read committed weekly meeting] We think we could implement this without predicate locking by using CPut to write a tombstone in regions that don't contain the row.

michae2 commented 3 months ago

Here's a demonstration of the problem. With this diff applied:

diff --git a/pkg/sql/opt/exec/execbuilder/relational.go b/pkg/sql/opt/exec/execbuilder/relational.go
index e6ecfde3c12..01a6750ef2d 100644
--- a/pkg/sql/opt/exec/execbuilder/relational.go
+++ b/pkg/sql/opt/exec/execbuilder/relational.go
@@ -3105,11 +3105,13 @@ func (b *Builder) buildLocking(toLock opt.TableID, locking opt.Locking) (opt.Loc
                                "cannot execute SELECT %s in a read-only transaction", locking.Strength.String(),
                        )
                }
-               if locking.Form == tree.LockPredicate {
-                       return opt.Locking{}, unimplemented.NewWithIssuef(
-                               110873, "explicit unique checks are not yet supported under read committed isolation",
-                       )
-               }
+               /*
+                       if locking.Form == tree.LockPredicate {
+                               return opt.Locking{}, unimplemented.NewWithIssuef(
+                                       110873, "explicit unique checks are not yet supported under read committed isolation",
+                               )
+                       }
+               */
                // Check if we can actually use shared locks here, or we need to use
                // non-locking reads instead.
                if locking.Strength == tree.ForShare || locking.Strength == tree.ForKeyShare {
diff --git a/pkg/sql/upsert.go b/pkg/sql/upsert.go
index 56d270b9ca5..3d03d6cd5eb 100644
--- a/pkg/sql/upsert.go
+++ b/pkg/sql/upsert.go
@@ -13,6 +13,7 @@ package sql
 import (
        "context"
        "sync"
+       "time"

        "github.com/cockroachdb/cockroach/pkg/sql/catalog"
        "github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
@@ -180,6 +181,10 @@ func (n *upsertNode) processSourceRow(params runParams, rowVals tree.Datums) err
                rowVals = rowVals[:ord]
        }

+       if n.run.tw.tableWriterBase.desc.GetName() == "xy" {
+               time.Sleep(5 * time.Second)
+       }
+
        // Process the row. This is also where the tableWriter will accumulate
        // the row for later.
        return n.run.tw.row(params.ctx, rowVals, pm, n.run.traceKV)

Start a multi-region demo cluster:

./cockroach demo --multitenant=false --nodes=6 --global

Create a regional by row table named "xy":

ALTER DATABASE defaultdb PRIMARY REGION "us-east1";
ALTER DATABASE defaultdb ADD REGION "us-west1";
CREATE TABLE xy (x INT PRIMARY KEY, y INT) LOCALITY REGIONAL BY ROW;

Connect to the cluster from two different regions. In both regions, issue simultaneous UPSERTs to table xy under read committed isolation:

-- from us-east1
SET default_transaction_isolation = 'READ COMMITTED';
UPSERT INTO xy VALUES (1, 1);

-- from us-west1
SET default_transaction_isolation = 'READ COMMITTED';
UPSERT INTO xy VALUES (1, 2);

Now there should be two rows with x=1:

demo@127.0.0.1:26257/defaultdb> SELECT * FROM xy;
  x | y
----+----
  1 | 2
  1 | 1
(2 rows)
mw5h commented 2 weeks ago

Breaking this into multiple parts for added visibility. This is about INSERT & UPDATE. I'll open a new issue to track UPSERT and ON CONFLICT, both of which make use of arbiter indexes.