Merge #148105

148105: opt: avoid delete cascade fast path for regional by row tables r=mgartner a=DrewKimball

#### opt,testcat: allow defining the region column explicitly in opt tests

This commit adds support for explicitly defining the `crdb_region` column
for REGIONAL BY ROW tables in optimizer tests. This will allow testing
scenarios like a computed region column.

Epic: None

Release note: None

#### opt: avoid delete cascade fast path for regional by row tables

We have a fast path for cascading deletes where the filters from the
parent table are applied directly to the child table instead of joining
from a buffer to the child table. This can result in an efficient
`DeleteRange` operation in some cases, but in others, can result in
a "less constrained" scan on the child table. In particular, it can
result in visiting more regions than necessary for a regional-by-row
child table.

This commit prevents the fast path from applying for regional-by-row
child tables unless the region column is restricted to a single value.
It is possible to revert to the old behavior by disabling a new session
var: `optimizer_disable_cross_region_cascade_fast_path_for_rbr_tables`.

Informs: #146705

Release note (performance improvement): The optimizer will no longer
apply a fast-path to deletes cascading to regional-by-row tables. This
prevents the cascading delete from accessing more regions than necessary.

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

Author: craig[bot]

pkg/ccl/logictestccl/testdata/logic_test/regional_by_row_query_behavior

@@ -3179,11 +3179,17 @@ vectorized: true
 │   └── • delete
 │       │ from: user_settings_cascades
 │       │
-│       └── • scan
-│             missing stats
-│             table: user_settings_cascades@user_settings_cascades_user_id_idx
-│             spans: [/'ap-southeast-2'/'5ebfedee-0dcf-41e6-a315-5fa0b51b9882' - /'ap-southeast-2'/'5ebfedee-0dcf-41e6-a315-5fa0b51b9882'] [/'ca-central-1'/'5ebfedee-0dcf-41e6-a315-5fa0b51b9882' - /'ca-central-1'/'5ebfedee-0dcf-41e6-a315-5fa0b51b9882'] [/'us-east-1'/'5ebfedee-0dcf-41e6-a315-5fa0b51b9882' - /'us-east-1'/'5ebfedee-0dcf-41e6-a315-5fa0b51b9882']
-│             locking strength: for update
+│       └── • lookup join
+│           │ table: user_settings_cascades@user_settings_cascades_user_id_idx
+│           │ equality: (crdb_region, id) = (crdb_region, user_id)
+│           │
+│           └── • distinct
+│               │ estimated row count: 100
+│               │ distinct on: id, crdb_region
+│               │
+│               └── • scan buffer
+│                     estimated row count: 100
+│                     label: buffer 1000000
 │
 └── • constraint-check
     │

pkg/sql/exec_util.go

@@ -4194,6 +4194,10 @@ func (m *sessionDataMutator) SetUseImprovedRoutineDependencyTracking(val bool) {
 	m.data.UseImprovedRoutineDependencyTracking = val
 }
 
+func (m *sessionDataMutator) SetOptimizerDisableCrossRegionCascadeFastPathForRBRTables(val bool) {
+	m.data.OptimizerDisableCrossRegionCascadeFastPathForRBRTables = val
+}
+
 // Utility functions related to scrubbing sensitive information on SQL Stats.
 
 // quantizeCounts ensures that the Count field in the

pkg/sql/logictest/testdata/logic_test/information_schema

