Merge #148465 #149570 #149718 #149786 #149793

148465: opt: convert parameterized lookup joins to placeholder scans r=mgartner a=mgartner

Parameterized lookup joins, which are used in generic query plans, are
now converted to placeholder scans in some cases. These placeholder
scans are more efficient because they cater to a narrower set of
features than general-purpose lookup joins and because they natively
support vectorization. See the new exploration rule for more details.

Release note: None


149570: cli: improve debug.zip transaction_contention_events query r=alyshanjahani-crl a=alyshanjahani-crl

Improvements to this query were made in #139735, however statement timeouts and memory budget exceeded errors were still occurring.

This commit improves the query dump by reducing the result set of the CTEs used in the query. Instead of retrieving all fingerprints from the statement_statistics table and using those in the joins, it first filters by the fingerprints present in the contention event registry.

Fixes: #146877

Release note (cli change): Improves the performance of the debug zip query that collects transaction_contention_events data, reducing the chances of "memory budget exceeded" or "query execution canceled due to statement timeout" errors.

149718: roachtest: fix hibernate nightly test r=rafiss a=rafiss

The previous version fails during installation with:
```
FAILURE: Build completed with 2 failures.

1: Task failed with an exception.
-----------
* What went wrong:
Execution failed for task ':hibernate-core:generateGraphParser'.
> Failed to load cache entry 7ed5a08bcdbd74c9347d61047c20a896 for task ':hibernate-core:generateGraphParser': Could not load from remote cache: Not in GZIP format

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Get more help at https://help.gradle.org.
==============================================================================

2: Task failed with an exception.
-----------
* What went wrong:
Execution failed for task ':hibernate-jpamodelgen:jaxb'.
> Failed to load cache entry dd3fbbdea5c65a87fb9d720c326dd64b for task ':hibernate-jpamodelgen:jaxb': Could not load from remote cache: Not in GZIP format

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Get more help at https://help.gradle.org.
==============================================================================
```


fixes #149404
fixes #149457
fixes #149455
fixes #149399
fixes #149346
fixes #149393
fixes #149400
fixes #149398
fixes #149406
fixes #149395

Release note: None

149786: roachtest: Fix JSON configuration file for db-console/endpoints r=alyshanjahani-crl a=alyshanjahani-crl

A previous PR modified one of the json configuration files for what endpoints to test. It forgot to include a comma...

Fixes: #149735, #149732
Release note: None

149793: roachtest: add schemachange/bulkingest variant for ADD COLUMN r=rafiss a=rafiss

This patch adds testing for ADD COLUMN backfills. This differs from the CREATE INDEX operation since ADD COLUMN populates an index in order, so it runs more quickly. Measuring this will help us observe improvements in CREATE INDEX.

informs #146571
Release note: None

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Alyshan Jahani <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>

Author: 4 people

pkg/cli/zip_table_registry.go

