multitenant: support crdb_internal.list_sql_keys_in_range() for secon…

…dary tenants

Fixes #95006

The crdb_internal.list_sql_keys_in_range builtin now uses the RangeDescIterator
and scopes the range span by the tenant span to prevent these errors:
```
ERROR: RangeIterator failed to seek to /Meta2/"\x00": rpc error: code = Unauthenticated desc = requested key /Meta2/"\x00" not fully contained in tenant keyspace /Tenant/1{2-3}
```

Release note: None

pkg/kv/kvclient/scan_meta.go

@@ -38,41 +38,3 @@ func ScanMetaKVs(ctx context.Context, txn *kv.Txn, span roachpb.Span) ([]kv.KeyV
 	}
 	return kvs, nil
 }
-
-// GetRangeWithID returns the RangeDescriptor with the requested id, or nil if
-// no such range exists. Note that it performs a scan over the meta.
-func GetRangeWithID(
-	ctx context.Context, txn *kv.Txn, id roachpb.RangeID,
-) (*roachpb.RangeDescriptor, error) {
-	// Scan the range meta K/V's to find the target range. We do this in a
-	// chunk-wise fashion to avoid loading all ranges into memory.
-	var ranges []kv.KeyValue
-	var err error
-	var rangeDesc roachpb.RangeDescriptor
-	const chunkSize = 100
-	metaStart := keys.RangeMetaKey(keys.MustAddr(keys.MinKey).Next())
-	metaEnd := keys.MustAddr(keys.Meta2Prefix.PrefixEnd())
-	for {
-		// Scan a batch of ranges.
-		ranges, err = txn.Scan(ctx, metaStart, metaEnd, chunkSize)
-		if err != nil {
-			return nil, err
-		}
-		// If no results were returned, then exit.
-		if len(ranges) == 0 {
-			break
-		}
-		for _, r := range ranges {
-			if err := r.ValueProto(&rangeDesc); err != nil {
-				return nil, err
-			}
-			// Look for a range that matches the target range ID.
-			if rangeDesc.RangeID == id {
-				return &rangeDesc, nil
-			}
-		}
-		// Set the next starting point to after the last key in this batch.
-		metaStart = keys.MustAddr(ranges[len(ranges)-1].Key.Next())
-	}
-	return nil, nil
-}

pkg/rpc/auth_tenant.go

@@ -147,6 +147,13 @@ func (a tenantAuthorizer) authorize(
 	}
 }
 
+func checkSpanBounds(rSpan, tenSpan roachpb.RSpan) error {
+	if !tenSpan.ContainsKeyRange(rSpan.Key, rSpan.EndKey) {
+		return authErrorf("requested key span %s not fully contained in tenant keyspace %s", rSpan, tenSpan)
+	}
+	return nil
+}
+
 // authBatch authorizes the provided tenant to invoke the Batch RPC with the
 // provided args.
 func (a tenantAuthorizer) authBatch(tenID roachpb.TenantID, args *roachpb.BatchRequest) error {
@@ -195,10 +202,7 @@ func (a tenantAuthorizer) authBatch(tenID roachpb.TenantID, args *roachpb.BatchR
 		return authError(err.Error())
 	}
 	tenSpan := tenantPrefix(tenID)
-	if !tenSpan.ContainsKeyRange(rSpan.Key, rSpan.EndKey) {
-		return authErrorf("requested key span %s not fully contained in tenant keyspace %s", rSpan, tenSpan)
-	}
-	return nil
+	return checkSpanBounds(rSpan, tenSpan)
 }
 
 func (a tenantAuthorizer) authGetRangeDescriptors(
@@ -229,10 +233,7 @@ func (a tenantAuthorizer) authRangeFeed(
 		return authError(err.Error())
 	}
 	tenSpan := tenantPrefix(tenID)
-	if !tenSpan.ContainsKeyRange(rSpan.Key, rSpan.EndKey) {
-		return authErrorf("requested key span %s not fully contained in tenant keyspace %s", rSpan, tenSpan)
-	}
-	return nil
+	return checkSpanBounds(rSpan, tenSpan)
 }
 
 // authGossipSubscription authorizes the provided tenant to invoke the
@@ -440,10 +441,7 @@ func validateSpan(tenID roachpb.TenantID, sp roachpb.Span) error {
 	if err != nil {
 		return authError(err.Error())
 	}
-	if !tenSpan.ContainsKeyRange(rSpan.Key, rSpan.EndKey) {
-		return authErrorf("requested key span %s not fully contained in tenant keyspace %s", rSpan, tenSpan)
-	}
-	return nil
+	return checkSpanBounds(rSpan, tenSpan)
 }
 
 // contextWithClientTenant inserts a tenant identifier in the context,

