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


##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java:
##########
@@ -645,14 +656,70 @@ private static boolean 
extractHasContainers(HugeVertexStep<?> newStep,
 
     private static boolean canExtractHasContainers(HugeGraph graph,
                                                    HasContainerHolder holder) {
-        for (HasContainer has : holder.getHasContainers()) {
+        // Keep unsafe labels and their sibling properties for local filtering.
+        if (hasUnsafeLabelPredicate(holder)) {
+            return false;
+        }
+        List<HasContainer> hasContainers = holder.getHasContainers();
+        for (HasContainer has : hasContainers) {
             if (!canExtractHasContainer(graph, has)) {
                 return false;
             }
         }
         return true;
     }
 
+    private static boolean hasUnsafeLabelInChain(Step<?, ?> step,
+                                                 boolean 
followPositiveLabelOr) {
+        // Partial pushdown can lose candidates before local label filtering.
+        // FIXME: Restore selective pushdown when every candidate schema label
+        // has compatible index coverage for extracted property predicates.
+        while (step instanceof HasStep ||

Review Comment:
   Fixed in 2efd58d.
   
   `hasUnsafeLabelInChain()` now inspects local children of an unflattened 
`OrStep` / `AndStep` / `NotStep` when the linear walk stops there. For a 
mixed-key filter such as `has("city", "Beijing").or(__.has(T.label, 
P.neq("author")), __.has("status", "active"))`, the preceding property 
predicate stays local, so `collectMatchedIndexes()` cannot drop labels that 
lack that property index.
   
   Coverage added:
   - `TraversalUtilOptimizeTest` for both `HugeGraphStep` and `HugeVertexStep`
   - `VertexCoreTest` / `EdgeCoreTest` end-to-end regressions with the 
cross-label fixture
   
   Still out of scope here: non-connective steps such as `dedup()` / `order()` 
sitting between an indexed property and a later negative label. That is the 
same incomplete-prefix class, but it needs a real prefix walker rather than 
growing the step whitelist. I will take that in a follow-up PR.



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