Merge #75582 #75732

75582: opt: push limit into FK and self-joins in more cases r=mgartner a=mgartner

#### opt: more accurate join multiplicity with FKs and self-joins

When calculating join multiplicity in the presence of a foreign key, it
was previously possible for the incorrect base table to be selected from
the right expression, in the case that the right expression contained a
self-join. The base table selected was incorrect because it did not
contain the equality column from the join filter. This prevented the
optimizer from determining that these joins preserve all rows from the
left side of the join, which could prevent further optimization of a
query.

Now, only the right equality columns are passed to `checkForeignKeyCase`
which ensures that the correct base table is selected. This is safe
because `verifyFiltersAreValidEqualities` has already checked that the
right equality columns are unfiltered in the right expression, so the
same checks in `checkForeignKeyCase` are redundant.

Release note (performance improvement): The optimizer better optimizes
queries that include both foreign key joins and self-joins.

#### opt: push limit into FK and self-joins in more cases

Previously, a constant equality condition pushed into both sides of a
foreign key join or self-join would prevent a limit from being pushed
into the left side of the join. This was because the multiplicity
builder could not determine that right filter would not remove any
values also removed by the left filter. Without the join being labelled
as left-preserving, the limit could not be pushed down.

The multiplicity builder has been updated to recognize a few additional
cases where left rows are preserved in foreign key joins and self-joins,
allowing a limit to be pushed into the left side of the join. Currently,
the multiplicity builder only recognizes cases where corresponding left
and right columns are held equal to the same constant value. It is
possible to extend this to more complex inequalities and boolean
expressions, but this is left as a TODO for now.

Fixes #74419

Release note (performance improvement): A LIMIT can now be pushed below
a foreign key join or self-join in more cases, which may result in more
efficient query plans.


75732: sql: pass statement request id to backend instead of statement fingerprint r=THardy98 a=THardy98

Previously, the frontend passed a statement
fingerprint to the API to update the corresponding statement diagnostic
request to an expired state. Now we pass the statement diagnostic ID
directly. This resolves a SQL parsing error that occurs when a statement
fingerprint contains single-quotes (i.e. when a statement fingerprint
contains placeholders).

Original PR with discussions: #74818
Resolves: #74226

Release note: None

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Thomas Hardy <[email protected]>

docs/generated/http/full.md