pkg/sql/faketreeeval/evalctx.go

@@ -454,6 +454,13 @@ func (ep *DummyEvalPlanner) IsANSIDML() bool {
 	return false
 }
 
+// GetRangeDescByID is part of the eval.Planner interface.
+func (ep *DummyEvalPlanner) GetRangeDescByID(
+	context.Context, roachpb.RangeID,
+) (rangeDesc roachpb.RangeDescriptor, err error) {
+	return
+}
+
 // DummyPrivilegedAccessor implements the tree.PrivilegedAccessor interface by returning errors.
 type DummyPrivilegedAccessor struct{}
 

pkg/sql/logictest/testdata/logic_test/sql_keys

@@ -1,4 +1,5 @@
-# LogicTest: local
+# LogicTest: local 3node-tenant
+# tenant-cluster-setting-override-opt: allow-split-at-for-secondary-tenants
 
 # This test depends on table ID's being stable, so add new tests at the bottom
 # of the file.
@@ -15,6 +16,14 @@ SELECT range_id FROM [SHOW RANGES FROM TABLE t] OFFSET 1 LIMIT 1
 let $tableid
 SELECT id FROM system.namespace WHERE name = 't'
 
+# wait until split happens on secondary tenant's end key or list_sql_keys_in_range will fail
+onlyif config 3node-tenant
+query TT
+SELECT start_key, end_key FROM [SHOW RANGES]
+----
+/Tenant/10                /Tenant/10/Table/106/1/0
+/Tenant/10/Table/106/1/0  /Tenant/11
+
 # Without any data in the table, there shouldn't be any keys in the range.
 query T
 SELECT key FROM crdb_internal.list_sql_keys_in_range($rangeid)
@@ -26,20 +35,36 @@ INSERT INTO t VALUES (1, 1), (2, 2)
 
 # List out all of the keys in this range. The values themselves are
 # different on each run of the test due to metadata stored in the value.
+onlyif config local
 query T
 SELECT key FROM crdb_internal.list_sql_keys_in_range($rangeid)
 ----
 /Table/106/1/1/0
 /Table/106/1/2/0
 
+onlyif config 3node-tenant
+query T
+SELECT key FROM crdb_internal.list_sql_keys_in_range($rangeid)
+----
+/Tenant/10/Table/106/1/1/0
+/Tenant/10/Table/106/1/2/0
+
 # List out all of the keys in this range. The values themselves are
 # different on each run of the test due to metadata stored in the value.
+onlyif config local
 query T
 SELECT crdb_internal.pretty_key(key, 0) FROM crdb_internal.scan(crdb_internal.table_span($tableid))
 ----
 /106/1/1/0
 /106/1/2/0
 
+onlyif config 3node-tenant
+query T
+SELECT crdb_internal.pretty_key(key, 0) FROM crdb_internal.scan(crdb_internal.table_span($tableid))
+----
+/10/Table/106/1/1/0
+/10/Table/106/1/2/0
+
 # An error should be returned when an invalid range ID is specified.
 statement error pq: range with ID 1000000 not found
 SELECT key FROM crdb_internal.list_sql_keys_in_range(1000000)
