Merge #111884 #111923

111884: cast: remove invalid casts for REFCURSOR r=DrewKimball a=DrewKimball

#### cast: remove invalid casts for REFCURSOR

The initial implementation for the `REFCURSOR` data type included
assignment casts from and explicit casts to every other type. In postgres,
`REFCURSOR` has only the following valid casts:
1. Explicit casts from each string type.
2. Assignment casts to each string type.

This patch aligns the casts for `REFCURSOR` with those of postgres, and
adds according tests.

Informs #111560

Release note: None

111923: kv: dequeue request from lock table wait-queues on scan error r=nvanbenschoten a=nvanbenschoten

Informs #111352.
Informs #111530.
Informs #111564.
Informs #111893.

In 8205b43, we added a case to the lock table where a request's initial scan could throw an error. This was not being handled properly if the request had already entered any other lock wait-queues. In these cases, the request's entries in those wait-queues would be abandoned and the locks would get stuck. This commit fixes that issue by dequeuing the request from the lock table when throwing an error from ScanAndEnqueue.

This was one of the causes of the recent kvnemesis instability, but we believe that there is at least one other issue that is still causing timeouts.

Release note: None

Co-authored-by: Drew Kimball <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>

pkg/kv/kvserver/concurrency/lock_table.go

@@ -3785,6 +3785,9 @@ func (t *lockTableImpl) ScanAndEnqueue(req Request, guard lockTableGuard) (lockT
 
 	err := g.resumeScan(true /* notify */)
 	if err != nil {
+		// We're not returning the guard on this error path, so we need to
+		// release the guard in case it has already entered any wait-queues.
+		t.Dequeue(g)
 		return nil, kvpb.NewError(err)
 	}
 	if g.notRemovableLock != nil {

pkg/kv/kvserver/concurrency/testdata/lock_table/shared_locks

@@ -1071,6 +1071,76 @@ num=2
     active: true req: 53, strength: Shared, txn: 00000000-0000-0000-0000-000000000003
    distinguished req: 50
 
+# ------------------------------------------------------------------------------
+# Sub-test for lock promotion. When an initial call to ScanAndEnqueue returns an
+# error, the request is dequeued from any other wait-queues that it may have
+# entered.
+# ------------------------------------------------------------------------------
+
+clear
+----
+num=0
+
+new-request r=req56 txn=txn1 ts=10 spans=exclusive@a
+----
+
+scan r=req56
+----
+start-waiting: false
+
+acquire r=req56 k=a durability=u strength=exclusive
+----
+num=1
+ lock: "a"
+  holder: txn: 00000000-0000-0000-0000-000000000001 epoch: 0, iso: Serializable, ts: 10.000000000,0, info: unrepl [(str: Exclusive seq: 0)]
+
+dequeue r=req56
+----
+num=1
+ lock: "a"
+  holder: txn: 00000000-0000-0000-0000-000000000001 epoch: 0, iso: Serializable, ts: 10.000000000,0, info: unrepl [(str: Exclusive seq: 0)]
+
+new-request r=req57 txn=txn2 ts=10 spans=shared@b
+----
+
+scan r=req57
+----
+start-waiting: false
+
+acquire r=req57 k=b durability=u strength=shared
+----
+num=2
+ lock: "a"
+  holder: txn: 00000000-0000-0000-0000-000000000001 epoch: 0, iso: Serializable, ts: 10.000000000,0, info: unrepl [(str: Exclusive seq: 0)]
+ lock: "b"
+  holder: txn: 00000000-0000-0000-0000-000000000002 epoch: 0, iso: Serializable, info: unrepl [(str: Shared seq: 0)]
+
+dequeue r=req57
+----
+num=2
+ lock: "a"
+  holder: txn: 00000000-0000-0000-0000-000000000001 epoch: 0, iso: Serializable, ts: 10.000000000,0, info: unrepl [(str: Exclusive seq: 0)]
+ lock: "b"
+  holder: txn: 00000000-0000-0000-0000-000000000002 epoch: 0, iso: Serializable, info: unrepl [(str: Shared seq: 0)]
+
+# Mark txn1 as aborted so that the next two requests immediately acquire claims
+# on key "a" when scanning.
+pushed-txn-updated txn=txn1 status=aborted
+----
+
+new-request r=req58 txn=txn2 ts=10 spans=exclusive@a,c
+----
+
+scan r=req58
+----
+lock promotion from Shared to Exclusive is not allowed
+
+print
+----
+num=1
+ lock: "b"
+  holder: txn: 00000000-0000-0000-0000-000000000002 epoch: 0, iso: Serializable, info: unrepl [(str: Shared seq: 0)]
+
 # TODO(arul): (non-exhaustive list) of shared lock state transitions that aren't
 # currently supported (and we need to add support for):
 #

pkg/sql/logictest/testdata/logic_test/refcursor

@@ -28,7 +28,7 @@ statement ok
 ALTER TABLE xy ADD COLUMN curs REFCURSOR;
 
 statement ok
-INSERT INTO xy VALUES (1, 2, 'foo');
+INSERT INTO xy VALUES (1, 2, 'foo'::REFCURSOR);
 
 query IIT
 SELECT * FROM xy;
@@ -38,14 +38,14 @@ SELECT * FROM xy;
 # Alter a column type to REFCURSOR.
 statement ok
 DROP TABLE IF EXISTS xy;
-CREATE TABLE xy (x INT, y INT);
+CREATE TABLE xy (x INT, y TEXT);
 SET enable_experimental_alter_column_type_general=true;
 
 statement ok
 ALTER TABLE xy ALTER COLUMN y TYPE REFCURSOR;
 
 statement ok
-INSERT INTO xy VALUES (1, 'bar');
+INSERT INTO xy VALUES (1, 'bar'::REFCURSOR);
 
 query IT
 SELECT * FROM xy;
@@ -60,10 +60,10 @@ INSERT INTO xy VALUES (3, 4);
 
 # Create a partial index that uses the REFCURSOR type.
 statement ok
-CREATE INDEX part ON xy (x) WHERE y::REFCURSOR < '3';
+CREATE INDEX part ON xy (x) WHERE y::TEXT::REFCURSOR < '3';
 
 query II
-SELECT * FROM xy@part WHERE y::REFCURSOR < '3';
+SELECT * FROM xy@part WHERE y::TEXT::REFCURSOR < '3';
 ----
 1  2
 
@@ -74,7 +74,7 @@ INSERT INTO xy VALUES (1, 2);
 
 # Add a check constraint that uses the REFCURSOR type.
 statement ok
-ALTER TABLE xy ADD CONSTRAINT bar CHECK (y::REFCURSOR < 'baz');
+ALTER TABLE xy ADD CONSTRAINT bar CHECK (y::TEXT::REFCURSOR < 'baz');
 
 query II
 SELECT * FROM xy;
@@ -88,7 +88,7 @@ statement ok
 CREATE TYPE typ AS (x INT, y REFCURSOR);
 
 query T
-SELECT (100, 'bar')::typ;
+SELECT (100, 'bar'::REFCURSOR)::typ;
 ----
 (100,bar)
 
@@ -97,7 +97,7 @@ subtest function
 # Function that returns REFCURSOR.
 statement ok
 CREATE FUNCTION f() RETURNS REFCURSOR AS $$
-  SELECT 'foo';
+  SELECT 'foo'::REFCURSOR;
 $$ LANGUAGE SQL;
 
 query T
@@ -111,7 +111,7 @@ DROP FUNCTION f;
 statement ok
 CREATE FUNCTION f() RETURNS REFCURSOR AS $$
   BEGIN
-    RETURN 'foo';
+    RETURN 'foo'::REFCURSOR;
   END
 $$ LANGUAGE PLpgSQL;
 
@@ -130,7 +130,7 @@ CREATE FUNCTION f(curs REFCURSOR) RETURNS INT AS $$
 $$ LANGUAGE SQL;
 
 query I
-SELECT f('foo');
+SELECT f('foo'::REFCURSOR);
 ----
 0
 
@@ -145,7 +145,7 @@ CREATE FUNCTION f(curs REFCURSOR) RETURNS INT AS $$
 $$ LANGUAGE PLpgSQL;
 
 query I
-SELECT f('foo');
+SELECT f('foo'::REFCURSOR);
 ----
 0
 
@@ -186,7 +186,7 @@ DROP FUNCTION f;
 # Function that returns a composite type with REFCURSOR component.
 statement ok
 CREATE FUNCTION f() RETURNS typ AS $$
-  SELECT (1, 'foo');
+  SELECT (1, 'foo'::REFCURSOR);
 $$ LANGUAGE SQL;
 
 query T
@@ -200,7 +200,7 @@ DROP FUNCTION f;
 statement ok
 CREATE FUNCTION f() RETURNS typ AS $$
   BEGIN
-    RETURN (1, 'foo');
+    RETURN (1, 'foo'::REFCURSOR);
   END
 $$ LANGUAGE PLpgSQL;
 
@@ -221,4 +221,188 @@ foo
 statement error pgcode 42601 syntax error
 SELECT 'foo'::REFCURSOR(2);
 
+# Testing casts.
+statement ok
+CREATE TABLE implicit_types (
+  a TEXT, b CHAR, c VARCHAR, d NAME, e INT, f FLOAT, g DECIMAL, h BOOL,
+  i INTERVAL, j DATE, k TIMESTAMP, l REFCURSOR
+);
+
+# Only the string types can be explicitly cast to REFCURSOR.
+subtest explicit_cast_to
+
+query T
+SELECT NULL::REFCURSOR;
+----
+NULL
+
+query T
+SELECT 'foo'::TEXT::REFCURSOR;
+----
+foo
+
+query T
+SELECT 'a'::CHAR::REFCURSOR;
+----
+a
+
+query T
+SELECT 'foo'::VARCHAR::REFCURSOR;
+----
+foo
+
+query T
+SELECT 'foo'::NAME::REFCURSOR;
+----
+foo
+
+statement error pgcode 42846 pq: invalid cast: int -> refcursor
+SELECT 1::INT::REFCURSOR;
+
+statement error pgcode 42846 pq: invalid cast: float -> refcursor
+SELECT 1.0::FLOAT::REFCURSOR;
+
+statement error pgcode 42846 pq: invalid cast: decimal -> refcursor
+SELECT 1.0::DECIMAL::REFCURSOR;
+
+statement error pgcode 42846 pq: invalid cast: bool -> refcursor
+SELECT False::REFCURSOR;
+
+statement error pgcode 42846 pq: invalid cast: interval -> refcursor
+SELECT '34h2s'::INTERVAL::REFCURSOR;
+
+statement error pgcode 42846 pq: invalid cast: date -> refcursor
+SELECT '2015-08-30'::DATE::REFCURSOR;
+
+statement error pgcode 42846 pq: invalid cast: timestamp -> refcursor
+SELECT '2015-08-30 03:34:45.34567'::TIMESTAMP::REFCURSOR;
+
+# Only the string types can be explicitly cast from REFCURSOR.
+subtest explicit_cast_from
+
+query T
+SELECT 'foo'::REFCURSOR::TEXT;
+----
+foo
+
+query T
+SELECT 'a'::REFCURSOR::CHAR;
+----
+a
+
+query T
+SELECT 'foo'::REFCURSOR::VARCHAR;
+----
+foo
+
+query T
+SELECT 'foo'::REFCURSOR::NAME;
+----
+foo
+
+statement error pgcode 42846 pq: invalid cast: refcursor -> int
+SELECT '1'::REFCURSOR::INT;
+
+statement error pgcode 42846 pq: invalid cast: refcursor -> float
+SELECT '1.0'::REFCURSOR::FLOAT;
+
+statement error pgcode 42846 pq: invalid cast: refcursor -> decimal
+SELECT '1.0'::REFCURSOR::DECIMAL;
+
+statement error pgcode 42846 pq: invalid cast: refcursor -> bool
+SELECT 'False'::REFCURSOR::BOOL;
+
+statement error pgcode 42846 pq: invalid cast: refcursor -> interval
+SELECT '34h2s'::REFCURSOR::INTERVAL;
+
+statement error pgcode 42846 pq: invalid cast: refcursor -> date
+SELECT '2015-08-30'::REFCURSOR::DATE;
+
+statement error pgcode 42846 pq: invalid cast: refcursor -> timestamp
+SELECT '2015-08-30 03:34:45.34567'::REFCURSOR::TIMESTAMP;
+
+# There are no implicit casts to REFCURSOR.
+subtest implicit_cast_to
+
+statement ok
+INSERT INTO implicit_types(l) VALUES (NULL);
+
+statement error pgcode 42804 pq: value type string doesn't match type refcursor of column \"l\"
+INSERT INTO implicit_types(l) VALUES ('foo'::TEXT);
+
+statement error pgcode 42804 pq: value type char doesn't match type refcursor of column \"l\"
+INSERT INTO implicit_types(l) VALUES ('a'::CHAR);
+
+statement error pgcode 42804 pq: value type varchar doesn't match type refcursor of column \"l\"
+INSERT INTO implicit_types(l) VALUES ('foo'::VARCHAR);
+
+statement error pgcode 42804 pq: value type name doesn't match type refcursor of column \"l\"
+INSERT INTO implicit_types(l) VALUES ('foo'::NAME);
+
+statement error pgcode 42804 pq: value type int doesn't match type refcursor of column \"l\"
+INSERT INTO implicit_types(l) VALUES (1::INT);
+
+statement error pgcode 42804 pq: value type float doesn't match type refcursor of column \"l\"
+INSERT INTO implicit_types(l) VALUES (1.0::FLOAT);
+
+statement error pgcode 42804 pq: value type decimal doesn't match type refcursor of column \"l\"
+INSERT INTO implicit_types(l) VALUES (1.0::DECIMAL);
+
+statement error pgcode 42804 pq: value type bool doesn't match type refcursor of column \"l\"
+INSERT INTO implicit_types(l) VALUES (False::BOOL);
+
+statement error pgcode 42804 pq: value type interval doesn't match type refcursor of column \"l\"
+INSERT INTO implicit_types(l) VALUES ('34h2s'::INTERVAL);
+
+statement error pgcode 42804 pq: value type date doesn't match type refcursor of column \"l\"
+INSERT INTO implicit_types(l) VALUES ('2015-08-30'::DATE);
+
+statement error pgcode 42804 pq: value type timestamp doesn't match type refcursor of column \"l\"
+INSERT INTO implicit_types(l) VALUES ('2015-08-30 03:34:45.34567'::TIMESTAMP);
+
+# Only the strings types can be implicitly casted from refcursor.
+subtest implicit_cast_from
+
+statement ok
+INSERT INTO implicit_types(a) VALUES ('foo'::REFCURSOR);
+
+statement ok
+INSERT INTO implicit_types(b) VALUES ('a'::REFCURSOR);
+
+statement ok
+INSERT INTO implicit_types(c) VALUES ('foo'::REFCURSOR);
+
+statement ok
+INSERT INTO implicit_types(d) VALUES ('foo'::REFCURSOR);
+
+statement error pgcode 42804 pq: value type refcursor doesn't match type int of column \"e\"
+INSERT INTO implicit_types(e) VALUES ('1'::REFCURSOR);
+
+statement error pgcode 42804 pq: value type refcursor doesn't match type float of column \"f\"
+INSERT INTO implicit_types(f) VALUES ('1.0'::REFCURSOR);
+
+statement error pgcode 42804 pq: value type refcursor doesn't match type decimal of column \"g\"
+INSERT INTO implicit_types(g) VALUES ('1.0'::REFCURSOR);
+
+statement error pgcode 42804 pq: value type refcursor doesn't match type bool of column \"h\"
+INSERT INTO implicit_types(h) VALUES ('False'::REFCURSOR);
+
+statement error pgcode 42804 pq: value type refcursor doesn't match type interval of column \"i\"
+INSERT INTO implicit_types(i) VALUES ('34h2s'::REFCURSOR);
+
+statement error pgcode 42804 pq: value type refcursor doesn't match type date of column \"j\"
+INSERT INTO implicit_types(j) VALUES ('2015-08-30'::REFCURSOR);
+
+statement error pgcode 42804 pq: value type refcursor doesn't match type timestamp of column \"k\"
+INSERT INTO implicit_types(k) VALUES ('2015-08-30 03:34:45.34567'::REFCURSOR);
+
+query TTTTIRRBTTTT rowsort
+SELECT * FROM implicit_types;
+----
+NULL  NULL  NULL  NULL  NULL  NULL  NULL  NULL  NULL  NULL  NULL  NULL
+foo   NULL  NULL  NULL  NULL  NULL  NULL  NULL  NULL  NULL  NULL  NULL
+NULL  a     NULL  NULL  NULL  NULL  NULL  NULL  NULL  NULL  NULL  NULL
+NULL  NULL  foo   NULL  NULL  NULL  NULL  NULL  NULL  NULL  NULL  NULL
+NULL  NULL  NULL  foo   NULL  NULL  NULL  NULL  NULL  NULL  NULL  NULL
+
 subtest end