Merge #40417 #40441 #40472

40417: sql: Remove unnecessary constraints check in show create r=rohany a=rohany

The show create table function would not display a zone configuration
if the constraints list was empty. However, this isn't a valid check
because the constraints list could be empty but other fields set.

Release note: None

40441: storage: assert zero HardState.Commit on replica creation r=nvanbenschoten a=nvanbenschoten

An uninitialized replica should never have a commit index in its
HardState. And yet, we observed in #40213 that (likely) due to a
missing Range deletion tombstone, this invariant can be violated
in practice.

This assertion will allow us to catch this kind of violation without
the need for a MsgVote to race with a MsgSnap, hopefully making the
bug implied by #40213 easier to track down.

Release note: None

40472: opt: detect zero-cardinality semi/anti joins r=rytaft a=rytaft

This commit adds two normalization rules to detect zero-cardinality
semi and anti joins, and replace them with an empty `Values`.

It also updates the stats estimate to ensure that the row count of semi
and anti joins is not zero unless the cardinality is zero.

Fixes #40456
Fixes #40440
Fixes #40394

Release note: None

Co-authored-by: Rohan Yadav <rohany@alumni.cmu.edu>
Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Co-authored-by: Rebecca Taft <becca@cockroachlabs.com>

pkg/ccl/logictestccl/testdata/logic_test/zone

@@ -593,3 +593,26 @@ PARTITION p1 OF INDEX "my database".public.t2@x1       ALTER PARTITION p1 OF IND
                                                        num_replicas = 1
 PARTITION p1 OF INDEX "my database".public.t2@x2       ALTER PARTITION p1 OF INDEX "my database".public.t2@x2 CONFIGURE ZONE USING
                                                        num_replicas = 1
+
+# regression for #40417
+statement ok
+CREATE TABLE t40417 (x INT PRIMARY KEY)
+
+statement ok
+ALTER TABLE t40417 PARTITION BY LIST (x) ( PARTITION p1 VALUES IN (1));
+
+statement ok
+ALTER PARTITION p1 OF TABLE t40417 CONFIGURE ZONE USING num_replicas = 1
+
+query TT
+SHOW CREATE TABLE t40417
+----
+t40417  CREATE TABLE t40417 (
+        x INT8 NOT NULL,
+        CONSTRAINT "primary" PRIMARY KEY (x ASC),
+        FAMILY "primary" (x)
+) PARTITION BY LIST (x) (
+  PARTITION p1 VALUES IN ((1))
+);
+ALTER PARTITION p1 OF INDEX "my database".public.t40417@primary CONFIGURE ZONE USING
+  num_replicas = 1

pkg/sql/crdb_internal.go

