Merge #150188 #151770

150188: sql: refactor lease helper for single-descriptor lookup r=bghal a=bghal

Adds a helper for single-descriptor use.

Epic: CRDB-49398

Part of: #138691

Release note: None


151770: rowenc: unskip JSON in TestEncDatumCompare r=yuzefovich a=yuzefovich

We introduced the key-encoding of JSONs some time ago, so we no longer need to skip this type in `TestEncDatumCompare`.

Epic: None
Release note: None

Co-authored-by: Brendan Gerrity <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>

Author: 3 people

pkg/sql/catalog/lease/lease.go

@@ -144,8 +144,8 @@ func (m *Manager) WaitForNoVersion(
 	return nil
 }
 
-// maybeGetDescriptorsWithoutValidation gets a descriptor without validating
-// from the KV layer.
+// maybeGetDescriptorsWithoutValidation gets descriptors without validating from
+// the KV layer.
 func (m *Manager) maybeGetDescriptorsWithoutValidation(
 	ctx context.Context, ids descpb.IDs, existenceExpected bool,
 ) (catalog.Descriptors, error) {
@@ -158,6 +158,7 @@ func (m *Manager) maybeGetDescriptorsWithoutValidation(
 		if err != nil {
 			return err
 		}
+
 		for _, id := range ids {
 			desc := c.LookupDescriptor(id)
 			if desc == nil {
@@ -177,6 +178,19 @@ func (m *Manager) maybeGetDescriptorsWithoutValidation(
 	return descs, nil
 }
 
+// maybeGetDescriptorWithoutValidation gets a descriptor without validating
+// from the KV layer.
+func (m *Manager) maybeGetDescriptorWithoutValidation(
+	ctx context.Context, id descpb.ID, existenceExpected bool,
+) (catalog.Descriptor, error) {
+	descArr, err := m.maybeGetDescriptorsWithoutValidation(ctx, descpb.IDs{id}, existenceExpected)
+	if err != nil {
+		return nil, err
+	}
+
+	return descArr[0], nil
+}
+
 // countDescriptorsHeldBySessionIDs can be used to make sure certain nodes
 // (sessions) observe the existence of a given set of descriptors. Assuming the given
 // sessions are still alive.
@@ -453,27 +467,28 @@ func (m *Manager) WaitForInitialVersion(
 // invariant that no new leases for desc.Version-1 will be granted once
 // desc.Version exists.
 //
-// If the descriptor is not found, an error will be returned. The error
-// can be detected by using errors.Is(err, catalog.ErrDescriptorNotFound).
+// If the descriptor is not found, an error will be returned.
 func (m *Manager) WaitForOneVersion(
 	ctx context.Context,
 	id descpb.ID,
 	regions regionliveness.CachedDatabaseRegions,
 	retryOpts retry.Options,
-) (desc catalog.Descriptor, _ error) {
+) (catalog.Descriptor, error) {
 	// Increment the long wait gauge for wait for one version, if this function
 	// takes longer than the lease duration.
 	decAfterWait := m.IncGaugeAfterLeaseDuration(GaugeWaitForOneVersion)
 	defer decAfterWait()
 	wsTracker := startWaitStatsTracker(ctx)
 	defer wsTracker.end()
+
+	var desc catalog.Descriptor
 	for lastCount, r := 0, retry.Start(retryOpts); r.Next(); {
 		var err error
-		var descArr catalog.Descriptors
-		if descArr, err = m.maybeGetDescriptorsWithoutValidation(ctx, descpb.IDs{id}, true); err != nil {
+		desc, err = m.maybeGetDescriptorWithoutValidation(ctx, id, true)
+		if err != nil {
 			return nil, err
 		}
-		desc = descArr[0]
+
 		// Check to see if there are any leases that still exist on the previous
 		// version of the descriptor.
 		now := m.storage.clock.Now()
@@ -492,6 +507,7 @@ func (m *Manager) WaitForOneVersion(
 			log.Infof(ctx, "waiting for %d leases to expire: desc=%v", detail.count, descs)
 		}
 	}
+
 	return desc, nil
 }
 

pkg/sql/rowenc/encoded_datum_test.go

@@ -215,8 +215,14 @@ func TestEncDatumCompare(t *testing.T) {
 
 	for _, typ := range types.OidToType {
 		switch typ.Family() {
-		case types.AnyFamily, types.UnknownFamily, types.ArrayFamily, types.JsonFamily, types.TupleFamily, types.VoidFamily,
-			types.TSQueryFamily, types.TSVectorFamily, types.PGVectorFamily, types.TriggerFamily, types.JsonpathFamily:
+		case types.AnyFamily, types.UnknownFamily, types.ArrayFamily,
+			types.TupleFamily, types.TSQueryFamily, types.TSVectorFamily,
+			types.PGVectorFamily, types.TriggerFamily, types.JsonpathFamily:
+			// These types don't have key-encoding.
+			continue
+		case types.VoidFamily:
+			// There is no point in testing VOID type as it only supports a
+			// single value.
 			continue
 		case types.CollatedStringFamily:
 			typ = types.MakeCollatedString(types.String, *randgen.RandCollationLocale(rng))