imbajin commented on code in PR #3034:
URL: https://github.com/apache/hugegraph/pull/3034#discussion_r3281725258
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java:
##########
@@ -166,38 +170,219 @@ 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;
- for (HasContainer has : holder.getHasContainers()) {
- if (!GraphStep.processHasContainerIds(newStep, has)) {
- newStep.addHasContainer(has);
- }
+ if (extractHasContainers(newStep, holder)) {
+ TraversalHelper.copyLabels(step, step.getPreviousStep(),
false);
+ traversal.removeStep(step);
}
- TraversalHelper.copyLabels(step, step.getPreviousStep(),
false);
- traversal.removeStep(step);
}
- } while (step instanceof HasStep || step instanceof NoOpBarrierStep);
+ step = nextStep;
+ }
}
public static void extractHasContainer(HugeVertexStep<?> newStep,
Traversal.Admin<?, ?> traversal) {
Step<?, ?> step = newStep;
do {
+ Step<?, ?> nextStep = step.getNextStep();
if (step instanceof HasStep) {
HasContainerHolder holder = (HasContainerHolder) step;
- for (HasContainer has : holder.getHasContainers()) {
- newStep.addHasContainer(has);
+ if (extractHasContainers(newStep, holder)) {
+ TraversalHelper.copyLabels(step, step.getPreviousStep(),
false);
+ traversal.removeStep(step);
}
- TraversalHelper.copyLabels(step, step.getPreviousStep(),
false);
- traversal.removeStep(step);
}
- step = step.getNextStep();
+ step = nextStep;
} while (step instanceof HasStep || step instanceof NoOpBarrierStep);
}
+ private static boolean extractHasContainers(HugeGraphStep<?, ?> newStep,
+ HasContainerHolder holder) {
+ HugeGraph graph = TraversalUtil.tryGetGraph(newStep);
+ List<HasContainer> hasContainers = new
ArrayList<>(holder.getHasContainers());
+ HugeType type = newStep.returnsVertex() ? HugeType.VERTEX :
HugeType.EDGE;
+ Set<Id> labelIds = collectLabelIds(graph, type,
newStep.getHasContainers(),
+ hasContainers);
+ for (HasContainer has : hasContainers) {
+ if (!canExtractHasContainer(graph, type, labelIds, has)) {
+ continue;
+ }
+ if (!GraphStep.processHasContainerIds(newStep, has)) {
+ newStep.addHasContainer(has);
+ }
+ holder.removeHasContainer(has);
Review Comment:
Thanks for adjusting the direction. To clarify my previous comment: I do not
mean that we should choose freely between the two shapes. For this PR, I would
prefer the minimal option A.
The original issue is small and focused: `Text` range predicates should not
be extracted into the backend query path. So the safest fix here is to add the
narrow condition that blocks only that unsafe case, while keeping the existing
extraction behavior unchanged for the rest.
```text
Preferred shape for this PR
HasStep(vp4 < "", age = 1)
|
v
if any extracted container is Text + range:
keep this HasStep in traversal layer
avoid backend text-range extraction
No new index-planner logic
No mixed HasStep split/remove behavior
No broader optimizer contract change
```
This may leave a small performance loss for the uncommon case where one
`has()` step mixes a text-range predicate with other safe predicates, but it
keeps the patch proportional to the bug and avoids introducing new correctness
risks around label context, index matching, composite index prefixes, paging,
or unique/search/numeric index behavior.
PS: the more aggressive split-and-remove optimization can still be
considered later
--
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]
