opt: fix With and WithScan properties

The `With` and `WithScan` property builders start by copying the input
props and then correcting things. This is error-prone: we have in time
identified multiple things that were being incorrectly inherited.

This change rewrites the code to start with empty properties and then
fill things in explicitly (like most other operators). Known problems
that are fixed by this change:
 - `CanMutate` was incorrectly set for `WithScan`
 - Rule props like `PruneCols` and `WithUses` were incorrectly
   inherited by `With`.

Release note: None

pkg/sql/opt/memo/logical_props_builder.go

@@ -712,7 +712,7 @@ func (b *logicalPropsBuilder) buildBasicProps(e opt.Expr, cols opt.ColList, rel
 
 func (b *logicalPropsBuilder) buildWithProps(with *WithExpr, rel *props.Relational) {
 	// Copy over the props from the input.
-	*rel = *with.Input.Relational()
+	inputProps := with.Input.Relational()
 
 	BuildSharedProps(b.mem, with, &rel.Shared)
 
@@ -723,77 +723,72 @@ func (b *logicalPropsBuilder) buildWithProps(with *WithExpr, rel *props.Relation
 
 	// Output Columns
 	// --------------
-	// Passed through from the call above to b.buildProps.
+	// Inherited from the input expression.
+	rel.OutputCols = inputProps.OutputCols
 
 	// Not Null Columns
 	// ----------------
-	// Passed through from the call above to b.buildProps.
+	// Inherited from the input expression.
+	rel.NotNullCols = inputProps.NotNullCols
 
 	// Outer Columns
 	// -------------
-	// Passed through from the call above to b.buildProps.
+	// The outer columns are the union of the outer columns from Binding or Input,
+	// which is what is computed by the call to BuildSharedProps.
 
 	// Functional Dependencies
 	// -----------------------
-	// Passed through from the call above to b.buildProps.
+	rel.FuncDeps = inputProps.FuncDeps
 
 	// Cardinality
 	// -----------
-	// Passed through from the call above to b.buildProps.
+	rel.Cardinality = inputProps.Cardinality
 
 	// Statistics
 	// ----------
-	// Passed through from the call above to b.buildProps.
+	// Inherited from the input expression.
+	rel.Stats = inputProps.Stats
 }
 
-func (b *logicalPropsBuilder) buildWithScanProps(ref *WithScanExpr, rel *props.Relational) {
-	// WithScan inherits most of the logical properties of the expression it
-	// references.
-	*rel = *ref.BindingProps
-
-	// Things like PruneCols are not valid here.
-	rel.Rule = props.Relational{}.Rule
-
-	// Has Placeholder
-	// ---------------
-	// Overwrite this from the copied props.
-	rel.HasPlaceholder = false
+func (b *logicalPropsBuilder) buildWithScanProps(withScan *WithScanExpr, rel *props.Relational) {
+	BuildSharedProps(b.mem, withScan, &rel.Shared)
+	bindingProps := withScan.BindingProps
 
 	// Side Effects
 	// ------------
-	// Overwrite this from the copied props.
-	rel.CanHaveSideEffects = false
+	// WithScan has no side effects (even if the original expression had them).
 
 	// Output Columns
 	// --------------
-	rel.OutputCols = ref.OutCols.ToSet()
+	rel.OutputCols = withScan.OutCols.ToSet()
 
 	// Not Null Columns
 	// ----------------
-	rel.NotNullCols = translateColSet(rel.NotNullCols, ref.InCols, ref.OutCols)
+	rel.NotNullCols = translateColSet(bindingProps.NotNullCols, withScan.InCols, withScan.OutCols)
 
 	// Outer Columns
 	// -------------
-	rel.OuterCols = opt.ColSet{}
+	// No outer columns.
 
 	// Functional Dependencies
 	// -----------------------
-	rel.FuncDeps = props.FuncDepSet{}
-	rel.FuncDeps.CopyFrom(&ref.BindingProps.FuncDeps)
-	for i := range ref.InCols {
-		rel.FuncDeps.AddEquivalency(ref.InCols[i], ref.OutCols[i])
+	// Inherit dependencies from the referenced expression (remapping the
+	// columns).
+	rel.FuncDeps.CopyFrom(&bindingProps.FuncDeps)
+	for i := range withScan.InCols {
+		rel.FuncDeps.AddEquivalency(withScan.InCols[i], withScan.OutCols[i])
 	}
-	rel.FuncDeps.ProjectCols(ref.OutCols.ToSet())
+	rel.FuncDeps.ProjectCols(withScan.OutCols.ToSet())
 
 	// Cardinality
 	// -----------
-	// Copied from the referenced expression.
+	// Inherit from the referenced expression.
+	rel.Cardinality = bindingProps.Cardinality
 
 	// Statistics
 	// ----------
-	rel.Stats = props.Statistics{}
 	if !b.disableStats {
-		b.sb.buildWithScan(ref, rel)
+		b.sb.buildWithScan(withScan, rel)
 	}
 }
 

pkg/sql/opt/memo/testdata/logprops/with

@@ -9,7 +9,6 @@ with &1 (foo)
  ├── columns: x:3(int!null) y:4(int)
  ├── key: (3)
  ├── fd: (3)-->(4)
- ├── cte-uses: map[1:1]
  ├── scan xy
  │    ├── columns: xy.x:1(int!null) xy.y:2(int)
  │    ├── key: (1)
@@ -70,7 +69,6 @@ with &1 (foo)
  ├── side-effects
  ├── key: ()
  ├── fd: ()-->(3)
- ├── cte-uses: map[1:1]
  ├── project
  │    ├── columns: "?column?":1(int!null)
  │    ├── cardinality: [1 - 1]
@@ -113,7 +111,6 @@ with &1 (foo)
  ├── has-placeholder
  ├── key: ()
  ├── fd: ()-->(3)
- ├── cte-uses: map[1:1]
  ├── project
  │    ├── columns: int8:1(int)
  │    ├── cardinality: [1 - 1]
@@ -171,7 +168,6 @@ inner-join-apply
  │    ├── cardinality: [1 - 1]
  │    ├── key: ()
  │    ├── fd: ()-->(3)
- │    ├── cte-uses: map[1:1]
  │    ├── project
  │    │    ├── columns: "?column?":2(int)
  │    │    ├── outer: (1)

pkg/sql/opt/norm/testdata/rules/with

@@ -6,7 +6,6 @@ with &1 (foo)
  ├── cardinality: [2 - 2]
  ├── stats: [rows=2]
  ├── cost: 0.11
- ├── cte-uses: map[1:2]
  ├── project
  │    ├── columns: "?column?":1(int!null)
  │    ├── cardinality: [1 - 1]
@@ -257,7 +256,6 @@ with &1 (foo)
  ├── cost: 0.25
  ├── key: ()
  ├── fd: ()-->(3-6)
- ├── cte-uses: map[1:2 2:2]
  ├── values
  │    ├── columns: "?column?":1(int!null)
  │    ├── cardinality: [1 - 1]
@@ -425,7 +423,6 @@ with &1 (foo)
       │    ├──  a.s:4(string) => s:14(string)
       │    └──  a.j:5(jsonb) => j:15(jsonb)
       ├── cardinality: [1 - 1]
-      ├── mutations
       ├── key: ()
       └── fd: ()-->(11-15)
 

pkg/sql/opt/props/logical.go

@@ -435,6 +435,9 @@ func (s *Shared) Verify() {
 	if s.HasCorrelatedSubquery && !s.HasSubquery {
 		panic(errors.AssertionFailedf("HasSubquery cannot be false if HasCorrelatedSubquery is true"))
 	}
+	if s.CanMutate && !s.CanHaveSideEffects {
+		panic(errors.AssertionFailedf("CanHaveSideEffects cannot be false if CanMutate is true"))
+	}
 }
 
 // Verify runs consistency checks against the relational properties, in order to