opt: fix LTREE <@ index constraint spans

Fixes #156478

Release note (bug fix): A bug has been fixed that caused incorrect
results for queries that filter indexed `LTREE` columns with the `<@`
(contained-by) operator. This bug has been present since v25.4.0.

Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Michael Erickson <michae2@cockroachlabs.com>

Author: 3 people

pkg/sql/logictest/testdata/logic_test/ltree

@@ -350,4 +350,21 @@ SELECT lca(''::LTREE, 'A.B.C'::LTREE)
 ----
 NULL
 
+# Regression test for #156478. Correct constraint spans should be built for the
+# <@ (contained-by) operator.
+statement ok
+CREATE TABLE t156478 (
+  k INT PRIMARY KEY,
+  l LTREE,
+  INDEX idx (l),
+  INDEX idx_desc (l DESC)
+)
+
+statement ok
+INSERT INTO t156478 VALUES (1, 'foo.bar_'::LTREE)
+
+query empty
+SELECT * FROM t156478@idx WHERE l <@ 'foo.bar'::LTREE
 
+query empty
+SELECT * FROM t156478@idx_desc WHERE l <@ 'foo.bar'::LTREE

pkg/sql/opt/exec/execbuilder/testdata/select_index

@@ -1943,4 +1943,4 @@ vectorized: true
 • scan
   missing stats
   table: t5@t5_a_idx
-  spans: [/'A.B.C' - /'A.B.D')
+  spans: [/'A.B.C' - /'A.B.C-')

pkg/sql/opt/idxconstraint/index_constraints.go

