bitflicker64 commented on code in PR #3243:
URL: https://github.com/apache/hugegraph/pull/3243#discussion_r4140682907
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphIndexTransaction.java:
##########
@@ -805,9 +814,57 @@ private Set<MatchedIndex>
collectMatchedIndexes(ConditionQuery query) {
matchedIndexes.add(index);
}
}
+ // An invalid value returns no results after an index is found. Keep
+ // that result before deciding whether index coverage is complete.
+ if (requireCompleteCoverage && !matchedIndexes.isEmpty() &&
+ validQueryConditionValues(this.graph(), query)) {
+ 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());
+ }
+ 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) {
+ if (covered.contains(label.id()) ||
+ !label.properties().containsAll(requiredProperties)) {
+ continue;
+ }
+ if (label instanceof EdgeLabel) {
+ EdgeLabel edgeLabel = (EdgeLabel) label;
+ // Base labels have no direct edges; their property indexes
+ // include edges of their sub-labels (see updateEdgeIndex()).
+ if (edgeLabel.isFather() ||
+ edgeLabel.hasFather() &&
covered.contains(edgeLabel.fatherId())) {
+ continue;
+ }
+ }
+ // Authorization filters elements after the backend query. Do not
+ // disclose an implicitly discovered schema label in this error.
+ throw new NoIndexException("Incomplete index coverage for
properties %s; " +
Review Comment:
Important: This throw breaks the shipped `hugegraph-example` Example1, which
the PR does not update. `Example1.main()` calls `testLeftIndexProcess()`
(Example1.java:64), and that method runs two unlabeled property queries against
the schema from `loadSchema()`:
- line 482, `V().has("age", 28)`: `person` matches `personByAge`, but
`author` also declares `age` (line 137) and only has the search index
`authorByLived`. `collectMatchedIndex()` returns null for `author`,
`author.properties()` contains `age`, so this loop throws.
- line 485, `V().has("city", "Hangzhou")`: `person` matches `personByCity`,
but `FridgeSensor` declares `city` (line 150) with no index. The primary-key
shortcut in `GraphTransaction.optimizeQuery()` needs a label condition, so the
query reaches this check and throws.
On master both queries return the indexed `person` result (empty, which is
what the method's `assert`s check). At 0de33eda `main()` stops with `Incomplete
index coverage` before `thread(graph)` runs. The example module has no CI job,
so the green checks do not cover it. Example2 and PerfExample3/4 are fully
indexed and are not affected.
Requested change: add `.hasLabel("person")` to the two queries at
Example1.java:482 and :485 (the method tests left-index cleanup on `person`),
or add `age` and `city` indexes on `author` and `FridgeSensor` in
`loadSchema()`, the same way this PR completed the TinkerPop fixtures in
TestGraph.
--
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]
