bitflicker64 commented on code in PR #2994:
URL: https://github.com/apache/hugegraph/pull/2994#discussion_r3901394222
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java:
##########
@@ -645,14 +656,67 @@ 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 || step instanceof NoOpBarrierStep) {
Review Comment:
‼️ A `RangeGlobalStep` between the property filter and the label filter
defeats this pre-scan, and results go silently incomplete.
This walk covers only `HasStep` and `NoOpBarrierStep`, so `limit()`,
`range()` and `skip()` end it. The extraction loop at line 180 stops there too,
but by then it has already pushed the earlier property filter into
`HugeGraphStep`, and `extractRange()` pushes the range down after it.
Run on the fixture `testQueryByNonEqLabelAndIndexedPropertyAcrossBarrier`
builds (memory backend, `initPersonIndex(true)`, `init5Persons()`, plus a `fan`
vertex with `city=Beijing` and no city index):
```
g.V().has("city","Beijing").barrier().has(T.label, P.neq("author"))
[HugeGraphStep(vertex,[]), HasStep([city.eq(Beijing)]), NoOpBarrierStep,
HasStep([~label.neq(author)])]
-> [unindexed-city-fan, James, Tom Cat, Lisa]
g.V().has("city","Beijing").skip(0).has(T.label, P.neq("author"))
[HugeGraphStep(Vertex,[city.eq(Beijing)]), RangeGlobalStep(0,MAX),
HasStep([~label.neq(author)])]
-> [James, Tom Cat, Lisa]
```
`unindexed-city-fan` is gone. `.limit(10)` and `.limit(1000)` give the same
three names, so this is not range truncation: the vertex is never fetched,
because `city=Beijing` went into an index plan covering only labels that have a
city index. `.skip(0)` is a semantic no-op and is already enough to trigger it.
Requested change: extend this walk to the steps the pushdown itself crosses,
at minimum `RangeGlobalStep`, and `IdentityStep` which
`positiveLabelOnlyOrStepAfter()` at line 283 already walks. Please add a
`VertexCoreTest` regression mirroring
`testQueryByNonEqLabelAndIndexedPropertyAcrossBarrier` with `.limit(n)` above
the match count in place of `.barrier()`, and the `EdgeCoreTest` counterpart.
For context: master is incomplete for both shapes, so this is a gap in the
new guard rather than a regression against master.
--
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]