@@ -375,13 +375,15 @@ func (c *indexConstraintCtx) makeSpansForSingleColumnDatum(
 
 	case opt.ContainedByOp:
 		if l, ok := datum.(*tree.DLTree); ok {
-			if l.LTree.Compare(ltree.Empty) == 0 {
-				// Empty LTree shouldn't be constrained.
-				// TODO: This could be constrained by excluding NULLs.
+			end, ok := l.LTree.NextSibling()
+			if !ok {
+				// An empty LTree represents the root of the tree, so it
+				// includes all non-NULL LTrees.
+				// TODO(mgartner): This could be constrained by excluding NULLs.
 				break
 			}
 			startKey := constraint.MakeKey(l)
-			endKey := constraint.MakeKey(tree.NewDLTree(l.LTree.NextSibling()))
+			endKey := constraint.MakeKey(tree.NewDLTree(end))
 			c.singleSpan(
 				offset, startKey, includeBoundary, endKey, excludeBoundary,
 				c.columns[offset].Descending(),

pkg/sql/opt/idxconstraint/testdata/ltree

@@ -25,7 +25,7 @@ a @> NULL
 index-constraints vars=(a ltree) index=a
 a <@ 'A.B.C'
 ----
-[/'A.B.C' - /'A.B.D')
+[/'A.B.C' - /'A.B.C-')
 
 index-constraints vars=(a ltree) index=a
 a <@ 'z'
@@ -53,23 +53,23 @@ a @> '-' OR a @> 'foo.bar'
 index-constraints vars=(a ltree) index=a
 a <@ '-' OR a <@ 'foo.bar'
 ----
-[/'-' - /'0')
-[/'foo.bar' - /'foo.bas')
+[/'-' - /'--')
+[/'foo.bar' - /'foo.bar-')
 
 index-constraints vars=(a ltree) index=a
 a <@ 'A.B.C' OR a @> 'A.B'
 ----
 [/'' - /'']
 [/'A' - /'A']
 [/'A.B' - /'A.B']
-[/'A.B.C' - /'A.B.D')
+[/'A.B.C' - /'A.B.C-')
 
 index-constraints vars=(a ltree) index=a
 a @> 'A.B.C' OR a <@ 'A.B'
 ----
 [/'' - /'']
 [/'A' - /'A']
-[/'A.B' - /'A.C')
+[/'A.B' - /'A.B-')
 
 index-constraints vars=(a ltree) index=a
 a <@ 'A.B.C' AND a @> 'A.B.C.D.E'

pkg/util/ltree/ltree.go

@@ -17,6 +17,7 @@ import (
 
 const (
 	PathSeparator = "."
+	minCharacter  = "-"
 	// Postgres imposes a 65535 limit on the number of labels in a ltree.
 	maxNumOfLabels = 65535
 	// Postgres docs mention labels must be less than 256 bytes, but in practice,
@@ -297,44 +298,17 @@ func LCA(ltrees []T) (_ T, isNull bool) { // lint: uppercase function OK
 }
 
 // NextSibling returns a LTree with a lexicographically incremented last label.
-// This is different from the next lexicographic LTree. This is mainly used for
-// defining a key-encoded span for ancestry operators.
-// Note that this could produce a LTREE with more labels than the maximum
-// allowed.
-// Example: 'A.B' -> 'A.C'
-func (lt T) NextSibling() T {
+// If the LTree is empty, it returns ok=false. An empty LTree represents the
+// root of the tree, so it has no siblings.
+//
+// This is mainly used for defining a key-encoded span for ancestry operators.
+//
+// Example: 'A.B' -> 'A.B-'
+func (lt T) NextSibling() (_ T, ok bool) {
 	if lt.Len() == 0 {
-		return Empty
+		return T{}, false
 	}
-	lastLabel := lt.path[len(lt.path)-1]
-	nextLabel := incrementLabel(lastLabel)
-
-	newLTree := lt.Copy()
-	newLTree.path[newLTree.Len()-1] = nextLabel
-	return newLTree
-}
-
-var nextCharMap = map[byte]byte{
-	'-': '0',
-	'9': 'A',
-	'Z': '_',
-	'_': 'a',
-	'z': 0,
-}
-
-func incrementLabel(label string) string {
-	nextLabel := []byte(label)
-	nextChar, ok := nextCharMap[nextLabel[len(nextLabel)-1]]
-
-	if ok && nextChar == 0 {
-		// Technically, this could mean exceeding the length of the label.
-		return string(append(nextLabel, '-'))
-	}
-
-	if !ok {
-		nextChar = nextLabel[len(nextLabel)-1] + 1
-	}
-
-	nextLabel[len(nextLabel)-1] = nextChar
-	return string(nextLabel)
+	newLT := lt.Copy()
+	newLT.path[newLT.Len()-1] = newLT.path[newLT.Len()-1] + minCharacter
+	return newLT, true
 }

pkg/util/ltree/ltree_test.go

@@ -153,28 +153,33 @@ func TestCompare(t *testing.T) {
 	}
 }
 
-func TestNext(t *testing.T) {
+func TestNextSibling(t *testing.T) {
 	tests := []struct {
 		input    string
 		expected string
+		ok       bool
 	}{
-		{"a", "b"},
-		{"a.b", "a.c"},
-		{"a.z", "a.z-"},
-		{"-", "0"},
-		{"9", "A"},
-		{"Z", "_"},
-		{"_", "a"},
+		{"", "", false},
+		{"a", "a-", true},
+		{"a.b", "a.b-", true},
+		{"a.z", "a.z-", true},
+		{"-", "--", true},
+		{"9", "9-", true},
+		{"Z", "Z-", true},
+		{"_", "_-", true},
 	}
 
 	for _, tc := range tests {
 		a, err := ParseLTree(tc.input)
 		if err != nil {
 			t.Fatalf("unexpected error parsing %q: %v", tc.input, err)
 		}
-		next := a.NextSibling()
-		if next.String() != tc.expected {
-			t.Errorf("expected next of %q to be %q, got %q", tc.input, tc.expected, next.String())
+		next, ok := a.NextSibling()
+		if ok != tc.ok {
+			t.Errorf("expected ok=%v for input %q, got %v", tc.ok, tc.input, ok)
+		}
+		if n := next.String(); n != tc.expected {
+			t.Errorf("expected %q for input %q, got %q", tc.expected, tc.input, n)
 		}
 	}
 }