Merge #143579 #147545 #147548 #147627

143579: sql,plpgsql: fix case sensitivity in composite element assignment r=DrewKimball a=sousa16

#### sql,plpgsql: fix case sensitivity in composite element assignment

Previously, the logic that resolved an element of a composite-typed PL/pgSQL
variable for assignment normalized the element name after it was already
normalized. This caused every field to be treated as lowercase, making it
impossible to perform an assignment like `NEW."FOO_Bar" = 100`. This commit
removes the extra normalization and replaces it with a conversion to string.

Fixes #142083

Release note (sql change): Assigning to an element of a composite-typed
variable in a PL/pgSQL routine now respects case-sensitivity rules. For
example, a field named `"FOO_Bar"` can be assigned like `NEW."FOO_Bar" = 100`.

147545: crosscluster/physical: deflake TestTenantStreamingFailback r=kev-cao a=msbutler

Previously, this test would fail with a scary fingerprint mismatch violation error, but it was because one test path did not wait for the fingerprint time to get replicated.

Fixes #146444

Release note: none

147548: crosscluster/physical: skip TestAlterTenantAddReader under deadlock r=kev-cao a=msbutler

Informs #146788

Epic: none

147627: sql: skip TestIndexBackfillerResumePreservesProgress under deadlock/race r=rafiss a=rafiss

Epic: None
Release note: None

Co-authored-by: João Sousa <[email protected]>
Co-authored-by: Michael Butler <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>

Author: 4 people

pkg/ccl/logictestccl/testdata/logic_test/plpgsql_assign

@@ -121,6 +121,97 @@ CREATE FUNCTION f(val xy) RETURNS xy AS $$
   END
 $$ LANGUAGE PLpgSQL;
 
+subtest assign_elem_case_sensitivity
+
+# Composite type with unquoted (lowercase) fields.
+statement ok
+CREATE TYPE comp1 AS (foo INT, bar INT);
+
+statement ok
+CREATE FUNCTION f1(val comp1) RETURNS comp1 AS $$
+  BEGIN
+    val.foo := 10;
+    val.bar := 20;
+    RETURN val;
+  END
+$$ LANGUAGE PLpgSQL;
+
+query T
+SELECT f1(ROW(1, 2));
+----
+(10,20)
+
+statement ok
+DROP FUNCTION f1;
+
+# Composite type with quoted (uppercase) fields.
+statement ok
+CREATE TYPE comp2 AS ("FOO" INT, "BAR" INT);
+
+statement ok
+CREATE FUNCTION f2(val comp2) RETURNS comp2 AS $$
+  BEGIN
+    val."FOO" := 100;
+    val."BAR" := 200;
+    RETURN val;
+  END
+$$ LANGUAGE PLpgSQL;
+
+query T
+SELECT f2(ROW(1, 2)::comp2);
+----
+(100,200)
+
+statement ok
+DROP FUNCTION f2;
+
+# Attempt to assign using wrong case (should fail).
+statement error pgcode 42703 pq: record "val" has no field "FOO"
+CREATE FUNCTION f3(val comp1) RETURNS comp1 AS $$
+  BEGIN
+    val."FOO" := 1;
+    RETURN val;
+  END
+$$ LANGUAGE PLpgSQL;
+
+statement error pgcode 42703 pq: record "val" has no field "foo"
+CREATE FUNCTION f4(val comp2) RETURNS comp2 AS $$
+  BEGIN
+    val.foo := 1;
+    RETURN val;
+  END
+$$ LANGUAGE PLpgSQL;
+
+# Mixed quoted/unquoted field names in composite type.
+statement ok
+CREATE TYPE comp3 AS (foo INT, "BAR" INT);
+
+statement ok
+CREATE FUNCTION f5(val comp3) RETURNS comp3 AS $$
+  BEGIN
+    val.foo := 5;
+    val."BAR" := 6;
+    RETURN val;
+  END
+$$ LANGUAGE PLpgSQL;
+
+query T
+SELECT f5(ROW(0, 0)::comp3);
+----
+(5,6)
+
+statement ok
+DROP FUNCTION f5;
+
+statement ok
+DROP TYPE comp1;
+
+statement ok
+DROP TYPE comp2;
+
+statement ok
+DROP TYPE comp3;
+
 subtest end
 
 # Ordinal parameter references should reflect updates made by variable

