Skip to content

fix(server): handle repeated range predicates correctly - #3122

Merged
imbajin merged 5 commits into
apache:masterfrom
LegendPei:fix/repeated-range-predicates
Jul 28, 2026
Merged

fix(server): handle repeated range predicates correctly#3122
imbajin merged 5 commits into
apache:masterfrom
LegendPei:fix/repeated-range-predicates

Conversation

@LegendPei

@LegendPei LegendPei commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Purpose of the PR

Fix incorrect empty results when multiple predicates target the same range-indexed property.

Main Changes

  • Cache the range-index validation result per element and property.
  • Avoid consuming the same index value multiple times for repeated predicates.
  • Add regression coverage for repeated predicates, including reversed predicate order.

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:
    • mvn test -pl hugegraph-server/hugegraph-test -am -P core-test,rocksdb -Dtest=EdgeCoreTest -DfailIfNoTests=false

Does this PR potentially affect the following parts?

Documentation Status

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

@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working tests Add or improve test cases labels Jul 27, 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. Summary: The query-lifetime range-match cache reuses an incomplete first-batch result and causes valid elements to be omitted. Evidence: three independent review lanes, exact-head static tracing, and repeated failures of VertexCoreTest.testRemoveLeftRangeIndex across memory, RocksDB, HBase, macOS, and HStore checks.

@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. Summary: The correctness fix introduces an allocation on every condition-filtered element, including queries that never use range-index bookkeeping. Evidence: exact-head static inspection and targeted memory-profile path analysis; the two focused correctness tests pass.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes incorrect empty results when multiple has() predicates target the same range-indexed property (issue #3083), by preventing range-index validation from “consuming” the same index value multiple times during per-element filtering in ConditionQuery.

Changes:

  • Cache range-index validation results per element + property while evaluating conditions, so repeated predicates on the same range-indexed property don’t invalidate otherwise-matching elements.
  • Adjust Element2IndexValueMap.checkRangeIndex() to accept a per-element cache map and reuse previously computed match outcomes for the same property.
  • Add a regression test covering repeated predicates and reversed predicate order.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/EdgeCoreTest.java Adds a regression test for repeated range predicates on the same edge property.
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/ConditionQuery.java Introduces per-element caching for range-index validation to avoid double-consuming index values for repeated predicates.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@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: The cross-batch correctness and non-index allocation regressions are fixed; the remaining range-index allocation concern is already covered by an existing thread, so no duplicate inline comment is added. Evidence: three independent exact-head review lanes, EdgeCoreTest ran 169 tests with 0 failures/errors and 11 skipped, and visible exact-head checks are green.

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Jul 28, 2026
@imbajin imbajin changed the title fix(core): handle repeated range predicates correctly fix(server): handle repeated range predicates correctly Jul 28, 2026
@imbajin
imbajin merged commit b9710a7 into apache:master Jul 28, 2026
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files. tests Add or improve test cases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Wrong result for repeated predicates

3 participants