Skip to content

Commit 47eac7e

Browse files
craig[bot]paulniziolek
andcommitted
Merge #151740
151740: sql: remove LTREE prev logic r=paulniziolek a=paulniziolek This was not needed & was incorrect, messing up LTREE comparisons. We should only rely on LTREE's `Compare` method. Informs: #44657 Epic: CRDB-148 Release note: None Co-authored-by: Paul Niziolek <[email protected]>
2 parents 7c9cfa3 + bb8d870 commit 47eac7e

File tree

3 files changed

+1
-169
lines changed

3 files changed

+1
-169
lines changed

pkg/sql/sem/tree/datum.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4547,11 +4547,7 @@ func (d *DLTree) Compare(ctx context.Context, cmpCtx CompareContext, other Datum
45474547

45484548
// Prev implements the Datum interface.
45494549
func (d *DLTree) Prev(ctx context.Context, cmpCtx CompareContext) (Datum, bool) {
4550-
prevLTree, ok := d.LTree.Prev()
4551-
if !ok {
4552-
return nil, false
4553-
}
4554-
return NewDLTree(prevLTree), true
4550+
return nil, false
45554551
}
45564552

45574553
// Next implements the Datum interface.

pkg/util/ltree/ltree.go

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -131,27 +131,6 @@ func (lt T) Copy() T {
131131
return T{path: copiedLabels}
132132
}
133133

134-
// Prev returns the lexicographically previous LTree and a bool
135-
// indicating whether it exists.
136-
func (lt T) Prev() (T, bool) {
137-
if lt.Len() == 0 {
138-
return Empty, false
139-
}
140-
141-
lastLabel := lt.path[lt.Len()-1]
142-
if l := prevLabel(lastLabel); l != "" {
143-
result := lt.Copy()
144-
result.path[lt.Len()-1] = l
145-
return result, true
146-
}
147-
148-
if lt.Len() > 1 {
149-
return T{path: lt.path[:lt.Len()-1]}, true
150-
}
151-
152-
return Empty, true
153-
}
154-
155134
// validateLabel checks if a label is valid and returns an error if it is not,
156135
// otherwise, it returns nil.
157136
// A label is valid if it:
@@ -173,43 +152,7 @@ func validateLabel(l string) error {
173152
return nil
174153
}
175154

176-
// prevLabel returns the lexicographically previous label or empty string if
177-
// none exists.
178-
func prevLabel(s string) string {
179-
if len(s) == 0 {
180-
return ""
181-
}
182-
183-
lastChar := s[len(s)-1]
184-
if prev := prevChar(lastChar); prev != 0 {
185-
return s[:len(s)-1] + string(prev)
186-
}
187-
188-
if len(s) > 1 {
189-
return s[:len(s)-1]
190-
}
191-
192-
return ""
193-
}
194-
195155
// isValidChar returns true if the character is valid in an LTree label.
196156
func isValidChar(c byte) bool {
197157
return (c >= '0' && c <= '9') || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c == '_') || (c == '-')
198158
}
199-
200-
var prevCharMap = map[byte]byte{
201-
'-': 0,
202-
'0': '-',
203-
'A': '9',
204-
'_': 'Z',
205-
'a': '_',
206-
}
207-
208-
// prevChar returns the previous valid character assuming a given valid
209-
// character, or 0 if none exists.
210-
func prevChar(c byte) byte {
211-
if prev, ok := prevCharMap[c]; ok {
212-
return prev
213-
}
214-
return c - 1
215-
}

pkg/util/ltree/ltree_test.go

Lines changed: 0 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -152,110 +152,3 @@ func TestCompare(t *testing.T) {
152152
}
153153
}
154154
}
155-
156-
func TestPrev(t *testing.T) {
157-
tests := []struct {
158-
input string
159-
expected string
160-
doesNotExist bool
161-
}{
162-
{
163-
input: "",
164-
expected: "",
165-
doesNotExist: true,
166-
},
167-
{
168-
input: "b",
169-
expected: "a",
170-
},
171-
{
172-
input: "a",
173-
expected: "_",
174-
},
175-
{
176-
input: "_",
177-
expected: "Z",
178-
},
179-
{
180-
input: "Z",
181-
expected: "Y",
182-
},
183-
{
184-
input: "A",
185-
expected: "9",
186-
},
187-
{
188-
input: "9",
189-
expected: "8",
190-
},
191-
{
192-
input: "0",
193-
expected: "-",
194-
},
195-
{
196-
input: "-",
197-
expected: "",
198-
},
199-
{
200-
input: "ab",
201-
expected: "aa",
202-
},
203-
{
204-
input: "a-",
205-
expected: "a",
206-
},
207-
{
208-
input: "--",
209-
expected: "-",
210-
},
211-
{
212-
input: "A.B",
213-
expected: "A.A",
214-
},
215-
{
216-
input: "A.A",
217-
expected: "A.9",
218-
},
219-
{
220-
input: "A.-",
221-
expected: "A",
222-
},
223-
{
224-
input: "a.b.c",
225-
expected: "a.b.b",
226-
},
227-
{
228-
input: "-.-.a",
229-
expected: "-.-._",
230-
},
231-
{
232-
input: "-.-.-",
233-
expected: "-.-",
234-
},
235-
{
236-
input: "abc.def",
237-
expected: "abc.dee",
238-
},
239-
{
240-
input: "A",
241-
expected: "9",
242-
},
243-
}
244-
245-
for _, tc := range tests {
246-
input, err := ParseLTree(tc.input)
247-
if err != nil {
248-
t.Fatalf("unexpected error parsing input %q: %v", tc.input, err)
249-
}
250-
251-
got, ok := input.Prev()
252-
253-
if ok && tc.doesNotExist {
254-
t.Errorf("expected result to not exist, but got %q", got.String())
255-
}
256-
257-
if ok && tc.expected != got.String() {
258-
t.Errorf("expected %q, got %q", tc.expected, got.String())
259-
}
260-
}
261-
}

0 commit comments

Comments
 (0)