Copilot commented on code in PR #3039:
URL: https://github.com/apache/hugegraph/pull/3039#discussion_r3336153966


##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java:
##########
@@ -166,11 +168,23 @@ public static void trySetGraph(Step<?, ?> step, HugeGraph 
graph) {
 
     public static void extractHasContainer(HugeGraphStep<?, ?> newStep,
                                            Traversal.Admin<?, ?> traversal) {
-        Step<?, ?> step = newStep;
-        do {
-            step = step.getNextStep();
+        Step<?, ?> step = newStep.getNextStep();
+        while (step instanceof HasStep || step instanceof NoOpBarrierStep) {
+            Step<?, ?> nextStep = step.getNextStep();
             if (step instanceof HasStep) {
                 HasContainerHolder holder = (HasContainerHolder) step;
+                /*
+                 * Range/neq predicates before match() may trigger a no-index
+                 * query after MatchStep reorders filters. Keep known-indexed
+                 * boolean predicates pushed down, and leave the rest for
+                 * TinkerPop to evaluate.
+                 */
+                if (followedByMatchStep(step) &&
+                    hasUnusableMatchPredicate(newStep, holder)) {
+                    extractUsableHasContainers(newStep, holder);
+                    step = nextStep;
+                    continue;
+                }

Review Comment:
   When the traversal is followed by `match()` and 
`hasUnusableMatchPredicate()` is true, the code extracts *some* `HasContainer`s 
into `HugeGraphStep` but leaves the original `HasStep` unchanged in the 
traversal. Since TinkerPop often accumulates multiple `has()` calls into a 
single `HasStep`, this can cause the extracted predicates to be evaluated twice 
(once by backend pushdown and again by the remaining `HasStep`). Besides 
redundant work, this risks semantic drift if backend predicate conversion 
differs from TinkerPop's `HasStep` evaluation (e.g., boolean predicate rewrites 
in `convCompare2BooleanUserpropRelation()`).
   
   Consider removing only the extracted `HasContainer`s from the `HasStep` (and 
removing the step entirely if it becomes empty), so each predicate is evaluated 
exactly once.



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