@@ -69,5 +94,10 @@ SELECT count(key), count(DISTINCT key) FROM crdb_internal.scan(crdb_internal.tab
 
 # Regression test for not closing the generator builtin if it encounters an
 # error in Start() (#87248).
+onlyif config local
 statement error failed to verify keys for Scan
 SELECT crdb_internal.scan('\xff':::BYTES, '\x3f5918':::BYTES);
+
+onlyif config 3node-tenant
+statement error not fully contained in tenant keyspace
+SELECT crdb_internal.scan('\xff':::BYTES, '\x3f5918':::BYTES);

pkg/sql/opt/BUILD.bazel

@@ -61,6 +61,7 @@ go_test(
     args = ["-test.timeout=55s"],
     embed = [":opt"],
     deps = [
+        "//pkg/roachpb",
         "//pkg/settings/cluster",
         "//pkg/sql/catalog/descpb",
         "//pkg/sql/opt/cat",

pkg/sql/opt/metadata_test.go

@@ -16,6 +16,7 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt"
@@ -461,3 +462,10 @@ func (f *fakeGetMultiregionConfigPlanner) GetMultiregionConfig(
 func (f *fakeGetMultiregionConfigPlanner) IsANSIDML() bool {
 	return false
 }
+
+// GetRangeDescByID is part of the eval.Planner interface.
+func (ep *fakeGetMultiregionConfigPlanner) GetRangeDescByID(
+	context.Context, roachpb.RangeID,
+) (rangeDesc roachpb.RangeDescriptor, err error) {
+	return
+}

pkg/sql/region_util.go

@@ -2397,6 +2397,29 @@ func (p *planner) IsANSIDML() bool {
 	return p.stmt.IsANSIDML()
 }
 
+// GetRangeDescByID is part of the eval.Planner interface.
+func (p *planner) GetRangeDescByID(
+	ctx context.Context, rangeID roachpb.RangeID,
+) (rangeDesc roachpb.RangeDescriptor, _ error) {
+	execCfg := p.execCfg
+	tenantSpan := execCfg.Codec.TenantSpan()
+	rangeDescIterator, err := execCfg.RangeDescIteratorFactory.NewIterator(ctx, tenantSpan)
+	if err != nil {
+		return rangeDesc, err
+	}
+	for rangeDescIterator.Valid() {
+		rangeDesc = rangeDescIterator.CurRangeDescriptor()
+		if rangeDesc.RangeID == rangeID {
+			break
+		}
+		rangeDescIterator.Next()
+	}
+	if !rangeDescIterator.Valid() {
+		return rangeDesc, errors.Newf("range with ID %d not found", rangeID)
+	}
+	return rangeDesc, nil
+}
+
 // OptimizeSystemDatabase is part of the eval.RegionOperator interface.
 func (p *planner) OptimizeSystemDatabase(ctx context.Context) error {
 	globalTables := []string{

pkg/sql/sem/builtins/generator_builtins.go

@@ -2230,6 +2230,7 @@ type rangeKeyIterator struct {
 	// by the constructor of the rangeKeyIterator.
 	rangeID roachpb.RangeID
 	spanKeyIterator
+	planner eval.Planner
 }
 
 var _ eval.ValueGenerator = &rangeKeyIterator{}
@@ -2246,12 +2247,14 @@ func makeRangeKeyIterator(
 	if !isAdmin {
 		return nil, pgerror.Newf(pgcode.InsufficientPrivilege, "user needs the admin role to view range data")
 	}
+	planner := evalCtx.Planner
 	rangeID := roachpb.RangeID(tree.MustBeDInt(args[0]))
 	return &rangeKeyIterator{
 		spanKeyIterator: spanKeyIterator{
-			acc: evalCtx.Planner.Mon().MakeBoundAccount(),
+			acc: planner.Mon().MakeBoundAccount(),
 		},
 		rangeID: rangeID,
+		planner: planner,
 	}, nil
 }
 
@@ -2261,17 +2264,14 @@ func (rk *rangeKeyIterator) ResolvedType() *types.T {
 }
 
 // Start implements the tree.ValueGenerator interface.
-func (rk *rangeKeyIterator) Start(ctx context.Context, txn *kv.Txn) error {
+func (rk *rangeKeyIterator) Start(ctx context.Context, txn *kv.Txn) (err error) {
 	// Scan the range meta K/V's to find the target range. We do this in a
 	// chunk-wise fashion to avoid loading all ranges into memory.
-	rangeDesc, err := kvclient.GetRangeWithID(ctx, txn, rk.rangeID)
+	rangeDesc, err := rk.planner.GetRangeDescByID(ctx, rk.rangeID)
 	if err != nil {
 		return err
 	}
-	if rangeDesc == nil {
-		return errors.Newf("range with ID %d not found", rk.rangeID)
-	}
-	rk.spanKeyIterator.span = roachpb.Span{Key: rangeDesc.StartKey.AsRawKey(), EndKey: rangeDesc.EndKey.AsRawKey()}
+	rk.span = rangeDesc.KeySpan().AsRawSpanWithNoLocals()
 	return rk.spanKeyIterator.Start(ctx, txn)
 }
 

pkg/sql/sem/eval/deps.go

@@ -357,6 +357,9 @@ type Planner interface {
 	// statements, SELECT, UPDATE, INSERT, DELETE, or an EXPLAIN of one of these
 	// statements.
 	IsANSIDML() bool
+
+	// GetRangeDescByID gets the RangeDescriptor by the specified RangeID.
+	GetRangeDescByID(context.Context, roachpb.RangeID) (roachpb.RangeDescriptor, error)
 }
 
 // InternalRows is an iterator interface that's exposed by the internal