@@ -207,6 +207,7 @@ type Memo struct {
 	internal                                   bool
 	usePre_25_2VariadicBuiltins                bool
 	useExistsFilterHoistRule                   bool
+	disableSlowCascadeFastPathForRBRTables     bool
 
 	// txnIsoLevel is the isolation level under which the plan was created. This
 	// affects the planning of some locking operations, so it must be included in
@@ -311,6 +312,7 @@ func (m *Memo) Init(ctx context.Context, evalCtx *eval.Context) {
 		internal:                                   evalCtx.SessionData().Internal,
 		usePre_25_2VariadicBuiltins:                evalCtx.SessionData().UsePre_25_2VariadicBuiltins,
 		useExistsFilterHoistRule:                   evalCtx.SessionData().OptimizerUseExistsFilterHoistRule,
+		disableSlowCascadeFastPathForRBRTables:     evalCtx.SessionData().OptimizerDisableCrossRegionCascadeFastPathForRBRTables,
 		txnIsoLevel:                                evalCtx.TxnIsoLevel,
 	}
 	m.metadata.Init()
@@ -488,6 +490,7 @@ func (m *Memo) IsStale(
 		m.internal != evalCtx.SessionData().Internal ||
 		m.usePre_25_2VariadicBuiltins != evalCtx.SessionData().UsePre_25_2VariadicBuiltins ||
 		m.useExistsFilterHoistRule != evalCtx.SessionData().OptimizerUseExistsFilterHoistRule ||
+		m.disableSlowCascadeFastPathForRBRTables != evalCtx.SessionData().OptimizerDisableCrossRegionCascadeFastPathForRBRTables ||
 		m.txnIsoLevel != evalCtx.TxnIsoLevel {
 		return true, nil
 	}

pkg/sql/logictest/testdata/logic_test/pg_catalog

@@ -579,6 +579,11 @@ func TestMemoIsStale(t *testing.T) {
 	evalCtx.SessionData().OptimizerUseExistsFilterHoistRule = false
 	notStale()
 
+	evalCtx.SessionData().OptimizerDisableCrossRegionCascadeFastPathForRBRTables = true
+	stale()
+	evalCtx.SessionData().OptimizerDisableCrossRegionCascadeFastPathForRBRTables = false
+	notStale()
+
 	// User no longer has access to view.
 	catalog.View(tree.NewTableNameWithSchema("t", catconstants.PublicSchemaName, "abcview")).Revoked = true
 	_, err = o.Memo().IsStale(ctx, &evalCtx, catalog)

pkg/sql/logictest/testdata/logic_test/show_source

@@ -225,6 +225,39 @@ func (mb *mutationBuilder) tryNewOnDeleteFastCascadeBuilder(
 		return nil, false
 	}
 
+	// For a REGIONAL BY ROW child table, if the region column is part of the FK,
+	// check that it is constrained to a single value. If this is not the case,
+	// the fast path can be suboptimal, since joining against the parent buffer
+	// will constrain the region column to a single constant value.
+	if childTab.IsRegionalByRow() &&
+		mb.b.evalCtx.SessionData().OptimizerDisableCrossRegionCascadeFastPathForRBRTables {
+		// The regional column is the first in every index, including the primary.
+		regionalColOrd := childTab.Index(cat.PrimaryIndex).Column(0).Ordinal()
+		var regionalColID opt.ColumnID
+		for i, colID := range fkCols {
+			if fk.OriginColumnOrdinal(childTab, i) == regionalColOrd {
+				regionalColID = colID
+				break
+			}
+		}
+		if regionalColID != 0 {
+			regionalColIsConstrained := false
+			for i := range filters {
+				if eq, isEq := filters[i].Condition.(*memo.EqExpr); isEq {
+					if v, leftIsVar := eq.Left.(*memo.VariableExpr); leftIsVar && v.Col == regionalColID {
+						if opt.IsConstValueOp(eq.Right) {
+							regionalColIsConstrained = true
+							break
+						}
+					}
+				}
+			}
+			if !regionalColIsConstrained {
+				return nil, false
+			}
+		}
+	}
+
 	var visited intsets.Fast
 	parentTabID := parentTab.ID()
 	childTabID := childTab.ID()

pkg/sql/opt/memo/memo.go

@@ -115,15 +115,27 @@ func (tc *Catalog) CreateTable(stmt *tree.CreateTable) *Table {
 		// so use type STRING instead.
 		oid := types.String.Oid()
 
-		evalCtx := eval.MakeTestingEvalContext(cluster.MakeTestingClusterSettings())
-		crdbRegionDef :=
-			multiregion.RegionalByRowDefaultColDef(
-				oid,
-				multiregion.RegionalByRowGatewayRegionDefaultExpr(oid),
-				multiregion.MaybeRegionalByRowOnUpdateExpr(&evalCtx, oid),
-			)
-		crdbRegionDef.Type = types.String
-		stmt.Defs = append(stmt.Defs, crdbRegionDef)
+		// Add a region column only if one doesn't already exist.
+		hasRegionCol := false
+		for _, def := range stmt.Defs {
+			if colDef, ok := def.(*tree.ColumnTableDef); ok {
+				if colDef.Name == tree.RegionalByRowRegionDefaultColName {
+					hasRegionCol = true
+					break
+				}
+			}
+		}
+		if !hasRegionCol {
+			evalCtx := eval.MakeTestingEvalContext(cluster.MakeTestingClusterSettings())
+			crdbRegionDef :=
+				multiregion.RegionalByRowDefaultColDef(
+					oid,
+					multiregion.RegionalByRowGatewayRegionDefaultExpr(oid),
+					multiregion.MaybeRegionalByRowOnUpdateExpr(&evalCtx, oid),
+				)
+			crdbRegionDef.Type = types.String
+			stmt.Defs = append(stmt.Defs, crdbRegionDef)
+		}
 		tab.implicitRBRIndexElem =
 			&tree.IndexElem{
 				Column: tree.RegionalByRowRegionDefaultColName,