Skip to content

Commit f80c374

Browse files
spmallettespmallette
committed
Merge branch 'comparator-fix' into 3.8-dev
2 parents fbd35ed + 964d6f4 commit f80c374

File tree

3 files changed

+123
-4
lines changed

3 files changed

+123
-4
lines changed

‎CHANGELOG.asciidoc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ This release also includes changes from <<release-3-7-6, 3.7.6>>.
2828
* Fixed bug in Gremlin orderability semantics for `OffsetDateTime`.
2929
* Fixed bug in Javascript translation for UUID.
3030
* Fixed bug in pre-repeat() `emit()/until()` where `emit()` and `until()` traversers weren't added to the results.
31+
* Fixed bug in `GremlinValueComparator` that prevented collections of differing sizes from being comparable.
3132
* Expose serialization functions for alternative transport protocols in gremlin-go
3233
* Improved Gremlint formatting to keep the first argument for a step on the same line if line breaks were required to meet max line length.
3334
* Improved Gremlint formatting to do greedy argument packing when possible so that more arguments can appear on a single line.
@@ -37,7 +38,7 @@ This release also includes changes from <<release-3-7-6, 3.7.6>>.
3738
3839
This release also includes changes from <<release-3-7-5, 3.7.5>>.
3940
40-
* Added a Gremln MCP server.
41+
* Added a Gremlin MCP server.
4142
* Added the Air Routes 1.0 dataset to the set of available samples packaged with distributions.
4243
* Added a minimal distribution for `tinkergraph-gremlin` using the `min` classifier that doesn't include the sample datasets.
4344
* Removed `AggregateLocalStep` and `aggregate(Scope, String)`, and renamed `AggregateGlobalStep` to `AggregateStep`.

‎gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/GremlinValueComparator.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -369,9 +369,6 @@ private static boolean contentsComparable(Iterator fi, Iterator si) {
369369
return false;
370370
}
371371
}
372-
if (fi.hasNext() || si.hasNext()) {
373-
return false;
374-
}
375372
return true;
376373
}
377374

‎gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/CompareTest.java‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
package org.apache.tinkerpop.gremlin.process.traversal;
2020

21+
import org.apache.tinkerpop.gremlin.process.traversal.step.util.MutablePath;
2122
import org.javatuples.Pair;
2223
import org.junit.Rule;
2324
import org.junit.Test;
@@ -29,6 +30,7 @@
2930
import java.math.BigInteger;
3031
import java.util.ArrayList;
3132
import java.util.Arrays;
33+
import java.util.Collections;
3234
import java.util.List;
3335

