Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ private static boolean equals(final Object first,
*
* @param first is actual value, might be Number/Date or String, It is
* probably that the `first` is serialized to String.
* @param second is value in query condition, must be Number/Date
* @param second is value in query condition, must be Number/Date/Boolean
* @return the value 0 if first is numerically equal to second;
* a value less than 0 if first is numerically less than
* second; and a value greater than 0 if first is
Expand All @@ -208,6 +208,8 @@ private static int compare(final Object first, final Object second) {
(Number) second);
} else if (second instanceof Date) {
return compareDate(first, (Date) second);
} else if (second instanceof Boolean) {
return compareBoolean(first, (Boolean) second);
}

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

private static int compareBoolean(Object first, Boolean second) {
if (first instanceof Boolean) {
return Boolean.compare((Boolean) first, second);
}

throw new IllegalArgumentException(String.format(
"Can't compare between %s(%s) and %s(%s)",
first, first == null ? null :
first.getClass().getSimpleName(),
second, second.getClass().getSimpleName()));
}

private void checkBaseType(Object value, Class<?> clazz) {
if (!clazz.isInstance(value)) {
String valueClass = value == null ? "null" :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -427,9 +427,26 @@ private static boolean validRange(Relation low, Relation high) {
if (low == null || high == null) {
return true;
}
return compare(low, high) < 0 || compare(low, high) == 0 &&
low.relation() == Condition.RelationType.GTE &&
high.relation() == Condition.RelationType.LTE;
int compared = compare(low, high);
if (compared > 0) {
return false;
}
if (compared == 0) {
return low.relation() == Condition.RelationType.GTE &&
high.relation() == Condition.RelationType.LTE;
}
return !emptyBooleanRange(low, high);
}

private static boolean emptyBooleanRange(Relation low, Relation high) {
if (!(low.value() instanceof Boolean) ||
!(high.value() instanceof Boolean)) {
return false;
}
return Boolean.FALSE.equals(low.value()) &&
Boolean.TRUE.equals(high.value()) &&
low.relation() == Condition.RelationType.GT &&
high.relation() == Condition.RelationType.LT;
}

private static boolean validEq(Relation eq, Relation low, Relation high) {
Expand Down Expand Up @@ -507,6 +524,9 @@ private static int compare(Relation first, Relation second) {
return NumericUtil.compareNumber(firstValue, (Number) secondValue);
} else if (firstValue instanceof Date && secondValue instanceof Date) {
return ((Date) firstValue).compareTo((Date) secondValue);
} else if (firstValue instanceof Boolean &&
secondValue instanceof Boolean) {
return Boolean.compare((Boolean) firstValue, (Boolean) secondValue);
} else {
throw new IllegalArgumentException(String.format("Can't compare between %s and %s",
first, second));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import org.apache.hugegraph.structure.HugeElement;
import org.apache.hugegraph.structure.HugeProperty;
import org.apache.hugegraph.type.HugeType;
import org.apache.hugegraph.type.define.DataType;
import org.apache.hugegraph.type.define.Directions;
import org.apache.hugegraph.type.define.HugeKeys;
import org.apache.hugegraph.util.CollectionUtil;
Expand Down Expand Up @@ -419,9 +420,9 @@ public static Condition convOr(HugeGraph graph,
return cond;
}

private static Condition.Relation convCompare2Relation(HugeGraph graph,
HugeType type,
HasContainer has) {
private static Condition convCompare2Relation(HugeGraph graph,
HugeType type,
HasContainer has) {
assert type.isGraph();
BiPredicate<?, ?> bp = has.getPredicate().getBiPredicate();
assert bp instanceof Compare;
Expand Down Expand Up @@ -459,16 +460,21 @@ private static Condition.Relation convCompare2SyspropRelation(HugeGraph graph,
}
}

private static Condition.Relation convCompare2UserpropRelation(HugeGraph graph,
HugeType type,
HasContainer has) {
private static Condition convCompare2UserpropRelation(HugeGraph graph,
HugeType type,
HasContainer has) {
BiPredicate<?, ?> bp = has.getPredicate().getBiPredicate();
assert bp instanceof Compare;

String key = has.getKey();
PropertyKey pkey = graph.propertyKey(key);
Id pkeyId = pkey.id();
Object value = validPropertyValue(has.getValue(), pkey);
if (pkey.dataType() == DataType.BOOLEAN &&
value instanceof Boolean) {
return convCompare2BooleanUserpropRelation((Compare) bp, pkeyId,
(Boolean) value);
}

switch ((Compare) bp) {
case eq:
Expand All @@ -488,6 +494,31 @@ private static Condition.Relation convCompare2UserpropRelation(HugeGraph graph,
}
}

private static Condition convCompare2BooleanUserpropRelation(Compare compare,
Id key,
Boolean value) {
switch (compare) {
case eq:
return Condition.eq(key, value);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ neq 未被规范化,可能无法利用二级索引

neq(true) 语义等价于 eq(false)neq(false) 等价于 eq(true)。当前直接透传 Condition.neq(key, value) 到 backend,而 neq 通常走全扫描 + 过滤,无法命中 secondary index(例如 strikeByArrested)。

该 PR 的目标之一是让 boolean 谓词走正确的索引路径,neq 也应一并处理:

case neq:
    return Condition.eq(key, !value);

这样 neq(true)eq(false),可以命中 secondary index。

case neq:
return Condition.eq(key, !value);
case gt:
return value ? Condition.in(key, ImmutableList.of()) :
Condition.eq(key, true);
case gte:
return value ? Condition.eq(key, true) :
Condition.in(key, ImmutableList.of(false, true));
Comment thread
contrueCT marked this conversation as resolved.
case lt:
return value ? Condition.eq(key, false) :
Condition.in(key, ImmutableList.of());
case lte:
return value ? Condition.in(key, ImmutableList.of(false, true)) :
Condition.eq(key, false);
default:
throw new AssertionError(compare);
}
}

private static Condition convRelationType2Relation(HugeGraph graph,
HugeType type,
HasContainer has) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7117,6 +7117,165 @@ public void testQueryEdgeWithNullablePropertyInCompositeIndex() {
Assert.assertEquals(1, (int) el.get(0).value("id"));
}

@Test
public void testQueryEdgeByBooleanRangePredicate() {
HugeGraph graph = graph();
initStrikeIndex();
graph.schema().indexLabel("strikeByArrested").onE("strike").secondary()
.by("arrested").create();

Vertex louise = graph.addVertex(T.label, "person", "name", "Louise",
"city", "Beijing", "age", 21);
Vertex sean = graph.addVertex(T.label, "person", "name", "Sean",
"city", "Beijing", "age", 23);
long current = System.currentTimeMillis();
louise.addEdge("strike", sean, "id", 1,
"timestamp", current, "place", "park",
"tool", "shovel", "reason", "jeer",
"arrested", false);
louise.addEdge("strike", sean, "id", 2,
"timestamp", current + 1, "place", "street",
"tool", "shovel", "reason", "jeer",
"arrested", true);

List<Edge> hasLtEdges = graph.traversal().E()
.has("arrested", P.lt(true))
.toList();
Assert.assertEquals(1, hasLtEdges.size());
Assert.assertEquals(1, (int) hasLtEdges.get(0).value("id"));

List<Edge> whereEdges = graph.traversal().E()
.where(__.has("arrested", P.lt(true)))
.toList();
Assert.assertEquals(1, whereEdges.size());
Assert.assertEquals(1, (int) whereEdges.get(0).value("id"));

List<Edge> matchEdges = graph.traversal().E()
.match(__.as("start")
.where(__.has("arrested",
P.lt(true)))
.as("matched"))
.<Edge>select("matched")
.toList();
Assert.assertEquals(1, matchEdges.size());
Assert.assertEquals(1, (int) matchEdges.get(0).value("id"));

List<Edge> hasNeqTrueEdges = graph.traversal().E()
.has("arrested", P.neq(true))
.toList();
Assert.assertEquals(1, hasNeqTrueEdges.size());
Assert.assertEquals(1, (int) hasNeqTrueEdges.get(0).value("id"));

List<Edge> hasNeqFalseEdges = graph.traversal().E()
.has("arrested", P.neq(false))
.toList();
Assert.assertEquals(1, hasNeqFalseEdges.size());
Assert.assertEquals(2, (int) hasNeqFalseEdges.get(0).value("id"));

List<Edge> hasLteFalseEdges = graph.traversal().E()
.has("arrested", P.lte(false))
.toList();
Assert.assertEquals(1, hasLteFalseEdges.size());
Assert.assertEquals(1, (int) hasLteFalseEdges.get(0).value("id"));

List<Edge> hasGtFalseEdges = graph.traversal().E()
.has("arrested", P.gt(false))
.toList();
Assert.assertEquals(1, hasGtFalseEdges.size());
Assert.assertEquals(2, (int) hasGtFalseEdges.get(0).value("id"));

List<Edge> hasGteTrueEdges = graph.traversal().E()
.has("arrested", P.gte(true))
.toList();
Assert.assertEquals(1, hasGteTrueEdges.size());
Assert.assertEquals(2, (int) hasGteTrueEdges.get(0).value("id"));

List<Edge> hasGteFalseEdges = graph.traversal().E()
.has("arrested", P.gte(false))
.toList();
Assert.assertEquals(2, hasGteFalseEdges.size());
Set<Integer> gteFalseIds = new HashSet<>();
for (Edge edge : hasGteFalseEdges) {
gteFalseIds.add(edge.value("id"));
}
Assert.assertEquals(ImmutableSet.of(1, 2), gteFalseIds);

List<Edge> hasLteTrueEdges = graph.traversal().E()
.has("arrested", P.lte(true))
.toList();
Assert.assertEquals(2, hasLteTrueEdges.size());
Set<Integer> lteTrueIds = new HashSet<>();
for (Edge edge : hasLteTrueEdges) {
lteTrueIds.add(edge.value("id"));
}
Assert.assertEquals(ImmutableSet.of(1, 2), lteTrueIds);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 缺少 neq 谓词的端到端测试,且仅覆盖了 Edge

  • P.neq(true) / P.neq(false) 没有集成测试,而上面的 neq 实现路径与其他谓词不同(如果接受上述 neq 规范化建议,需要补充验证)
  • 所有新增集成测试均在 EdgeCoreTest,建议同步在 VertexCoreTest 中添加对应的 boolean range 覆盖,确认 TraversalUtil.convCompare2UserpropRelationHugeType.VERTEX 也正常工作

Assert.assertEquals(0, graph.traversal().E()
.has("arrested", P.lt(false))
.toList().size());
Assert.assertEquals(0, graph.traversal().E()
.has("arrested", P.gt(true))
.toList().size());
}

@Test
public void testQueryEdgeByBooleanRangePredicateWithoutNullableProperty() {
HugeGraph graph = graph();
initStrikeIndex();
graph.schema().indexLabel("strikeByHurt").onE("strike").secondary()
.by("hurt").create();

Vertex louise = graph.addVertex(T.label, "person", "name", "Louise",
"city", "Beijing", "age", 21);
Vertex sean = graph.addVertex(T.label, "person", "name", "Sean",
"city", "Beijing", "age", 23);
long current = System.currentTimeMillis();
louise.addEdge("strike", sean, "id", 1,
"timestamp", current, "place", "park",
"tool", "shovel", "reason", "jeer",
"hurt", false, "arrested", false);
louise.addEdge("strike", sean, "id", 2,
"timestamp", current + 1, "place", "street",
"tool", "shovel", "reason", "jeer",
"hurt", true, "arrested", true);
louise.addEdge("strike", sean, "id", 3,
"timestamp", current + 2, "place", "mall",
"tool", "shovel", "reason", "jeer",
"arrested", false);

List<Edge> gteFalseEdges = graph.traversal().E()
.has("hurt", P.gte(false))
.toList();
Assert.assertEquals(2, gteFalseEdges.size());
Set<Integer> gteFalseIds = new HashSet<>();
for (Edge edge : gteFalseEdges) {
gteFalseIds.add(edge.value("id"));
}
Assert.assertEquals(ImmutableSet.of(1, 2), gteFalseIds);

List<Edge> lteTrueEdges = graph.traversal().E()
.has("hurt", P.lte(true))
.toList();
Assert.assertEquals(2, lteTrueEdges.size());
Set<Integer> lteTrueIds = new HashSet<>();
for (Edge edge : lteTrueEdges) {
lteTrueIds.add(edge.value("id"));
}
Assert.assertEquals(ImmutableSet.of(1, 2), lteTrueIds);

List<Edge> lteFalseEdges = graph.traversal().E()
.has("hurt", P.lte(false))
.toList();
Assert.assertEquals(1, lteFalseEdges.size());
Assert.assertEquals(1, (int) lteFalseEdges.get(0).value("id"));

List<Edge> neqTrueEdges = graph.traversal().E()
.has("hurt", P.neq(true))
.toList();
Assert.assertEquals(1, neqTrueEdges.size());
Assert.assertEquals(1, (int) neqTrueEdges.get(0).value("id"));
}

@Test
public void testQueryEdgeByPage() {
Assume.assumeTrue("Not support paging",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6470,6 +6470,34 @@ public void testQueryVertexByPropertyWithEmptyString() {
Assert.assertEquals("", vertex.value("city"));
}

@Test
public void testQueryVertexByBooleanNeqPredicate() {
HugeGraph graph = graph();
graph.schema().indexLabel("languageByDynamic").onV("language")
.secondary().by("dynamic").create();

graph.addVertex(T.label, "language", "name", "java",
"dynamic", true);
graph.addVertex(T.label, "language", "name", "rust",
"dynamic", false);
graph.addVertex(T.label, "language", "name", "c");
this.commitTx();

List<Vertex> neqTrueVertices = graph.traversal().V()
.hasLabel("language")
.has("dynamic", P.neq(true))
.toList();
Assert.assertEquals(1, neqTrueVertices.size());
Assert.assertEquals("rust", neqTrueVertices.get(0).value("name"));

List<Vertex> neqFalseVertices = graph.traversal().V()
.hasLabel("language")
.has("dynamic", P.neq(false))
.toList();
Assert.assertEquals(1, neqFalseVertices.size());
Assert.assertEquals("java", neqFalseVertices.get(0).value("name"));
}

@Test
public void testQueryVertexBeforeAfterUpdateMultiPropertyWithIndex() {
HugeGraph graph = graph();
Expand Down
Loading
Loading