contrueCT commented on code in PR #3243:
URL: https://github.com/apache/hugegraph/pull/3243#discussion_r4290638287
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphIndexTransaction.java:
##########
@@ -699,6 +739,59 @@ private Set<MatchedIndex>
collectMatchedIndexes(ConditionQuery query) {
return matchedIndexes;
}
+ private static boolean isIndexQueryCandidate(SchemaLabel label, Set<Id>
properties) {
+ // Base edge labels describe sub-labels and have no direct records.
+ return label.properties().containsAll(properties) &&
+ !(label instanceof EdgeLabel && ((EdgeLabel) label).isFather());
+ }
+
+ private Set<Id> requiredIndexProperties(ConditionQuery query) {
+ Set<Id> requiredProperties = query.userpropKeys();
+ if (this.graph().readMode().showOlap()) {
+ // OLAP properties are shared rather than declared on every label.
+ requiredProperties.removeIf(id ->
this.graph().propertyKey(id).olap());
+ }
+ return requiredProperties;
+ }
+
+ private void checkIndexCoverage(ConditionQuery query,
+ List<? extends SchemaLabel> labels,
+ Set<MatchedIndex> indexes) {
+ // Check at execution, before creating any lazy index iterator. Cleanup
+ // callers only need matching indexes and must not require full
coverage.
+ Set<Id> requiredProperties = this.requiredIndexProperties(query);
+ if (requiredProperties.isEmpty()) {
+ // A shared OLAP index can match several labels, while MatchedIndex
+ // deduplicates those matches by index labels alone.
+ return;
+ }
+ Set<Id> covered = new HashSet<>();
+ for (MatchedIndex index : indexes) {
+ covered.add(index.schemaLabel().id());
+ }
+ for (SchemaLabel label : labels) {
Review Comment:
I reproduced the exact fixture and traversal against the production code at
`4a9427c3`, after rebuilding/installing the reactor to exclude stale local
dependencies. The reported exception does not occur on either Memory or
RocksDB. No production-code change was needed; commit `83ac53545` adds the
[regression](https://github.com/apache/hugegraph/blob/83ac535454310e53b4357be40b68b036a67c4284/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/SourceIndexCoverageTest.java#L310).
Fixture: `name` is TEXT; both `indexed` and `unindexed` declare it and use
automatic IDs; only `indexed.name` has a secondary index. Data contains
indexed/alice, unindexed/alice, and indexed/bob.
- Control `g.V().has("name", "alice")` throws `NoIndexException` containing
`Incomplete index coverage`.
- The exact reported query `g.V().hasLabel(P.neq("unindexed")).has("name",
"alice")` returns only indexed/alice.
- After applying strategies, the source `HugeGraphStep` has no pushed
HasContainers. The backend index cursor tracker records 0 opens and 0 closes.
The relevant earlier branch is
[extractHasContainer()](https://github.com/apache/hugegraph/blob/4a9427c30668eca628c746d5a6e6d0db3c5ed7d7/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java#L259):
detecting the unsafe label predicate calls
[prepareLocalHasContainers()](https://github.com/apache/hugegraph/blob/4a9427c30668eca628c746d5a6e6d0db3c5ed7d7/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java#L784)
and returns. That retains the ordinary property predicate locally as well as
the negative label. Consequently, this traversal scans vertices and applies
both filters locally; it never enters the claimed property-index coverage path.
Both the exact-query regression and the invalid-value regression passed on
each backend (2 tests each). The complete SourceIndexCoverageTest also passed:
Memory 30 tests / 3 skipped, RocksDB 30 tests / 0 skipped, zero
failures/errors. Java 17 full-reactor compilation passed.
This evidence is limited to the stated root traversal; it does not assert
indexed execution or cover every possible negative-label composition. Please
let me know if the report depends on a different traversal shape.
##########
docs/server-runtime.md:
##########
@@ -251,6 +251,34 @@ without a preceding REST graph query, and subsequent REST
queries must remain us
Gremlin scripts still cannot read restricted system properties such as
`java.version`
or disable the security manager.
+### Property-query index coverage
+
+A global property query such as `g.V().has("name", "alice")` or
+`g.E().has("weight", 0.5)` requires compatible indexes on every visible
candidate
+label. A candidate declares all queried non-OLAP properties; even a label with
no
+records requires coverage. Base edge labels have no direct records; their
indexes
+can cover sub-labels. Shared OLAP indexes follow the graph read mode.
+
+| Candidate state | Query behavior |
+| --- | --- |
+| Every visible candidate has compatible indexes | The query combines the
indexed results. Composite and joint index plans may serve different labels in
the same query. |
+| A visible candidate lacks a compatible index | The query fails with
`NoIndexException`, including when other candidates are hidden or deleting. |
Review Comment:
Fixed in `83ac53545`. The [coverage contract and
table](https://github.com/apache/hugegraph/blob/83ac535454310e53b4357be40b68b036a67c4284/docs/server-runtime.md#L254)
now qualify the error for values passing the property-type check and document
the ordering explicitly: with visible candidates remaining, a matched index
plus an invalid value returns empty before coverage validation; no matching
index still raises `NoIndexException`.
The existing
[testInvalidValuePrecedesPartialCoverageError](https://github.com/apache/hugegraph/blob/83ac535454310e53b4357be40b68b036a67c4284/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/SourceIndexCoverageTest.java#L341)
checks all three cases: invalid value without an index, invalid value with
partial coverage, and valid value with partial coverage. It was rerun
successfully on both Memory and RocksDB.
--
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]
