Skip to content

Commit e018889

Browse files
alex268alex268
committed
Update impelementation of compareTo for Value
1 parent 85785c5 commit e018889

File tree

14 files changed

+145
-495
lines changed

14 files changed

+145
-495
lines changed

‎table/src/main/java/tech/ydb/table/values/DecimalValue.java‎

Lines changed: 16 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -272,39 +272,27 @@ public ValueProtos.Value toPb() {
272272
@Override
273273
public int compareTo(Value<?> other) {
274274
if (other == null) {
275-
throw new IllegalArgumentException("Cannot compare with null value");
275+
throw new NullPointerException("Cannot compare with null value");
276276
}
277277

278-
// Handle comparison with OptionalValue
279278
if (other instanceof OptionalValue) {
280-
OptionalValue otherOptional = (OptionalValue) other;
281-
282-
// Check that the item type matches this decimal type
283-
if (!getType().equals(otherOptional.getType().getItemType())) {
284-
throw new IllegalArgumentException(
285-
"Cannot compare DecimalValue with OptionalValue of different item type: " +
286-
getType() + " vs " + otherOptional.getType().getItemType());
287-
}
288-
289-
// Non-empty value is greater than empty optional
290-
if (!otherOptional.isPresent()) {
291-
return 1;
279+
OptionalValue optional = (OptionalValue) other;
280+
if (!optional.isPresent()) {
281+
throw new NullPointerException("Cannot compare value " + this + " with NULL");
292282
}
293-
294-
// Compare with the wrapped value
295-
return compareTo(otherOptional.get());
283+
return compareTo(optional.get());
296284
}
297285

298286
if (!(other instanceof DecimalValue)) {
299287
throw new IllegalArgumentException("Cannot compare DecimalValue with " + other.getClass().getSimpleName());
300288
}
301289

302-
DecimalValue otherDecimal = (DecimalValue) other;
290+
DecimalValue decimal = (DecimalValue) other;
303291

304292
// Handle special values first
305-
if (isNan() || otherDecimal.isNan()) {
293+
if (isNan() || decimal.isNan()) {
306294
// NaN is not comparable, but we need to provide a consistent ordering
307-
if (isNan() && otherDecimal.isNan()) {
295+
if (isNan() && decimal.isNan()) {
308296
return 0;
309297
}
310298
if (isNan()) {
@@ -313,38 +301,39 @@ public int compareTo(Value<?> other) {
313301
return -1;
314302
}
315303

316-
if (isInf() && otherDecimal.isInf()) {
304+
if (isInf() && decimal.isInf()) {
317305
return 0;
318306
}
319307
if (isInf()) {
320308
return 1; // Positive infinity is greater than any finite value
321309
}
322-
if (otherDecimal.isInf()) {
310+
if (decimal.isInf()) {
323311
return -1;
324312
}
325313

326-
if (isNegativeInf() && otherDecimal.isNegativeInf()) {
314+
if (isNegativeInf() && decimal.isNegativeInf()) {
327315
return 0;
328316
}
329317
if (isNegativeInf()) {
330318
return -1; // Negative infinity is less than any finite value
331319
}
332-
if (otherDecimal.isNegativeInf()) {
320+
321+
if (decimal.isNegativeInf()) {
333322
return 1;
334323
}
335324

336325
// Compare finite values
337-
if (isNegative() != otherDecimal.isNegative()) {
326+
if (isNegative() != decimal.isNegative()) {
338327
return isNegative() ? -1 : 1;
339328
}
340329

341330
// Both have the same sign, compare magnitudes
342-
int highComparison = Long.compareUnsigned(high, otherDecimal.high);
331+
int highComparison = Long.compareUnsigned(high, decimal.high);
343332
if (highComparison != 0) {
344333
return isNegative() ? -highComparison : highComparison;
345334
}
346335

347-
int lowComparison = Long.compareUnsigned(low, otherDecimal.low);
336+
int lowComparison = Long.compareUnsigned(low, decimal.low);
348337
return isNegative() ? -lowComparison : lowComparison;
349338
}
350339

‎table/src/main/java/tech/ydb/table/values/DictValue.java‎

Lines changed: 19 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
package tech.ydb.table.values;
22

3-
import java.util.ArrayList;
43
import java.util.Collection;
54
import java.util.HashMap;
6-
import java.util.List;
75
import java.util.Map;
86
import java.util.Set;
7+
import java.util.TreeSet;
98

109
import javax.annotation.Nullable;
1110

@@ -122,106 +121,42 @@ public ValueProtos.Value toPb() {
122121
@Override
123122
public int compareTo(Value<?> other) {
124123
if (other == null) {
125-
throw new IllegalArgumentException("Cannot compare with null value");
124+
throw new NullPointerException("Cannot compare with null value");
126125
}
127126

128-
// Handle comparison with OptionalValue
129127
if (other instanceof OptionalValue) {
130-
OptionalValue otherOptional = (OptionalValue) other;
131-
132-
// Check that the item type matches this dict type
133-
if (!getType().equals(otherOptional.getType().getItemType())) {
134-
throw new IllegalArgumentException(
135-
"Cannot compare DictValue with OptionalValue of different item type: " +
136-
getType() + " vs " + otherOptional.getType().getItemType());
128+
OptionalValue optional = (OptionalValue) other;
129+
if (!optional.isPresent()) {
130+
throw new NullPointerException("Cannot compare value " + this + " with NULL");
137131
}
138-
139-
// Non-empty value is greater than empty optional
140-
if (!otherOptional.isPresent()) {
141-
return 1;
142-
}
143-
144-
// Compare with the wrapped value
145-
return compareTo(otherOptional.get());
132+
return compareTo(optional.get());
146133
}
147134

148-
if (!(other instanceof DictValue)) {
149-
throw new IllegalArgumentException("Cannot compare DictValue with " + other.getClass().getSimpleName());
135+
if (!type.equals(other.getType())) {
136+
throw new IllegalArgumentException("Cannot compare value " + type + " with " + other.getType());
150137
}
151138

152139
DictValue otherDict = (DictValue) other;
153140

154-
// Convert to sorted lists for lexicographical comparison
155-
List<Map.Entry<Value<?>, Value<?>>> thisEntries = new ArrayList<>(items.entrySet());
156-
List<Map.Entry<Value<?>, Value<?>>> otherEntries = new ArrayList<>(otherDict.items.entrySet());
141+
// Sort entries by keys
142+
Set<Value<?>> keys = new TreeSet<>();
143+
keys.addAll(items.keySet());
144+
keys.addAll(otherDict.keySet());
157145

158-
// Sort entries by key first, then by value
159-
thisEntries.sort((e1, e2) -> {
160-
int keyComparison = compareValues(e1.getKey(), e2.getKey());
161-
if (keyComparison != 0) {
162-
return keyComparison;
163-
}
164-
return compareValues(e1.getValue(), e2.getValue());
165-
});
166-
167-
otherEntries.sort((e1, e2) -> {
168-
int keyComparison = compareValues(e1.getKey(), e2.getKey());
169-
if (keyComparison != 0) {
170-
return keyComparison;
146+
for (Value<?> key: keys) {
147+
if (!otherDict.items.containsKey(key)) {
148+
return 1;
171149
}
172-
return compareValues(e1.getValue(), e2.getValue());
173-
});
174-
175-
// Compare sorted entries lexicographically
176-
int minLength = Math.min(thisEntries.size(), otherEntries.size());
177-
for (int i = 0; i < minLength; i++) {
178-
Map.Entry<Value<?>, Value<?>> thisEntry = thisEntries.get(i);
179-
Map.Entry<Value<?>, Value<?>> otherEntry = otherEntries.get(i);
180-
181-
int keyComparison = compareValues(thisEntry.getKey(), otherEntry.getKey());
182-
if (keyComparison != 0) {
183-
return keyComparison;
150+
if (!items.containsKey(key)) {
151+
return -1;
184152
}
185153

186-
int valueComparison = compareValues(thisEntry.getValue(), otherEntry.getValue());
154+
int valueComparison = items.get(key).compareTo(otherDict.items.get(key));
187155
if (valueComparison != 0) {
188156
return valueComparison;
189157
}
190158
}
191159

192-
// If we reach here, one dict is a prefix of the other
193-
// The shorter dict comes first
194-
return Integer.compare(thisEntries.size(), otherEntries.size());
195-
}
196-
197-
private static int compareValues(Value<?> a, Value<?> b) {
198-
// Handle null values
199-
if (a == null && b == null) {
200-
return 0;
201-
}
202-
if (a == null) {
203-
return -1;
204-
}
205-
if (b == null) {
206-
return 1;
207-
}
208-
209-
// Check that the types are the same
210-
if (!a.getType().equals(b.getType())) {
211-
throw new IllegalArgumentException("Cannot compare values of different types: " +
212-
a.getType() + " vs " + b.getType());
213-
}
214-
215-
// Use the actual compareTo method of the values
216-
if (a instanceof Comparable && b instanceof Comparable) {
217-
try {
218-
return ((Comparable<Value<?>>) a).compareTo((Value<?>) b);
219-
} catch (ClassCastException e) {
220-
// Fall back to error
221-
}
222-
}
223-
224-
throw new IllegalArgumentException("Cannot compare values of different types: " +
225-
a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName());
160+
return 0;
226161
}
227162
}

‎table/src/main/java/tech/ydb/table/values/ListValue.java‎

Lines changed: 9 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -117,80 +117,27 @@ public ValueProtos.Value toPb() {
117117
@Override
118118
public int compareTo(Value<?> other) {
119119
if (other == null) {
120-
throw new IllegalArgumentException("Cannot compare with null value");
120+
throw new NullPointerException("Cannot compare with null value");
121121
}
122122

123-
// Handle comparison with OptionalValue
124123
if (other instanceof OptionalValue) {
125-
OptionalValue otherOptional = (OptionalValue) other;
126-
127-
// Check that the item type matches this list type
128-
if (!getType().equals(otherOptional.getType().getItemType())) {
129-
throw new IllegalArgumentException(
130-
"Cannot compare ListValue with OptionalValue of different item type: " +
131-
getType() + " vs " + otherOptional.getType().getItemType());
132-
}
133-
134-
// Non-empty value is greater than empty optional
135-
if (!otherOptional.isPresent()) {
136-
return 1;
124+
OptionalValue optional = (OptionalValue) other;
125+
if (!optional.isPresent()) {
126+
throw new NullPointerException("Cannot compare value " + this + " with NULL");
137127
}
138-
139-
// Compare with the wrapped value
140-
return compareTo(otherOptional.get());
128+
return compareTo(optional.get());
141129
}
142130

143-
if (!(other instanceof ListValue)) {
144-
throw new IllegalArgumentException("Cannot compare ListValue with " + other.getClass().getSimpleName());
145-
}
131+
ListValue list = (ListValue) other;
146132

147-
ListValue otherList = (ListValue) other;
148-
149-
// Compare elements lexicographically
150-
int minLength = Math.min(items.length, otherList.items.length);
133+
int minLength = Math.min(items.length, list.items.length);
151134
for (int i = 0; i < minLength; i++) {
152-
Value<?> thisItem = items[i];
153-
Value<?> otherItem = otherList.items[i];
154-
155-
int itemComparison = compareValues(thisItem, otherItem);
135+
int itemComparison = items[i].compareTo(list.items[i]);
156136
if (itemComparison != 0) {
157137
return itemComparison;
158138
}
159139
}
160140

161-
// If we reach here, one list is a prefix of the other
162-
// The shorter list comes first
163-
return Integer.compare(items.length, otherList.items.length);
164-
}
165-
166-
private static int compareValues(Value<?> a, Value<?> b) {
167-
// Handle null values
168-
if (a == null && b == null) {
169-
return 0;
170-
}
171-
if (a == null) {
172-
return -1;
173-
}
174-
if (b == null) {
175-
return 1;
176-
}
177-
178-
// Check that the types are the same
179-
if (!a.getType().equals(b.getType())) {
180-
throw new IllegalArgumentException("Cannot compare values of different types: " +
181-
a.getType() + " vs " + b.getType());
182-
}
183-
184-
// Use the actual compareTo method of the values
185-
if (a instanceof Comparable && b instanceof Comparable) {
186-
try {
187-
return ((Comparable<Value<?>>) a).compareTo((Value<?>) b);
188-
} catch (ClassCastException e) {
189-
// Fall back to error
190-
}
191-
}
192-
193-
throw new IllegalArgumentException("Cannot compare values of different types: " +
194-
a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName());
141+
return Integer.compare(items.length, list.items.length);
195142
}
196143
}

‎table/src/main/java/tech/ydb/table/values/NullValue.java‎

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,31 +36,19 @@ public ValueProtos.Value toPb() {
3636
@Override
3737
public int compareTo(Value<?> other) {
3838
if (other == null) {
39-
throw new IllegalArgumentException("Cannot compare with null value");
39+
throw new NullPointerException("Cannot compare with null value");
4040
}
4141

42-
// Handle comparison with OptionalValue
4342
if (other instanceof OptionalValue) {
44-
OptionalValue otherOptional = (OptionalValue) other;
45-
46-
// Check that the item type matches this null type
47-
if (!getType().equals(otherOptional.getType().getItemType())) {
48-
throw new IllegalArgumentException(
49-
"Cannot compare NullValue with OptionalValue of different item type: " +
50-
getType() + " vs " + otherOptional.getType().getItemType());
51-
}
52-
53-
// Non-empty value is greater than empty optional
54-
if (!otherOptional.isPresent()) {
55-
return 1;
43+
OptionalValue optional = (OptionalValue) other;
44+
if (!optional.isPresent()) {
45+
return 0;
5646
}
57-
58-
// Compare with the wrapped value
59-
return compareTo(otherOptional.get());
47+
return compareTo(optional.get());
6048
}
6149

62-
if (!(other instanceof NullValue)) {
63-
throw new IllegalArgumentException("Cannot compare NullValue with " + other.getClass().getSimpleName());
50+
if (!getType().equals(other.getType())) {
51+
throw new IllegalArgumentException("Cannot compare value " + getType() + " with " + other.getType());
6452
}
6553

6654
// All NullValue instances are equal

0 commit comments

Comments
 (0)