Skip to content

fix(server): clarify condition resolution semantics for label queries - #2994

Open
contrueCT wants to merge 46 commits into
apache:masterfrom
contrueCT:task/improve-condition-query-semantics
Open

fix(server): clarify condition resolution semantics for label queries#2994
contrueCT wants to merge 46 commits into
apache:masterfrom
contrueCT:task/improve-condition-query-semantics

Conversation

@contrueCT

@contrueCT contrueCT commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Purpose of the PR

ConditionQuery.condition() historically combines several meanings in one API:

  • no matching condition
  • EQ/IN conditions whose intersection is empty
  • one resolved value
  • the raw list from a sole IN relation
  • an exception when several relations still resolve to multiple values

This PR preserves that legacy behavior, adds explicit condition-resolution APIs, and
migrates the high-risk LABEL call sites to semantics that match each caller.

Visual overview

Condition resolution semantics and label-query optimization for PR #2994

The diagram contrasts strict and tolerant single-value resolution and shows why
negative or ambiguous label predicates stay local to preserve complete results.
Eligible positive EQ/IN label predicates can still use label indexes; the diagram
is a summary, not an exhaustive query-plan description.

Main Changes

Make condition resolution explicit

  • containsCondition(HugeKeys key) reports any top-level relation for the system key.
    Its Object implementation is private; the existing operator-based overload remains public.
  • containsConditionValues(key) reports whether a top-level EQ/IN relation exists,
    including an empty IN relation.
  • conditionValues(key) returns the resolved EQ/IN intersection. Pair it with
    containsConditionValues(key) when absence and an empty intersection must differ.
  • conditionValue(key) returns null for an empty result, returns the value for a
    singleton, and rejects a multi-value result.
  • singleConditionValueOrNull(key) returns the value only for a singleton and returns
    null for both empty and multi-value results.
  • condition(key) remains backward-compatible, including returning the raw list for a
    sole IN relation.

Migrate label-sensitive callers

  • Use strict conditionValue() semantics where serializers and sort-key paths require
    one resolved label.
  • Use singleConditionValueOrNull() where an optimization is valid only for exactly one
    resolved label.
  • Resolve label intersections when collecting matched indexes, including multi-label and
    conflicting-label queries.
  • Apply the same semantics to graph/index transactions, traversers, and HStore.
  • Keep RamTable's multi-label adjacency fast path for nonempty valid label candidates;
    flatten them into per-label queries and reject empty or unsupported conditions.

Preserve correctness for negative-label predicates

A downstream unsafe label predicate, including one after a range, side effect, or
inside a child traversal, keeps candidate-filtering predicates local. This avoids
losing matches from labels without equivalent index coverage.

The fallback handles query controls and SEARCH predicates separately:

  • ~page is consumed as query metadata. The backend page is bounded while local
    filters and range steps keep their order. A filtered page can be empty while its
    cursor still points to more data; callers must follow the cursor to exhaustion.
  • In this fallback, Text.contains() in the filter chain directly following the source
    (including barriers, but before range/limit/order boundaries) uses the same analyzer
    and term matcher as SEARCH indexes,
    including (word), (word1|word2), and analyzed text. For example, searching
    body = "alpha" with Text.contains("(alpha)") still matches when a negative label
    follows limit() while the SEARCH predicate precedes it. A Text.contains() after
    range/limit/order keeps plain substring semantics. The adapted local container
    preserves the original predicate tree;
    its graph-specific runtime matcher is transient and rebuilt after cloning,
    deserialization, predicate changes, or graph rebinding.
  • HugeGraph.searchPredicate(text) creates this matcher without exposing graph
    configuration; the authorization proxy verifies graph access before delegation.

This fallback intentionally changes missing-index behavior: a defined but unindexed
property query such as g.V().has("unindexedProp", "x").hasLabel(P.neq("author"))
can scan candidates and filter locally instead of raising NoIndexException.
This preserves complete results, but may increase latency and backend work. Existing
capacity checks are not a universal scan-work bound, and a final limit bounds matches,
not all examined candidates. Explicit-ID and adjacency queries can retain narrower
candidate sources.

See negative-label query behavior and limits
for the user-facing contract, capacity exceptions, and paging guidance. The image and
note are hosted in gist and do not depend on the contributor's fork.

Follow-ups are tracked separately:

Verifying these changes

Regression coverage includes:

  • absent, empty, singleton, conflicting, and multi-value condition resolution
  • a sole raw IN relation and non-EQ/IN label predicates
  • single-label and multi-label edge sort-key queries
  • negative-label queries next to indexed properties, across barriers, and with mixed
    connectives or multiple label containers
  • matched-index collection for joint labels with indexed properties
  • vertex and outgoing-edge paging with negative labels, including continuation through empty
    filtered pages without missing or duplicate IDs
  • SEARCH positive controls, explicit terms, analyzed text, and matching unindexed labels
  • real offset/limit ordering, aggregate contents, and indexed interference labels
  • singleton/duplicate IN compatibility, serializer and RamTable contracts, and
    non-admin access to the SEARCH matcher
  • unindexed-property fallback versus NoIndexException, candidate-capacity enforcement,
    and explicit-ID lookups
  • SEARCH predicate structure/hash stability, Java serialization before and after use,
    clone isolation, predicate mutation, and graph rebinding
  • RamTable multi-label IN/OUT/BOTH results, duplicate labels, and unsafe candidate rejection

Targeted verification (run on the SSH test host; no coverage/style skips):

mvn test -pl hugegraph-server/hugegraph-test -am \
  -P unit-test,memory -Dsurefire.failIfNoSpecifiedTests=false \
  -Dtest='TraversalUtilOptimizeTest,QueryTest,BinarySerializerTest,TextSerializerTest,GraphTransactionTest,CachedGraphTransactionTest,HugeGraphAuthProxyTest#testSearchPredicateDoesNotRequireAdminConfigAccess,org.apache.hugegraph.unit.cache.RamTableTest#testMatchedLabelCandidateContract+testQueryByMultipleLabels+testMatchedRejectsUnsafeLabelCandidates'

