opt: Fix and improve zone-based index selection

This PR fixes a couple of related issues with zone-based index selection.

First, placeholder zones were incorrectly discarded. Placeholder zones are
used when there are indexes with Constraints/Leaseholder Preferences on a
table that doesn't have either of those things. The fix is to merge the
indexes placeholder into the table zone.

Second, the optimizer required that a prefix of the locality tiers match
constraints. That is no longer required after this commit. For example, if
locality=region=us;dc=east, then it now matches any of these constraint
sets equally well:

  [+locality=region=us;+dc=east]
  [+dc=east]
  [+dc=east,+dc=west]

A missing constraint match is OK (i.e. region=us), as long as a more specific
locality tier matches (i.e. dc=east).

Fixes cockroachdb#36642
Fixes cockroachdb#36644

Release note: None

pkg/ccl/logictestccl/testdata/logic_test/zone

@@ -209,6 +209,86 @@ scan  ·      ·
 statement ok
 DEALLOCATE p
 
-# test for #36348; place this at the bottom of this file.
+
+# ------------------------------------------------------------------------------
+# Regression for issue #36642. Optimizer picked wrong index when the index had
+# constraints / lease preferences, but the table had no zone config.
+# ------------------------------------------------------------------------------
+
+statement ok
+CREATE TABLE t36642 (
+    k INT PRIMARY KEY,
+    v STRING,
+    INDEX secondary (k) STORING (v),
+    INDEX tertiary (k) STORING (v)
+);
+
+statement ok
+ALTER INDEX t36642@secondary CONFIGURE ZONE USING lease_preferences='[[+region=test,+dc=dc1]]'
+
+query TTT retry
+EXPLAIN SELECT * FROM t36642 WHERE k=10
+----
+scan  ·      ·
+·     table  t36642@secondary
+·     spans  /10-/11
+
+statement ok
+ALTER INDEX t36642@tertiary CONFIGURE ZONE USING lease_preferences='[[+region=test,+dc=dc1]]'
+
+statement ok
+ALTER INDEX t36642@secondary CONFIGURE ZONE USING lease_preferences='[[+region=test,+dc=dc2]]'
+
+query TTT retry
+EXPLAIN SELECT * FROM t36642 WHERE k=10
+----
+scan  ·      ·
+·     table  t36642@tertiary
+·     spans  /10-/11
+
+
+# ------------------------------------------------------------------------------
+# Regression for issue #36644. Allow matching constraints for leading locality
+# tiers to be omitted.
+# ------------------------------------------------------------------------------
+
+statement ok
+CREATE TABLE t36644 (
+    k INT PRIMARY KEY,
+    v STRING,
+    INDEX secondary (k) STORING (v),
+    INDEX tertiary (k) STORING (v)
+);
+
+statement ok
+ALTER INDEX t36644@secondary
+CONFIGURE ZONE USING constraints='[+region=test]', lease_preferences='[[+dc=dc1]]'
+
+query TTT retry
+EXPLAIN SELECT * FROM t36644 WHERE k=10
+----
+scan  ·      ·
+·     table  t36644@secondary
+·     spans  /10-/11
+
+statement ok
+ALTER INDEX t36644@secondary CONFIGURE ZONE USING lease_preferences='[[+dc=dc3]]'
+
+statement ok
+ALTER INDEX t36644@tertiary
+CONFIGURE ZONE USING constraints='[+region=test]', lease_preferences='[[+dc=dc1]]'
+
+query TTT retry
+EXPLAIN SELECT * FROM t36644 WHERE k=10
+----
+scan  ·      ·
+·     table  t36644@tertiary
+·     spans  /10-/11
+
+
+# ------------------------------------------------------------------------------
+# Regression test for #36348; place this at the bottom of this file.
+# ------------------------------------------------------------------------------
+
 statement ok
 DROP INDEX t@secondary

pkg/config/system.go

@@ -45,11 +45,6 @@ var (
 	SplitAtIDHook func(uint32, *SystemConfig) bool
 )
 
-type zoneEntry struct {
-	zone        *ZoneConfig
-	placeholder *ZoneConfig
-}
-
 // SystemConfig embeds a SystemConfigEntries message which contains an
 // entry for every system descriptor (e.g. databases, tables, zone
 // configs). It also has a map from object ID to unmarshaled zone
@@ -62,15 +57,15 @@ type SystemConfig struct {
 	SystemConfigEntries
 	mu struct {
 		syncutil.RWMutex
-		zoneCache        map[uint32]zoneEntry
+		zoneCache        map[uint32]*ZoneConfig
 		shouldSplitCache map[uint32]bool
 	}
 }
 
 // NewSystemConfig returns an initialized instance of SystemConfig.
 func NewSystemConfig() *SystemConfig {
 	sc := &SystemConfig{}
-	sc.mu.zoneCache = map[uint32]zoneEntry{}
+	sc.mu.zoneCache = map[uint32]*ZoneConfig{}
 	sc.mu.shouldSplitCache = map[uint32]bool{}
 	return sc
 }
