Merge cockroachdb#132768

132768: sql: arbiter index support under read committed isolation r=mw5h a=mw5h

Previously, a read committed transaction could not perform UPSERT or INSERT .. ON CONFLICT statements against tables with implicit partitioning (e.g. regional by row tables) because the SQL engine couldn't enforce uniqueness across all partitions. A prior patch added the ability to enforce uniqueness in limited circumstances by writing a tombstone to all partitions.

This patch extends the tombstone mechanism to arbiter indexes, used by UPSERT and INSERT .. ON CONFLICT. Rather than trying to prevent races with conflicting writes on the arbiter index, the SQL engine now writes tombstones to the arbiter index. The tombstones cause the KV layer to throw a "write too old" retryable error on conflict, causing the arbiter read to be retried and the conflicting value to be seen.

The majority of this patch is a test that forces a race condition between the arbiter read and a conflicting write for a variety of arbiter index reads. The idea is to:

* start the arbiter read.
* suspend the statement.
* perform a conflicting write.
* complete the UPSERT/INSERT .. ON CONFLICT
* verify the results are as expected.

Fixes: cockroachdb#129835
Release note (sql change): UPSERT and INSERT .. ON CONFLICT statements are now supported on Regional By Row tables under READ COMMITTED isolation.

Co-authored-by: Matt White <[email protected]>

pkg/ccl/logictestccl/testdata/logic_test/partitioning_implicit_read_committed

@@ -22,7 +22,7 @@ CREATE TABLE t_double (
 )
 
 # Test that we don't allow writes to tables with multiple partition columns.
-statement error pgcode 0A000 pq: unimplemented: explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 0A000 pq: unimplemented: unique without index constraint under non-serializable isolation levels
 INSERT INTO t_double VALUES (1, 'one', 'one', 10), (2, 'two', 'two', 20)
 
 statement ok
@@ -37,7 +37,7 @@ CREATE TABLE t_int (
 )
 
 # Test that we don't allow writes to tables with non-enum partition columns.
-statement error pgcode 0A000 pq: unimplemented: explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 0A000 pq: unimplemented: unique without index constraint under non-serializable isolation levels
 INSERT INTO t_int VALUES (1, 1, 10), (2, 2, 20)
 
 statement ok
@@ -141,9 +141,36 @@ scan t
  └── check constraint expressions
       └── a IN ('one', 'two', 'three', 'four', 'five')
 
+statement ok
+CREATE TABLE overwrite (
+  pk INT PRIMARY KEY,
+  a part_type,
+  b INT,
+  FAMILY (pk, a, b)
+) PARTITION ALL BY LIST(a) (
+  PARTITION one VALUES IN ('one'),
+  PARTITION two VALUES IN ('two'),
+  PARTITION three VALUES IN ('three'),
+  PARTITION four VALUES IN ('four'),
+  PARTITION five VALUES IN ('five')
+)
+
 statement ok
 SET tracing = kv
 
+# Test a blind write.
+statement ok
+UPSERT INTO overwrite VALUES (1, 'two', 3);
+
+query T
+SELECT message FROM [SHOW TRACE FOR SESSION] WHERE message LIKE 'CPut%'
+----
+CPut /Table/111/1/"@"/1/0 -> /TUPLE/3:3:Int/3
+CPut /Table/111/1/" "/1/0 -> nil (tombstone)
+CPut /Table/111/1/"\x80"/1/0 -> nil (tombstone)
+CPut /Table/111/1/"\xa0"/1/0 -> nil (tombstone)
+CPut /Table/111/1/"\xc0"/1/0 -> nil (tombstone)
+
 statement ok
 INSERT INTO t VALUES (1, 'two', 3, 4, 5)
 
@@ -162,9 +189,23 @@ UPDATE t SET pk = 1 WHERE c = 6;
 statement error pgcode 23505 pq: duplicate key value violates unique constraint "t_c_key"
 UPDATE t SET c = 4 WHERE pk = 2
 
+statement ok
+UPSERT INTO t VALUES (1, 'five', 3, 4, 15)
+
+statement ok
+INSERT INTO t VALUES (1, 'three', 3, 4, 15) ON CONFLICT DO NOTHING
+
+statement ok
+INSERT INTO t VALUES (1, 'one', 3, 4, 5) ON CONFLICT (pk) DO UPDATE SET d = t.d + 10
+
 query T
 SELECT message FROM [SHOW TRACE FOR SESSION] WHERE message LIKE 'CPut%'
 ----
