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


##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java:
##########
@@ -188,6 +192,10 @@ public static void extractHasContainer(HugeVertexStep<?> 
newStep,
         do {
             if (step instanceof HasStep) {
                 HasContainerHolder holder = (HasContainerHolder) step;
+                if (!canExtractHasContainers(TraversalUtil.getGraph(newStep),
+                                             holder.getHasContainers())) {
+                    break;
+                }

Review Comment:
   Same concern as the HugeGraphStep overload: calling `getGraph(newStep)` can 
throw for traversals without an attached graph (known to happen in some 
`__`/child traversals per tinkerpop#1699 workarounds), and `break` prevents 
extracting later `HasStep`s. Use `tryGetGraph` with a null-safe fallback, and 
consider extracting only safe `HasContainer`s (leaving unsafe ones in the 
`HasStep`) instead of all-or-nothing stopping extraction.



##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java:
##########
@@ -171,6 +171,10 @@ public static void extractHasContainer(HugeGraphStep<?, ?> 
newStep,
             step = step.getNextStep();
             if (step instanceof HasStep) {
                 HasContainerHolder holder = (HasContainerHolder) step;
+                if (!canExtractHasContainers(TraversalUtil.getGraph(newStep),
+                                             holder.getHasContainers())) {
+                    break;
+                }

Review Comment:
   `TraversalUtil.getGraph(newStep)` can throw when the traversal has no graph 
attached (this repo already acknowledges such cases in HugeVertexStepStrategy 
via `tryGetGraph(...)` for tinkerpop#1699). Consider using 
`tryGetGraph(newStep)` and, when it’s null, skipping the text-range check (or 
conservatively avoiding extraction) rather than throwing. Also, using `break` 
here stops extracting any subsequent `HasStep`s and keeps otherwise-extractable 
filters in the traversal; prefer selectively extracting only safe 
`HasContainer`s and only removing the `HasStep` if all its containers were 
extracted.



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