@@ -1215,7 +1215,8 @@ CREATE TABLE crdb_internal.create_statements (
 				if err != nil {
 					return err
 				}
-				if len(zoneConfig.Constraints) > 0 {
+				// If all constraints are default, then don't show anything.
+				if !zoneConfig.Equal(config.ZoneConfig{}) {
 					sqlString := string(tree.MustBeDString(row[2]))
 					zoneConfigStmts[tableName] = append(zoneConfigStmts[tableName], sqlString)
 				}

pkg/sql/opt/memo/statistics_builder.go

@@ -852,8 +852,10 @@ func (sb *statisticsBuilder) buildJoin(
 			s.RowCount = leftStats.RowCount
 
 		case opt.AntiJoinOp, opt.AntiJoinApplyOp:
-			s.RowCount = 0
-			s.Selectivity = 0
+			// Don't set the row count to 0 since we can't guarantee that the
+			// cardinality is 0.
+			s.RowCount = epsilon
+			s.Selectivity = epsilon
 		}
 		return
 	}

pkg/sql/opt/memo/testdata/logprops/join

@@ -1417,3 +1417,16 @@ project
  │    └── filters (true)
  └── projections
       └── variable: mn.m [type=int, outer=(13)]
+
+# Regression test #40456.
+opt
+SELECT NULL
+FROM uv
+WHERE NOT EXISTS(SELECT uv.u);
+----
+values
+ ├── columns: "?column?":5(unknown!null)
+ ├── cardinality: [0 - 0]
+ ├── key: ()
+ ├── fd: ()-->(5)
+ └── prune: (5)

pkg/sql/opt/memo/testdata/stats/join

@@ -1380,3 +1380,65 @@ full-join (hash)
            │                   ├── fd: ()-->(7)
            │                   └── (NULL,) [type=tuple{unknown}]
            └── false [type=bool]
+
+expr
+(SemiJoin
+    (Values
+      [ (Tuple [ (Const 1) (Const 2) ] "tuple{int}" ) ]
+      [ (Cols [ (NewColumn "a" "int") (NewColumn "b" "int") ]) ]
+    )
+    (Scan [ (Table "uv") (Cols "u,v,rowid") ])
+    []
+    []
+)
+----
+semi-join (hash)
+ ├── columns: a:1(int!null) b:2(int!null)
+ ├── cardinality: [0 - 1]
+ ├── stats: [rows=1]
+ ├── key: ()
+ ├── fd: ()-->(1,2)
+ ├── values
+ │    ├── columns: a:1(int!null) b:2(int!null)
+ │    ├── cardinality: [1 - 1]
+ │    ├── stats: [rows=1]
+ │    ├── key: ()
+ │    ├── fd: ()-->(1,2)
+ │    └── (1, 2) [type=tuple{int}]
+ ├── scan uv
+ │    ├── columns: u:3(int) v:4(int!null) rowid:5(int!null)
+ │    ├── stats: [rows=10000]
+ │    ├── key: (5)
+ │    └── fd: (5)-->(3,4)
+ └── filters (true)
+
+expr
+(AntiJoin
+    (Values
+      [ (Tuple [ (Const 1) (Const 2) ] "tuple{int}" ) ]
+      [ (Cols [ (NewColumn "a" "int") (NewColumn "b" "int") ]) ]
+    )
+    (Scan [ (Table "uv") (Cols "u,v,rowid") ])
+    []
+    []
+)
+----
+anti-join (hash)
+ ├── columns: a:1(int!null) b:2(int!null)
+ ├── cardinality: [0 - 1]
+ ├── stats: [rows=1e-10]
+ ├── key: ()
+ ├── fd: ()-->(1,2)
+ ├── values
+ │    ├── columns: a:1(int!null) b:2(int!null)
+ │    ├── cardinality: [1 - 1]
+ │    ├── stats: [rows=1]
+ │    ├── key: ()
+ │    ├── fd: ()-->(1,2)
+ │    └── (1, 2) [type=tuple{int}]
+ ├── scan uv
+ │    ├── columns: u:3(int) v:4(int!null) rowid:5(int!null)
+ │    ├── stats: [rows=10000]
+ │    ├── key: (5)
+ │    └── fd: (5)-->(3,4)
+ └── filters (true)

pkg/sql/opt/norm/rules/join.opt

@@ -325,6 +325,16 @@
 =>
 $left
 
+# SimplifyZeroCardinalitySemiJoin converts a SemiJoin operator to an empty
+# Values when it's known that the right input never returns any rows.
+[SimplifyZeroCardinalitySemiJoin, Normalize]
+(SemiJoin | SemiJoinApply
+    $left:*
+    $right:* & (HasZeroRows $right)
+)
+=>
+(ConstructEmptyValues (OutputCols $left))
+
 # EliminateAntiJoin discards an AntiJoin operator when it's known that the right
 # input never returns any rows.
 [EliminateAntiJoin, Normalize]
@@ -335,6 +345,18 @@ $left
 =>
 $left
 
+# SimplifyZeroCardinalityAntiJoin converts an AntiJoin operator to an empty
+# Values when it's known that the right input never returns zero rows, and
+# there is no join condition.
+[SimplifyZeroCardinalityAntiJoin, Normalize]
+(AntiJoin | AntiJoinApply
+    $left:*
+    $right:* & ^(CanHaveZeroRows $right)
+    []
+)
+=>
+(ConstructEmptyValues (OutputCols $left))
+
 # EliminateJoinNoColsLeft eliminates an InnerJoin with a one row, zero column
 # left input set. These can be produced when a Values, scalar GroupBy, or other
 # one-row operator's columns are never used.

pkg/sql/opt/norm/testdata/rules/join

@@ -1841,6 +1841,27 @@ scan a
  ├── key: (1)
  └── fd: (1)-->(2-5)
 
+opt expect=EliminateSemiJoin
+SELECT * FROM a WHERE EXISTS(VALUES (k))
+----
+scan a
+ ├── columns: k:1(int!null) i:2(int) f:3(float!null) s:4(string) j:5(jsonb)
+ ├── key: (1)
+ └── fd: (1)-->(2-5)
+
+# --------------------------------------------------
+# SimplifyZeroCardinalitySemiJoin
+# --------------------------------------------------
+# TODO(justin): figure out if there's a good way to make this still apply.
+opt disable=SimplifyZeroCardinalityGroup expect=SimplifyZeroCardinalitySemiJoin
+SELECT * FROM a WHERE EXISTS(SELECT * FROM (VALUES (k)) OFFSET 1)
+----
+values
+ ├── columns: k:1(int!null) i:2(int!null) f:3(float!null) s:4(string!null) j:5(jsonb!null)
+ ├── cardinality: [0 - 0]
+ ├── key: ()
+ └── fd: ()-->(1-5)
+
 # --------------------------------------------------
 # EliminateAntiJoin
 # --------------------------------------------------
@@ -1853,6 +1874,27 @@ scan a
  ├── key: (1)
  └── fd: (1)-->(2-5)
 
+# --------------------------------------------------
+# SimplifyZeroCardinalityAntiJoin
+# --------------------------------------------------
+opt expect=SimplifyZeroCardinalityAntiJoin
+SELECT * FROM a WHERE NOT EXISTS(SELECT count(*) FROM b WHERE x=k)
+----
+values
+ ├── columns: k:1(int!null) i:2(int!null) f:3(float!null) s:4(string!null) j:5(jsonb!null)
+ ├── cardinality: [0 - 0]
+ ├── key: ()
+ └── fd: ()-->(1-5)
+
+opt expect=SimplifyZeroCardinalityAntiJoin
+SELECT * FROM a WHERE NOT EXISTS(VALUES (k))
+----
+values
+ ├── columns: k:1(int!null) i:2(int!null) f:3(float!null) s:4(string!null) j:5(jsonb!null)
+ ├── cardinality: [0 - 0]
+ ├── key: ()
+ └── fd: ()-->(1-5)
+
 # --------------------------------------------------
 # EliminateJoinNoColsLeft
 # --------------------------------------------------

pkg/storage/stateloader/stateloader.go

@@ -574,7 +574,7 @@ func (rsl StateLoader) LoadHardState(
 
 // SetHardState overwrites the HardState.
 func (rsl StateLoader) SetHardState(
-	ctx context.Context, batch engine.Writer, st raftpb.HardState,
+	ctx context.Context, batch engine.Writer, hs raftpb.HardState,
 ) error {
 	// "Blind" because ms == nil and timestamp == hlc.Timestamp{}.
 	return engine.MVCCBlindPutProto(
@@ -583,7 +583,7 @@ func (rsl StateLoader) SetHardState(
 		nil, /* ms */
 		rsl.RaftHardStateKey(),
 		hlc.Timestamp{}, /* timestamp */
-		&st,
+		&hs,
 		nil, /* txn */
 	)
 }

pkg/storage/store.go

@@ -4006,6 +4006,17 @@ func (s *Store) tryGetOrCreateReplica(
 	s.mu.uninitReplicas[repl.RangeID] = repl
 	s.mu.Unlock()
 
+	// An uninitiazlied replica should have an empty HardState.Commit at
+	// all times. Failure to maintain this invariant indicates corruption.
+	// And yet, we have observed this in the wild. See #40213.
+	if hs, err := repl.mu.stateLoader.LoadHardState(ctx, s.Engine()); err != nil {
+		repl.mu.Unlock()
+		repl.raftMu.Unlock()
+		return nil, false, err
+	} else if hs.Commit != 0 {
+		log.Fatalf(ctx, "found non-zero HardState.Commit on uninitialized replica %s. HS=%+v", repl, hs)
+	}
+
 	desc := &roachpb.RangeDescriptor{
 		RangeID: rangeID,
 		// TODO(bdarnell): other fields are unknown; need to populate them from

pkg/storage/store_test.go

@@ -2863,7 +2863,8 @@ func TestStoreRemovePlaceholderOnRaftIgnored(t *testing.T) {
 	s := tc.store
 	ctx := context.Background()
 
-	// Clobber the existing range so we can test nonoverlapping placeholders.
+	// Clobber the existing range and recreated it with an uninitialized
+	// descriptor so we can test nonoverlapping placeholders.
 	repl1, err := s.GetReplica(1)
 	if err != nil {
 		t.Fatal(err)
@@ -2874,13 +2875,21 @@ func TestStoreRemovePlaceholderOnRaftIgnored(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	uninitDesc := roachpb.RangeDescriptor{RangeID: repl1.Desc().RangeID}
 	cv := s.ClusterSettings().Version.Version().Version
 	if _, err := stateloader.WriteInitialState(
-		ctx, s.Engine(), enginepb.MVCCStats{}, *repl1.Desc(), roachpb.Lease{},
+		ctx, s.Engine(), enginepb.MVCCStats{}, uninitDesc, roachpb.Lease{},
 		hlc.Timestamp{}, cv, stateloader.TruncatedStateUnreplicated,
 	); err != nil {
 		t.Fatal(err)
 	}
+	uninitRepl1, err := NewReplica(&uninitDesc, s, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := s.addReplicaToRangeMapLocked(uninitRepl1); err != nil {
+		t.Fatal(err)
+	}
 
 	// Generate a minimal fake snapshot.
 	snapData := &roachpb.RaftSnapshotData{}