+CPut /Table/111/1/"@"/1/0 -> /TUPLE/3:3:Int/3
+CPut /Table/111/1/" "/1/0 -> nil (tombstone)
+CPut /Table/111/1/"\x80"/1/0 -> nil (tombstone)
+CPut /Table/111/1/"\xa0"/1/0 -> nil (tombstone)
+CPut /Table/111/1/"\xc0"/1/0 -> nil (tombstone)
 CPut /Table/110/1/"@"/1/0 -> /TUPLE/3:3:Int/3/1:4:Int/4/1:5:Int/5
 CPut /Table/110/1/" "/1/0 -> nil (tombstone)
 CPut /Table/110/1/"\x80"/1/0 -> nil (tombstone)
@@ -211,3 +252,18 @@ CPut /Table/110/2/" "/4/0 -> nil (tombstone)
 CPut /Table/110/2/"@"/4/0 -> nil (tombstone)
 CPut /Table/110/2/"\x80"/4/0 -> nil (tombstone)
 CPut /Table/110/2/"\xc0"/4/0 -> nil (tombstone)
+CPut /Table/110/1/"\xc0"/1/0 -> /TUPLE/3:3:Int/3/1:4:Int/4/1:5:Int/15
+CPut /Table/110/1/" "/1/0 -> nil (tombstone)
+CPut /Table/110/1/"@"/1/0 -> nil (tombstone)
+CPut /Table/110/1/"\x80"/1/0 -> nil (tombstone)
+CPut /Table/110/1/"\xa0"/1/0 -> nil (tombstone)
+CPut /Table/110/2/" "/4/0 -> nil (tombstone)
+CPut /Table/110/2/"@"/4/0 -> nil (tombstone)
+CPut /Table/110/2/"\x80"/4/0 -> nil (tombstone)
+CPut /Table/110/2/"\xa0"/4/0 -> nil (tombstone)
+
+query ITIIIT
+SELECT * FROM t ORDER BY pk
+----
+1  five  3  4  25  NULL
+2  four  3  6  5   NULL

pkg/ccl/logictestccl/testdata/logic_test/regional_by_row_read_committed

@@ -112,22 +112,18 @@ INSERT INTO river VALUES ('ap-southeast-2', 'Skykomish', 'Snoqualmie')
 
 # Test conflicting INSERT ON CONFLICT DO NOTHING.
 
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement ok
 INSERT INTO university (name, mascot, postal_code)
 VALUES ('Thompson Rivers', 'wolves', 'V2C 0C8'), ('Evergreen State', 'geoducks', '98505')
 ON CONFLICT (mascot) DO NOTHING
 
-# TODO(mw5h): Temporary until ON CONFLICT works
-statement ok
-INSERT INTO university (name, mascot, postal_code) VALUES ('Evergreen State', 'geoducks', '98505')
-
 query TTT
 SELECT name, mascot, postal_code FROM university ORDER BY name
 ----
 Evergreen State  geoducks  98505
 Western Oregon   wolves    97361
 
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement ok
 INSERT INTO volcano VALUES
 ('Mount Adams', 'Fought over Loowit and was transformed by Saghalie.', 'POINT(-121.490895 46.202412)', 'ap-southeast-2'),
 ('Mount St. Helens', 'Fair maiden Loowit could not choose between Wyeast and Pahto and was transformed by Saghalie.', 'POINT(-122.1944 46.1912)', 'ap-southeast-2')
@@ -136,7 +132,8 @@ ON CONFLICT (origin) DO NOTHING
 query TTT
 SELECT name, origin, location FROM volcano ORDER BY name
 ----
-Mount Hood  Fought over Loowit and was transformed by Saghalie.  0101000020E6100000909E2287886C5EC08236397CD2AF4640
+Mount Hood        Fought over Loowit and was transformed by Saghalie.                                            0101000020E6100000909E2287886C5EC08236397CD2AF4640
+Mount St. Helens  Fair maiden Loowit could not choose between Wyeast and Pahto and was transformed by Saghalie.  0101000020E6100000EA95B20C718C5EC0637FD93D79184740
 
 statement ok
 INSERT INTO city VALUES ('Vancouver', 'The Big Smoke', 'BC'), ('Salem', 'Cherry City', 'OR')