# Repeat with core-test,rocksdb for a persistent backend.
mvn test -pl hugegraph-server/hugegraph-test -am \
  -P core-test,memory -Dsurefire.failIfNoSpecifiedTests=false \
  -Dtest='VertexCoreTest#testUnindexedPropertyBeforeNegativeLabel+testLocalConnectiveStringIds+testLocalContainsBeforeNegativeLabel+testSearchBeforeDownstreamNegativeLabel+testPageBeforeDownstreamNegativeLabel+testNegativeLabelPreservesRangeAndAggregate,EdgeCoreTest#testQueryEdgesByNonEqLabel*+testQueryOutEdgesByMultiLabelsAndSortKey+testPageOutEdgesWithNegativeLabel*'

mvn editorconfig:format
mvn clean compile -Dmaven.javadoc.skip=true
  • 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 shown above.

Does this PR potentially affect the following parts?

The public Java API of ConditionQuery gains explicit resolution methods, and
HugeGraph.searchPredicate(text) exposes the existing SEARCH matching semantics for
local traversal filters. No REST API, configuration, or dependency changes are included.

Documentation Status

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

The API semantics are documented in Javadocs. The linked user note documents the
full-scan fallback, missing-index behavior, capacity limits, paging, and local SEARCH.
Publication to the HugeGraph documentation website is still pending; the gist and
repository note do not imply that the website has been updated.

@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Apr 13, 2026
@contrueCT contrueCT changed the title improve(query): clarify condition resolution semantics for label queries fix(query): clarify condition resolution semantics for label queries Apr 19, 2026
@codecov

codecov Bot commented May 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 23.38308% with 308 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.76%. Comparing base (3681148) to head (459a2b2).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
...he/hugegraph/traversal/optimize/TraversalUtil.java 14.53% 229 Missing and 18 partials ⚠️
...apache/hugegraph/backend/query/ConditionQuery.java 59.01% 15 Missing and 10 partials ⚠️
...he/hugegraph/backend/tx/GraphIndexTransaction.java 28.57% 10 Missing and 5 partials ⚠️
...g/apache/hugegraph/backend/store/ram/RamTable.java 0.00% 10 Missing ⚠️
...he/hugegraph/backend/store/hstore/HstoreStore.java 0.00% 3 Missing ⚠️
.../org/apache/hugegraph/auth/HugeGraphAuthProxy.java 0.00% 2 Missing ⚠️
.../src/main/java/org/apache/hugegraph/HugeGraph.java 0.00% 1 Missing ⚠️
...n/java/org/apache/hugegraph/StandardHugeGraph.java 0.00% 1 Missing ⚠️
...hugegraph/backend/serializer/BinarySerializer.java 50.00% 1 Missing ⚠️
...e/hugegraph/backend/serializer/TextSerializer.java 50.00% 1 Missing ⚠️
... and 2 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2994      +/-   ##
============================================
- Coverage     37.77%   37.76%   -0.01%     
- Complexity     6560     6621      +61     
============================================
  Files           800      800              
  Lines         68960    69323     +363     
  Branches       9166     9274     +108     
============================================
+ Hits          26052    26183     +131     
- Misses        39841    40042     +201     
- Partials       3067     3098      +31     

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

@contrueCT
contrueCT force-pushed the task/improve-condition-query-semantics branch 2 times, most recently from 4c42786 to cc9af24 Compare May 26, 2026 12:29

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

I found one correctness issue in the latest revision. The CI failures were posted separately as a PR-level reminder.

CI/status checks are failing on the latest head (cc9af24929e42af1c90e1f55f3e60adc351e0318). Could you check the failed jobs before the next review round?

Failed checks include:

@contrueCT
contrueCT force-pushed the task/improve-condition-query-semantics branch from cc9af24 to 2e82f83 Compare May 30, 2026 10:20

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

I don't see a clear blocking correctness issue in the latest head, and the previous LABEL-resolution comments look addressed. One remaining merge risk is that the latest checks are still red: hstore failed in VertexCoreTest#testQueryByDateProperty.

Since this PR also touches HstoreStore, could you rerun or clarify whether the hstore failure is an existing flaky/environment issue?

contrueCT added 5 commits June 5, 2026 12:59
Add explicit condition resolution APIs to ConditionQuery while preserving the legacy condition() behavior. Introduce containsCondition(Object), conditionValues(Object), and conditionValue(Object) so callers can distinguish missing, empty, unique, and multi-value results without overloading null semantics.

Migrate LABEL-specific consumers in graph/index transactions, serializers, traversers, and stores to use the new APIs for unique-label resolution and conservative fallback behavior. Extend QueryTest and VertexCoreTest to cover absent, conflicting, and multi-value label conditions as well as collectMatchedIndexes() behavior for multi-label and conflicting label queries.
@contrueCT
contrueCT force-pushed the task/improve-condition-query-semantics branch from 94408b7 to b10e3c2 Compare June 5, 2026 05:10
@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 5, 2026
@contrueCT
contrueCT force-pushed the task/improve-condition-query-semantics branch from 801923a to ebc31c8 Compare June 5, 2026 18:06
@contrueCT

contrueCT commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for your patience. The hstore CI failure exposed an existing latent issue in hstore's range-index query path. For range-index scans with limit/paging, the upper layer assumed that backend scan results were globally ordered by the range-index key and that the returned page state could be reused as a HugeGraph range cursor. In hstore, multi-node/tablet scans can return entries in backend iterator order, and the page state is an internal storage cursor, so those assumptions may lead to unstable ordering or skipped results. This PR keeps the fix intentionally scoped: hstore range-index queries whose visible result depends on limit/offset/paging are sorted and sliced in the index layer, while unbounded scans still use the original streaming path to avoid disturbing count, joint-index, and cleanup paths. I think this is enough for the current PR, but the underlying hstore scan/page-state contract should be handled in a dedicated follow-up, ideally by defining whether range scans must be globally ordered and fixing the hstore iterator/page-state semantics at the storage-client layer.

@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: HStore range-index offset queries can skip too many sorted results. Evidence: static review of GraphIndexTransaction/query offset handling.

@contrueCT

Copy link
Copy Markdown
Contributor Author

Thanks. I fixed this by resetting scanQuery.offset(0L) before the full sorted range-index scan, so the fallback now reads the complete matched range first and lets the original query apply offset/limit only once after sorting. I also added range-offset coverage to VertexCoreTest#testQueryByDateProperty to guard the double-skip case. Local checks passed with git diff --check, hugegraph-core compile, and VertexCoreTest#testQueryByDateProperty under the rocksdb core-test profile.

