Skip to content

Commit e108076

Browse files
contrueCTcontrueCT
authored
fix(server): normalize bool range predicates in gremlin filters (apache#2991)
1 parent de8781e commit e108076

9 files changed

Lines changed: 555 additions & 15 deletions

File tree

‎hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/Condition.java‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ private static boolean equals(final Object first,
195195
*
196196
* @param first is actual value, might be Number/Date or String, It is
197197
* probably that the `first` is serialized to String.
198-
* @param second is value in query condition, must be Number/Date
198+
* @param second is value in query condition, must be Number/Date/Boolean
199199
* @return the value 0 if first is numerically equal to second;
200200
* a value less than 0 if first is numerically less than
201201
* second; and a value greater than 0 if first is
@@ -208,6 +208,8 @@ private static int compare(final Object first, final Object second) {
208208
(Number) second);
209209
} else if (second instanceof Date) {
210210
return compareDate(first, (Date) second);
211+
} else if (second instanceof Boolean) {
212+
return compareBoolean(first, (Boolean) second);
211213
}
212214

213215
throw new IllegalArgumentException(String.format(
@@ -230,6 +232,18 @@ private static int compareDate(Object first, Date second) {
230232
second, second.getClass().getSimpleName()));
231233
}
232234

235+
private static int compareBoolean(Object first, Boolean second) {
236+
if (first instanceof Boolean) {
237+
return Boolean.compare((Boolean) first, second);
238+
}
239+
240+
throw new IllegalArgumentException(String.format(
241+
"Can't compare between %s(%s) and %s(%s)",
242+
first, first == null ? null :
243+
first.getClass().getSimpleName(),
244+
second, second.getClass().getSimpleName()));
245+
}
246+
233247
private void checkBaseType(Object value, Class<?> clazz) {
234248
if (!clazz.isInstance(value)) {
235249
String valueClass = value == null ? "null" :

‎hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/ConditionQueryFlatten.java‎

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,23 @@ private static Condition flattenIn(Condition condition, boolean supportIn) {
115115
}
116116
case AND:
117117
Condition.And and = (Condition.And) condition;
118-
return new Condition.And(flattenIn(and.left(), supportIn),
119-
flattenIn(and.right(), supportIn));
118+
Condition andLeft = flattenIn(and.left(), supportIn);
119+
Condition andRight = flattenIn(and.right(), supportIn);
120+
if (andLeft == null || andRight == null) {
121+
return null;
122+
}
123+
return new Condition.And(andLeft, andRight);
120124
case OR:
121125
Condition.Or or = (Condition.Or) condition;
122-
return new Condition.Or(flattenIn(or.left(), supportIn),
123-
flattenIn(or.right(), supportIn));
126+
Condition orLeft = flattenIn(or.left(), supportIn);
127+
Condition orRight = flattenIn(or.right(), supportIn);
128+
if (orLeft == null) {
129+
return orRight;
130+
}
131+
if (orRight == null) {
132+
return orLeft;
133+
}
134+
return new Condition.Or(orLeft, orRight);
124135
default:
125136
throw new AssertionError(String.format("Wrong condition type: '%s'",
126137
condition.type()));
@@ -427,9 +438,26 @@ private static boolean validRange(Relation low, Relation high) {
427438
if (low == null || high == null) {
428439
return true;
429440
}
430-
return compare(low, high) < 0 || compare(low, high) == 0 &&
431-
low.relation() == Condition.RelationType.GTE &&
432-
high.relation() == Condition.RelationType.LTE;
441+
int compared = compare(low, high);
442+
if (compared > 0) {
443+
return false;
444+
}
445+
if (compared == 0) {
446+
return low.relation() == Condition.RelationType.GTE &&
447+
high.relation() == Condition.RelationType.LTE;
448+
}
449+
return !emptyBooleanRange(low, high);
450+
}
451+
452+
private static boolean emptyBooleanRange(Relation low, Relation high) {
453+
if (!(low.value() instanceof Boolean) ||
454+
!(high.value() instanceof Boolean)) {
455+
return false;
456+
}
457+
return Boolean.FALSE.equals(low.value()) &&
458+
Boolean.TRUE.equals(high.value()) &&
459+
low.relation() == Condition.RelationType.GT &&
460+
high.relation() == Condition.RelationType.LT;
433461
}
434462

435463
private static boolean validEq(Relation eq, Relation low, Relation high) {
@@ -507,6 +535,9 @@ private static int compare(Relation first, Relation second) {
507535
return NumericUtil.compareNumber(firstValue, (Number) secondValue);
508536
} else if (firstValue instanceof Date && secondValue instanceof Date) {
509537
return ((Date) firstValue).compareTo((Date) secondValue);
538+
} else if (firstValue instanceof Boolean &&
539+
secondValue instanceof Boolean) {
540+
return Boolean.compare((Boolean) firstValue, (Boolean) secondValue);
510541
} else {
511542
throw new IllegalArgumentException(String.format("Can't compare between %s and %s",
512543
first, second));

‎hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import org.apache.hugegraph.structure.HugeElement;
4747
import org.apache.hugegraph.structure.HugeProperty;
4848
import org.apache.hugegraph.type.HugeType;
49+
import org.apache.hugegraph.type.define.DataType;
4950
import org.apache.hugegraph.type.define.Directions;
5051
import org.apache.hugegraph.type.define.HugeKeys;
5152
import org.apache.hugegraph.util.CollectionUtil;
@@ -419,9 +420,9 @@ public static Condition convOr(HugeGraph graph,
419420
return cond;
420421
}
421422

422-
private static Condition.Relation convCompare2Relation(HugeGraph graph,
423-
HugeType type,
424-
HasContainer has) {
423+
private static Condition convCompare2Relation(HugeGraph graph,
424+
HugeType type,
425+
HasContainer has) {
425426
assert type.isGraph();
426427
BiPredicate<?, ?> bp = has.getPredicate().getBiPredicate();
427428
assert bp instanceof Compare;
@@ -459,16 +460,21 @@ private static Condition.Relation convCompare2SyspropRelation(HugeGraph graph,
459460
}
460461
}
461462

462-
private static Condition.Relation convCompare2UserpropRelation(HugeGraph graph,
463-
HugeType type,
464-
HasContainer has) {
463+
private static Condition convCompare2UserpropRelation(HugeGraph graph,
464+
HugeType type,
465+
HasContainer has) {
465466
BiPredicate<?, ?> bp = has.getPredicate().getBiPredicate();
466467
assert bp instanceof Compare;
467468

468469
String key = has.getKey();
469470
PropertyKey pkey = graph.propertyKey(key);
470471
Id pkeyId = pkey.id();
471472
Object value = validPropertyValue(has.getValue(), pkey);
473+
if (pkey.dataType() == DataType.BOOLEAN &&
474+
value instanceof Boolean) {
475+
return convCompare2BooleanUserpropRelation((Compare) bp, pkeyId,
476+
(Boolean) value);
477+
}
472478

473479
switch ((Compare) bp) {
474480
case eq:
@@ -488,6 +494,31 @@ private static Condition.Relation convCompare2UserpropRelation(HugeGraph graph,
488494
}
489495
}
490496

497+
private static Condition convCompare2BooleanUserpropRelation(Compare compare,
498+
Id key,
499+
Boolean value) {
500+
switch (compare) {
501+
case eq:
502+
return Condition.eq(key, value);
503+
case neq:
504+
return Condition.eq(key, !value);
505+
case gt:
506+
return value ? Condition.in(key, ImmutableList.of()) :
507+
Condition.eq(key, true);
508+
case gte:
509+
return value ? Condition.eq(key, true) :
510+
Condition.in(key, ImmutableList.of(false, true));
511+
case lt:
512+
return value ? Condition.eq(key, false) :
513+
Condition.in(key, ImmutableList.of());
514+
case lte:
515+
return value ? Condition.in(key, ImmutableList.of(false, true)) :
516+
Condition.eq(key, false);
517+
default:
518+
throw new AssertionError(compare);
519+
}
520+
}
521+
491522
private static Condition convRelationType2Relation(HugeGraph graph,
492523
HugeType type,
493524
HasContainer has) {

‎hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/EdgeCoreTest.java‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7140,6 +7140,165 @@ public void testQueryEdgeWithNullablePropertyInCompositeIndex() {
71407140
Assert.assertEquals(1, (int) el.get(0).value("id"));
71417141
}
71427142

7143+
@Test
7144+
public void testQueryEdgeByBooleanRangePredicate() {
7145+
HugeGraph graph = graph();
7146+
initStrikeIndex();
7147+
graph.schema().indexLabel("strikeByArrested").onE("strike").secondary()
7148+
.by("arrested").create();
7149+
7150+
Vertex louise = graph.addVertex(T.label, "person", "name", "Louise",
7151+
"city", "Beijing", "age", 21);
7152+
Vertex sean = graph.addVertex(T.label, "person", "name", "Sean",
7153+
"city", "Beijing", "age", 23);
7154+
long current = System.currentTimeMillis();
7155+
louise.addEdge("strike", sean, "id", 1,
7156+
"timestamp", current, "place", "park",
7157+
"tool", "shovel", "reason", "jeer",
7158+
"arrested", false);
7159+
louise.addEdge("strike", sean, "id", 2,
7160+
"timestamp", current + 1, "place", "street",
7161+
"tool", "shovel", "reason", "jeer",
7162+
"arrested", true);
7163+
7164+
List<Edge> hasLtEdges = graph.traversal().E()
7165+
.has("arrested", P.lt(true))
7166+
.toList();
7167+
Assert.assertEquals(1, hasLtEdges.size());
7168+
Assert.assertEquals(1, (int) hasLtEdges.get(0).value("id"));
7169+
7170+
List<Edge> whereEdges = graph.traversal().E()
7171+
.where(__.has("arrested", P.lt(true)))
7172+
.toList();
7173+
Assert.assertEquals(1, whereEdges.size());
7174+
Assert.assertEquals(1, (int) whereEdges.get(0).value("id"));
7175+
7176+
List<Edge> matchEdges = graph.traversal().E()
7177+
.match(__.as("start")
7178+
.where(__.has("arrested",
7179+
P.lt(true)))
7180+
.as("matched"))
7181+
.<Edge>select("matched")
7182+
.toList();
7183+
Assert.assertEquals(1, matchEdges.size());
7184+
Assert.assertEquals(1, (int) matchEdges.get(0).value("id"));
7185+
7186+
List<Edge> hasNeqTrueEdges = graph.traversal().E()
7187+
.has("arrested", P.neq(true))
7188+
.toList();
7189+
Assert.assertEquals(1, hasNeqTrueEdges.size());
7190+
Assert.assertEquals(1, (int) hasNeqTrueEdges.get(0).value("id"));
7191+
7192+
List<Edge> hasNeqFalseEdges = graph.traversal().E()
7193+
.has("arrested", P.neq(false))
7194+
.toList();
7195+
Assert.assertEquals(1, hasNeqFalseEdges.size());
7196+
Assert.assertEquals(2, (int) hasNeqFalseEdges.get(0).value("id"));
7197+
7198+
List<Edge> hasLteFalseEdges = graph.traversal().E()
7199+
.has("arrested", P.lte(false))
7200+
.toList();
7201+
Assert.assertEquals(1, hasLteFalseEdges.size());
7202+
Assert.assertEquals(1, (int) hasLteFalseEdges.get(0).value("id"));
7203+
7204+
List<Edge> hasGtFalseEdges = graph.traversal().E()
7205+
.has("arrested", P.gt(false))
7206+
.toList();
7207+
Assert.assertEquals(1, hasGtFalseEdges.size());
7208+
Assert.assertEquals(2, (int) hasGtFalseEdges.get(0).value("id"));
7209+
7210+
List<Edge> hasGteTrueEdges = graph.traversal().E()
7211+
.has("arrested", P.gte(true))
7212+
.toList();
7213+
Assert.assertEquals(1, hasGteTrueEdges.size());
7214+
Assert.assertEquals(2, (int) hasGteTrueEdges.get(0).value("id"));
7215+
7216+
List<Edge> hasGteFalseEdges = graph.traversal().E()
7217+
.has("arrested", P.gte(false))
7218+
.toList();
7219+
Assert.assertEquals(2, hasGteFalseEdges.size());
7220+
Set<Integer> gteFalseIds = new HashSet<>();
7221+
for (Edge edge : hasGteFalseEdges) {
7222+
gteFalseIds.add(edge.value("id"));
7223+
}
7224+
Assert.assertEquals(ImmutableSet.of(1, 2), gteFalseIds);
7225+
7226+
List<Edge> hasLteTrueEdges = graph.traversal().E()
7227+
.has("arrested", P.lte(true))
7228+
.toList();
7229+
Assert.assertEquals(2, hasLteTrueEdges.size());
7230+
Set<Integer> lteTrueIds = new HashSet<>();
7231+
for (Edge edge : hasLteTrueEdges) {
7232+
lteTrueIds.add(edge.value("id"));
7233+
}
7234+
Assert.assertEquals(ImmutableSet.of(1, 2), lteTrueIds);
7235+
7236+
Assert.assertEquals(0, graph.traversal().E()
7237+
.has("arrested", P.lt(false))
7238+
.toList().size());
7239+
Assert.assertEquals(0, graph.traversal().E()
7240+
.has("arrested", P.gt(true))
7241+
.toList().size());
7242+
}
7243+
7244+
@Test
7245+
public void testQueryEdgeByBooleanRangePredicateWithoutNullableProperty() {
7246+
HugeGraph graph = graph();
7247+
initStrikeIndex();
7248+
graph.schema().indexLabel("strikeByHurt").onE("strike").secondary()
7249+
.by("hurt").create();
7250+
7251+
Vertex louise = graph.addVertex(T.label, "person", "name", "Louise",
7252+
"city", "Beijing", "age", 21);
7253+
Vertex sean = graph.addVertex(T.label, "person", "name", "Sean",
7254+
"city", "Beijing", "age", 23);
7255+
long current = System.currentTimeMillis();
7256+
louise.addEdge("strike", sean, "id", 1,
7257+
"timestamp", current, "place", "park",
7258+
"tool", "shovel", "reason", "jeer",
7259+
"hurt", false, "arrested", false);
7260+
louise.addEdge("strike", sean, "id", 2,
7261+
"timestamp", current + 1, "place", "street",
7262+
"tool", "shovel", "reason", "jeer",
7263+
"hurt", true, "arrested", true);
7264+
louise.addEdge("strike", sean, "id", 3,
7265+
"timestamp", current + 2, "place", "mall",
7266+
"tool", "shovel", "reason", "jeer",
7267+
"arrested", false);
7268+
7269+
List<Edge> gteFalseEdges = graph.traversal().E()
7270+
.has("hurt", P.gte(false))
7271+
.toList();
7272+
Assert.assertEquals(2, gteFalseEdges.size());
7273+
Set<Integer> gteFalseIds = new HashSet<>();
7274+
for (Edge edge : gteFalseEdges) {
7275+
gteFalseIds.add(edge.value("id"));
7276+
}
7277+
Assert.assertEquals(ImmutableSet.of(1, 2), gteFalseIds);
7278+
7279+
List<Edge> lteTrueEdges = graph.traversal().E()
7280+
.has("hurt", P.lte(true))
7281+
.toList();
7282+
Assert.assertEquals(2, lteTrueEdges.size());
7283+
Set<Integer> lteTrueIds = new HashSet<>();
7284+
for (Edge edge : lteTrueEdges) {
7285+
lteTrueIds.add(edge.value("id"));
7286+
}
7287+
Assert.assertEquals(ImmutableSet.of(1, 2), lteTrueIds);
7288+
7289+
List<Edge> lteFalseEdges = graph.traversal().E()
7290+
.has("hurt", P.lte(false))
7291+
.toList();
7292+
Assert.assertEquals(1, lteFalseEdges.size());
7293+
Assert.assertEquals(1, (int) lteFalseEdges.get(0).value("id"));
7294+
7295+
List<Edge> neqTrueEdges = graph.traversal().E()
7296+
.has("hurt", P.neq(true))
7297+
.toList();
7298+
Assert.assertEquals(1, neqTrueEdges.size());
7299+
Assert.assertEquals(1, (int) neqTrueEdges.get(0).value("id"));
7300+
}
7301+
71437302
@Test
71447303
public void testQueryEdgeByPage() {
71457304
Assume.assumeTrue("Not support paging",

0 commit comments

Comments
 (0)