@@ -160,10 +157,10 @@ us-east-1       Fraser     Strait of Georgia
 
 # Test conflicting UPSERT.
 
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 23505 pq: duplicate key value violates unique constraint "university_mascot_key"
 UPSERT INTO university (name, mascot, postal_code) VALUES ('Thompson Rivers', 'wolves', 'V2C 0C8')
 
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 23505 pq: duplicate key value violates unique constraint "volcano_origin_key"
 UPSERT INTO volcano VALUES
 ('Mount Adams', 'Fought over Loowit and was transformed by Saghalie.', 'POINT(-121.490895 46.202412)', 'ap-southeast-2')
 
@@ -178,7 +175,7 @@ UPSERT INTO river VALUES ('us-east-1', 'Fraser', 'Salish Sea')
 statement error pgcode 23505 pq: duplicate key value violates unique constraint "university_mascot_key"
 UPDATE university SET mascot = 'wolves' WHERE name = 'Evergreen State'
 
-statement ok
+statement error pgcode 23505 pq: duplicate key value violates unique constraint "volcano_origin_key"
 UPDATE volcano SET origin = 'Fought over Loowit and was transformed by Saghalie.' WHERE name = 'Mount St. Helens'
 
 statement error pgcode 23505 pq: duplicate key value violates unique constraint "city_pkey"\nDETAIL: Key \(name, state_or_province\)=\('Vancouver', 'BC'\) already exists\.
@@ -189,12 +186,12 @@ UPDATE river SET region = 'us-east-1', outlet = 'Salish Sea' WHERE name = 'Skyko
 
 # Test conflicting INSERT ON CONFLICT DO UPDATE.
 
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 23505 pq: duplicate key value violates unique constraint "university_pkey"
 INSERT INTO university (name, mascot, postal_code)
 VALUES ('Thompson Rivers', 'wolves', 'V2C 0C8'), ('Oregon Tech', 'owls', '97601')
 ON CONFLICT (mascot) DO UPDATE SET name = 'Evergreen State', mascot = 'banana slugs'
 
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 23505 pq: duplicate key value violates unique constraint "volcano_pkey"
 INSERT INTO volcano VALUES
 ('Mount Adams', 'Fought over Loowit and was transformed by Saghalie.', 'POINT(-121.490895 46.202412)', 'ap-southeast-2'),
 ('Mount Garibaldi', 'Lightning from thunderbird eyes struck the ground.', 'POINT(-123.004722 49.850278)', 'us-east-1')

pkg/ccl/logictestccl/testdata/logic_test/unique_read_committed

@@ -25,19 +25,19 @@ SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL READ COMMITTED
 
 # Test non-conflicting INSERT.
 
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 0A000 pq: unimplemented: unique without index constraint under non-serializable isolation levels
 INSERT INTO voyage VALUES ('caspian', 'hercules', 'argonauts', 'golden fleece')
 
 # Test the (quest, crew) uniqueness constraint.
 
 # The Argonauts searching for the golden fleece should fail the (quest, crew)
 # uniqueness check, even with a different sea.
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 0A000 pq: unimplemented: unique without index constraint under non-serializable isolation levels
 INSERT INTO voyage
 VALUES (DEFAULT, 'odysseus', 'nobody', 'penelope'), ('black', 'jason', 'argonauts', 'golden fleece')
 
 # Only Odysseus should be inserted.
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 0A000 pq: unimplemented: unique without index constraint under non-serializable isolation levels
 INSERT INTO voyage
 VALUES ('mediterranean', 'odysseus', 'nobody', 'penelope'), ('black', 'jason', 'argonauts', 'golden fleece')
 ON CONFLICT (quest, crew) DO NOTHING
@@ -49,11 +49,11 @@ SELECT * FROM voyage ORDER BY hero, crew, quest
 # Test the (hero) uniqueness constraint.
 
 # Hercules should fail the (hero) uniqueness check, even with a different sea.
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 0A000 pq: unimplemented: unique without index constraint under non-serializable isolation levels
 INSERT INTO voyage (hero, quest) VALUES ('perseus', 'medusa'), ('hercules', 'geryon')
 
 # Only Perseus should be inserted.
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 0A000 pq: unimplemented: unique without index constraint under non-serializable isolation levels
 INSERT INTO voyage (hero, quest) VALUES ('perseus', 'medusa'), ('hercules', 'geryon')
 ON CONFLICT (hero) DO NOTHING
 
