imbajin commented on code in PR #2991:
URL: https://github.com/apache/hugegraph/pull/2991#discussion_r3147578899
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/Condition.java:
##########
@@ -230,6 +232,20 @@ private static int compareDate(Object first, Date second) {
second, second.getClass().getSimpleName()));
}
+ private static int compareBoolean(Object first, Boolean second) {
+ if (first == null) {
Review Comment:
⚠️ **null 静默转为 false，与 compareDate 行为不一致**
`compareDate` 遇到非 Date 的 `first` 时直接 throw，但 `compareBoolean` 将 `null` 静默设为
`false`。这会掩盖两类问题：
1. 属性不存在（null = 未设置）与属性值为 false 在语义上是不同的
2. 如果 `first` 是 null，后面的 `first.getClass().getSimpleName()` 本会 NPE，这里只是绕过了
NPE，没有真正解决问题
建议对齐 `compareDate` 的风格，同时修复 NPE 隐患：
```java
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()));
}
```
如果业务上确实需要 null→false 的语义（某些 backend 存储缺省值为 null），请在注释中说明原因。
##########
hugegraph-struct/src/main/java/org/apache/hugegraph/query/Condition.java:
##########
@@ -472,6 +474,20 @@ private static int compareDate(Object first, Date second) {
second, second.getClass().getSimpleName()));
}
+ private static int compareBoolean(Object first, Boolean second) {
+ if (first == null) {
Review Comment:
⚠️ 同上：`hugegraph-struct` 的 `compareBoolean` 有相同的 null→false
静默处理问题。两份拷贝需保持一致，建议同步修改。
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java:
##########
@@ -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);
Review Comment:
⚠️ **`neq` 未被规范化，可能无法利用二级索引**
`neq(true)` 语义等价于 `eq(false)`，`neq(false)` 等价于 `eq(true)`。当前直接透传
`Condition.neq(key, value)` 到 backend，而 neq 通常走全扫描 + 过滤，无法命中 secondary index（例如
`strikeByArrested`）。
该 PR 的目标之一是让 boolean 谓词走正确的索引路径，`neq` 也应一并处理：
```java
case neq:
return Condition.eq(key, !value);
```
这样 `neq(true)` → `eq(false)`，可以命中 secondary index。
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/EdgeCoreTest.java:
##########
@@ -7117,6 +7117,147 @@ 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> 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);
+
Review Comment:
🧹 **缺少 `neq` 谓词的端到端测试，且仅覆盖了 Edge**
- `P.neq(true)` / `P.neq(false)` 没有集成测试，而上面的 neq 实现路径与其他谓词不同（如果接受上述 neq
规范化建议，需要补充验证）
- 所有新增集成测试均在 `EdgeCoreTest`，建议同步在 `VertexCoreTest` 中添加对应的 boolean range
覆盖，确认 `TraversalUtil.convCompare2UserpropRelation` 对 `HugeType.VERTEX` 也正常工作
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