@@ -3687,7 +3687,7 @@ Support status: [reserved](#support-status)
 
 | Field | Type | Label | Description | Support status |
 | ----- | ---- | ----- | ----------- | -------------- |
-| statement_fingerprint | [string](#cockroach.server.serverpb.CancelStatementDiagnosticsReportRequest-string) |  |  | [reserved](#support-status) |
+| request_id | [int64](#cockroach.server.serverpb.CancelStatementDiagnosticsReportRequest-int64) |  |  | [reserved](#support-status) |
 
 
 

pkg/server/serverpb/status.proto

@@ -1286,7 +1286,7 @@ message CreateStatementDiagnosticsReportResponse {
 }
 
 message CancelStatementDiagnosticsReportRequest {
-  string statement_fingerprint = 1;
+  int64 request_id = 1 [(gogoproto.customname) = "RequestID"];
 }
 
 message CancelStatementDiagnosticsReportResponse {

pkg/server/statement_diagnostics_requests.go

@@ -106,7 +106,7 @@ func (s *statusServer) CancelStatementDiagnosticsReport(
 	}
 
 	var response serverpb.CancelStatementDiagnosticsReportResponse
-	err := s.stmtDiagnosticsRequester.CancelRequest(ctx, req.StatementFingerprint)
+	err := s.stmtDiagnosticsRequester.CancelRequest(ctx, req.RequestID)
 	if err != nil {
 		response.Canceled = false
 		response.Error = err.Error()

pkg/server/status.go

@@ -396,7 +396,7 @@ type StmtDiagnosticsRequester interface {
 	// CancelRequest updates an entry in system.statement_diagnostics_requests
 	// for tracing a query with the given fingerprint to be expired (thus,
 	// canceling any new tracing for it).
-	CancelRequest(ctx context.Context, stmtFingerprint string) error
+	CancelRequest(ctx context.Context, requestID int64) error
 }
 
 // newStatusServer allocates and returns a statusServer.

pkg/sql/opt/memo/multiplicity_builder.go

@@ -122,8 +122,8 @@ func deriveUnfilteredCols(in RelExpr) opt.ColSet {
 		right := t.Child(1).(RelExpr)
 		multiplicity := GetJoinMultiplicity(t)
 
-		// Use the UnfilteredCols to determine whether unfiltered columns can be
-		// passed through.
+		// Use the join's multiplicity to determine whether unfiltered columns
+		// can be passed through.
 		if multiplicity.JoinPreservesLeftRows(t.Op()) {
 			unfilteredCols.UnionWith(deriveUnfilteredCols(left))
 		}
@@ -234,13 +234,18 @@ func filtersMatchLeftRowsAtMostOnce(left, right RelExpr, filters FiltersExpr) bo
 //      must come from the same foreign key.
 //
 // In both the self-join and the foreign key cases, the left columns must be
-// not-null, and the right columns must be unfiltered.
+// not-null, and the right columns must be either unfiltered, or the left and
+// right must be Select expressions where the left side filters imply the right
+// side filters and right columns are unfiltered in the right Select's input
+// (see condition #3b in the comment for verifyFiltersAreValidEqualities).
 //
-// Why do the left columns have to be not-null and the right columns
-// unfiltered? In both the self-join and the foreign-key cases, a non-null
-// value in the left column guarantees a corresponding value in the right
-// column. As long as no nulls have been added to the left column and no values
-// have been removed from the right, this property will be valid.
+// Why do the left columns have to be non-null, and the right columns unfiltered
+// or filtered identically as their corresponding left column? In both the
+// self-join and the foreign-key cases, a non-null value in the left column
+// guarantees a corresponding value in the right column. As long as no nulls
+// have been added to the left column and no values have been removed from the
+// right that have not also been removed from the left, this property will be
+// valid.
 //
 // Why do all foreign key columns in the foreign key case have to come from the
 // same foreign key? Equalities on different foreign keys may each be
@@ -267,7 +272,8 @@ func filtersMatchAllLeftRows(left, right RelExpr, filters FiltersExpr) bool {
 			filters,
 		)
 	}
-	if !verifyFiltersAreValidEqualities(left, right, filters) {
+	rightEqualityCols, ok := verifyFiltersAreValidEqualities(left, right, filters)
+	if !ok {
 		return false
 	}
 	if checkSelfJoinCase(left.Memo().Metadata(), filters) {
@@ -276,41 +282,50 @@ func filtersMatchAllLeftRows(left, right RelExpr, filters FiltersExpr) bool {
 	}
 	// Case 2b.
 	return checkForeignKeyCase(
-		left.Memo().Metadata(), left.Relational().NotNullCols, deriveUnfilteredCols(right), filters)
+		left.Memo().Metadata(),
+		left.Relational().NotNullCols,
+		rightEqualityCols,
+		filters,
+	)
 }
 
-// verifyFiltersAreValidEqualities returns true when all of the following
-// conditions are satisfied:
-// 1. All filters are equalities.
-// 2. All equalities directly compare two columns.
-// 3. All equalities contain one column from the left not-null columns, and one
-//    column from the right unfiltered columns.
-// 4. All equality columns come from a base table.
-// 5. All left columns come from a single table, and all right columns come from
-//    a single table.
-func verifyFiltersAreValidEqualities(left, right RelExpr, filters FiltersExpr) bool {
+// verifyFiltersAreValidEqualities returns the set of equality columns in the
+// right relation and true when all the following conditions are satisfied:
+//
+//   1. All filters are equalities.
+//   2. All equalities directly compare two columns.
+//   3. All equalities x=y (or y=x) have x as a left non-null column and y as a
+//      right column, and either:
+//      a. y is an unfiltered column in the right expression, or
+//      b. both the left and right expressions are Selects; the left side
+//         filters imply the right side filters when replacing x with y; and y
+//         is an unfiltered column in the right Select's input.
+//   4. All equality columns come from a base table.
+//   5. All left columns come from a single table, and all right columns come
+//      from a single table.
+//
+// Returns ok=false if any of these conditions are unsatisfied.
+func verifyFiltersAreValidEqualities(
+	left, right RelExpr, filters FiltersExpr,
+) (rightEqualityCols opt.ColSet, ok bool) {
 	md := left.Memo().Metadata()
 
 	var leftTab, rightTab opt.TableID
 	leftNotNullCols := left.Relational().NotNullCols
 	rightUnfilteredCols := deriveUnfilteredCols(right)
-	if rightUnfilteredCols.Empty() {
-		// There are no unfiltered columns from the right input.
-		return false
-	}
 
 	for i := range filters {
 		eq, _ := filters[i].Condition.(*EqExpr)
 		if eq == nil {
 			// Condition #1: Conjunct is not an equality comparison.
-			return false
+			return opt.ColSet{}, false
 		}
 
 		leftVar, _ := eq.Left.(*VariableExpr)
 		rightVar, _ := eq.Right.(*VariableExpr)
 		if leftVar == nil || rightVar == nil {
 			// Condition #2: Conjunct does not directly compare two columns.
-			return false
+			return opt.ColSet{}, false
 		}
 
 		leftColID := leftVar.Col
@@ -319,10 +334,22 @@ func verifyFiltersAreValidEqualities(left, right RelExpr, filters FiltersExpr) b
 		// Normalize leftColID to come from leftColIDs.
 		if !leftNotNullCols.Contains(leftColID) {
 			leftColID, rightColID = rightColID, leftColID
+			if !leftNotNullCols.Contains(leftColID) {
+				// Condition #3: Left column is not guaranteed to be non-null.
+				return opt.ColSet{}, false
+			}
 		}
-		if !leftNotNullCols.Contains(leftColID) || !rightUnfilteredCols.Contains(rightColID) {
-			// Condition #3: Columns don't come from both the left and right ColSets.
-			return false
+
+		switch {
+		case rightUnfilteredCols.Contains(rightColID):
+		// Condition #3a: the right column is unfiltered.
+		case rightHasSingleFilterThatMatchesLeft(left, right, leftColID, rightColID):
+		// Condition #3b: The left and right are Selects where the left filters
+		// imply the right filters when replacing the left column with the right
+		// column, and the right column is unfiltered in the right Select's
+		// input.
+		default:
+			return opt.ColSet{}, false
 		}
 
 		if leftTab == 0 || rightTab == 0 {
@@ -331,16 +358,92 @@ func verifyFiltersAreValidEqualities(left, right RelExpr, filters FiltersExpr) b
 			rightTab = md.ColumnMeta(rightColID).Table
 			if leftTab == 0 || rightTab == 0 {
 				// Condition #4: Columns don't come from base tables.
-				return false
+				return opt.ColSet{}, false
 			}
 		}
 		if leftTab != md.ColumnMeta(leftColID).Table || rightTab != md.ColumnMeta(rightColID).Table {
 			// Condition #5: The filter conditions reference more than one table from
 			// each side.
-			return false
+			return opt.ColSet{}, false
 		}
+
+		rightEqualityCols.Add(rightColID)
 	}
-	return true
+
+	return rightEqualityCols, true
+}
+
+// rightHasSingleFilterThatMatchesLeft returns true if:
+//
+//   1. Both left and right are Select expressions.
+//   2. rightCol is unfiltered in right's input.
+//   3. The left Select has a filter in the form leftCol=const.
+//   4. The right Select has a single filter in the form rightCol=const where
+//      the const value is the same as the const value in (2).
+//
+// This function is used by verifyFiltersAreValidEqualities to try to prove that
+// every row in the left input of a join will have a match in the right input
+// (see condition #3b in the comment of verifyFiltersAreValidEqualities).
+//
+// TODO(mgartner): Extend this to return true when the left filters imply the
+// right filters, after remapping leftCol to rightCol in the left filters. For
+// example, leftCol<10 implies rightCol<20 when leftCol and rightCol are held
+// equal by the join filters. This may be a good opportunity to reuse
+// partialidx.Implicator. Be aware that it might not be possible to simply
+// replace columns in a filter when one of the columns has a composite type.
+func rightHasSingleFilterThatMatchesLeft(left, right RelExpr, leftCol, rightCol opt.ColumnID) bool {
+	leftSelect, ok := left.(*SelectExpr)
+	if !ok {
+		return false
+	}
+	rightSelect, ok := right.(*SelectExpr)
+	if !ok {
+		return false
+	}
+
+	// Return false if the right column has been filtered in the input to
+	// rightSelect.
+	rightUnfilteredCols := deriveUnfilteredCols(rightSelect.Input)
+	if !rightUnfilteredCols.Contains(rightCol) {
+		return false
+	}
+
+	// Return false if rightSelect has more than one filter.
+	if len(rightSelect.Filters) > 1 {
+		return false
+	}
+
+	// constValueForCol searches for an expression in the form
+	// (Eq (Var col) Const) and returns the Const expression, if one is found.
+	constValueForCol := func(filters FiltersExpr, col opt.ColumnID) (_ *ConstExpr, ok bool) {
+		var constant *ConstExpr
+		for i := range filters {
+			if !filters[i].ScalarProps().OuterCols.Contains(col) {
+				continue
+			}
+			eq, _ := filters[i].Condition.(*EqExpr)
+			if eq == nil {
+				continue
+			}
+			v, _ := eq.Left.(*VariableExpr)
+			c, _ := eq.Right.(*ConstExpr)
+			if v == nil || v.Col != col || c == nil {
+				continue
+			}
+			constant = c
+		}
+		return constant, constant != nil
+	}
+
+	leftConst, ok := constValueForCol(leftSelect.Filters, leftCol)
+	if !ok {
+		return false
+	}
+	rightConst, ok := constValueForCol(rightSelect.Filters, rightCol)
+	if !ok {
+		return false
+	}
+	return leftConst == rightConst
 }
 
 // checkSelfJoinCase returns true if all equalities in the given FiltersExpr
@@ -374,7 +477,9 @@ func checkForeignKeyCase(
 ) bool {
 	if rightUnfilteredCols.Empty() {
 		// There are no unfiltered columns from the right; a valid foreign key
-		// relation is not possible.
+		// relation is not possible. This check, which is a duplicate of a check
+		// in verifyFiltersAreValidEqualities, is necessary in the case of a
+		// cross-join because verifyFiltersAreValidEqualities is not called.
 		return false
 	}
 
@@ -419,7 +524,7 @@ func checkForeignKeyCase(
 				rightColID := rightTableID.ColumnID(rightColOrd)
 				if !leftNotNullCols.Contains(leftColID) {
 					// The left column isn't a left not-null output column. It can't be
-					// used in a filter (since it isn't an output column) but the foreign key may still be valid.but it may still
+					// used in a filter (since it isn't an output column) but it may still
 					// be provably not null.
 					//
 					// A column that isn't in leftNotNullCols is guaranteed not-null if it
@@ -444,8 +549,11 @@ func checkForeignKeyCase(
 					continue
 				}
 				if !rightUnfilteredCols.Contains(rightColID) {
-					// The right column isn't guaranteed to be unfiltered. It can't be
-					// used in a filter.
+					// The right column isn't guaranteed to be unfiltered. It
+					// can't be used in a filter. This check, which is a
+					// duplicate of a check in verifyFiltersAreValidEqualities,
+					// is necessary in the case of a cross-join because
+					// verifyFiltersAreValidEqualities is not called.
 					continue
 				}
 				if filtersHaveEquality(filters, leftColID, rightColID) {