@@ -559,26 +559,43 @@ var zipInternalTablesPerCluster = DebugZipTableRegistry{
 	},
 	"crdb_internal.transaction_contention_events": {
 		customQueryUnredacted: `
-with fingerprint_queries as (
-	SELECT distinct fingerprint_id, metadata->> 'query' as query  
-	FROM system.statement_statistics
+WITH contention_fingerprints AS (
+    -- First, extract all relevant fingerprints from contention events
+    SELECT DISTINCT waiting_stmt_fingerprint_id, blocking_txn_fingerprint_id
+    FROM crdb_internal.transaction_contention_events
+    WHERE blocking_txn_fingerprint_id != '\x0000000000000000'
 ),
-transaction_fingerprints as (
-		SELECT distinct fingerprint_id, transaction_fingerprint_id
-		FROM system.statement_statistics
-), 
-transaction_queries as (
-		SELECT tf.transaction_fingerprint_id, array_agg(fq.query) as queries
-		FROM fingerprint_queries fq
-		JOIN transaction_fingerprints tf on tf.fingerprint_id = fq.fingerprint_id
-		GROUP BY tf.transaction_fingerprint_id
+fingerprint_queries AS (
+    -- Only fetch statement data for fingerprints that appear in contention events
+    SELECT DISTINCT fingerprint_id, metadata->>'query' as query
+    FROM system.statement_statistics ss
+    WHERE EXISTS (
+        SELECT 1 FROM contention_fingerprints cf
+        WHERE cf.waiting_stmt_fingerprint_id = ss.fingerprint_id
+    )
+),
+transaction_fingerprints AS (
+    -- Only fetch transaction data for fingerprints that appear in contention events
+    SELECT DISTINCT fingerprint_id, transaction_fingerprint_id
+    FROM system.statement_statistics ss
+    WHERE EXISTS (
+        SELECT 1 FROM contention_fingerprints cf
+        WHERE cf.waiting_stmt_fingerprint_id = ss.fingerprint_id
+    )
+),
+transaction_queries AS (
+    -- Build transaction queries only for relevant transactions
+    SELECT tf.transaction_fingerprint_id, array_agg(fq.query) as queries
+    FROM fingerprint_queries fq
+    JOIN transaction_fingerprints tf ON tf.fingerprint_id = fq.fingerprint_id
+    GROUP BY tf.transaction_fingerprint_id
 )
 SELECT collection_ts,
        contention_duration,
        waiting_txn_id,
        waiting_txn_fingerprint_id,
        waiting_stmt_fingerprint_id,
-       fq.query                      AS waiting_stmt_query,
+       fq.query AS waiting_stmt_query,
        blocking_txn_id,
        blocking_txn_fingerprint_id,
        tq.queries AS blocking_txn_queries_unordered,

pkg/cmd/roachtest/tests/db-console/admin_endpoints.json

@@ -77,7 +77,7 @@
     },
     {
       "url": "/_admin/v1/rangelog",
-      "method": "GET"
+      "method": "GET",
       "skip": "https://github.com/cockroachdb/cockroach/pull/148112#issuecomment-2960322577"
     },
     {

pkg/cmd/roachtest/tests/hibernate.go

@@ -23,7 +23,7 @@ var hibernateReleaseTagRegex = regexp.MustCompile(`^(?P<major>\d+)\.(?P<minor>\d
 
 // WARNING: DO NOT MODIFY the name of the below constant/variable without approval from the docs team.
 // This is used by docs automation to produce a list of supported versions for ORM's.
-var supportedHibernateTag = "6.6.0"
+var supportedHibernateTag = "6.6.20"
 
 type hibernateOptions struct {
 	testName string

pkg/cmd/roachtest/tests/schemachange.go

@@ -328,17 +328,32 @@ func makeIndexAddTpccTest(
 	}
 }
 
+// bulkIngestSchemaChangeOp represents the type of schema change operation to
+// perform  during bulk ingestion testing.
+type bulkIngestSchemaChangeOp string
+
+const (
+	// createIndexOp runs CREATE INDEX, which adds keys in unsorted order.
+	createIndexOp bulkIngestSchemaChangeOp = "create_index"
+	// addColumnOp runs ADD COLUMN, which adds keys in sorted order.
+	addColumnOp bulkIngestSchemaChangeOp = "add_column"
+)
+
 func registerSchemaChangeBulkIngest(r registry.Registry) {
 	// Allow a long running time to account for runs that use a
 	// cockroach build with runtime assertions enabled.
-	r.Add(makeSchemaChangeBulkIngestTest(r, 12, 4_000_000_000, 5*time.Hour))
+	r.Add(makeSchemaChangeBulkIngestTest(r, 12, 4_000_000_000, 5*time.Hour, createIndexOp))
+	r.Add(makeSchemaChangeBulkIngestTest(r, 12, 4_000_000_000, 5*time.Hour, addColumnOp))
 }
 
 func makeSchemaChangeBulkIngestTest(
-	r registry.Registry, numNodes, numRows int, length time.Duration,
+	r registry.Registry,
+	numNodes, numRows int,
+	length time.Duration,
+	operation bulkIngestSchemaChangeOp,
 ) registry.TestSpec {
 	return registry.TestSpec{
-		Name:             "schemachange/bulkingest",
+		Name:             fmt.Sprintf("schemachange/bulkingest/nodes=%d/rows=%d/%s", numNodes, numRows, operation),
 		Owner:            registry.OwnerSQLFoundations,
 		Benchmark:        true,
 		Cluster:          r.MakeClusterSpec(numNodes, spec.WorkloadNode(), spec.SSD(4)),
@@ -350,26 +365,18 @@ func makeSchemaChangeBulkIngestTest(
 			// The histogram tracks the total elapsed time for the CREATE INDEX operation.
 			totalElapsed := histogram.Elapsed
 
-			// Calculate the approximate data size for the index.
-			// The index is on (payload, a) where payload is 40 bytes.
-			// Approximate size per row for index: 40 bytes (payload) + 8 bytes (a) = 48 bytes
-			rowsIndexed := int64(numRows)
-			bytesPerRow := int64(48)
-			mb := int64(1 << 20)
-			dataSizeInMB := (rowsIndexed * bytesPerRow) / mb
-
-			// Calculate throughput in MB/s per node.
-			indexDuration := int64(totalElapsed / 1000) // Convert to seconds.
-			if indexDuration == 0 {
-				indexDuration = 1 // Avoid division by zero.
+			// Calculate throughput in rows/sec per node.
+			schemaChangeDuration := int64(totalElapsed / 1000) // Convert to seconds.
+			if schemaChangeDuration == 0 {
+				schemaChangeDuration = 1 // Avoid division by zero.
 			}
-			avgRatePerNode := roachtestutil.MetricPoint(float64(dataSizeInMB) / float64(int64(numNodes)*indexDuration))
+			avgRatePerNode := roachtestutil.MetricPoint(float64(numRows) / float64(int64(numNodes)*schemaChangeDuration))
 
 			return roachtestutil.AggregatedPerfMetrics{
 				{
 					Name:           fmt.Sprintf("%s_throughput", test),
 					Value:          avgRatePerNode,
-					Unit:           "MB/s/node",
+					Unit:           "rows/s/node",
 					IsHigherBetter: true,
 				},
 			}, nil
@@ -427,7 +434,7 @@ func makeSchemaChangeBulkIngestTest(
 				defer db.Close()
 
 				if !c.IsLocal() {
-					// Wait for the load generator to run for a few minutes before creating the index.
+					// Wait for the load generator to run for a few minutes before performing the schema change.
 					sleepInterval := time.Minute * 5
 					maxSleep := length / 2
 					if sleepInterval > maxSleep {
@@ -436,16 +443,28 @@ func makeSchemaChangeBulkIngestTest(
 					time.Sleep(sleepInterval)
 				}
 
-				t.L().Printf("Creating index")
-				// Tick once before starting the index creation.
+				// Tick once before starting the schema change.
 				tickHistogram()
 				before := timeutil.Now()
-				if _, err := db.Exec(`CREATE INDEX payload_a ON bulkingest.bulkingest (payload, a)`); err != nil {
+
+				var stmt string
+				switch operation {
+				case createIndexOp:
+					t.L().Printf("Creating index")
+					stmt = `CREATE INDEX payload_a ON bulkingest.bulkingest (payload, a)`
+				case addColumnOp:
+					t.L().Printf("Adding column")
+					stmt = `ALTER TABLE bulkingest.bulkingest ADD COLUMN new_column INT NOT NULL DEFAULT 42`
+				default:
+					t.Fatalf("Unknown operation: %s", operation)
+				}
+
+				if _, err := db.Exec(stmt); err != nil {
 					t.Fatal(err)
 				}
-				// Tick once after the index creation to capture the total elapsed time.
+				// Tick once after the schema change to capture the total elapsed time.
 				tickHistogram()
-				t.L().Printf("CREATE INDEX took %v\n", timeutil.Since(before))
+				t.L().Printf("%s took %v\n", stmt, timeutil.Since(before))
 				return nil
 			})
 

pkg/sql/opt/exec/execbuilder/mutation.go

@@ -1233,7 +1233,7 @@ func shouldApplyImplicitLockingToUpdateOrDeleteInput(
 	var toLockIndexes intsets.Fast
 	// Try to match the mutation's input expression against the pattern:
 	//
-	//   [Project]* [IndexJoin] (Scan | LookupJoin [LookupJoin] Values)
+	//   [Project]* [IndexJoin] (Scan | PlaceholderScan | LookupJoin [LookupJoin] Values)
 	//
 	// The IndexJoin will only be present if the base expression is a Scan, but
 	// making it an optional prefix to the LookupJoins makes the logic simpler.
@@ -1247,6 +1247,9 @@ func shouldApplyImplicitLockingToUpdateOrDeleteInput(
 	case *memo.ScanExpr:
 		toLockIndexes.Add(t.Index)
 		toLock = t.Table
+	case *memo.PlaceholderScanExpr:
+		toLockIndexes.Add(t.Index)
+		toLock = t.Table
 	case *memo.LookupJoinExpr:
 		toLockIndexes.Add(t.Index)
 		if innerJoin, ok := t.Input.(*memo.LookupJoinExpr); ok && innerJoin.Table == t.Table {

pkg/sql/opt/exec/execbuilder/testdata/delete

@@ -500,28 +500,20 @@ quality of service: regular
 │ from: t137352
 │ auto commit
 │
-└── • lookup join
-    │ sql nodes: <hidden>
-    │ kv nodes: <hidden>
-    │ regions: <hidden>
-    │ actual row count: 0
-    │ KV time: 0µs
-    │ KV rows decoded: 0
-    │ KV bytes read: 0 B
-    │ KV gRPC calls: 0
-    │ execution time: 0µs
-    │ estimated max memory allocated: 0 B
-    │ table: t137352@t137352_pkey
-    │ equality: ($1) = (k)
-    │ equality cols are key
-    │ locking strength: for update
-    │
-    └── • values
-          sql nodes: <hidden>
-          regions: <hidden>
-          actual row count: 1
-          execution time: 0µs
-          size: 1 column, 1 row
+└── • scan
+      sql nodes: <hidden>
+      kv nodes: <hidden>
+      regions: <hidden>
+      actual row count: 0
+      KV time: 0µs
+      KV rows decoded: 0
+      KV bytes read: 0 B
+      KV gRPC calls: 0
+      estimated max memory allocated: 0 B
+      missing stats
+      table: t137352@t137352_pkey
+      spans: [/1 - /1]
+      locking strength: for update
 
 statement ok
 INSERT INTO t137352 VALUES (1, 10, 100);

pkg/sql/opt/exec/execbuilder/testdata/update

@@ -1025,28 +1025,20 @@ quality of service: regular
 │
 └── • render
     │
-    └── • lookup join
-        │ sql nodes: <hidden>
-        │ kv nodes: <hidden>
-        │ regions: <hidden>
-        │ actual row count: 0
-        │ KV time: 0µs
-        │ KV rows decoded: 0
-        │ KV bytes read: 0 B
-        │ KV gRPC calls: 0
-        │ execution time: 0µs
-        │ estimated max memory allocated: 0 B
-        │ table: t137352@t137352_pkey
-        │ equality: ($1) = (k)
-        │ equality cols are key
-        │ locking strength: for update
-        │
-        └── • values
-              sql nodes: <hidden>
-              regions: <hidden>
-              actual row count: 1
-              execution time: 0µs
-              size: 1 column, 1 row
+    └── • scan
+          sql nodes: <hidden>
+          kv nodes: <hidden>
+          regions: <hidden>
+          actual row count: 0
+          KV time: 0µs
+          KV rows decoded: 0
+          KV bytes read: 0 B
+          KV gRPC calls: 0
+          estimated max memory allocated: 0 B
+          missing stats
+          table: t137352@t137352_pkey
+          spans: [/1 - /1]
+          locking strength: for update
 
 statement ok
 INSERT INTO t137352 VALUES (1, 2, 3);
@@ -1297,29 +1289,20 @@ quality of service: regular
                 │ equality: (k) = (a)
                 │ pred: a = 1
                 │
-                └── • lookup join
-                    │ sql nodes: <hidden>
-                    │ kv nodes: <hidden>
-                    │ regions: <hidden>
-                    │ actual row count: 1
-                    │ KV time: 0µs
-                    │ KV rows decoded: 1
-                    │ KV pairs read: 2
-                    │ KV bytes read: 8 B
-                    │ KV gRPC calls: 1
-                    │ execution time: 0µs
-                    │ estimated max memory allocated: 0 B
-                    │ estimated row count: 1
-                    │ table: t137352@t137352_pkey
-                    │ equality: ($1) = (k)
-                    │ equality cols are key
-                    │
-                    └── • values
-                          sql nodes: <hidden>
-                          regions: <hidden>
-                          actual row count: 1
-                          execution time: 0µs
-                          size: 1 column, 1 row
+                └── • scan
+                      sql nodes: <hidden>
+                      kv nodes: <hidden>
+                      regions: <hidden>
+                      actual row count: 1
+                      KV time: 0µs
+                      KV rows decoded: 1
+                      KV pairs read: 2
+                      KV bytes read: 8 B
+                      KV gRPC calls: 1
+                      estimated max memory allocated: 0 B
+                      estimated row count: 1
+                      table: t137352@t137352_pkey
+                      spans: [/1 - /1]
 
 statement ok
 DELETE FROM t137352 WHERE true;

pkg/sql/opt/exec/execbuilder/testdata/virtual

@@ -539,4 +539,3 @@ vectorized: true
 
 statement ok
 RESET disallow_full_table_scans;
-

pkg/sql/opt/memo/logical_props_builder.go

@@ -203,6 +203,15 @@ func (b *logicalPropsBuilder) buildScanProps(scan *ScanExpr, rel *props.Relation
 	}
 }
 
+// buildPlaceholderScanProps is unimplemented. Placeholder expressions are only created
+// in two places:
+//
+//  1. The placeholder fast-path which entirely skips optimization.
+//  2. The ConvertParameterizedLookupJoinToPlaceholderScan exploration rule
+//     which always adds the placeholder scan to an existing memo group, for
+//     which logical properties have already been built.
+//
+// In both cases this function is never called.
 func (b *logicalPropsBuilder) buildPlaceholderScanProps(
 	scan *PlaceholderScanExpr, rel *props.Relational,
 ) {

pkg/sql/opt/norm/factory_test.go

@@ -87,9 +87,8 @@ func TestCopyAndReplace(t *testing.T) {
 
 	if e, err := o.Optimize(); err != nil {
 		t.Fatal(err)
-	} else if e.Op() != opt.ProjectOp || e.Child(0).Op() != opt.LookupJoinOp {
-		t.Errorf("expected optimizer to choose a (project (lookup-join)), not (%v (%v))",
-			e.Op(), e.Child(0).Op())
+	} else if e.Op() != opt.PlaceholderScanOp {
+		t.Errorf("expected optimizer to choose a placeholder scan, not %v", e.Op())
 	}
 
 	m := o.Factory().DetachMemo()