Skip to content

fix(core): keep unsafe range filters local#3068

Open
LegendPei wants to merge 4 commits into
apache:masterfrom
LegendPei:fix/issue-2929-connective-range
Open

fix(core): keep unsafe range filters local#3068
LegendPei wants to merge 4 commits into
apache:masterfrom
LegendPei:fix/issue-2929-connective-range

Conversation

@LegendPei

@LegendPei LegendPei commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Purpose of the PR

Main Changes

  • Avoid pushing unsafe non-indexed range/neq predicates into HugeGraphStep when followed by match/connective label filters.
  • Still push down usable predicates, such as label filters and indexed conditions, to keep backend filtering where it is safe.
  • Keep remaining unsafe predicates as local HasStep filters.
  • Prevent count extraction from crossing a remaining local HasStep, so local filters are evaluated before count().
  • Add a regression test for connective label filtering with a non-indexed range predicate.

Verifying these changes

  • Trivial rework / code cleanup without any test coverage. (No Need)
  • Already covered by existing tests, such as (please modify tests here).
  • Need tests and can be verified as follows:
    • xxx

Does this PR potentially affect the following parts?

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need

@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jun 25, 2026
imbajin

This comment was marked as outdated.

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Jun 25, 2026

@imbajin imbajin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: yes. The current head still misses the same unsafe range + connective label class for label-only or() filters. I verified this on a temporary detached worktree by adding a minimal regression traversal using or(__.hasLabel("el2"), __.hasLabel("el3")); the PR head still throws NoIndexException, while the existing and(__.hasLabel("el2")) regression passes.

@dosubot dosubot Bot removed the lgtm This PR has been approved by a maintainer label Jun 25, 2026
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.57377% with 63 lines in your changes missing coverage. Please review.
✅ Project coverage is 31.96%. Comparing base (86e0a66) to head (13df957).

Files with missing lines Patch % Lines
...he/hugegraph/traversal/optimize/TraversalUtil.java 55.90% 46 Missing and 10 partials ⚠️
...rsal/optimize/HugeConnectiveLabelStepStrategy.java 88.23% 3 Missing and 3 partials ⚠️
...rg/apache/hugegraph/auth/HugeFactoryAuthProxy.java 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3068      +/-   ##
============================================
- Coverage     36.57%   31.96%   -4.62%     
- Complexity      338      500     +162     
============================================
  Files           804      816      +12     
  Lines         68417    69738    +1321     
  Branches       8983     9226     +243     
============================================
- Hits          25024    22291    -2733     
- Misses        40706    44968    +4262     
+ Partials       2687     2479     -208     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dosubot dosubot Bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Jun 25, 2026

@imbajin imbajin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: no. Summary: No obvious issues found in the current head. Evidence: git diff --check origin/master...HEAD; JDK 11 mvn -pl hugegraph-server/hugegraph-test -am -P core-test,rocksdb -Dtest=CountStrategyCoreTest,TraversalUtilOptimizeTest -DfailIfNoTests=false test passed 51 tests; latest-head checks passed.

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Two query behaviors that produce the same result are inconsistent.

2 participants