@contrueCT

Copy link
Copy Markdown
Contributor Author

CI follow-up: the Store retry passed completely (11m40s), confirming the earlier failure was the pre-checkout Zulu JDK download timeout. The HStore lane then failed twice, including repository-side rerun attempt 2, at the exact failure already tracked in #3180: VertexCoreTest#testQueryByJointIndexesWithSearchAndTwoRangeIndexesAndWithin, expected:<3> but was:<1>. The same PR code passed the full HStore lane in run 33718055523, and #3180 documents the identical intermittent failure on multiple master heads. All other visible GitHub Actions checks on current head are green. I do not think skipping/changing the assertion or folding the separate cross-partition range/search bug into this PR would be appropriate. Could a maintainer please rerun only the HStore job once more? That should also allow its coverage report to upload so Codecov can produce the final aggregate status.

@SebastianGruza

Copy link
Copy Markdown

Cross-backend results for this branch (head 5cb9a51), from the same run reported in #3090: 35 label-semantics queries (neq / without / within / conflicting labels / or / and / not, negative labels across barrier() and sideEffect(), incoming edges from multi-label sources, vertices with label + range index; edge labels with sort keys) executed through REST/Gremlin against HStore (PD + 3 stores) and RocksDB on the same server build, compared as sets of element ids.

Before — master 98477f0, both backends:

g.V().has('age',gte(30)).hasLabel(without('person','robot'))
  -> Can't do index query with [LABEL != 5, LABEL != 6] and [12 >= 30]
g.V().hasLabel(without('person')).barrier().has('age',gte(60))
  -> Don't accept query based on properties [age] that are not indexed in any label, may not match range/not-equal

Both throw on RocksDB as well, so this is server-side condition resolution, not a backend issue. The other 33 cases are identical sets on both backends already on master.