pkg/ccl/logictestccl/testdata/logic_test/triggers

@@ -4247,6 +4247,125 @@ SELECT id, a, b FROM tab_141810 ORDER BY id
 3  1     2
 4  NULL  NULL
 
+# ==============================================================================
+# Test case sensitivity of column names in triggers
+# ==============================================================================
+
+subtest case_sensitivity
+
+# Setup: clean up any existing tables and functions
+statement ok
+DROP TABLE IF EXISTS t1, t2, t3;
+
+statement ok
+DROP TRIGGER IF EXISTS tr1 ON t1;
+DROP TRIGGER IF EXISTS tr2 ON t1;
+DROP TRIGGER IF EXISTS tr3 ON t1;
+-- tr4 failed to create, so no need to drop
+
+statement ok
+DROP TRIGGER IF EXISTS tr5 ON t2;
+DROP TRIGGER IF EXISTS tr6 ON t2;
+DROP TRIGGER IF EXISTS tr7 ON t2;
+-- tr8 failed to create, so no need to drop
+
+statement ok
+DROP TRIGGER IF EXISTS tr12 ON t3;
+-- tr9, tr10, tr11 failed to create, so no need to drop
+
+# Setup: Create tables with different casing
+statement ok
+CREATE TABLE t1 (foo_bar INT);
+CREATE TABLE t2 ("foo_bar" INT);
+CREATE TABLE t3 ("FOO_BAR" INT);
+
+# Trigger functions with different quoting/casing styles
+statement ok
+CREATE FUNCTION f1() RETURNS TRIGGER LANGUAGE PLpgSQL AS $$
+  BEGIN
+    NEW.foo_bar := 1;
+    RETURN NEW;
+  END;
+$$;
+
+statement ok
+CREATE FUNCTION f2() RETURNS TRIGGER LANGUAGE PLpgSQL AS $$
+  BEGIN
+    NEW.FOO_BAR := 1;
+    RETURN NEW;
+  END;
+$$;
+
+statement ok
+CREATE FUNCTION f3() RETURNS TRIGGER LANGUAGE PLpgSQL AS $$
+  BEGIN
+    NEW."foo_bar" := 1;
+    RETURN NEW;
+  END;
+$$;
+
+statement ok
+CREATE FUNCTION f4() RETURNS TRIGGER LANGUAGE PLpgSQL AS $$
+  BEGIN
+    NEW."FOO_BAR" := 1;
+    RETURN NEW;
+  END;
+$$;
+
+# Attach each function to each table and check success/failure
+
+# Unquoted table
+statement ok
+CREATE TRIGGER tr1 BEFORE INSERT ON t1 FOR EACH ROW EXECUTE FUNCTION f1();
+
+statement ok
+CREATE TRIGGER tr2 BEFORE INSERT ON t1 FOR EACH ROW EXECUTE FUNCTION f2();
+
+statement ok
+CREATE TRIGGER tr3 BEFORE INSERT ON t1 FOR EACH ROW EXECUTE FUNCTION f3();
+
+statement error pgcode 42703 pq: record "new" has no field "FOO_BAR"
+CREATE TRIGGER tr4 BEFORE INSERT ON t1 FOR EACH ROW EXECUTE FUNCTION f4();
+
+# Quoted lowercase column
+statement ok
+CREATE TRIGGER tr5 BEFORE INSERT ON t2 FOR EACH ROW EXECUTE FUNCTION f1();
+
+statement ok
+CREATE TRIGGER tr6 BEFORE INSERT ON t2 FOR EACH ROW EXECUTE FUNCTION f2();
+
+statement ok
+CREATE TRIGGER tr7 BEFORE INSERT ON t2 FOR EACH ROW EXECUTE FUNCTION f3();
+
+statement error pgcode 42703 pq: record "new" has no field "FOO_BAR"
+CREATE TRIGGER tr8 BEFORE INSERT ON t2 FOR EACH ROW EXECUTE FUNCTION f4();
+
+# Quoted uppercase column
+statement error pgcode 42703 pq: record "new" has no field "foo_bar"
+CREATE TRIGGER tr9 BEFORE INSERT ON t3 FOR EACH ROW EXECUTE FUNCTION f1();
+
+statement error pgcode 42703 pq: record "new" has no field "foo_bar"
+CREATE TRIGGER tr10 BEFORE INSERT ON t3 FOR EACH ROW EXECUTE FUNCTION f2();
+
+statement error pgcode 42703 pq: record "new" has no field "foo_bar"
+CREATE TRIGGER tr11 BEFORE INSERT ON t3 FOR EACH ROW EXECUTE FUNCTION f3();
+
+statement ok
+CREATE TRIGGER tr12 BEFORE INSERT ON t3 FOR EACH ROW EXECUTE FUNCTION f4();
+
+# Drop all tables
+statement ok
+DROP TABLE t1;
+DROP TABLE t2;
+DROP TABLE t3;
+
+# Drop all functions
+statement ok
+DROP FUNCTION f1();
+DROP FUNCTION f2();
+DROP FUNCTION f3();
+DROP FUNCTION f4();
+
 subtest end
 
 # Until #135311 can be fixed, disallow TG_ARGV references by default. There is

