From b5e20416f8941f4a56edbef6dce81d6e3bf9c175 Mon Sep 17 00:00:00 2001
From: Oliver Tan <otan@cockroachlabs.com>
Date: Mon, 7 Aug 2023 15:33:07 +1000
Subject: [PATCH 1/8] roachtest/awsdms: run once a week instead

Save a bit of mad dosh by running awsdms once a weekly instead of daily.
We don't need this tested every week.

Epic: None

Release note: None
---
 pkg/cmd/roachtest/tests/awsdms.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/cmd/roachtest/tests/awsdms.go b/pkg/cmd/roachtest/tests/awsdms.go
index fc411f4b0ff7..046ac12cf1ef 100644
--- a/pkg/cmd/roachtest/tests/awsdms.go
+++ b/pkg/cmd/roachtest/tests/awsdms.go
@@ -192,7 +192,7 @@ func registerAWSDMS(r registry.Registry) {
 		Owner:   registry.OwnerMigrations,
 		Cluster: r.MakeClusterSpec(1),
 		Leases:  registry.MetamorphicLeases,
-		Tags:    registry.Tags(`default`, `awsdms`, `aws`),
+		Tags:    registry.Tags(`weekly`, `aws-weekly`),
 		Run:     runAWSDMS,
 	})
 }

From 895b19518d0b49c14ef07a721e11bf8c6460c60b Mon Sep 17 00:00:00 2001
From: Yahor Yuzefovich <yahor@cockroachlabs.com>
Date: Mon, 7 Aug 2023 13:13:33 -0700
Subject: [PATCH 2/8] rowexec: add log scopes to all tests

Release note: None
---
 pkg/sql/rowexec/aggregator_test.go              | 1 +
 pkg/sql/rowexec/backfiller_test.go              | 2 ++
 pkg/sql/rowexec/distinct_test.go                | 1 +
 pkg/sql/rowexec/filterer_test.go                | 2 ++
 pkg/sql/rowexec/hashjoiner_test.go              | 5 +++++
 pkg/sql/rowexec/inverted_expr_evaluator_test.go | 7 +++++++
 pkg/sql/rowexec/inverted_filterer_test.go       | 2 ++
 pkg/sql/rowexec/inverted_joiner_test.go         | 4 ++++
 pkg/sql/rowexec/mergejoiner_test.go             | 2 ++
 pkg/sql/rowexec/ordinality_test.go              | 1 +
 pkg/sql/rowexec/processors_test.go              | 5 +++++
 pkg/sql/rowexec/project_set_test.go             | 1 +
 pkg/sql/rowexec/sorter_test.go                  | 2 ++
 pkg/sql/rowexec/stats_test.go                   | 2 ++
 pkg/sql/rowexec/tablereader_test.go             | 3 ++-
 pkg/sql/rowexec/values_test.go                  | 2 ++
 pkg/sql/rowexec/windower_test.go                | 2 ++
 pkg/sql/rowexec/zigzagjoiner_test.go            | 7 +++++--
 18 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/pkg/sql/rowexec/aggregator_test.go b/pkg/sql/rowexec/aggregator_test.go
