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 #36642
Fixes #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

@@ -48,6 +48,17 @@ var (
 type zoneEntry struct {
 	zone        *ZoneConfig
 	placeholder *ZoneConfig
+
+	// combined merges the zone and placeholder configs into a combined config.
+	// If both have subzone information, the placeholder information is preferred.
+	// This may never happen, but while the existing code gives preference to the
+	// placeholder, there appear to be no guarantees that there can be no overlap.
+	//
+	// TODO(andyk): Use the combined value everywhere early in 19.2, so there's
+	// enough bake time to ensure this is OK to do. Until then, only use the
+	// combined value in GetZoneConfigForObject, which is only used by the
+	// optimizer.
+	combined *ZoneConfig
 }
 
 // SystemConfig embeds a SystemConfigEntries message which contains an
@@ -270,15 +281,22 @@ func (s *SystemConfig) GetZoneConfigForKey(key roachpb.RKey) (*ZoneConfig, error
 	return s.getZoneConfigForKey(objectID, keySuffix)
 }
 
-// GetZoneConfigForObject returns the zone ID for a given object ID.
+// GetZoneConfigForObject returns the combined zone config for the given object
+// identifier.
+// 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) GetZoneConfigForObject(id uint32) (*ZoneConfig, error) {
 	entry, err := s.getZoneEntry(id)
 	if err != nil {
 		return nil, err
 	}
-	return entry.zone, nil
+	return entry.combined, nil
 }
 
+// getZoneEntry returns the zone entry for the given object ID. In the fast
+// path, the zone is already in the cache, and is directly returned. Otherwise,
+// getZoneEntry will hydrate new ZoneConfig(s) from the SystemConfig and install
+// them as an entry in the cache.
 func (s *SystemConfig) getZoneEntry(id uint32) (zoneEntry, error) {
 	s.mu.RLock()
 	entry, ok := s.mu.zoneCache[id]
@@ -294,7 +312,16 @@ func (s *SystemConfig) getZoneEntry(id uint32) (zoneEntry, error) {
 		return zoneEntry{}, err
 	}
 	if zone != nil {
-		entry := zoneEntry{zone: zone, placeholder: placeholder}
+		entry := zoneEntry{zone: zone, placeholder: placeholder, combined: zone}
+		if placeholder != nil {
+			// Merge placeholder with zone by copying over subzone information.
+			// Placeholders should only define the Subzones and SubzoneSpans fields.
+			combined := *zone
+			combined.Subzones = placeholder.Subzones
+			combined.SubzoneSpans = placeholder.SubzoneSpans
+			entry.combined = &combined
+		}
+
 		if cache {
 			s.mu.Lock()
 			s.mu.zoneCache[id] = entry
@@ -314,16 +341,16 @@ func (s *SystemConfig) getZoneConfigForKey(id uint32, keySuffix []byte) (*ZoneCo
 		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(&indexSubzone.Config)
 				}
-				subzone.Config.InheritFromParent(*entry.zone)
+				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)
+				subzone.Config.InheritFromParent(&indexSubzone.Config)
 			}
-			subzone.Config.InheritFromParent(*entry.zone)
+			subzone.Config.InheritFromParent(entry.zone)
 			return &subzone.Config, nil
 		}
 		return entry.zone, 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)