pkg/crosscluster/physical/alter_replication_job_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/jobutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
+	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
@@ -116,6 +117,8 @@ func TestAlterTenantAddReader(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
+	skip.UnderDeadlock(t, "flakes with deadlock detector")
+
 	ctx := context.Background()
 	args := replicationtestutils.DefaultTenantStreamingClustersArgs
 

pkg/crosscluster/physical/stream_ingestion_job_test.go

@@ -199,21 +199,19 @@ func TestTenantStreamingFailback(t *testing.T) {
 	var ts1 string
 	sqlA.QueryRow(t, "SELECT cluster_logical_timestamp()").Scan(&ts1)
 
+	t.Logf("waiting for g@%s", ts1)
+	replicationtestutils.WaitUntilReplicatedTime(t,
+		replicationtestutils.DecimalTimeToHLC(t, ts1),
+		sqlB,
+		jobspb.JobID(consumerGJobID))
+
 	// Randomize query execution to verify fast failback works for both
 	// `COMPLETE REPLICATION TO LATEST` and `COMPLETE REPLICATION TO SYSTEM TIME`
 	rng, _ := randutil.NewPseudoRand()
 	if rng.Intn(2) == 0 {
-		t.Logf("waiting for g@%s", ts1)
-		replicationtestutils.WaitUntilReplicatedTime(t,
-			replicationtestutils.DecimalTimeToHLC(t, ts1),
-			sqlB,
-			jobspb.JobID(consumerGJobID))
-
 		t.Logf("completing replication on g@%s", ts1)
 		sqlB.Exec(t, fmt.Sprintf("ALTER VIRTUAL CLUSTER g COMPLETE REPLICATION TO SYSTEM TIME '%s'", ts1))
 	} else {
-		t.Log("waiting for initial scan on g")
-		replicationtestutils.WaitUntilStartTimeReached(t, sqlB, jobspb.JobID(consumerGJobID))
 		t.Log("completing replication on g to latest")
 		sqlB.Exec(t, "ALTER VIRTUAL CLUSTER g COMPLETE REPLICATION TO LATEST")
 	}

pkg/sql/indexbackfiller_test.go

@@ -43,6 +43,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
+	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
@@ -575,6 +576,8 @@ INSERT INTO foo VALUES (1), (10), (100);
 func TestIndexBackfillerResumePreservesProgress(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
+	skip.UnderDeadlock(t, "slow timing sensitive test")
+	skip.UnderRace(t, "slow timing sensitive test")
 
 	ctx := context.Background()
 	backfillProgressCompletedCh := make(chan []roachpb.Span)

pkg/sql/opt/optbuilder/plpgsql.go

@@ -1608,7 +1608,7 @@ const noIndirection = ""
 func (b *plpgsqlBuilder) handleIndirectionForAssign(
 	inScope *scope, typ *types.T, ident ast.Variable, indirection tree.Name, val tree.Expr,
 ) opt.ScalarExpr {
-	elemName := indirection.Normalize()
+	elemName := string(indirection)
 
 	// We do not yet support qualifying a variable with a block label.
 	b.checkBlockLabelReference(elemName)