@@ -63,27 +63,27 @@ SELECT * FROM voyage ORDER BY hero, crew, quest
 
 # Test conflicting UPSERT.
 
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 0A000 pq: unimplemented: unique without index constraint under non-serializable isolation levels
 UPSERT INTO voyage VALUES ('black', 'jason', 'argonauts', 'golden fleece')
 
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 0A000 pq: unimplemented: unique without index constraint under non-serializable isolation levels
 UPSERT INTO voyage (hero, quest) VALUES ('hercules', 'geryon')
 
 # Test conflicting UPDATE.
 
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 0A000 pq: unimplemented: unique without index constraint under non-serializable isolation levels
 UPDATE voyage SET crew = 'argonauts', quest = 'golden fleece' WHERE hero = 'perseus'
 
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 0A000 pq: unimplemented: unique without index constraint under non-serializable isolation levels
 UPDATE voyage SET hero = 'hercules' WHERE hero = 'odysseus'
 
 # Test conflicting INSERT ON CONFLICT DO UPDATE.
 
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 0A000 pq: unimplemented: unique without index constraint under non-serializable isolation levels
 INSERT INTO voyage VALUES ('black', 'jason', 'argonauts', 'golden fleece')
 ON CONFLICT (quest, crew) DO UPDATE SET quest = 'penelope', crew = 'nobody'
 
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 0A000 pq: unimplemented: unique without index constraint under non-serializable isolation levels
 INSERT INTO voyage (hero, quest) VALUES ('hercules', 'geryon')
 ON CONFLICT (hero) DO UPDATE SET hero = 'perseus'
 
@@ -100,5 +100,5 @@ CREATE TABLE titan (
   FAMILY (name, domain, children)
 )
 
-statement error pgcode 0A000 explicit unique checks are not yet supported under read committed isolation
+statement error pgcode 0A000 pq: unimplemented: unique without index constraint under non-serializable isolation levels
 INSERT INTO titan VALUES ('cronus', 'time', ARRAY['zeus', 'hera', 'hades', 'poseidon', 'demeter', 'hestia'])

pkg/sql/distsql_spec_exec_factory.go

@@ -1014,6 +1014,7 @@ func (e *distSQLSpecExecFactory) ConstructUpsert(
 	updateCols exec.TableColumnOrdinalSet,
 	returnCols exec.TableColumnOrdinalSet,
 	checks exec.CheckOrdinalSet,
+	uniqueWithTombstoneIndexes cat.IndexOrdinals,
 	autoCommit bool,
 ) (exec.Node, error) {
 	return nil, unimplemented.NewWithIssue(47473, "experimental opt-driven distsql planning: upsert")

pkg/sql/exec_util.go

@@ -1670,6 +1670,10 @@ type ExecutorTestingKnobs struct {
 	// due to some other condition. We can't set the probability to 0 since
 	// that would disable the feature entirely.
 	DisableProbabilisticSampling bool
+
+	// AfterArbiterRead, if set, will be called after each row read from an arbiter index
+	// for an UPSERT or INSERT.
+	AfterArbiterRead func()
 }
 
 // PGWireTestingKnobs contains knobs for the pgwire module.

pkg/sql/insert.go

@@ -19,6 +19,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/rowcontainer"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlerrors"
+	"github.com/cockroachdb/cockroach/pkg/util/buildutil"
 	"github.com/cockroachdb/errors"
 )
 
@@ -307,6 +308,13 @@ func (n *insertNode) BatchedNext(params runParams) (bool, error) {
 			break
 		}
 
+		if buildutil.CrdbTestBuild {
+			// This testing knob allows us to suspend execution to force a race condition.
+			if fn := params.ExecCfg().TestingKnobs.AfterArbiterRead; fn != nil {
+				fn()
+			}
+		}
+
 		// Process the insertion for the current source row, potentially
 		// accumulating the result row for later.
 		if err := n.run.processSourceRow(params, n.source.Values()); err != nil {

pkg/sql/insert_fast_path.go

@@ -23,6 +23,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/span"
+	"github.com/cockroachdb/cockroach/pkg/util/buildutil"
 	"github.com/cockroachdb/cockroach/pkg/util/intsets"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/errors"
@@ -463,6 +464,14 @@ func (n *insertFastPathNode) BatchedNext(params runParams) (bool, error) {
 				return false, err
 			}
 		}
+
+		if buildutil.CrdbTestBuild {
+			// This testing knob allows us to suspend execution to force a race condition.
+			if fn := params.ExecCfg().TestingKnobs.AfterArbiterRead; fn != nil {
+				fn()
+			}
+		}
+
 		// Process the insertion for the current source row, potentially
 		// accumulating the result row for later.
 		if err := n.run.processSourceRow(params, inputRow); err != nil {