3436
import static org.apache.tinkerpop.gremlin.util.CollectionUtil.asList;
@@ -249,6 +251,117 @@ public static Iterable<Object[]> data() {
249251
{Compare.gt, asMap(1.0, "foo", 2.0, "bar"), asMap(2, "bar", 1, "foo"), false},
250252
{Compare.gte, asMap(1.0, "foo", 2.0, "bar"), asMap(2, "bar", 1, "foo"), true},
251253

254+
// Empty collections
255+
{Compare.eq, asList(), asList(), true},
256+
{Compare.neq, asList(), asList(), false},
257+
{Compare.lt, asList(), asList(), false},
258+
{Compare.lte, asList(), asList(), true},
259+
{Compare.gt, asList(), asList(), false},
260+
{Compare.gte, asList(), asList(), true},
261+
262+
{Compare.lt, asList(), asList(1), true},
263+
{Compare.gt, asList(1), asList(), true},
264+
265+
{Compare.eq, asSet(), asSet(), true},
266+
{Compare.lt, asSet(), asSet(1), true},
267+
{Compare.gt, asSet(1), asSet(), true},
268+
269+
{Compare.eq, asMap(), asMap(), true},
270+
{Compare.lt, asMap(), asMap(1, 1), true},
271+
{Compare.gt, asMap(1, 1), asMap(), true},
272+
273+
// Different sizes
274+
{Compare.lt, asList(1, 2, 3), asList(1, 2, 3, 4), true},
275+
{Compare.gt, asList(1, 2, 3, 4), asList(1, 2, 3), true},
276+
{Compare.lt, asList(1, 2, 4), asList(1, 2, 3, 4), false},
277+
{Compare.gt, asList(1, 2, 4), asList(1, 2, 3, 4), true},
278+
279+
{Compare.lt, asSet(1, 2, 3), asSet(1, 2, 3, 4), true},
280+
{Compare.gt, asSet(1, 2, 3, 4), asSet(1, 2, 3), true},
281+
{Compare.lt, asSet(1, 2, 4), asSet(1, 2, 3, 4), false},
282+
{Compare.gt, asSet(1, 2, 4), asSet(1, 2, 3, 4), true},
283+
284+
{Compare.lt, asMap(1, 1, 2, 2, 3, 3), asMap(1, 1, 2, 2, 3, 3, 4, 4), true},
285+
{Compare.gt, asMap(1, 1, 2, 2, 3, 3, 4, 4), asMap(1, 1, 2, 2, 3, 3), true},
286+
287+
// Type promotion within collections
288+
{Compare.eq, asList(1, 2), asList(1.0, 2.0), true},
289+
{Compare.lt, asList(1, 2), asList(1.0, 3.0), true},
290+
{Compare.eq, asSet(1, 2), asSet(2.0, 1.0), true},
291+
{Compare.eq, asMap(1, 1.0), asMap(1L, 1.0d), true},
292+
293+
// Lexicographical comparison with mixed types (but pairwise comparable)
294+
{Compare.lt, asList(1, "a"), asList(1, "b"), true},
295+
{Compare.lt, asList(1, "a"), asList(2, "a"), true},
296+
{Compare.gt, asList(1, "a"), asList(0, "a"), true},
297+
298+
// Sets and Maps with mixed types (Orderability used for sorting)
299+
{Compare.lt, asSet(1, "a"), asSet(1, "b"), true},
300+
{Compare.eq, asSet(1, "a"), asSet("a", 1), true},
301+
{Compare.gt, asMap(1, "a", "z", 2), asMap(1, "a", "y", 2), true},
302+
303+
// Incomparable elements (cross-type) in collections
304+
{Compare.eq, asList(1, "a"), asList(1, 1), false},
305+
{Compare.neq, asList(1, "a"), asList(1, 1), true},
306+
{Compare.lt, asList(1, "a"), asList(1, 1), false},
307+
{Compare.gt, asList(1, "a"), asList(1, 1), false},
308+
309+
{Compare.eq, asSet(1, "a"), asSet(1, 2.0), false},
310+
{Compare.lt, asSet(1, "a"), asSet(1, 2.0), false},
311+
312+
// Nested collections
313+
{Compare.lt, asList(asList(1, 2)), asList(asList(1, 3)), true},
314+
{Compare.lt, asList(asList(1, 2)), asList(asList(1, 2, 3)), true},
315+
{Compare.gt, asList(asList(1, 2)), asList(asList(1, 1)), true},
316+
317+
// Nulls in collections
318+
{Compare.eq, asList(1, null), asList(1, null), true},
319+
{Compare.eq, asList(1, null), asList(1, 2), false},
320+
{Compare.lt, asList(1, null), asList(1, 2), false},
321+
{Compare.gt, asList(1, null), asList(1, 2), false},
322+
323+
// Paths
324+
{Compare.eq, p(), p(), true},
325+
{Compare.neq, p(), p(), false},
326+
{Compare.lt, p(), p(), false},
327+
{Compare.lte, p(), p(), true},
328+
{Compare.gt, p(), p(), false},
329+
{Compare.gte, p(), p(), true},
330+
331+
{Compare.lt, p(), p(1), true},
332+
{Compare.gt, p(1), p(), true},
333+
334+
// Lexicographical comparison (pairwise)
335+
{Compare.lt, p(1, "a"), p(1, "b"), true},
336+
{Compare.lt, p(1, "a"), p(2, "a"), true},
337+
{Compare.gt, p(1, "a"), p(0, "a"), true},
338+
339+
// Different sizes
340+
{Compare.lt, p(1, 2, 3), p(1, 2, 3, 4), true},
341+
{Compare.gt, p(1, 2, 3, 4), p(1, 2, 3), true},
342+
343+
// Incomparable elements (cross-type) in paths
344+
{Compare.eq, p(1, "a"), p(1, 1), false},
345+
{Compare.neq, p(1, "a"), p(1, 1), true},
346+
{Compare.lt, p(1, "a"), p(1, 1), false},
347+
{Compare.gt, p(1, "a"), p(1, 1), false},
348+
349+
// Nested collections in paths
350+
{Compare.lt, p(asList(1, 2)), p(asList(1, 3)), true},
351+
{Compare.lt, p(asList(1, 2)), p(asList(1, 2, 3)), true},
352+
{Compare.gt, p(asList(1, 2)), p(asList(1, 1)), true},
353+
354+
// Nulls in paths
355+
{Compare.eq, p(1, null), p(1, null), true},
356+
{Compare.eq, p(1, null), p(1, 2), false},
357+
{Compare.lt, p(1, null), p(1, 2), false},
358+
{Compare.gt, p(1, null), p(1, 2), false},
359+
360+
// Cross-type Path vs List (must be false/error per semantics)
361+
{Compare.eq, p(1, 2), asList(1, 2), false},
362+
{Compare.neq, p(1, 2), asList(1, 2), true},
363+
{Compare.lt, p(1, 2), asList(1, 2), false},
364+
{Compare.gt, p(1, 2), asList(1, 2), false},
252365
}));
253366
// Compare Numbers of mixed types.
254367
final List<Object> one = Arrays.asList(1, 1l, 1d, 1f, BigDecimal.ONE, BigInteger.ONE);
@@ -433,4 +546,12 @@ static class C extends B {
433546

434547
static class D extends B {
435548
}
549+
550+
private static Path p(final Object... objects) {
551+
final Path path = MutablePath.make();
552+
for (final Object obj : objects) {
553+
path.extend(obj, Collections.emptySet());
554+
}
555+
return path;
556+
}
436557
}

0 commit comments

Comments
 (0)