@@ -272,61 +267,64 @@ func (s *SystemConfig) GetZoneConfigForKey(key roachpb.RKey) (*ZoneConfig, error
 
 // GetZoneConfigForObject returns the zone ID for a given object ID.
 func (s *SystemConfig) GetZoneConfigForObject(id uint32) (*ZoneConfig, error) {
-	entry, err := s.getZoneEntry(id)
-	if err != nil {
-		return nil, err
-	}
-	return entry.zone, nil
+	return s.getCachedZone(id)
 }
 
-func (s *SystemConfig) getZoneEntry(id uint32) (zoneEntry, error) {
+// getCachedZone returns the ZoneConfig for the given object ID. In the fast
+// path, the zone is already in the cache, and is directly returned. Otherwise,
+// getCachedZone will hydrate a new ZoneConfig from the SystemConfig and install
+// it in the cache.
+//
+// NOTE: any subzones from the zone placeholder will be automatically merged
+// into the cached zone so the caller doesn't need special-case handling code.
+func (s *SystemConfig) getCachedZone(id uint32) (*ZoneConfig, error) {
 	s.mu.RLock()
-	entry, ok := s.mu.zoneCache[id]
+	existing, ok := s.mu.zoneCache[id]
 	s.mu.RUnlock()
 	if ok {
-		return entry, nil
+		return existing, nil
 	}
 	testingLock.Lock()
 	hook := ZoneConfigHook
 	testingLock.Unlock()
 	zone, placeholder, cache, err := hook(s, id)
 	if err != nil {
-		return zoneEntry{}, err
+		return nil, err
 	}
 	if zone != nil {
-		entry := zoneEntry{zone: zone, placeholder: placeholder}
+		if placeholder != nil {
+			// Merge placeholder with zone by copying over subzone information.
+			// Placeholders should only define the Subzones and SubzoneSpans fields.
+			copy := *zone
+			copy.Subzones = placeholder.Subzones
+			copy.SubzoneSpans = placeholder.SubzoneSpans
+			zone = &copy
+		}
+
 		if cache {
 			s.mu.Lock()
-			s.mu.zoneCache[id] = entry
+			s.mu.zoneCache[id] = zone
 			s.mu.Unlock()
 		}
-		return entry, nil
+		return zone, nil
 	}
-	return zoneEntry{}, nil
+	return nil, nil
 }
 
 func (s *SystemConfig) getZoneConfigForKey(id uint32, keySuffix []byte) (*ZoneConfig, error) {
-	entry, err := s.getZoneEntry(id)
+	zone, err := s.getCachedZone(id)
 	if err != nil {
 		return nil, err
 	}
-	if entry.zone != nil {
-		if entry.placeholder != nil {
-			if subzone := entry.placeholder.GetSubzoneForKeySuffix(keySuffix); subzone != nil {
-				if indexSubzone := entry.placeholder.GetSubzone(subzone.IndexID, ""); indexSubzone != nil {
-					subzone.Config.InheritFromParent(indexSubzone.Config)
-				}
-				subzone.Config.InheritFromParent(*entry.zone)
-				return &subzone.Config, nil
-			}
-		} else if subzone := entry.zone.GetSubzoneForKeySuffix(keySuffix); subzone != nil {
-			if indexSubzone := entry.zone.GetSubzone(subzone.IndexID, ""); indexSubzone != nil {
-				subzone.Config.InheritFromParent(indexSubzone.Config)
+	if zone != nil {
+		if subzone := zone.GetSubzoneForKeySuffix(keySuffix); subzone != nil {
+			if indexSubzone := zone.GetSubzone(subzone.IndexID, ""); indexSubzone != nil {
+				subzone.Config.InheritFromParent(&indexSubzone.Config)
 			}
-			subzone.Config.InheritFromParent(*entry.zone)
+			subzone.Config.InheritFromParent(zone)
 			return &subzone.Config, nil
 		}
-		return entry.zone, nil
+		return zone, nil
 	}
 	return DefaultZoneConfigRef(), nil
 }

pkg/config/zone.go

@@ -505,7 +505,7 @@ func (z *ZoneConfig) Validate() error {
 }
 
 // InheritFromParent hydrates a zones missing fields from its parent.
-func (z *ZoneConfig) InheritFromParent(parent ZoneConfig) {
+func (z *ZoneConfig) InheritFromParent(parent *ZoneConfig) {
 	if z.NumReplicas == nil {
 		if parent.NumReplicas != nil {
 			z.NumReplicas = proto.Int32(*parent.NumReplicas)