bitflicker64 commented on code in PR #3243:
URL: https://github.com/apache/hugegraph/pull/3243#discussion_r4115746808
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphIndexTransaction.java:
##########
@@ -805,9 +814,49 @@ private Set<MatchedIndex>
collectMatchedIndexes(ConditionQuery query) {
matchedIndexes.add(index);
}
}
+ if (requireCompleteCoverage && !matchedIndexes.isEmpty()) {
+ this.checkIndexCoverage(query, schemaLabels, matchedIndexes);
+ }
return matchedIndexes;
}
+ 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 = 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());
Review Comment:
Important: I agree with the existing comment on this line, and the passing
CI does not contradict it. With GraphReadMode.ALL, `g.V().has("wcc", "a")`
leaves `requiredProperties` empty after this `removeIf`, so every vertex label
passes the `containsAll` check below. `collectMatchedIndex()` adds the same
global OLAP index label to every schema label, and
`MatchedIndex.equals()`/`hashCode()` compare only `indexLabels`, so the set
keeps one entry and `covered` holds one label id. The loop then throws
`NoIndexException` for the next vertex label. On master the same query returns
the matching vertex.
`VertexCoreTest.testAddOlapSecondaryProperties` runs exactly this query, but
it only runs when `System.getProperty("backend")` is `"hstore"`.
`hugegraph-test/pom.xml` sets that property for the `api-test` execution only,
not `core-test`, so the OLAP core tests are skipped in the memory, RocksDB,
HBase and HStore jobs at this head (the hstore job reports CoreTestSuite 830
run, 48 skipped). Green CI therefore says nothing about this path.
Requested change: treat an OLAP index label (`il.olap()`) as covering every
candidate vertex label, or skip the per-label loop when `requiredProperties` is
empty after this line. Add a multi-label OLAP-only regression in
`SourceIndexCoverageTest` guarded only by `supportsOlapProperties()`, so it
runs on 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]