After — master + this branch (+ #3184 and the since-merged #3182): 35/35 identical sets; the two queries above return 0 and 100 vertices respectively, identically on both backends.

Version axis (RocksDB master vs RocksDB with the branch, backend held constant): 168 of 174 queries across all four sections (sort-key pushdown, range-index paging, label semantics, within × search × range) return identical id sets; the only differences are the two without() cases above (the remaining 4 are REST string-predicate rejections present on both versions). So on this corpus the branch changes visible results exactly where it intends to and nowhere else.

Caveat: the "after" run was the combined branch, not #2994 alone; #3184 only touches the HStore pushdown path and #3182 is now in master, so attribution of the two fixes to this PR is by elimination. I can run #2994 alone on request.

Raw reports (ids included) and the case list: https://github.com/SebastianGruza/hugegraph-oracle-suiteresults/compare_master.txt, results/compare_master-oracle_vs_combined-oracle.txt, finding F9 in docs/findings.md. Happy to re-run on the next head — ping me.

@contrueCT
contrueCT marked this pull request as ready for review September 5, 2026 04:35

@bitflicker64 bitflicker64 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.

Blocking: no. Summary: The condition-resolution split reads well and all 35 earlier review threads are resolved at this head; I have one error-handling regression in the new local-filter path, one gap in the unsafe-label gate that the PR does not close, and one minor redundancy. Evidence: mvn -o -q compile -pl hugegraph-server/hugegraph-core -am passed; all 24 GitHub checks are green at 0ce61d0; findings below come from static review of git diff 98477f0f..0ce61d0 and the surrounding TraversalUtil / GraphIndexTransaction code. Not verified by a local test run.

@bitflicker64 bitflicker64 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.

Blocking: yes. Summary: The ConditionQuery decomposition and the LABEL call-site migration are sound, and they fix a real family of latent ClassCastExceptions on multi-label queries. All three findings are in the new TraversalUtil unsafe-label gate; the first one turns g.V().hasId(x).limit(n).hasLabel(P.neq(...)) from a point lookup into a full table scan that cannot terminate early, which fails outright above Query.DEFAULT_CAPACITY.

Evidence: static review of head 15e052f4 against merge base 36811483, plus the TinkerPop 3.5.1 HasContainer sources (testLabel is protected and non-final and clone() preserves the subclass, so LocalLabelHasContainer is safe). I did not build or run the tests, so findings 1 and 2 are reasoning over HugeGraphStep.makeQuery() and BackendEntryIterator.checkCapacity() rather than measured queries; an execution-plan assertion in TraversalUtilOptimizeTest would settle both.

holder.removeHasContainer(has);
continue;
}
if (T.id.getAccessor().equals(has.getKey())) {

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.

‼️ ~id never reaches the source step, so a point lookup becomes an unbounded full scan

Evidence: on this path no has container reaches newStep.hasContainers (only ~page at line 690 and the paging range at line 734 are pushed). HugeGraphStep.makeQuery() then takes the this.hasContainers.isEmpty() branch and builds new Query(type), and vertices() takes the !hasIds() branch. At the merge base the same container went through GraphStep.processHasContainerIds(), which set graphStep.ids and gave graph.vertices(this.ids).

So g.V().hasId(one).limit(10).hasLabel(P.neq("other")), the shape asserted at VertexCoreTest.java:9268, scans the whole vertex table. The surviving HasStep also blocks extractRange(), so limit(10) is not pushed either, and since exactly one vertex can match, the RangeGlobalStep drains the scan to exhaustion. Query starts at DEFAULT_CAPACITY = 800000 (Query.java:51, 97) and BackendEntryIterator.checkCapacity() throws LimitExceedException past it, so above that size this query stops returning the vertex at all. The neighbouring g.V().hasId(one).toList() at line 9266 keeps its point lookup, so the test is green either way.

The gate's rationale does not apply to ~id: an id fetch is label-agnostic and complete, so keeping it local buys nothing.

Requested change: for HugeGraphStep sources, still call GraphStep.processHasContainerIds(newStep, has) on T.id containers and fall back to this local rewrite only when it returns false. Please assert the resulting step ids in a regression, so the plan is pinned and not just the result set.

// steps don't reliably expose element identity, so stay conservative.
// FIXME: Restore selective pushdown when every candidate schema label
// has compatible index coverage for extracted property predicates.
List<Step> steps = traversal.getSteps();

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.

⚠️ The gate fires for label predicates that can never filter this step's output

This is the cost side of what I asked for on 2026-09-02 and 2026-09-05, not a request to undo it. The traversal-level condition and the getParent() walk are the right shape; the ask here is to bound their breadth.

Evidence: the walk enumerates every remaining step of the traversal, plus each ancestor's remaining steps and all their children, and hasUnsafeLabelPredicate() (line 916) inspects only the key and the predicate. Nothing tracks which elements a step filters. For

g.V().has("city", "Beijing").out().has(T.label, P.neq("author"))

the label predicate applies to the out() results and never to the g.V() candidates, yet the gate fires, prepareLocalHasContainers() leaves city local, and HugeGraphStep.makeQuery() builds new Query(VERTEX). An indexed lookup becomes a full vertex scan, with the same DEFAULT_CAPACITY ceiling noted above. The .out().where(__.not(__.hasLabel("author"))) variant behaves the same way through hasUnsafeLabelInChildren() line 884.

Requested change: bound the forward scan to steps that still operate on the source step's own elements, stopping at element-changing steps such as VertexStep/EdgeVertexStep/PropertiesStep, while keeping the walk alive for select()/path()-shaped steps that can reintroduce earlier elements. If that is follow-up work, please say in the FIXME above that a negative label anywhere in the traversal disables all pushdown at every source step, so the cost is recorded rather than implied to be narrow.


ConditionQuery cq = (ConditionQuery) query;
if (cq.condition(HugeKeys.LABEL) != null && cq.resultType().isEdge()) {
if (cq.singleConditionValueOrNull(HugeKeys.LABEL) != null &&

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.

🧹 This is migrated, but its sibling gate below is the one production condition(HugeKeys.LABEL) left

Evidence: queryNeedsPostFilter() at line 2024 still reads

boolean edgeIndexWithLabel =
        cq.resultType().isEdge() &&
        cq.optimized() == OptimizedType.INDEX &&
        cq.condition(HugeKeys.LABEL) != null;

It is byte-identical at the merge base and outside every hunk of this PR, but it now disagrees with the branch you migrated here. For a sole LABEL IN [a, b], condition() returns the raw list (ConditionQuery.java:293-297), so edgeIndexWithLabel is true and the query is reported as needing no post-filter, while this branch correctly falls through to cq.test(elem). condition() can also throw IllegalStateException (ConditionQuery.java:305) from inside that caching decision where singleConditionValueOrNull() returns null.

No wrong data is served today: the production consumer is CachedGraphTransaction (lines 322, 399, 424), and what it caches was already filtered by super.queryEdgesFromBackend(). This is about the two gates reading the same way and about the exception surface.

Requested change: as a follow-up, since line 2024 is outside this diff, migrate it to singleConditionValueOrNull(HugeKeys.LABEL) != null, or leave a comment there recording that the legacy semantics are deliberate.

@SebastianGruza

Copy link
Copy Markdown

Measurement of head ac641c6 against origin/master 36811483, 2026-09-06 — follow-up to my 2026-09-03 comment, this time on #2994 alone: the head contains current master, so pr2994 = master + this PR and nothing else (#3184 was added only in the variant used for the hstore axis). Same method: the same REST/Gremlin queries against hstore (PD + 3 store nodes) and against rocksdb built from the same tree, compared as sets of element ids. Raw reports, plans and reproduction commands: hugegraph-validation, tag pr-2994-2026-09-06. Independent work, not affiliated with the project.

1. Backend axis (hstore vs rocksdb, same server code). 174 suite cases plus 155 new shapes taken from this review (barrier-like steps, mixed-key or(), child traversals in both directions, point lookups, SEARCH, element-changing steps, ~page). On pr2994 + #3184: 170 OK, 4 BOTH-ERR (REST string range predicates, same as master), no MISMATCH; the new shapes 92/92 OK, explain() plans 20/20 identical. The only difference between the variant with and without #3184 is one #3090 shape.

2. Version axis (rocksdb master vs rocksdb pr2994). Suite: 168 OK and exactly two cases where master throws and the PR answers (the same without() cases as on 2026-09-03). In the new shapes the PR changes the result of 18 queries, and every one of them is the same defect on master: score is range-indexed on person only; 60 of the 300 robot vertices have score >= 40.

Query master pr2994
g.V().has('score',gte(40)).hasLabel(neq('person')) 60 60
same with limit(100000), skip(0), range(0,100000), aggregate('x'), coin(1.0) or barrier() between the property and the label 0 60
g.V().has('score',gte(40)).not(hasLabel('person')), also with limit() 0 60
g.V().has('score',gte(40)).where(__.not(__.hasLabel('person'))), also with limit() 0 60
g.V('a').union(__.V().has('score',gte(40))).hasLabel(neq('person')), flatMap, repeat().times(1) 0 60
g.V().has('score',gte(40)).or(hasLabel(neq('person')), has('cnt',5)) 180 240

Master pushes score into the person index and filters the label locally, so the robots are lost silently, no error; without the barrier-like step the same master returns 60. The PR returns the complete set in every shape, identically on hstore. Two more cases where master throws (has('cnt',5).limit().hasLabel(neq(..)), outE('deal').hasLabel(neq('flow'))) become correct empty sets. ~page with a downstream negative label: master rejects it (Invalid paging traversal, 12 cases), the PR pages it and the union of all pages equals the unpaged set for page sizes 7, 50 and 500 on both backends.

3. Plan and cost axis (the two findings of @bitflicker64 from 2026-09-06). On top of the suite data I loaded 1 000 000 big vertices without any property index on rocksdb, which is what a real schema looks like, and recorded times plus explain().

Point lookups (finding 1): at ac641c6 the plan is identical to master, [HugeGraphStep(vertex,[big00000010]), RangeGlobalStep(0,10), HasStep([~label.neq(mark)])], 3 to 10 ms for g.V('x').limit(10).hasLabel(neq(..)), g.V().hasId('x').limit(10)... and the two-id variant. From my side this one is closed.

Gate breadth (finding 2), on 1 M vertices:

Query master pr2994
g.V().has('age',gte(60)).limit(100000).hasLabel(neq('person')) 8 ms, incomplete result (as in section 2) Too many records(must <= 800000) for the query: Query * from VERTEX
g.V().has('age',gte(60)).out().hasLabel(neq('person')) 17 ms, correct Too many records(...)
g.V().has('age',gte(60)).out().where(__.not(__.hasLabel('person'))) 18 ms, correct Too many records(...)
g.V().has('fname',Text.contains('gold')).limit(100000).hasLabel(neq('person')) 7 ms Too many records(...), plan HasStep(lambda)
g.V().has('age',gte(60)).hasLabel(neq('person')), no barrier Too many records(...) Too many records(...)

Row 1 is the deliberate trade-off the PR description records (a complete answer instead of a silently incomplete 8 ms one) and looks right to me. Rows 2 and 3 are a regression: the negative label applies to the out() vertices, never to the g.V() candidates, master answers correctly in 17 ms, and the PR fails above Query.DEFAULT_CAPACITY. On the small suite graph, where only person and robot exist and both carry an age index, the same shapes keep the pushdown, which is why the PR's own tests do not see it: the gate depends on whether every label in the schema has compatible index coverage.

In short: label semantics are correct and identical on both backends, the PR fixes a real class of silently incomplete answers, point lookups are preserved. The one thing I would close before merging is bounding the gate to steps that still operate on the source step's own elements, exactly in the spirit of finding 2. Happy to re-run the measurement on the next head, same script, same cluster.

@bitflicker64 bitflicker64 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.

Blocking: yes. Summary: On the new local-filter fallback, hasKey() and hasValue() containers are left in the local HasStep untranslated, so combining them with a label predicate the new check treats as unsafe returns an empty result set with no error. Evidence: prepareLocalHasContainers skips them at TraversalUtil.java:716 while canExtractHasContainer still pushes them down on the normal path, and TinkerPop 3.5.1 HasContainer.test(Element) then evaluates them as a property literally named key or value, which HugeVertex.properties(String...) never resolves.

Comment thread hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java Outdated
@SebastianGruza

Copy link
Copy Markdown

Re-measurement on fefe3ca, 2026-09-07 — same method and cluster as my comment of 2026-09-06, plus 27 new shapes for today's findings of @bitflicker64 (hasKey() / hasValue() and connective string ids next to a negative label). Report and raw data: tag pr-2994-2026-09-07.

Finding 2 of 2026-09-06 is closed. On 1 M vertices, g.V().has('age',gte(60)).out().hasLabel(neq('person')): master 17 ms, ac641c6 Too many records, fefe3ca 12 ms with plan HugeGraphStep(Vertex,[age.gte(60)]), HugeVertexStep(OUT,vertex), HasStep([~label.neq(person)]); the where(__.not(__.hasLabel(..))) variant 17 ms. Point lookups 2 ms. The full scans for has(indexed).limit().hasLabel(neq) are unchanged, as the PR description states.

Backend axis: suite 170 OK / 4 BOTH-ERR, 115/115 J8 shapes identical on hstore and rocksdb. Version axis (rocksdb): suite unchanged since 2026-09-06; the 18 score shapes stay fixed. Among them is the mixed-key or() with a child traversal that @imbajin asked about on 2026-09-01: g.V().has('score',gte(40)).or(hasLabel(neq('person')), has('cnt',5)) with score indexed on person only returns 180 on master and 240 on this head, the 60 missing robots included, identically on both backends.

hasKey / hasValue next to a negative label (rocksdb, master vs fefe3ca):

Query master fefe3ca
g.V().hasKey('age').hasLabel(neq('person')) 0 300
g.V().hasKey('score').hasLabel(neq('robot')) 0 3000
g.V().hasKey('fname').hasLabel(neq('person')) 0 200
g.V().hasValue(20).hasLabel(neq('person')) 0 5
g.V().hasLabel('firm').out('deal').hasKey('type').hasLabel(neq('person')) 0 100
g.V('a').outE().hasKey('amount').hasLabel(neq('flow')) 0 120
g.E().hasKey('amt').hasLabel(neq('flow')) 0 100
g.V().hasKey('age').limit(100000).hasLabel(neq('person')) Not support query: … containsk 300
g.V().hasId(within('p00010','r0001').and(neq('r0001'))).limit(10).hasLabel(neq('robot')) Not supported querying by id and conditions 1

A bare g.V().hasKey('age') throws on both versions and both backends (the backends do not run CONTAINS queries), but with a label predicate master answers with an empty set and no error. At fefe3ca every shape returns the complete set, identically on hstore. The one change of direction: hasKey('nope') is 0 on master and Undefined property key on the PR, which is the better answer.

Two observations on the new code, not blocking since master throws in both cases: g.V().hasId(within('big00000010','mark0').and(neq('mark0'))).limit(10).hasLabel(neq('mark')) is a full scan at 1 M (Too many records, plan HugeGraphStep(vertex,[]), HasStep([~id.and(within(..), neq(..))])), while the same shape without .and() is a 39 ms point lookup; the within list is a complete candidate set, so it could reach HugeGraphStep with the neq leaf attached as well. testLocalConnectiveStringIds pins the result set; a plan assertion would pin this too. hasKey() with a label is also a full scan, but there is no index to push there, so that is a limitation to record rather than a fix to request.

In short: from my side there is nothing left I would close before merging. At this head the label semantics are complete and identical on both backends, finding 2 is closed by measurement, and the PR fixes two classes of silently empty answers on master (score behind a barrier-like step, hasKey/hasValue next to a label).

@bitflicker64 bitflicker64 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.

Blocking: no. Summary: The ConditionQuery decomposition is behaviour-preserving (collectConditionValues/resolveConditionValues are lifted verbatim and every legacy fast path is intact) and the LABEL call-site migration removes a real family of latent ClassCastExceptions, because Id label = cq.condition(HugeKeys.LABEL) compiles to a checkcast Id and constructEdgesQuery() (GraphTransaction.java:1379) builds exactly the sole Condition.in(HugeKeys.LABEL, ...) that trips it. The earlier open threads look addressed at this head: ~id now reaches the source step via GraphStep.processHasContainerIds() (TraversalUtil.java:702-711), hasKey()/hasValue() go through the new LocalContainsStep, HugeGraph.searchPredicate is a default method, the unsafe-label gate walks ancestors and short-circuits on changesCurrentElement, and queryNeedsPostFilter() line 2024 is migrated. Three notes inline: the local-filter fallback also drops the NoIndexException guard, localSearchPredicate() builds a P around a capturing lambda, and RamTable now declines multi-label edge queries it could still serve. Evidence: mvn -o -q -pl hugegraph-server/hugegraph-core -am compile -DskipTests -Dcheckstyle.skip=true -Djacoco.skip=true at fefe3ca gives BUILD SUCCESS; gh -R apache/hugegraph pr checks 2994 shows all 24 functional lanes green with only codecov/patch and codecov/project red (28.66% patch coverage, 234 uncovered lines); the rest is static tracing of the exact-head diff against merge base 3681148. I could not run the PR's regression suites here — hugegraph-test does not build in this environment because hg-store-client fails Lombok annotation processing (cannot find symbol: variable log), unrelated to this PR — so all three findings are static and the paging/full-scan behaviour below is unreproduced at runtime.

@SebastianGruza

Copy link
Copy Markdown

e32a75f checked on the same lab, report under tag pr-2994-2026-09-08. No change in any result set against fefe3ca on either backend (suite 170 OK, 115/115 J8 shapes). On the contents of docs/negative-label-queries.md: on HStore (PD + 3 store nodes, 1 M vertices) every fallback shape, count() variants included, ends in Too many records(must <= 800000) after about 0.9 s exactly as on rocksdb, so the "where the execution path enforces them" caveat can be dropped for HStore. On the same graph, g.V().has('age',gte(60)).out().hasLabel(neq('person')).count() takes 2.5 s on master hstore and 49 ms on this head, because master pushes the label into HugeVertexStep and the store evaluates it per adjacency query. One thing to check on LocalSearchHasContainer: g.V().has('fname',Text.contains('gold')).limit(100000).hasLabel(neq('person')).explain() prints HasStep([fname.TEXT_CONTAINS(gold)]) on hstore but HasStep(lambda) on rocksdb with the same build; the result sets are identical, so it may just be toString() after the matcher was built, but I would rather report it than guess.

@bitflicker64 bitflicker64 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.

Blocking: no. Summary: The explicit condition-resolution API and the LABEL call-site migrations read correctly, and the negative-label gate now escapes at element-changing steps; the substantive point left is that the local-filtering path demotes positive hasLabel() containers along with the unsafe ones, turning a label-index lookup into a full scan, plus two nits on the new SEARCH and CONTAINS filters. Evidence: static read of the exact-head diff against merge base 3681148, and of TraversalUtil, HugeGraphStep, GraphIndexTransaction, RamTable, ConditionQueryFlatten and Condition at e32a75f.

if (predicates.stream().anyMatch(p ->
p.getBiPredicate() == Condition.RelationType.TEXT_CONTAINS)) {
HugeGraph graph = tryGetGraph(source);
if (graph == null) {

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.

🧹 The new doc promises analyzer matching that this branch does not give

docs/negative-label-queries.md states without qualification that local Text.contains() filters use the graph's SEARCH analyzer and exact term matcher. On this branch the container keeps Condition.RelationType.TEXT_CONTAINS, whose tester is ((String) v1).contains((String) v2) (Condition.java:94-97), so Text.contains("(alpha)") looks for the literal substring (alpha), parentheses included. The behaviour itself predates the PR (at the merge base canExtractHasContainer returned false on graph == null and left the same raw container behind), but the doc shipped here now describes the analyzed path as the rule.

Worth noting the gate is not what keeps the rewrite working: LocalSearchHasContainer takes no graph at construction (lines 914-916) and resolves one from property.element().graph() in testValue (lines 918-932), so installing it unconditionally would not reintroduce the getGraph() failure raised earlier in review.

Requested change: either drop the tryGetGraph gate so both cases match the doc, or qualify the doc's SEARCH paragraph and say here that matching degrades to substring on this path.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 2d53a55. Removed the graph-null gate: unbound child traversals now receive LocalSearchHasContainer too, and it resolves the analyzer from the runtime element as intended. Replaced the old keep-the-raw-predicate test with assertions that distinguish explicit-term SEARCH from substring matching. Tests cover graph/adjacency source shapes without a graph during extraction, mixed/nested predicates, serialization, cloning and graph rebinding, plus a core test using the real graph analyzer. Original predicate trees remain intact. The final targeted unit and Memory/RocksDB regressions passed; CI for the pushed commit is running.

@SebastianGruza

Copy link
Copy Markdown

Correction to my comment of 2026-09-08: the observation about HasStep(lambda) on rocksdb "with the same build" was my mistake, not a property of the PR. The rocksdb server in that run was still on the fefe3ca core jar (my dist-assembly script died before copying the rocksdb dist). The hstore columns and the master baseline of that report stand, the rocksdb columns do not; the report now carries an erratum and the cycle script refuses to run on a jar mismatch. Sorry for the noise.

2d53a55 measured with verified dists, report under tag pr-2994-2026-09-09. explain() plans are now identical on hstore and rocksdb for all 27 shapes, the result sets of all 133 J8 shapes as well; the suite is unchanged.

On the two notes of @bitflicker64 from 2026-09-08:

  • Positive label before an unsafe child, 1 M vertices: g.V().hasLabel('firm').where(__.out('deal').hasLabel(neq('person'))).count() hstore 14 ms, rocksdb 6 ms (master 16 and 7 ms), plan HugeGraphStep(Vertex,[~label.eq(firm)]), TraversalFilterStep([HugeVertexStep(OUT,[deal],vertex), HasStep([~label.neq(person)])]); the within('firm','robot') and has('type',gte(2)) variants (the property stays local next to the pushed label) are in the same range. The label-index lookup is back on both backends.
  • SEARCH analyzer in an unbound child: g.V().hasLabel('firm').where(__.out('deal').has('fname',Text.contains('(gold)')).hasLabel(neq('person'))) returns 0 on master (literal substring, parentheses included) and 1 on this head; same for Text.contains('(gold|silver)'). On the source step both return 80, so the difference is exactly the path the document describes.

From my side unchanged: nothing left to close before merging.

@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: One added unit regression does not run successfully with the repository's Mockito setup. Evidence: exact-head unit-test execution reproduced the failure; core Memory/Edge regressions passed.

@bitflicker64 bitflicker64 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.

Blocking: no. Summary: the ConditionQuery decomposition is behaviour-preserving (collectConditionValues and resolveConditionValues are lifted verbatim, every legacy fast path in condition() survives) and it removes a real family of latent ClassCastExceptions, for example BinarySerializer.java:677, where Id label = cq.condition(HugeKeys.LABEL) met a sole IN relation and got the raw List back. The four items below are maintainability, not correctness. Evidence: static review of the full diff at 2d53a556 against merge-base 36811483; gh api repos/apache/hugegraph/commits/2d53a556.../check-runs reports 22 green check runs at this head, with only codecov/patch and codecov/project red. Nothing was compiled or executed locally, so this is a static pass, and the open threads on this head (the TraversalUtilOptimizeTest.java:721 Mockito failure and the SEARCH predicates doc section) are acknowledged rather than re-litigated.

/**
* Returns whether there is any top-level relation for the specified key.
*/
public boolean containsCondition(Object key) {

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.

🧹 This overload is public but has one caller in the tree, and it widens an overload set whose members disagree about what the argument means.

git grep 'containsCondition(' at this head finds a single call to it: line 407, inside containsCondition(HugeKeys). The set now reads:

  • containsCondition(Object key) here and containsCondition(HugeKeys key) at line 406: is there a top-level relation on this key
  • containsCondition(Condition.RelationType type) at line 460: is there a top-level relation using this operator

Both pre-existing overloads take enums, so no other argument type compiled. With Object in the set, any reference type does and silently resolves to key matching, including Condition.Relation.key(), which is declared Object (Condition.java:739).

Requested change: make this method private. If a public key accessor is wanted for Id userprop keys, give it a distinct name such as containsConditionKey(Object), matching the containsConditionValues naming you introduced, rather than another overload. containsConditionValues(Object) at line 350 has the same single-delegate-caller shape but no competing overload, so it is only extra surface.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 5a7048f: containsCondition(Object) is now private. The HugeKeys and RelationType overloads remain public and retain key-based/operator-based behavior respectively. Added a regression test for visibility and both public overloads; it failed before the change and passes after it. All 43 targeted query/serializer/transaction tests and the full-module clean compile passed on the SSH test host. No renaming or broader API change is included.

return false;
}

private Set<Object> resolveConditionValues(List<Object> valuesEQ,

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.

🧹 A second ConditionQuery in this repo keeps the legacy semantics, and its copy of this loop drops conflicts.

hugegraph-struct/src/main/java/org/apache/hugegraph/query/ConditionQuery.java carries a near-duplicate condition(Object) (lines 463-524 at this head). It is the class the store side deserializes: hg-store-common/.../StoreQueryParam.java, hg-store-core/.../business/FilterIterator.java (ConditionQuery.fromBytes, line 49) and hg-store-node/.../grpc/query/stages/FilterStage.java all import org.apache.hugegraph.query.ConditionQuery.

Its intersection loop (lines 494-513) gates on intersectValues.isEmpty() instead of the initialized flag this method uses, so an emptied set is read as "not seeded yet". For three conflicting relations on one key, EQ a, EQ b, EQ c, the walk is {a}, then intersect [b] gives {}, then the third iteration takes the seed branch and re-seeds {c}; it returns c where this file returns null. Two conflicting relations happen to coincide, which is why it is easy to miss. The struct copy also has none of the explicit accessors you added here.

Nothing is broken today: git grep '\.condition(' over hugegraph-struct and hugegraph-store finds only Condition.java:770 and ConditionQuery.java:440, so there is no production caller. This PR is scoped to server, so I am not asking you to widen it.

Requested change: open a follow-up to port collectConditionValues/resolveConditionValues and the explicit accessors to the struct copy, and add a short comment on each copy naming the other so the two do not drift further.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Tracked separately in #3200. It covers the empty-intersection reseeding bug, explicit accessor parity, compatibility boundaries, and regression fixtures for both implementations. Commit 5a7048f adds a short cross-reference and the issue number on each copy. The struct implementation itself is intentionally unchanged in this PR.

// Scan the remaining traversal, its children and each ancestor's
// remaining steps for filters on child output. Arbitrary extension
// steps don't reliably expose element identity, so stay conservative.
// FIXME: Restore selective pushdown when every candidate schema label

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.

🧹 This FIXME marks the fallback as provisional but gives a reader no issue to follow.

It is the one tracked exit from the conservative behaviour the rest of the PR builds on: while it stands, a downstream unsafe label predicate disables property pushdown across element changes, ancestors and unknown extension steps, which is exactly the full-scan cost docs/negative-label-queries.md warns about. You already split the convContains2Relation caching out to #3196, so the convention is established here.

Requested change: file an issue for restoring selective pushdown once every candidate schema label can be shown to have compatible index coverage, and put its number on this line, // FIXME(#NNNN): ..., so the fallback has a tracked path back.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Opened #3201 for selective property pushdown with complete, predicate-compatible coverage of all candidate labels. Commit 5a7048f links it as FIXME(#3201). The issue includes traversal context/identity, schema lifecycle, incomplete-coverage counterexamples, paging/count/side-effect correctness, and RocksDB/HStore performance validation. This revision does not change the conservative fallback.

@@ -0,0 +1,53 @@
# Negative-label queries and local filtering

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.

🧹 Two fork URLs in the PR description point at this content and will break on merge.

Description line 98 links github.com/contrueCT/hugegraph/blob/e32a75ff.../docs/negative-label-queries.md, and line 37 embeds raw.githubusercontent.com/contrueCT/hugegraph/b3f5642.../docs/images/pr-2994-condition-resolution.png. Both die once that branch is deleted, and they are what a reader following the merged commit lands on. Requested change: point the link at the in-repo path, and either commit the diagram under docs/images/ or drop it.

Separately, docs/ at this head is BUILDING.md, CONTRIBUTING.md and this file, so it is a contributor-docs folder; HugeGraph user documentation is published from apache/hugegraph-doc. What this file describes is user-visible: g.V().has("unindexedProp", "x") still raises NoIndexException while the same query followed by hasLabel(P.neq("author")) now scans candidates, and the paging note changes what a correct client has to do. Once the SEARCH predicates section is settled on the other open thread, a follow-up in hugegraph-doc would put this in front of the users who hit it, which is what the checked Doc - Done box implies for an API-affecting change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed both contributor-fork URLs from the PR description. The existing diagram and the query-behavior note now live in this standalone gist: https://gist.github.com/contrueCT/1e44ef501d0db6d82dfe2b95849770a3 . The PR embeds its image/png raw URL, verified accessible and byte-identical to the original image; the note is identical to the repository document. The diagram caption clarifies the eligible positive-label exception. I also changed Doc - Done to Doc - TODO and explicitly noted that publication to the HugeGraph documentation website is still pending. The SHA-pinned fork URLs are currently accessible, so deletion of a branch was not itself proof of immediate breakage; the fork dependency is now removed regardless.

@bitflicker64 bitflicker64 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.

Blocking: no. Summary: the LABEL migration holds at 5a7048fd. queryIndex() calls query.checkFlattened() before any single-label routing (GraphIndexTransaction.java:386) and IN is in UNFLATTEN_TYPES (Condition.java:143-144), while ConditionQueryFlatten.convIn2Or() preserves IN only for OWNER_VERTEX/ID (:152-158), so no multi-value LABEL can reach conditionValue() or singleConditionValueOrNull() on the RamTable, serializer or index paths, including on backends with supportsQueryWithInCondition(). git grep 'condition(HugeKeys.LABEL)' -- '*/src/main/*' ':(exclude)*hugegraph-test*' returns no hits at this head, and the queryNeedsPostFilter() holdout is migrated at GraphTransaction.java:2024. I also checked that leaving a HasStep in place is what keeps extractRange() (:1288-1290), extractCount() (:1303) and HugeCountStepStrategy from stepping over the new local filters. Two comments below, neither blocking. Evidence: static review of the exact-head diff (23 files, +2789/-80) against merge-base 36811483; gh api repos/apache/hugegraph/commits/5a7048fd.../check-runs is green on every workflow, with codecov/patch (23.38%) and codecov/project the only red checks. No local build or test run.

public static void extractHasContainer(HugeVertexStep<?> newStep,
Traversal.Admin<?, ?> traversal) {
if (hasUnsafeLabelInTraversal(traversal, newStep)) {
prepareLocalHasContainers(newStep, traversal);

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.

⚠️ The edge side of this fallback has no paging regression, and on this path it replaces a hard rejection with a short page.

Evidence:

  • It is reached for g.V(v).outE().has('~page','').hasLabel(P.neq('knows')).limit(10): the folded HasStep fails isEqInLabelPredicate, so hasUnsafeLabelInTraversal returns true.
  • prepareLocalHasContainers() sends ~page through query.addHasContainer(has) (line 701), which HugeVertexStep.addHasContainer() turns into setPage() and returns (HugeVertexStep.java:213-217). The label takes the else at line 729 because source instanceof HugeGraphStep is false, so HugeVertexStep.hasContainers stays empty.
  • withEdgeCondition() and withVertexCondition() are !this.hasContainers.isEmpty() (HugeVertexStep.java:185-191), so E.checkArgument(!this.queryInfo().paging(), "Can't query by paging and filtering") (:173-176) cannot fire on this path. At merge-base 36811483 it did fire: canExtractHasContainer() returned true for any sysprop key (:658-660 there), so both ~page and ~label were pushed into the step and the traversal was rejected outright.
  • The short-page contract is deliberate and documented (docs/negative-label-queries.md:41-45, plus the range bound at lines 760-765), but the only regression for it is VertexCoreTest#testPageBeforeDownstreamNegativeLabel. The five new EdgeCoreTest#testQueryEdgesByNonEqLabel* cases cover barrier, range, mixed-key or and sideEffect, none with ~page.

Requested change: add an edge-side paging regression next to testPageBeforeDownstreamNegativeLabel that pages g.V(v).outE().has('~page', cursor) with a downstream negative label to cursor exhaustion and asserts no missing or duplicate edge ids, so the shape that used to be rejected is pinned.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 459a2b2. Added two outgoing-edge paging regressions for the negative label before and after limit(). Both traverse to cursor exhaustion, reject duplicate edge IDs and repeated cursors, assert the exact expected IDs, and explicitly require an empty filtered page with a continuation cursor. On the SSH test host, all 14 targeted RocksDB edge/vertex paging and negative-label tests passed with no skips; editorconfig formatting and full-module clean compile also passed. No optimizer behavior was changed. CI for the pushed commit is running.

Comment thread docs/negative-label-queries.md Outdated

## SEARCH predicates

Local `Text.contains()` filters use the graph's SEARCH analyzer and exact term

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.

🧹 This sentence never says which Text.contains() predicates count as local, and one ordinary position is not covered. The unbound-child cause raised on the earlier head is fixed in 2d53a556; this is a different position.

Evidence:

  • prepareLocalHasContainers() walks only while (step instanceof HasStep || step instanceof NoOpBarrierStep) (TraversalUtil.java:694) and installs LocalSearchHasContainer only for containers found in that walk (:742-751).
  • A Text.contains() sitting after a RangeGlobalStep or OrderGlobalStep, for example g.V().hasLabel(P.neq('author')).limit(10).has('body', Text.contains('(alpha)')), is never reached by that walk and keeps the raw Condition.RelationType.TEXT_CONTAINS predicate, which is a plain substring test with no (word) handling: return v1 != null && ((String) v1).contains((String) v2); (Condition.java:94-97).
  • This PR's own test pins the difference: Assert.assertFalse(Text.contains("(alpha)").test("alpha")) (TraversalUtilOptimizeTest.java:534).

Requested change: name the scope, for example "Text.contains() predicates in the filter chain directly following the source step use the graph's SEARCH analyzer and exact term matcher, including explicit (word) and (word1|word2) expressions; one placed after range() or order() keeps plain substring semantics."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 459a2b2. The SEARCH section now limits analyzer-based matching to the filter chain directly following the source, including barriers, and states that range/limit/order boundaries stop this adaptation. It includes the reported post-limit example and explains its literal substring semantics. The same clarification is included in the PR description and standalone gist; no SEARCH runtime behavior was changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improve]: clarify ConditionQuery.condition() semantics for missing, conflicting, and multi-value conditions

5 participants