bitflicker64 commented on code in PR #2994:
URL: https://github.com/apache/hugegraph/pull/2994#discussion_r3978442250


##########
docs/negative-label-queries.md:
##########
@@ -0,0 +1,53 @@
+# Negative-label queries and local filtering
+
+HugeGraph keeps property filters local when pushing them into an index could
+discard vertices or edges needed by a later label predicate. For example,
+`hasLabel(P.neq("author"))` includes other labels even if they do not have the 
same
+property indexes. This also applies to unsafe label predicates across barriers,
+ranges and child traversals where the optimizer cannot prove a narrower scope.
+
+## Result completeness changes the no-index behavior
+
+Consider a defined property `unindexedProp` with no property index:
+
+```groovy
+g.V().has("unindexedProp", "x")
+g.V().has("unindexedProp", "x").hasLabel(P.neq("author"))
+```
+
+The first query uses the indexed-property query path and raises
+`NoIndexException`. The second keeps the property predicate local and can scan
+vertices, returning matching non-author vertices. It does **not** use the
+missing-index exception as a fast-fail guard. This is intentional: selecting
+only labels with a matching index could silently omit valid results.
+
+This fallback can turn a selective index lookup into a full candidate scan,
+increasing latency and backend work even when very few results match. Adding an
+index to one label alone does not guarantee that this conservative fallback 
will
+use it. When possible, specify a known positive label with a suitable index, or
+start from explicit element IDs. Explicit-ID lookups and adjacent-element
+traversals can filter their own candidates locally; they do not necessarily 
scan
+the whole graph.
+
+## Limits and paging
+
+Existing query capacity checks still apply where the execution path enforces
+them. The default capacity is 800,000 records; a candidate scan can reach this
+limit before finding all matching results and raise `LimitExceedException`.
+This is not a universal work bound: the test-only Memory backend does not 
enforce
+scan capacity, some count paths disable capacity checks, and a final `limit()`
+bounds returned matches rather than all candidates examined.
+
+With `has("~page", cursor)`, the backend page is bounded before local 
filtering.
+A page may contain fewer matches than requested, or no matches at all, while
+still returning a continuation cursor. Continue until the cursor is exhausted;
+do not stop solely because the filtered page is empty. Backends without paging
+support cannot use this mechanism.
+
+## SEARCH predicates
+
+Local `Text.contains()` filters use the graph's SEARCH analyzer and exact term

Review Comment:
   🧹 This sentence never says which `Text.contains()` predicates count as 
local, and one ordinary position is not covered. The unbound-child cause raised 
on the earlier head is fixed in `2d53a556`; this is a different position.
   
   Evidence:
   
   - `prepareLocalHasContainers()` walks only `while (step instanceof HasStep 
|| step instanceof NoOpBarrierStep)` (`TraversalUtil.java:694`) and installs 
`LocalSearchHasContainer` only for containers found in that walk (`:742-751`).
   - A `Text.contains()` sitting after a `RangeGlobalStep` or 
`OrderGlobalStep`, for example 
`g.V().hasLabel(P.neq('author')).limit(10).has('body', 
Text.contains('(alpha)'))`, is never reached by that walk and keeps the raw 
`Condition.RelationType.TEXT_CONTAINS` predicate, which is a plain substring 
test with no `(word)` handling: `return v1 != null && ((String) 
v1).contains((String) v2);` (`Condition.java:94-97`).
   - This PR's own test pins the difference: 
`Assert.assertFalse(Text.contains("(alpha)").test("alpha"))` 
(`TraversalUtilOptimizeTest.java:534`).
   
   Requested change: name the scope, for example "`Text.contains()` predicates 
in the filter chain directly following the source step use the graph's SEARCH 
analyzer and exact term matcher, including explicit `(word)` and 
`(word1|word2)` expressions; one placed after `range()` or `order()` keeps 
plain substring semantics."



##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java:
##########
@@ -602,6 +627,10 @@ private static boolean hasOnlyRangePredicates(HasContainer 
has) {
 
     public static void extractHasContainer(HugeVertexStep<?> newStep,
                                            Traversal.Admin<?, ?> traversal) {
+        if (hasUnsafeLabelInTraversal(traversal, newStep)) {
+            prepareLocalHasContainers(newStep, traversal);

Review Comment:
   ⚠️ The edge side of this fallback has no paging regression, and on this path 
it replaces a hard rejection with a short page.
   
   Evidence:
   
   - It is reached for 
`g.V(v).outE().has('~page','').hasLabel(P.neq('knows')).limit(10)`: the folded 
`HasStep` fails `isEqInLabelPredicate`, so `hasUnsafeLabelInTraversal` returns 
true.
   - `prepareLocalHasContainers()` sends `~page` through 
`query.addHasContainer(has)` (line 701), which 
`HugeVertexStep.addHasContainer()` turns into `setPage()` and returns 
(`HugeVertexStep.java:213-217`). The label takes the `else` at line 729 because 
`source instanceof HugeGraphStep` is false, so `HugeVertexStep.hasContainers` 
stays empty.
   - `withEdgeCondition()` and `withVertexCondition()` are 
`!this.hasContainers.isEmpty()` (`HugeVertexStep.java:185-191`), so 
`E.checkArgument(!this.queryInfo().paging(), "Can't query by paging and 
filtering")` (`:173-176`) cannot fire on this path. At merge-base `36811483` it 
did fire: `canExtractHasContainer()` returned true for any sysprop key 
(`:658-660` there), so both `~page` and `~label` were pushed into the step and 
the traversal was rejected outright.
   - The short-page contract is deliberate and documented 
(`docs/negative-label-queries.md:41-45`, plus the range bound at lines 
760-765), but the only regression for it is 
`VertexCoreTest#testPageBeforeDownstreamNegativeLabel`. The five new 
`EdgeCoreTest#testQueryEdgesByNonEqLabel*` cases cover barrier, range, 
mixed-key `or` and `sideEffect`, none with `~page`.
   
   Requested change: add an edge-side paging regression next to 
`testPageBeforeDownstreamNegativeLabel` that pages `g.V(v).outE().has('~page', 
cursor)` with a downstream negative label to cursor exhaustion and asserts no 
missing or duplicate edge ids, so the shape that used to be rejected is pinned.



-- 
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]

Reply via email to