imbajin commented on code in PR #2991:
URL: https://github.com/apache/hugegraph/pull/2991#discussion_r3195695454
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/ConditionQueryFlattenTest.java:
##########
@@ -256,5 +256,107 @@ public void testFlattenWithNotIn() {
Collection<Condition> actual = queries.iterator().next().conditions();
Assert.assertEquals(expect, actual);
}
+
+ @Test
+ public void testFlattenWithBooleanRangeUpperBound() {
+ Id key = IdGenerator.of("c1");
+
+ ConditionQuery query = new ConditionQuery(HugeType.VERTEX);
+ query.query(Condition.lt(key, true));
+ query.query(Condition.lt(key, false));
+
+ List<ConditionQuery> queries = ConditionQueryFlatten.flatten(query);
+ Assert.assertEquals(1, queries.size());
+
+ Collection<Condition> actual = queries.iterator().next().conditions();
+ Assert.assertEquals(ImmutableList.of(Condition.lt(key, false)),
actual);
+ }
+
+ @Test
+ public void testFlattenWithBooleanRangeWindow() {
+ Id key = IdGenerator.of("c1");
+
+ ConditionQuery query = new ConditionQuery(HugeType.VERTEX);
+ query.query(Condition.gte(key, false));
+ query.query(Condition.lt(key, true));
+
+ List<ConditionQuery> queries = ConditionQueryFlatten.flatten(query);
+ Assert.assertEquals(1, queries.size());
+
+ Collection<Condition> actual = queries.iterator().next().conditions();
+ Assert.assertEquals(ImmutableList.of(Condition.gte(key, false),
+ Condition.lt(key, true)), actual);
+ }
+
+ @Test
+ public void testFlattenWithConflictingBooleanRange() {
+ Id key = IdGenerator.of("c1");
+
+ ConditionQuery query = new ConditionQuery(HugeType.VERTEX);
+ query.query(Condition.gt(key, false).and(Condition.lt(key, true)));
+
+ List<ConditionQuery> queries = ConditionQueryFlatten.flatten(query);
+ Assert.assertEquals(0, queries.size());
+ }
+
+ @Test
+ public void testFlattenWithImpossibleInInsideAnd() {
+ Id key = IdGenerator.of("c1");
+
+ ConditionQuery query = new ConditionQuery(HugeType.VERTEX);
+ query.query(Condition.in(key, ImmutableList.of())
+ .and(Condition.eq(key, true)));
+
+ List<ConditionQuery> queries = ConditionQueryFlatten.flatten(query);
+ Assert.assertEquals(0, queries.size());
+ }
+
+ @Test
+ public void testFlattenWithImpossibleInInsideOr() {
+ Id key = IdGenerator.of("c1");
+
+ ConditionQuery query = new ConditionQuery(HugeType.VERTEX);
+ Condition eq = Condition.eq(key, true);
+ query.query(Condition.in(key, ImmutableList.of()).or(eq));
+
+ List<ConditionQuery> queries = ConditionQueryFlatten.flatten(query);
+ Assert.assertEquals(1, queries.size());
+
+ Collection<Condition> actual = queries.iterator().next().conditions();
+ Assert.assertEquals(ImmutableList.of(eq), actual);
+ }
+
+ @Test
+ public void testFlattenWithImpossibleInInsideOrRight() {
+ Id key = IdGenerator.of("c1");
+
+ ConditionQuery query = new ConditionQuery(HugeType.VERTEX);
+ Condition eq = Condition.eq(key, true);
+ query.query(eq.or(Condition.in(key, ImmutableList.of())));
+
+ List<ConditionQuery> queries = ConditionQueryFlatten.flatten(query);
+ Assert.assertEquals(1, queries.size());
+
+ Collection<Condition> actual = queries.iterator().next().conditions();
+ Assert.assertEquals(ImmutableList.of(eq), actual);
+ }
Review Comment:
๐งน **Minor โ consider adding a deeper-nested regression test for `IN []`
null-propagation**
The new `testFlattenWithImpossibleInInsideAnd/Or/OrRight` cases all exercise
**2-level** trees (`AND(IN[], eq)` / `OR(IN[], eq)`). The new `flattenIn`
null-propagation is recursive and should handle arbitrary depth correctly, but
there's no direct assertion for 3+ level trees such as:
```java
// AND( OR(IN[], eq(a)), eq(b) ) โ eq(a) AND eq(b)
query.query(Condition.in(key, ImmutableList.of()).or(Condition.eq(key, "a"))
.and(Condition.eq(key, "b")));
```
**Why this matters:** the current implementation is correct today, but if
someone later refactors `flattenIn` (e.g. adds short-circuits, converts to
iterative, or caches intermediate nodes), deep-nested null propagation is easy
to break without a direct test signal. A single 3-level AND-over-OR case would
pin this behavior cheaply.
**Impact if not added:** no correctness regression today; moderate future
regression risk when `flattenIn` is touched. Happy for this to land as-is and
add in a follow-up if preferred.
--
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]