index d920c4973e91..dbb78bbe8b73 100644
--- a/pkg/sql/rowexec/aggregator_test.go
+++ b/pkg/sql/rowexec/aggregator_test.go
@@ -60,6 +60,7 @@ func aggregations(aggTestSpecs []aggTestSpec) []execinfrapb.AggregatorSpec_Aggre
 //	VARIANCE
 func TestAggregator(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	var (
 		col0              = []uint32{0}
diff --git a/pkg/sql/rowexec/backfiller_test.go b/pkg/sql/rowexec/backfiller_test.go
index 0439f3136b48..19bd4b001fee 100644
--- a/pkg/sql/rowexec/backfiller_test.go
+++ b/pkg/sql/rowexec/backfiller_test.go
@@ -30,6 +30,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
 	"github.com/cockroachdb/errors"
 	"github.com/stretchr/testify/require"
@@ -65,6 +66,7 @@ func TestingWriteResumeSpan(
 
 func TestWriteResumeSpan(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	ctx := context.Background()
 
diff --git a/pkg/sql/rowexec/distinct_test.go b/pkg/sql/rowexec/distinct_test.go
index 9e4e8e86eddb..d57825103b36 100644
--- a/pkg/sql/rowexec/distinct_test.go
+++ b/pkg/sql/rowexec/distinct_test.go
@@ -30,6 +30,7 @@ import (
 
 func TestDistinct(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	v := [15]rowenc.EncDatum{}
 	for i := range v {
diff --git a/pkg/sql/rowexec/filterer_test.go b/pkg/sql/rowexec/filterer_test.go
index 50cee8e05b76..c8a86816de64 100644
--- a/pkg/sql/rowexec/filterer_test.go
+++ b/pkg/sql/rowexec/filterer_test.go
@@ -30,6 +30,8 @@ import (
 
 func TestFilterer(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
 	v := [10]rowenc.EncDatum{}
 	for i := range v {
 		v[i] = rowenc.DatumToEncDatum(types.Int, tree.NewDInt(tree.DInt(i)))
diff --git a/pkg/sql/rowexec/hashjoiner_test.go b/pkg/sql/rowexec/hashjoiner_test.go
index 8bacc58bfab9..dad8e8f4aded 100644
--- a/pkg/sql/rowexec/hashjoiner_test.go
+++ b/pkg/sql/rowexec/hashjoiner_test.go
@@ -991,6 +991,7 @@ func mirrorJoinTypeAndOnExpr(
 
 func TestHashJoiner(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	testCases := hashJoinerTestCases()
 
@@ -1101,6 +1102,7 @@ func TestHashJoiner(t *testing.T) {
 
 func TestHashJoinerError(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	v := [10]rowenc.EncDatum{}
 	for i := range v {
@@ -1211,6 +1213,8 @@ func checkExpectedRows(
 // the consumer is draining.
 func TestHashJoinerDrain(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
 	v := [10]rowenc.EncDatum{}
 	for i := range v {
 		v[i] = rowenc.DatumToEncDatum(types.Int, tree.NewDInt(tree.DInt(i)))
@@ -1319,6 +1323,7 @@ func TestHashJoinerDrain(t *testing.T) {
 // joiner will drain both inputs.
 func TestHashJoinerDrainAfterBuildPhaseError(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	v := [10]rowenc.EncDatum{}
 	for i := range v {
diff --git a/pkg/sql/rowexec/inverted_expr_evaluator_test.go b/pkg/sql/rowexec/inverted_expr_evaluator_test.go
index 3d76cf00d55b..e2f304322852 100644
--- a/pkg/sql/rowexec/inverted_expr_evaluator_test.go
+++ b/pkg/sql/rowexec/inverted_expr_evaluator_test.go
@@ -18,6 +18,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/sql/inverted"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/stretchr/testify/require"
 )
 
@@ -89,6 +90,7 @@ func keyIndexesToString(indexes [][]KeyIndex) string {
 
 func TestSetContainerUnion(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	type testCase struct {
 		a        setContainer
@@ -111,6 +113,7 @@ func TestSetContainerUnion(t *testing.T) {
 
 func TestSetContainerIntersection(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	type testCase struct {
 		a        setContainer
@@ -139,6 +142,7 @@ type keyAndIndex struct {
 // Tests both invertedExprEvaluator and batchedInvertedExprEvaluator.
 func TestInvertedExpressionEvaluator(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	leaf1 := &spanExpression{
 		FactoredUnionSpans: []invertedSpan{{Start: []byte("a"), End: []byte("d")}},
@@ -302,6 +306,7 @@ func TestInvertedExpressionEvaluator(t *testing.T) {
 // overlapping spans.
 func TestFragmentedSpans(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	expr1 := inverted.SpanExpressionProto{
 		Node: spanExpression{
@@ -362,6 +367,8 @@ func (t *testPreFilterer) PreFilter(
 
 func TestInvertedExpressionEvaluatorPreFilter(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
 	// Setup expressions such that the same expression appears multiple times
 	// in a span.
 	leaf1 := &spanExpression{
diff --git a/pkg/sql/rowexec/inverted_filterer_test.go b/pkg/sql/rowexec/inverted_filterer_test.go
index 4b5bcb4adc5e..e71d0b69aaf9 100644
--- a/pkg/sql/rowexec/inverted_filterer_test.go
+++ b/pkg/sql/rowexec/inverted_filterer_test.go
@@ -23,6 +23,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/util/encoding"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
 )
 
 func intToEncodedInvertedVal(v int64) []byte {
@@ -38,6 +39,7 @@ func intSpanToEncodedSpan(start, end int64) inverted.SpanExpressionProto_Span {
 
 func TestInvertedFilterer(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	// Tests do simple intersection and reordering of columns, to exercise the
 	// plumbing in invertedFilterer -- all the heavy lifting for filtering is
diff --git a/pkg/sql/rowexec/inverted_joiner_test.go b/pkg/sql/rowexec/inverted_joiner_test.go
index 22f29a8f0d69..4ca0c27a627b 100644
--- a/pkg/sql/rowexec/inverted_joiner_test.go
+++ b/pkg/sql/rowexec/inverted_joiner_test.go
@@ -38,6 +38,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/encoding"
 	"github.com/cockroachdb/cockroach/pkg/util/json"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb"
 	"github.com/cockroachdb/errors"
@@ -199,6 +200,8 @@ func (jsonUnionExpr) PreFilter(_ inverted.EncVal, _ []interface{}, _ []bool) (bo
 
 func TestInvertedJoiner(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
 	ctx := context.Background()
 
 	s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
@@ -742,6 +745,7 @@ func TestInvertedJoiner(t *testing.T) {
 
 func TestInvertedJoinerDrain(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
 	defer s.Stopper().Stop(context.Background())
diff --git a/pkg/sql/rowexec/mergejoiner_test.go b/pkg/sql/rowexec/mergejoiner_test.go
index b9d6dee0bee6..4040a511a5b5 100644
--- a/pkg/sql/rowexec/mergejoiner_test.go
+++ b/pkg/sql/rowexec/mergejoiner_test.go
@@ -45,6 +45,7 @@ type mergeJoinerTestCase struct {
 
 func TestMergeJoiner(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	v := [10]rowenc.EncDatum{}
 	for i := range v {
@@ -761,6 +762,7 @@ func TestMergeJoiner(t *testing.T) {
 // Test that the joiner shuts down fine if the consumer is closed prematurely.
 func TestConsumerClosed(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	v := [10]rowenc.EncDatum{}
 	for i := range v {
diff --git a/pkg/sql/rowexec/ordinality_test.go b/pkg/sql/rowexec/ordinality_test.go
index 0beb075b6458..7921f5c4a550 100644
--- a/pkg/sql/rowexec/ordinality_test.go
+++ b/pkg/sql/rowexec/ordinality_test.go
@@ -30,6 +30,7 @@ import (
 
 func TestOrdinality(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	v := [15]rowenc.EncDatum{}
 	for i := range v {
diff --git a/pkg/sql/rowexec/processors_test.go b/pkg/sql/rowexec/processors_test.go
index 8dc856c4864c..9dea4d27b335 100644
--- a/pkg/sql/rowexec/processors_test.go
+++ b/pkg/sql/rowexec/processors_test.go
@@ -54,6 +54,7 @@ import (
 
 func TestPostProcess(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	v := [10]rowenc.EncDatum{}
 	for i := range v {
@@ -225,6 +226,7 @@ func TestPostProcess(t *testing.T) {
 
 func TestAggregatorSpecAggregationEquals(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	// Used for FilterColIdx *uint32.
 	colIdx1 := uint32(0)
@@ -315,6 +317,7 @@ func TestAggregatorSpecAggregationEquals(t *testing.T) {
 
 func TestProcessorBaseContext(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	// Use a custom context to distinguish it from the background one.
 	ctx := context.WithValue(context.Background(), struct{}{}, struct{}{})
@@ -428,6 +431,7 @@ func populateRangeCacheAndDisableBuffering(t *testing.T, db *gosql.DB, tableName
 // interesting to test is the integration between DistSQL and KV.
 func TestDrainingProcessorSwallowsUncertaintyError(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	// We're going to test by running a query that selects rows 1..10 with limit
 	// 5. Out of these, rows 1..5 are on node 1, 6..10 on node 2. We're going to
@@ -816,6 +820,7 @@ func TestUncertaintyErrorIsReturned(t *testing.T) {
 // instantiation of processors.
 func TestFlowConcurrentTxnUse(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	t.Run("TestSingleGoroutine", func(t *testing.T) {
 		flow := &flowinfra.FlowBase{}
diff --git a/pkg/sql/rowexec/project_set_test.go b/pkg/sql/rowexec/project_set_test.go
index 3ad08bb37235..3a53eba685f3 100644
--- a/pkg/sql/rowexec/project_set_test.go
+++ b/pkg/sql/rowexec/project_set_test.go
@@ -28,6 +28,7 @@ import (
 
 func TestProjectSet(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	v := [10]rowenc.EncDatum{}
 	for i := range v {
diff --git a/pkg/sql/rowexec/sorter_test.go b/pkg/sql/rowexec/sorter_test.go
index 170c42b91d15..09aace4ab32b 100644
--- a/pkg/sql/rowexec/sorter_test.go
+++ b/pkg/sql/rowexec/sorter_test.go
@@ -37,6 +37,7 @@ import (
 
 func TestSorter(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	v := [6]rowenc.EncDatum{}
 	for i := range v {
@@ -339,6 +340,7 @@ func TestSorter(t *testing.T) {
 // an invalid k-parameter.
 func TestSortInvalidLimit(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	spec := execinfrapb.SorterSpec{}
 	spec.Limit = 0
diff --git a/pkg/sql/rowexec/stats_test.go b/pkg/sql/rowexec/stats_test.go
index c6e78da648f3..513e01e24431 100644
--- a/pkg/sql/rowexec/stats_test.go
+++ b/pkg/sql/rowexec/stats_test.go
@@ -17,12 +17,14 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/testutils/distsqlutils"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
 )
 
 // TestInputStatCollector verifies that an inputStatCollector correctly collects
 // stats from an input.
 func TestInputStatCollector(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	const numRows = 100
 
diff --git a/pkg/sql/rowexec/tablereader_test.go b/pkg/sql/rowexec/tablereader_test.go
index 4f8fedc8e7fd..1ed3e352cd4f 100644
--- a/pkg/sql/rowexec/tablereader_test.go
+++ b/pkg/sql/rowexec/tablereader_test.go
@@ -45,8 +45,9 @@ import (
 
 func TestTableReader(t *testing.T) {
 	defer leaktest.AfterTest(t)()
-	ctx := context.Background()
+	defer log.Scope(t).Close(t)
 
+	ctx := context.Background()
 	s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
 	defer s.Stopper().Stop(ctx)
 
diff --git a/pkg/sql/rowexec/values_test.go b/pkg/sql/rowexec/values_test.go
index 177fda97b04c..443febbfa40d 100644
--- a/pkg/sql/rowexec/values_test.go
+++ b/pkg/sql/rowexec/values_test.go
@@ -31,6 +31,8 @@ import (
 
 func TestValuesProcessor(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
 	rng, _ := randutil.NewTestRand()
 	for _, numRows := range []int{0, 1, 10, 13, 15} {
 		for _, numCols := range []int{0, 1, 3} {
diff --git a/pkg/sql/rowexec/windower_test.go b/pkg/sql/rowexec/windower_test.go
index 8e5c148f8a62..d492ca7a9963 100644
--- a/pkg/sql/rowexec/windower_test.go
+++ b/pkg/sql/rowexec/windower_test.go
@@ -37,6 +37,8 @@ import (
 
 func TestWindowerAccountingForResults(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
 	ctx := context.Background()
 	st := cluster.MakeTestingClusterSettings()
 	monitor := mon.NewMonitorWithLimit(
diff --git a/pkg/sql/rowexec/zigzagjoiner_test.go b/pkg/sql/rowexec/zigzagjoiner_test.go
index 70cf4b8bafa8..59d39275bf89 100644
--- a/pkg/sql/rowexec/zigzagjoiner_test.go
+++ b/pkg/sql/rowexec/zigzagjoiner_test.go
@@ -30,6 +30,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb"
 	"github.com/stretchr/testify/require"
@@ -58,8 +59,9 @@ func encInt(i int) rowenc.EncDatum {
 
 func TestZigzagJoiner(t *testing.T) {
 	defer leaktest.AfterTest(t)()
-	ctx := context.Background()
+	defer log.Scope(t).Close(t)
 
+	ctx := context.Background()
 	s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
 	defer s.Stopper().Stop(ctx)
 
@@ -741,8 +743,9 @@ func TestZigzagJoiner(t *testing.T) {
 // is closed.
 func TestZigzagJoinerDrain(t *testing.T) {
 	defer leaktest.AfterTest(t)()
-	ctx := context.Background()
+	defer log.Scope(t).Close(t)
 
+	ctx := context.Background()
 	s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
 	defer s.Stopper().Stop(ctx)
 

From daab5110ba337aadd3fa1dff8e87639f6bba33d8 Mon Sep 17 00:00:00 2001
From: Yahor Yuzefovich <yahor@cockroachlabs.com>
Date: Mon, 7 Aug 2023 14:05:58 -0700
Subject: [PATCH 3/8] rowexec: fix TestUncertaintyErrorIsReturned under race

We just saw a case when `TestUncertaintyErrorIsReturned` failed under
race because we got a different DistSQL plan. This seems plausible in
case the range cache population (which the test does explicitly) isn't
quick enough for some reason, so this commit allows for the DistSQL plan
to match the expectation via `SucceedsSoon` (if we happen to get a bad
plan, then the following query execution should have the up-to-date
range cache).

Release note: None
---
 pkg/sql/rowexec/processors_test.go | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/pkg/sql/rowexec/processors_test.go b/pkg/sql/rowexec/processors_test.go
index 9dea4d27b335..1db8d43fbd4c 100644
--- a/pkg/sql/rowexec/processors_test.go
+++ b/pkg/sql/rowexec/processors_test.go
@@ -41,13 +41,13 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/distsqlutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
-	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
+	"github.com/cockroachdb/errors"
 	"github.com/jackc/pgx/v4"
 	"github.com/stretchr/testify/require"
 )
@@ -714,8 +714,6 @@ func TestUncertaintyErrorIsReturned(t *testing.T) {
 		// The default behavior is to enable uncertainty errors for a single random
 		// node.
 		overrideErrorOrigin []int
-		// if non-empty, this test will be skipped.
-		skip string
 	}{
 		{
 			query:           "SELECT * FROM t AS t1 JOIN t AS t2 ON t1.x = t2.x",
@@ -726,8 +724,8 @@ func TestUncertaintyErrorIsReturned(t *testing.T) {
 			expectedPlanURL: "Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJzElV1v2jwUgO_fX2Ed6VW3yZTY4TPSJKqSqWlpwgjTJlWoyojLvIY4sx0VVPHfpwBdIW0iwkWbCyTHh8ePfY5PHkH9icAC3x7Y52PE4zuBvoy8a3Rj_xgOzhwXfeg7_tj_OsDIvzgb2h_RNvTTJk6jMx9pghzXtUdo4HlX34bo0nPc7QxFnos0OV2gz0jT0-UEfb-wR_ZmpYFzZaOTPg9mMphb_58AhliEzA3mTIF1AwQwUMBgwgRDIsWUKSVkNvW4DnTCBVgGBh4nqc5eTzBMhWRgPYLmOmJgwTj4GbERC0Im6wZgCJkOeLTG656-Te7ZEjCciyidx8pCC4yysZ8E2ahWJwZMVhhEqp-XUDqYMbDIjpPTB8tY4cO1LgWPt1bmS6vlLQ8XgGEgxH2aoN-Cx0jEFuqRXdclRlI88LDQkOYMzSMNW4Xn9kKwsXeYgMFLdeaNexT3mrhnFsqaOdlWoeyzYxoLGTLJwj3ByeqV7biiJpJ6Nxf4ukojp9LdUyGHFxypWnB1YtTq9PCaI1XMdjLaeLOaaxxp2H6PmmvvydLDE00rJ5oatYOzTKto7Zxh882y3DzSsPMeWe5UaYMjphIRK3ZQ4zByK9VI1opYOGObvqVEKqdsKMV0HbsZemvQ-kXIlN7Mks3AiZ-mlJYsmP_7zuySSCmJFpPMPImWksw9EtkltfIks3x3RoXtNUpRzWISyZOapaRWMamRJ7WOPah2ntQuJXWKnWie1CkldYtJzType-zuOlm530Xi4ZaHYIGxfWqv_Dw9kP0hmKnszvm_xMMaO14m2Y25CyLFMFwH96zPNJNzHnOl-RQsLVO2Wv33NwAA___OinSA",
 		},
 		{
-			// This test reproduces 51458 and should be enabled once that issue is
-			// fixed.
+			// Reproduction of not propagating errors to all outputs of the
+			// hash router (#51458).
 			query:               "SELECT * FROM t JOIN onerow ON t.x = onerow.x",
 			expectedPlanURL:     "Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJy8lW9P4koUxt_fTzE5yY16M0inFPA2McEIN-JFcIFkNzHEjPQAjaXDzkwjxvDdN21xpYV2Kf7pC2WY098855knhxdQPz2wYdDqtC6HxPUngvzX792Qu9aP285Fu0uOm-3BcPCtQ8ng6uK2dULWpf_EdZpc99pdInyU4on0ukSfLsn5en26HJHvV61-KwZ32v-3yFHT5VPJ5_bfR0DBFw52-RwV2HfAgIIJFCoworCQYoxKCRluvUSFbWcJtkHB9ReBDr8eURgLiWC_gHa1h2DDkD942EfuoCwbQMFBzV0vwuuGvl884jNQuBReMPeVTZaUhOvBgoerUpkZMFpREIFeH_FGfngmM65mSWaDwWg1oqA0nyLYbEN3uwm2saIZ0t-4gS-kgxKdBHkUvvmnkh39X3E1uxauj7JsJaV6ONHHDXZyLt3pLPoEFHqBtkmD0YZJG5VU629tVVJtWe9oa4fmriiJRbme7n-nFCslpZ6QwvYPBysajjIzSmXzY_PBMtV_QT6qn5aPaqItc_9LMQtfimmUPvRGzEOlV5LHxAOwEf_bbmJj5rxLvpmSX8mU_wWBqn1aoGqZA2eHoj6qhfAV7jVPjNRJJRY2ic4UY9OUCOQYb6UYR7XxsheBovA4qHS8u160_dctpSXy-e-fgf1JVjbJKkaqZ5PO0iSWJhmbJDObxMw0ysxFnSVQRq5RlUMtZ8VIOZZXi5FyLP83TbIOtrySRlUPNWrr8vJJOUbVipFyjGJbOagVaM_cRG0ZlU-yskn1YqR6NolthbN-cBCscFxNPPF07zpgg7F-Sjv-vD4QvsCnKpyZg5l4irjD50U48SbcU0jhhj9iEzXKueu7SrtjsLUMcLX661cAAAD__3vS9aQ=",
 			overrideErrorOrigin: []int{0},
@@ -754,14 +752,13 @@ func TestUncertaintyErrorIsReturned(t *testing.T) {
 		}
 		for _, testCase := range testCases {
 			t.Run(testCase.query, func(t *testing.T) {
-				if testCase.skip != "" {
-					skip.IgnoreLint(t, testCase.skip)
-				}
 				func() {
 					_, err := defaultConn.Exec(ctx, fmt.Sprintf("set vectorize=%s", vectorizeOpt))
 					require.NoError(t, err)
-					func() {
-						// Check distsql plan.
+					// We allow for the DistSQL plan to be different for some
+					// time in case the range cache wasn't populated as we
+					// expected (we've seen this under race in #108250).
+					testutils.SucceedsSoon(t, func() error {
 						rows, err := defaultConn.Query(
 							ctx,
 							fmt.Sprintf("SELECT info FROM [EXPLAIN (DISTSQL, SHAPE) %s] WHERE info LIKE 'Diagram:%%'", testCase.query),
@@ -771,8 +768,14 @@ func TestUncertaintyErrorIsReturned(t *testing.T) {
 						rows.Next()
 						var actualPlanURL string
 						require.NoError(t, rows.Scan(&actualPlanURL))
-						require.Equal(t, testCase.expectedPlanURL, actualPlanURL)
-					}()
+						if testCase.expectedPlanURL != actualPlanURL {
+							return errors.Newf(
+								"DistSQL plans didn't match:\nexpected:%s\nactual: %s",
+								testCase.expectedPlanURL, actualPlanURL,
+							)
+						}
+						return nil
+					})
 
 					errorOrigin := []int{allNodeIdxs[rng.Intn(len(allNodeIdxs))]}
 					if testCase.overrideErrorOrigin != nil {

From eed5695314ef1a41a629513dd0f34db8eebb7663 Mon Sep 17 00:00:00 2001
From: Oliver Tan <otan@cockroachlabs.com>
Date: Tue, 8 Aug 2023 09:47:42 +1000
Subject: [PATCH 4/8] importer: fix stale comment on mysqlStrToDatum

Release note: None
Epic: None
---
 pkg/sql/importer/read_import_mysql.go | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/pkg/sql/importer/read_import_mysql.go b/pkg/sql/importer/read_import_mysql.go
index 4b49698bd532..887f25c8aa35 100644
--- a/pkg/sql/importer/read_import_mysql.go
+++ b/pkg/sql/importer/read_import_mysql.go
@@ -209,9 +209,10 @@ const (
 func mysqlStrToDatum(evalCtx *eval.Context, s string, desired *types.T) (tree.Datum, error) {
 	switch desired.Family() {
 	case types.BytesFamily:
-		//  mysql emits raw byte strings that do not use the same escaping as our ParseBytes
-		//	function expects, and the difference between ParseStringAs and
-		//	ParseDatumStringAs is whether or not it attempts to parse bytes.
+		// mysql emits raw byte strings that do not use the same escaping as our
+		// tree.ParseDBytes function expects, and the difference between
+		// tree.ParseAndRequireString and mysqlStrToDatum is whether or not it
+		// attempts to parse bytes.
 		return tree.NewDBytes(tree.DBytes(s)), nil
 	default:
 		res, _, err := tree.ParseAndRequireString(desired, s, evalCtx)

From 056c300112f038138dd295f0bf56a2515315903e Mon Sep 17 00:00:00 2001
From: Shivam Saraf <shivam.saraf@cockroachlabs.com>
Date: Thu, 30 Mar 2023 10:22:24 -0400
Subject: [PATCH 5/8] sql: replicating JSON empty array ordering found in
 Postgres

Currently, #97928 and #99275 are responsible for laying out a
lexicographical ordering for JSON columns to be forward indexable in
nature. This ordering is based on the rules posted by Postgres and is
in #99849.

However, Postgres currently sorts the empty JSON array before any other
JSON values. A Postgres bug report for this has been opened:
https://www.postgresql.org/message-id/17873-826fdc8bbcace4f1%40postgresql.org

This PR intends on replicating the Postgres behavior.

Fixes #105668

Epic: CRDB-24501

Release note: None
---
 docs/tech-notes/jsonb_forward_indexing.md     | 10 +++-
 .../logictest/testdata/logic_test/json_index  | 22 +++----
 pkg/sql/opt/exec/execbuilder/testdata/json    | 14 ++---
 pkg/sql/rowenc/keyside/json.go                |  2 +-
 pkg/util/encoding/encoding.go                 | 59 +++++++++++++------
 pkg/util/encoding/type_string.go              |  6 ++
 pkg/util/json/encoded.go                      | 12 +++-
 pkg/util/json/json.go                         | 44 ++++++++++++--
 pkg/util/json/json_test.go                    | 14 +++--
 9 files changed, 133 insertions(+), 50 deletions(-)

diff --git a/docs/tech-notes/jsonb_forward_indexing.md b/docs/tech-notes/jsonb_forward_indexing.md
index 732ceac1f6e4..f4b46830c8dc 100644
--- a/docs/tech-notes/jsonb_forward_indexing.md
+++ b/docs/tech-notes/jsonb_forward_indexing.md
@@ -44,10 +44,18 @@ The following rules were kept in mind while designing this form of encoding, as
 5. Objects with an equal number of key value pairs are compared in the order:
 `key1`, `value1`, `key2`, `value2`, ….
 
+**NOTE:** There is one exception to these rules, which is neither documented by
+Postgres, nor mentioned in the source code: empty arrays are the minimum JSON
+value. As far as we can tell, this is a Postgres bug that has existed for some
+time. We've decided to replicate this behavior to remain consistent with
+Postgres. We've filed a [Postgres bug report](https://www.postgresql.org/message-id/17873-826fdc8bbcace4f1%40postgresql.org)
+to track the issue.
+
 In order to satisfy property 1 at all times, tags are defined in an increasing order of bytes. 
 These tags will also have to be defined in a way where the tag representing an object is a large byte representation 
 for a hexadecimal value (such as 0xff) and the subsequent objects have a value 1 less than the previous one,
-where the ordering is described in point 1 above. 
+where the ordering is described in point 1 above. There is a special tag for empty JSON arrays
+in order to handle the special case of empty arrays being ordered before all other JSON values.
 
 Additionally, tags representing terminators will also be defined. There will be two terminators, one for the ascending designation and the other for the descending one, and will be required to denote the end of a key encoding of the following JSON values: Objects, Arrays, Number and Strings. JSON Boolean and JSON Null are not required to have the terminator since they do not have variable length encoding due to the presence of a single tag (as explained later in this document).
 
diff --git a/pkg/sql/logictest/testdata/logic_test/json_index b/pkg/sql/logictest/testdata/logic_test/json_index
index e27bd0e38ff0..2c1c076dd931 100644
--- a/pkg/sql/logictest/testdata/logic_test/json_index
+++ b/pkg/sql/logictest/testdata/logic_test/json_index
@@ -20,13 +20,13 @@ INSERT INTO t VALUES
 query T
 SELECT x FROM t ORDER BY x
 ----
+[]
 "a"
 "aa"
 "abcdefghi"
 "b"
 1
 100
-[]
 {"a": "b"}
 
 
@@ -38,13 +38,13 @@ INSERT INTO t VALUES
 query T
 SELECT x FROM t@t_pkey ORDER BY x
 ----
+[]
 "a"
 "aa"
 "abcdefghi"
 "b"
 1
 100
-[]
 {"a": "b"}
 
 # Use the index for point lookups.
@@ -77,12 +77,12 @@ query T
 SELECT x FROM t@t_pkey WHERE x > '1' ORDER BY x
 ----
 100
-[]
 {"a": "b"}
 
 query T
 SELECT x FROM t@t_pkey WHERE x < '1' ORDER BY x
 ----
+[]
 "a"
 "aa"
 "abcdefghi"
@@ -92,12 +92,12 @@ SELECT x FROM t@t_pkey WHERE x < '1' ORDER BY x
 query T
 SELECT x FROM t@t_pkey WHERE x > '1' OR x < '1' ORDER BY x
 ----
+[]
 "a"
 "aa"
 "abcdefghi"
 "b"
 100
-[]
 {"a": "b"}
 
 query T
@@ -109,12 +109,12 @@ query T
 SELECT x FROM t@t_pkey WHERE x > '1' OR x < '1' ORDER BY x DESC
 ----
 {"a": "b"}
-[]
 100
 "b"
 "abcdefghi"
 "aa"
 "a"
+[]
 
 # Adding more primitive JSON values.
 statement ok
@@ -129,6 +129,7 @@ INSERT INTO t VALUES
 query T
 SELECT x FROM t@t_pkey ORDER BY x
 ----
+[]
 null
 "Testing Punctuation?!."
 "a"
@@ -141,18 +142,17 @@ null
 100
 false
 true
-[]
 {"a": "b"}
 
 query T
 SELECT x FROM t@t_pkey WHERE x > 'true' ORDER BY x
 ----
-[]
 {"a": "b"}
 
 query T
 SELECT x FROM t@t_pkey WHERE x < 'false' ORDER BY x
 ----
+[]
 null
 "Testing Punctuation?!."
 "a"
@@ -330,12 +330,12 @@ query T
 SELECT x FROM t@t_pkey ORDER BY x
 ----
 NULL
+[]
 null
 "crdb"
 1
 false
 true
-[]
 [1, 2, 3]
 {}
 {"a": "b", "c": "d"}
@@ -346,24 +346,24 @@ SELECT x FROM t@t_pkey ORDER BY x DESC
 {"a": "b", "c": "d"}
 {}
 [1, 2, 3]
-[]
 true
 false
 1
 "crdb"
 null
+[]
 NULL
 
 # Test to show JSON Null is different from NULL.
 query T
 SELECT x FROM t@t_pkey WHERE x IS NOT NULL ORDER BY x
 ----
+[]
 null
 "crdb"
 1
 false
 true
-[]
 [1, 2, 3]
 {}
 {"a": "b", "c": "d"}
@@ -446,12 +446,12 @@ INSERT INTO t VALUES
 query T
 SELECT x FROM t@i ORDER BY x;
 ----
+[]
 null
 "crdb"
 1
 false
 true
-[]
 [null]
 [1]
 [{"a": "b"}]
diff --git a/pkg/sql/opt/exec/execbuilder/testdata/json b/pkg/sql/opt/exec/execbuilder/testdata/json
index 4d0adf966e3c..b03029d5db4f 100644
--- a/pkg/sql/opt/exec/execbuilder/testdata/json
+++ b/pkg/sql/opt/exec/execbuilder/testdata/json
@@ -205,7 +205,7 @@ vectorized: true
 • scan
   missing stats
   table: t@t_pkey
-  spans: [/'null' - /'null'] [/'""' - /'""'] [/'[]' - /'[]'] [/'{}' - /'{}']
+  spans: [/'[]' - /'[]'] [/'null' - /'null'] [/'""' - /'""'] [/'{}' - /'{}']
 
 # Multicolumn index, including JSONB
 
@@ -252,20 +252,20 @@ INSERT INTO composite VALUES (1, '1.00'::JSONB), (2, '1'::JSONB), (3, '2'::JSONB
  (4, '3.0'::JSONB), (5, '"a"'::JSONB)
 ----
 CPut /Table/108/1/1/0 -> /TUPLE/
-InitPut /Table/108/2/"G*\x02\x00\x00\x89\x88" -> /BYTES/0x2f0f0c200000002000000403348964
+InitPut /Table/108/2/"H*\x02\x00\x00\x89\x88" -> /BYTES/0x2f0f0c200000002000000403348964
 CPut /Table/108/1/2/0 -> /TUPLE/
-InitPut /Table/108/2/"G*\x02\x00\x00\x8a\x88" -> /BYTES/
+InitPut /Table/108/2/"H*\x02\x00\x00\x8a\x88" -> /BYTES/
 CPut /Table/108/1/3/0 -> /TUPLE/
-InitPut /Table/108/2/"G*\x04\x00\x00\x8b\x88" -> /BYTES/
+InitPut /Table/108/2/"H*\x04\x00\x00\x8b\x88" -> /BYTES/
 CPut /Table/108/1/4/0 -> /TUPLE/
-InitPut /Table/108/2/"G*\x06\x00\x00\x8c\x88" -> /BYTES/0x2f0f0c20000000200000040334891e
+InitPut /Table/108/2/"H*\x06\x00\x00\x8c\x88" -> /BYTES/0x2f0f0c20000000200000040334891e
 CPut /Table/108/1/5/0 -> /TUPLE/
-InitPut /Table/108/2/"F\x12a\x00\x01\x00\x8d\x88" -> /BYTES/
+InitPut /Table/108/2/"G\x12a\x00\x01\x00\x8d\x88" -> /BYTES/
 
 query T kvtrace
 SELECT j FROM composite where j = '1.00'::JSONB
 ----
-Scan /Table/108/2/"G*\x02\x00\x0{0"-1"}
+Scan /Table/108/2/"H*\x02\x00\x0{0"-1"}
 
 query T
 SELECT j FROM composite ORDER BY j;
diff --git a/pkg/sql/rowenc/keyside/json.go b/pkg/sql/rowenc/keyside/json.go
index d98dd74cc4e1..163ed3d43164 100644
--- a/pkg/sql/rowenc/keyside/json.go
+++ b/pkg/sql/rowenc/keyside/json.go
@@ -79,7 +79,7 @@ func decodeJSONKey(buf []byte, dir encoding.Direction) (json.JSON, []byte, error
 		}
 		buf = buf[1:] // removing the terminator
 		jsonVal = json.FromDecimal(dec)
-	case encoding.JSONArray, encoding.JSONArrayDesc:
+	case encoding.JSONArray, encoding.JSONArrayDesc, encoding.JsonEmptyArray, encoding.JsonEmptyArrayDesc:
 		jsonVal, buf, err = decodeJSONArray(buf, dir)
 		if err != nil {
 			return nil, nil, errors.NewAssertionErrorWithWrappedErrf(err, "could not decode JSON Array")
diff --git a/pkg/util/encoding/encoding.go b/pkg/util/encoding/encoding.go
index 9662e7f301a8..343471503c58 100644
--- a/pkg/util/encoding/encoding.go
+++ b/pkg/util/encoding/encoding.go
@@ -107,13 +107,18 @@ const (
 
 	// Defining different key markers, for the ascending designation,
 	// for handling different JSON values.
-	jsonNullKeyMarker   = voidMarker + 1
-	jsonStringKeyMarker = jsonNullKeyMarker + 1
-	jsonNumberKeyMarker = jsonStringKeyMarker + 1
-	jsonFalseKeyMarker  = jsonNumberKeyMarker + 1
-	jsonTrueKeyMarker   = jsonFalseKeyMarker + 1
-	jsonArrayKeyMarker  = jsonTrueKeyMarker + 1
-	jsonObjectKeyMarker = jsonArrayKeyMarker + 1
+
+	// Postgres currently has a special case (maybe a bug) where the empty JSON
+	// Array sorts before all other JSON values. See the bug report:
+	// https://www.postgresql.org/message-id/17873-826fdc8bbcace4f1%40postgresql.org
+	jsonEmptyArrayKeyMarker = voidMarker + 1
+	jsonNullKeyMarker       = jsonEmptyArrayKeyMarker + 1
+	jsonStringKeyMarker     = jsonNullKeyMarker + 1
+	jsonNumberKeyMarker     = jsonStringKeyMarker + 1
+	jsonFalseKeyMarker      = jsonNumberKeyMarker + 1
+	jsonTrueKeyMarker       = jsonFalseKeyMarker + 1
+	jsonArrayKeyMarker      = jsonTrueKeyMarker + 1
+	jsonObjectKeyMarker     = jsonArrayKeyMarker + 1
 
 	arrayKeyTerminator           byte = 0x00
 	arrayKeyDescendingTerminator byte = 0xFF
@@ -127,13 +132,14 @@ const (
 
 	// Defining different key markers, for the descending designation,
 	// for handling different JSON values.
-	jsonNullKeyDescendingMarker   = jsonObjectKeyMarker + 7
-	jsonStringKeyDescendingMarker = jsonNullKeyDescendingMarker - 1
-	jsonNumberKeyDescendingMarker = jsonStringKeyDescendingMarker - 1
-	jsonFalseKeyDescendingMarker  = jsonNumberKeyDescendingMarker - 1
-	jsonTrueKeyDescendingMarker   = jsonFalseKeyDescendingMarker - 1
-	jsonArrayKeyDescendingMarker  = jsonTrueKeyDescendingMarker - 1
-	jsonObjectKeyDescendingMarker = jsonArrayKeyDescendingMarker - 1
+	jsonEmptyArrayKeyDescendingMarker = jsonObjectKeyMarker + 8
+	jsonNullKeyDescendingMarker       = jsonEmptyArrayKeyDescendingMarker - 1
+	jsonStringKeyDescendingMarker     = jsonNullKeyDescendingMarker - 1
+	jsonNumberKeyDescendingMarker     = jsonStringKeyDescendingMarker - 1
+	jsonFalseKeyDescendingMarker      = jsonNumberKeyDescendingMarker - 1
+	jsonTrueKeyDescendingMarker       = jsonFalseKeyDescendingMarker - 1
+	jsonArrayKeyDescendingMarker      = jsonTrueKeyDescendingMarker - 1
+	jsonObjectKeyDescendingMarker     = jsonArrayKeyDescendingMarker - 1
 
 	// Terminators for JSON Key encoding.
 	jsonKeyTerminator           byte = 0x00
@@ -1789,6 +1795,9 @@ const (
 	JSONArrayDesc  Type = 39
 	JSONObject     Type = 40
 	JSONObjectDesc Type = 41
+	// Special case
+	JsonEmptyArray     Type = 42
+	JsonEmptyArrayDesc Type = 43
 )
 
 // typMap maps an encoded type byte to a decoded Type. It's got 256 slots, one
@@ -1849,6 +1858,10 @@ func slowPeekType(b []byte) Type {
 			return JSONArray
 		case m == jsonArrayKeyDescendingMarker:
 			return JSONArrayDesc
+		case m == jsonEmptyArrayKeyMarker:
+			return JsonEmptyArray
+		case m == jsonEmptyArrayKeyDescendingMarker:
+			return JsonEmptyArrayDesc
 		case m == jsonObjectKeyMarker:
 			return JSONObject
 		case m == jsonObjectKeyDescendingMarker:
@@ -2009,10 +2022,12 @@ func PeekLength(b []byte) (int, error) {
 		length, err := getArrayOrJSONLength(b[1:], dir, IsJSONKeyDone)
 		return 1 + length, err
 	case jsonArrayKeyMarker, jsonArrayKeyDescendingMarker,
-		jsonObjectKeyMarker, jsonObjectKeyDescendingMarker:
+		jsonObjectKeyMarker, jsonObjectKeyDescendingMarker,
+		jsonEmptyArrayKeyMarker, jsonEmptyArrayKeyDescendingMarker:
 		dir := Ascending
 		if (m == jsonArrayKeyDescendingMarker) ||
-			(m == jsonObjectKeyDescendingMarker) {
+			(m == jsonObjectKeyDescendingMarker) ||
+			(m == jsonEmptyArrayKeyDescendingMarker) {
 			dir = Descending
 		}
 		// removing the starter tag
@@ -3500,11 +3515,17 @@ func EncodeJSONTrueKeyMarker(buf []byte, dir Direction) []byte {
 
 // EncodeJSONArrayKeyMarker adds a JSON Array key encoding marker
 // to buf and returns the new buffer.
-func EncodeJSONArrayKeyMarker(buf []byte, dir Direction) []byte {
+func EncodeJSONArrayKeyMarker(buf []byte, dir Direction, arrayLength int64) []byte {
 	switch dir {
 	case Ascending:
+		if arrayLength == 0 {
+			return append(buf, jsonEmptyArrayKeyMarker)
+		}
 		return append(buf, jsonArrayKeyMarker)
 	case Descending:
+		if arrayLength == 0 {
+			return append(buf, jsonEmptyArrayKeyDescendingMarker)
+		}
 		return append(buf, jsonArrayKeyDescendingMarker)
 	default:
 		panic("invalid direction")
@@ -3621,7 +3642,7 @@ func ValidateAndConsumeJSONKeyMarker(buf []byte, dir Direction) ([]byte, Type, e
 	case Descending:
 		switch typ {
 		case JSONNullDesc, JSONNumberDesc, JSONStringDesc, JSONFalseDesc,
-			JSONTrueDesc, JSONArrayDesc, JSONObjectDesc:
+			JSONTrueDesc, JSONArrayDesc, JSONObjectDesc, JsonEmptyArrayDesc:
 			return buf[1:], typ, nil
 		default:
 			return nil, Unknown, errors.Newf("invalid type found %s", typ)
@@ -3629,7 +3650,7 @@ func ValidateAndConsumeJSONKeyMarker(buf []byte, dir Direction) ([]byte, Type, e
 	case Ascending:
 		switch typ {
 		case JSONNull, JSONNumber, JSONString, JSONFalse, JSONTrue, JSONArray,
-			JSONObject:
+			JSONObject, JsonEmptyArray:
 			return buf[1:], typ, nil
 		default:
 			return nil, Unknown, errors.Newf("invalid type found %s", typ)
diff --git a/pkg/util/encoding/type_string.go b/pkg/util/encoding/type_string.go
index 4bdb56a300a1..a6eaee2c3d5e 100644
--- a/pkg/util/encoding/type_string.go
+++ b/pkg/util/encoding/type_string.go
@@ -51,6 +51,8 @@ func _() {
 	_ = x[JSONArrayDesc-39]
 	_ = x[JSONObject-40]
 	_ = x[JSONObjectDesc-41]
+	_ = x[JsonEmptyArray-42]
+	_ = x[JsonEmptyArrayDesc-43]
 }
 
 func (i Type) String() string {
@@ -139,6 +141,10 @@ func (i Type) String() string {
 		return "JSONObject"
 	case JSONObjectDesc:
 		return "JSONObjectDesc"
+	case JsonEmptyArray:
+		return "JsonEmptyArray"
+	case JsonEmptyArrayDesc:
+		return "JsonEmptyArrayDesc"
 	default:
 		return "Type(" + strconv.FormatInt(int64(i), 10) + ")"
 	}
diff --git a/pkg/util/json/encoded.go b/pkg/util/json/encoded.go
index 24b8d19128b3..0afaf08a093d 100644
--- a/pkg/util/json/encoded.go
+++ b/pkg/util/json/encoded.go
@@ -606,10 +606,20 @@ func (j *jsonEncoded) AreKeysSorted() bool {
 	return decoded.AreKeysSorted()
 }
 
-func (j *jsonEncoded) Compare(other JSON) (int, error) {
+func (j *jsonEncoded) Compare(other JSON) (_ int, err error) {
 	if other == nil {
 		return -1, nil
 	}
+	// We must first check for the special case of empty arrays, which are the
+	// minimum JSON value.
+	switch {
+	case isEmptyArray(j) && isEmptyArray(other):
+		return 0, nil
+	case isEmptyArray(j):
+		return -1, nil
+	case isEmptyArray(other):
+		return 1, nil
+	}
 	if cmp := cmpJSONTypes(j.Type(), other.Type()); cmp != 0 {
 		return cmp, nil
 	}
diff --git a/pkg/util/json/json.go b/pkg/util/json/json.go
index 0327355ebae4..2113c64dfb36 100644
--- a/pkg/util/json/json.go
+++ b/pkg/util/json/json.go
@@ -579,9 +579,29 @@ func cmpJSONTypes(a Type, b Type) int {
 	return 0
 }
 
-func (j jsonNull) Compare(other JSON) (int, error)  { return cmpJSONTypes(j.Type(), other.Type()), nil }
-func (j jsonFalse) Compare(other JSON) (int, error) { return cmpJSONTypes(j.Type(), other.Type()), nil }
-func (j jsonTrue) Compare(other JSON) (int, error)  { return cmpJSONTypes(j.Type(), other.Type()), nil }
+// isEmptyArray returns true if j is a JSON array with length 0.
+func isEmptyArray(j JSON) bool {
+	return j.Type() == ArrayJSONType && j.Len() == 0
+}
+
+func (j jsonNull) Compare(other JSON) (int, error) {
+	if isEmptyArray(other) {
+		return 1, nil
+	}
+	return cmpJSONTypes(j.Type(), other.Type()), nil
+}
+func (j jsonFalse) Compare(other JSON) (int, error) {
+	if isEmptyArray(other) {
+		return 1, nil
+	}
+	return cmpJSONTypes(j.Type(), other.Type()), nil
+}
+func (j jsonTrue) Compare(other JSON) (int, error) {
+	if isEmptyArray(other) {
+		return 1, nil
+	}
+	return cmpJSONTypes(j.Type(), other.Type()), nil
+}
 
 func decodeIfNeeded(j JSON) (JSON, error) {
 	if enc, ok := j.(*jsonEncoded); ok {
@@ -595,6 +615,9 @@ func decodeIfNeeded(j JSON) (JSON, error) {
 }
 
 func (j jsonNumber) Compare(other JSON) (int, error) {
+	if isEmptyArray(other) {
+		return 1, nil
+	}
 	cmp := cmpJSONTypes(j.Type(), other.Type())
 	if cmp != 0 {
 		return cmp, nil
@@ -609,6 +632,9 @@ func (j jsonNumber) Compare(other JSON) (int, error) {
 }
 
 func (j jsonString) Compare(other JSON) (int, error) {
+	if isEmptyArray(other) {
+		return 1, nil
+	}
 	cmp := cmpJSONTypes(j.Type(), other.Type())
 	if cmp != 0 {
 		return cmp, nil
@@ -629,6 +655,14 @@ func (j jsonString) Compare(other JSON) (int, error) {
 }
 
 func (j jsonArray) Compare(other JSON) (int, error) {
+	switch {
+	case isEmptyArray(j) && isEmptyArray(other):
+		return 0, nil
+	case isEmptyArray(j):
+		return -1, nil
+	case isEmptyArray(other):
+		return 1, nil
+	}
 	cmp := cmpJSONTypes(j.Type(), other.Type())
 	if cmp != 0 {
 		return cmp, nil
@@ -660,6 +694,8 @@ func (j jsonArray) Compare(other JSON) (int, error) {
 }
 
 func (j jsonObject) Compare(other JSON) (int, error) {
+	// NOTE: There is no need to check if other is an empty array because all
+	// arrays are less than all objects, so the type comparison is sufficient.
 	cmp := cmpJSONTypes(j.Type(), other.Type())
 	if cmp != 0 {
 		return cmp, nil
@@ -1978,7 +2014,7 @@ func (j jsonTrue) EncodeForwardIndex(buf []byte, dir encoding.Direction) ([]byte
 }
 
 func (j jsonArray) EncodeForwardIndex(buf []byte, dir encoding.Direction) ([]byte, error) {
-	buf = encoding.EncodeJSONArrayKeyMarker(buf, dir)
+	buf = encoding.EncodeJSONArrayKeyMarker(buf, dir, int64(len(j)))
 	buf = encoding.EncodeJSONValueLength(buf, dir, int64(len(j)))
 
 	var err error
diff --git a/pkg/util/json/json_test.go b/pkg/util/json/json_test.go
index d7f6b59352f3..2e6c5fa1615d 100644
--- a/pkg/util/json/json_test.go
+++ b/pkg/util/json/json_test.go
@@ -66,6 +66,9 @@ func TestJSONOrdering(t *testing.T) {
 	// We test here that every element in order sorts before every one that comes
 	// after it, and is equal to itself.
 	sources := []string{
+		// In Postgres's sorting rules, the empty array comes before everything,
+		// even null.
+		`[]`,
 		`null`,
 		`"a"`,
 		`"aa"`,
@@ -76,10 +79,8 @@ func TestJSONOrdering(t *testing.T) {
 		`100`,
 		`false`,
 		`true`,
-		// In Postgres's sorting rules, the empty array comes before everything (even null),
-		// so this is a departure.
-		// Shorter arrays sort before longer arrays (this is the same as in Postgres).
-		`[]`,
+		// Shorter arrays sort before longer arrays (this is the same as in
+		// Postgres).
 		`[1]`,
 		`[2]`,
 		`[1, 2]`,
@@ -88,8 +89,9 @@ func TestJSONOrdering(t *testing.T) {
 		`{}`,
 		`{"a": 1}`,
 		`{"a": 2}`,
-		// In Postgres, keys which are shorter sort before keys which are longer. This
-		// is not true for us (right now). TODO(justin): unclear if it should be.
+		// In Postgres, keys which are shorter sort before keys which are
+		// longer. This is not true for us (right now).
+		// TODO(justin): unclear if it should be.
 		`{"aa": 1}`,
 		`{"b": 1}`,
 		`{"b": 2}`,

From f397c130b22872a2a8def4aa026ece936269b9ac Mon Sep 17 00:00:00 2001
From: Xiang Gu <xiang@cockroachlabs.com>
Date: Mon, 7 Aug 2023 12:35:26 -0400
Subject: [PATCH 6/8] schemachanger: Unskip some backup tests

Release note: None
---
 pkg/sql/schemachanger/sctest/backup.go    | 32 +++++------------------
 pkg/sql/schemachanger/sctest/framework.go |  1 -
 2 files changed, 6 insertions(+), 27 deletions(-)

diff --git a/pkg/sql/schemachanger/sctest/backup.go b/pkg/sql/schemachanger/sctest/backup.go
index 06a2c13b35da..215a892692d8 100644
--- a/pkg/sql/schemachanger/sctest/backup.go
+++ b/pkg/sql/schemachanger/sctest/backup.go
@@ -35,15 +35,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// TODO (xiang): Fix and unskip those flaky backup tests.
-func skipFlakyBackupTests(t *testing.T, path string) {
-	if strings.Contains(path, "alter_table_multiple_commands") ||
-		strings.Contains(path, "alter_table_alter_primary_key_drop_rowid") ||
-		strings.Contains(path, "drop_column_basic") {
-		skip.WithIssue(t, 108221, "skipping flaky tests")
-	}
-}
-
 // BackupSuccess tests that the schema changer can handle being backed up
 // any time during a successful schema change.
 func BackupSuccess(t *testing.T, path string, factory TestServerFactory) {
@@ -51,8 +42,6 @@ func BackupSuccess(t *testing.T, path string, factory TestServerFactory) {
 	skip.UnderStress(t)
 	skip.UnderRace(t)
 
-	skipFlakyBackupTests(t, path)
-
 	cumulativeTestForEachPostCommitStage(t, path, factory, func(t *testing.T, cs CumulativeTestCaseSpec) {
 		backupSuccess(t, factory, cs)
 	})
@@ -68,8 +57,6 @@ func BackupRollbacks(t *testing.T, path string, factory TestServerFactory) {
 	// and at least as expensive to run.
 	skip.UnderShort(t)
 
-	skipFlakyBackupTests(t, path)
-
 	cumulativeTestForEachPostCommitStage(t, path, factory, func(t *testing.T, cs CumulativeTestCaseSpec) {
 		backupRollbacks(t, factory, cs)
 	})
@@ -85,8 +72,6 @@ func BackupSuccessMixedVersion(t *testing.T, path string, factory TestServerFact
 	// and at least as expensive to run.
 	skip.UnderShort(t)
 
-	skipFlakyBackupTests(t, path)
-
 	factory = factory.WithMixedVersion()
 	cumulativeTestForEachPostCommitStage(t, path, factory, func(t *testing.T, cs CumulativeTestCaseSpec) {
 		backupSuccess(t, factory, cs)
@@ -103,8 +88,6 @@ func BackupRollbacksMixedVersion(t *testing.T, path string, factory TestServerFa
 	// and at least as expensive to run.
 	skip.UnderShort(t)
 
-	skipFlakyBackupTests(t, path)
-
 	factory = factory.WithMixedVersion()
 	cumulativeTestForEachPostCommitStage(t, path, factory, func(t *testing.T, cs CumulativeTestCaseSpec) {
 		backupRollbacks(t, factory, cs)
@@ -117,21 +100,17 @@ var runAllBackups = flag.Bool(
 	"if true, run all backups instead of a random subset",
 )
 
-// If the number of stages in the same phase exceeds skipThreshold, we enable
-// skipping such that the backup test for each stage is skipped with probability
-// skipRate.
-// Set runAllBackups to true to disable skipping altogether.
-const skipThreshold = 10
 const skipRate = .5
 
-func maybeRandomlySkip(t *testing.T, stageCountInPhase int) {
-	if !*runAllBackups && stageCountInPhase > skipThreshold && rand.Float64() < skipRate {
+func maybeRandomlySkip(t *testing.T) {
+	if !*runAllBackups && rand.Float64() < skipRate {
 		skip.IgnoreLint(t, "skipping due to randomness")
 	}
 }
 
 func backupSuccess(t *testing.T, factory TestServerFactory, cs CumulativeTestCaseSpec) {
-	maybeRandomlySkip(t, cs.StagesCount)
+	maybeRandomlySkip(t)
+	t.Parallel() // SAFE FOR TESTING (this comment is for the linter)
 	ctx := context.Background()
 	url := fmt.Sprintf("userfile://backups.public.userfiles_$user/data_%s_%d",
 		cs.Phase, cs.StageOrdinal)
@@ -240,7 +219,8 @@ func backupRollbacks(t *testing.T, factory TestServerFactory, cs CumulativeTestC
 	if cs.Phase != scop.PostCommitPhase {
 		return
 	}
-	maybeRandomlySkip(t, cs.StagesCount)
+	maybeRandomlySkip(t)
+	t.Parallel() // SAFE FOR TESTING (this comment is for the linter)
 	ctx := context.Background()
 	var urls atomic.Value
 	var dbForBackup atomic.Pointer[gosql.DB]
diff --git a/pkg/sql/schemachanger/sctest/framework.go b/pkg/sql/schemachanger/sctest/framework.go
index ed565317ec65..27f4e8a1be17 100644
--- a/pkg/sql/schemachanger/sctest/framework.go
+++ b/pkg/sql/schemachanger/sctest/framework.go
@@ -713,7 +713,6 @@ func cumulativeTestForEachPostCommitStage(
 			var hasFailed bool
 			for _, tc := range testCases {
 				fn := func(t *testing.T) {
-					t.Parallel() // SAFE FOR TESTING
 					tf(t, tc)
 				}
 				if hasFailed {

From eda4a6aba4e33af82dc706520c05054f0ae0b74a Mon Sep 17 00:00:00 2001
From: Rahul Aggarwal <rahul.aggarwal@cockroachlabs.com>
Date: Tue, 8 Aug 2023 13:21:32 -0400
Subject: [PATCH 7/8] go.mod: bump Pebble to fffe02a195e3

fffe02a1 db: simplify ScanInternal()
df7e2ae1 vfs: deflake TestDiskHealthChecking_Filesystem
ff5c929a Rate Limit Scan Statistics
af8c5f27 internal/cache: mark panic messages as redaction-safe

Epic: none
Release note: none
---
 DEPS.bzl                          | 6 +++---
 build/bazelutil/distdir_files.bzl | 2 +-
 go.mod                            | 2 +-
 go.sum                            | 4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/DEPS.bzl b/DEPS.bzl
index c3fd3de52bfa..96dd0e10c40b 100644
--- a/DEPS.bzl
+++ b/DEPS.bzl
@@ -1595,10 +1595,10 @@ def go_deps():
         patches = [
             "@com_github_cockroachdb_cockroach//build/patches:com_github_cockroachdb_pebble.patch",
         ],
-        sha256 = "f0319e618ed024b7d708e2c1f8cf0d4b9b7e4112943288ba6036e893a7f8c151",
-        strip_prefix = "github.com/cockroachdb/pebble@v0.0.0-20230807145728-40d3f411e45b",
+        sha256 = "0866be1de9e4ba30d2b03d1300ca796e88260e620671ab6099850dd576e074a8",
+        strip_prefix = "github.com/cockroachdb/pebble@v0.0.0-20230808154433-fffe02a195e3",
         urls = [
-            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230807145728-40d3f411e45b.zip",
+            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230808154433-fffe02a195e3.zip",
         ],
     )
     go_repository(
diff --git a/build/bazelutil/distdir_files.bzl b/build/bazelutil/distdir_files.bzl
index 335e46e6bd2b..e4eb42e23f87 100644
--- a/build/bazelutil/distdir_files.bzl
+++ b/build/bazelutil/distdir_files.bzl
@@ -320,7 +320,7 @@ DISTDIR_FILES = {
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/go-test-teamcity/com_github_cockroachdb_go_test_teamcity-v0.0.0-20191211140407-cff980ad0a55.zip": "bac30148e525b79d004da84d16453ddd2d5cd20528e9187f1d7dac708335674b",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/gostdlib/com_github_cockroachdb_gostdlib-v1.19.0.zip": "c4d516bcfe8c07b6fc09b8a9a07a95065b36c2855627cb3514e40c98f872b69e",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/logtags/com_github_cockroachdb_logtags-v0.0.0-20230118201751-21c54148d20b.zip": "ca7776f47e5fecb4c495490a679036bfc29d95bd7625290cfdb9abb0baf97476",
-    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230807145728-40d3f411e45b.zip": "f0319e618ed024b7d708e2c1f8cf0d4b9b7e4112943288ba6036e893a7f8c151",
+    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230808154433-fffe02a195e3.zip": "0866be1de9e4ba30d2b03d1300ca796e88260e620671ab6099850dd576e074a8",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/redact/com_github_cockroachdb_redact-v1.1.5.zip": "11b30528eb0dafc8bc1a5ba39d81277c257cbe6946a7564402f588357c164560",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/returncheck/com_github_cockroachdb_returncheck-v0.0.0-20200612231554-92cdbca611dd.zip": "ce92ba4352deec995b1f2eecf16eba7f5d51f5aa245a1c362dfe24c83d31f82b",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/sentry-go/com_github_cockroachdb_sentry_go-v0.6.1-cockroachdb.2.zip": "fbb2207d02aecfdd411b1357efe1192dbb827959e36b7cab7491731ac55935c9",
diff --git a/go.mod b/go.mod
index 434531f3324c..afedeb6829d5 100644
--- a/go.mod
+++ b/go.mod
@@ -116,7 +116,7 @@ require (
 	github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55
 	github.com/cockroachdb/gostdlib v1.19.0
 	github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b
-	github.com/cockroachdb/pebble v0.0.0-20230807145728-40d3f411e45b
+	github.com/cockroachdb/pebble v0.0.0-20230808154433-fffe02a195e3
 	github.com/cockroachdb/redact v1.1.5
 	github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd
 	github.com/cockroachdb/stress v0.0.0-20220803192808-1806698b1b7b
diff --git a/go.sum b/go.sum
index eec029d30833..de30406c04f9 100644
--- a/go.sum
+++ b/go.sum
@@ -493,8 +493,8 @@ github.com/cockroachdb/gostdlib v1.19.0/go.mod h1:+dqqpARXbE/gRDEhCak6dm0l14AaTy
 github.com/cockroachdb/logtags v0.0.0-20211118104740-dabe8e521a4f/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs=
 github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b h1:r6VH0faHjZeQy818SGhaone5OnYfxFR/+AzdY3sf5aE=
 github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs=
-github.com/cockroachdb/pebble v0.0.0-20230807145728-40d3f411e45b h1:ymYyDZy5WYRTBqPVYgy0XUW+gHx2HeRkyt1FJmNJVOo=
-github.com/cockroachdb/pebble v0.0.0-20230807145728-40d3f411e45b/go.mod h1:FN5O47SBEz5+kO9fG8UTR64g2WS1u5ZFCgTvxGjoSks=
+github.com/cockroachdb/pebble v0.0.0-20230808154433-fffe02a195e3 h1:LUfRb+Ibf/OrSFHSyjls7neeWBAIsK4d/SWkv7z1nLw=
+github.com/cockroachdb/pebble v0.0.0-20230808154433-fffe02a195e3/go.mod h1:FN5O47SBEz5+kO9fG8UTR64g2WS1u5ZFCgTvxGjoSks=
 github.com/cockroachdb/redact v1.1.3/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
 github.com/cockroachdb/redact v1.1.5 h1:u1PMllDkdFfPWaNGMyLD1+so+aq3uUItthCFqzwPJ30=
 github.com/cockroachdb/redact v1.1.5/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=

From c44ffa88e509720fbe873804cfa31b38966928ee Mon Sep 17 00:00:00 2001
From: Jayant Shrivastava <jayants@cockroachlabs.com>
Date: Tue, 8 Aug 2023 15:47:53 -0400
Subject: [PATCH 8/8] changefeedccl: deflake
 TestChangefeedSchemaChangeBackfillCheckpoint

Previously, the test `TestChangefeedSchemaChangeBackfillCheckpoint` would fail
because it would read a table span too early. A schema change using the delcarative
schema changer will update a table span to point to a new set of ranges. Previously,
this test would use the span from before the schema change, which is incorrect.
This change makes it use the span from after the schema change.

I stress tested this 30k times under the new schema changer and 10k times
under the legacy schema changer to ensure the test is not flaky anymore.

Fixes: https://github.com/cockroachdb/cockroach/issues/108084
Release note: None
Epic: None
---
 pkg/ccl/changefeedccl/changefeed_test.go | 53 +++++++++++++++---------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/pkg/ccl/changefeedccl/changefeed_test.go b/pkg/ccl/changefeedccl/changefeed_test.go
index 52083e5f4d62..6c5424d40c26 100644
--- a/pkg/ccl/changefeedccl/changefeed_test.go
+++ b/pkg/ccl/changefeedccl/changefeed_test.go
@@ -1944,15 +1944,17 @@ func TestChangefeedSchemaChangeBackfillCheckpoint(t *testing.T) {
 		changefeedbase.FrontierCheckpointMaxBytes.Override(
 			context.Background(), &s.Server.ClusterSettings().SV, maxCheckpointSize)
 
-		// Note the tableSpan to avoid resolved events that leave no gaps
-		fooDesc := desctestutils.TestingGetPublicTableDescriptor(
-			s.SystemServer.DB(), s.Codec, "d", "foo")
-		tableSpan := fooDesc.PrimaryIndexSpan(s.Codec)
+		var tableSpan roachpb.Span
+		refreshTableSpan := func() {
+			fooDesc := desctestutils.TestingGetPublicTableDescriptor(
+				s.SystemServer.DB(), s.Codec, "d", "foo")
+			tableSpan = fooDesc.PrimaryIndexSpan(s.Codec)
+		}
 
 		// FilterSpanWithMutation should ensure that once the backfill begins, the following resolved events
 		// that are for that backfill (are of the timestamp right after the backfill timestamp) resolve some
 		// but not all of the time, which results in a checkpoint eventually being created
-		haveGaps := false
+		numGaps := 0
 		var backfillTimestamp hlc.Timestamp
 		var initialCheckpoint roachpb.SpanGroup
 		var foundCheckpoint int32
@@ -1966,6 +1968,11 @@ func TestChangefeedSchemaChangeBackfillCheckpoint(t *testing.T) {
 			// timestamp such that all backfill spans have a timestamp of
 			// timestamp.Next().
 			if r.BoundaryType == expectedBoundaryType {
+				// NB: We wait until the schema change is public before looking
+				// up the table span. When using the declarative schema changer,
+				// the table span will be different before and after the schema
+				// change due to a primary index swap.
+				refreshTableSpan()
 				backfillTimestamp = r.Timestamp
 				return false, nil
 			}
@@ -1988,11 +1995,18 @@ func TestChangefeedSchemaChangeBackfillCheckpoint(t *testing.T) {
 				return !(backfillTimestamp.IsEmpty() || r.Timestamp.LessEq(backfillTimestamp.Next())), nil
 			}
 
-			// Only allow resolving if we definitely won't have a completely resolved table
-			if !r.Span.Equal(tableSpan) && haveGaps {
+			// At the end of a backfill, kv feed will emit a resolved span for the whole table.
+			// Filter this out because we would like to leave gaps.
+			if r.Span.Equal(tableSpan) {
+				return true, nil
+			}
+
+			// Ensure that we have at least 2 gaps, so when a second checkpoint happens later in this test,
+			// the second checkpoint can still leave at least one gap.
+			if numGaps >= 2 {
 				return rnd.Intn(10) > 7, nil
 			}
-			haveGaps = true
+			numGaps += 1
 			return true, nil
 		}
 
@@ -2021,7 +2035,7 @@ func TestChangefeedSchemaChangeBackfillCheckpoint(t *testing.T) {
 		// as well as the newly resolved ones
 		var secondCheckpoint roachpb.SpanGroup
 		foundCheckpoint = 0
-		haveGaps = false
+		numGaps = 0
 		knobs.FilterSpanWithMutation = func(r *jobspb.ResolvedSpan) (bool, error) {
 			// Stop resolving anything after second checkpoint set to avoid backfill completion
 			if secondCheckpoint.Len() > 0 {
@@ -2049,11 +2063,17 @@ func TestChangefeedSchemaChangeBackfillCheckpoint(t *testing.T) {
 
 			require.Falsef(t, initialCheckpoint.Encloses(r.Span), "second backfill should not resolve checkpointed span")
 
-			// Only allow resolving if we definitely won't have a completely resolved table
-			if !r.Span.Equal(tableSpan) && haveGaps {
+			// At the end of a backfill, kv feed will emit a resolved span for the whole table.
+			// Filter this out because we would like to leave at least one gap.
+			if r.Span.Equal(tableSpan) {
+				return true, nil
+			}
+
+			// Ensure there is at least one gap so that we can receive resolved spans later.
+			if numGaps >= 1 {
 				return rnd.Intn(10) > 7, nil
 			}
-			haveGaps = true
+			numGaps += 1
 			return true, nil
 		}
 
@@ -2092,15 +2112,10 @@ func TestChangefeedSchemaChangeBackfillCheckpoint(t *testing.T) {
 		// Pause job to avoid race on the resolved array
 		require.NoError(t, jobFeed.Pause())
 
-		// NB: With the declarative schema changer, there is a primary index swap,
-		// so the primary index span will change.
-		freshFooDesc := desctestutils.TestingGetPublicTableDescriptor(
-			s.SystemServer.DB(), s.Codec, "d", "foo")
-		tableSpanAfter := freshFooDesc.PrimaryIndexSpan(s.Codec)
-
 		// Verify that none of the resolved spans after resume were checkpointed.
+		t.Logf("Table Span: %s, Second Checkpoint: %v, Resolved Spans: %v", tableSpan, secondCheckpoint, resolved)
 		for _, sp := range resolved {
-			require.Falsef(t, !sp.Equal(tableSpanAfter) && secondCheckpoint.Contains(sp.Key), "span should not have been resolved: %s", sp)
+			require.Falsef(t, !sp.Equal(tableSpan) && secondCheckpoint.Contains(sp.Key), "span should not have been resolved: %s", sp)
 		}
 	}