contrueCT commented on code in PR #3243:
URL: https://github.com/apache/hugegraph/pull/3243#discussion_r4163105409
##########
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) {
Review Comment:
Fixed in 7f5a9173. Foreground planning now filters schema labels using the
outer request's showHidden/showDeleting flags before matching or validating
indexes; cleanup matching remains separate. Normal queries skip DELETING and
UNDELETED labels, while showDeleting requests still require coverage. I
reproduced both the removed-index coverage error and the still-present
DELETING-index validation error on the old code. Added vertex/edge visibility,
inherited origin flags, maintenance-query positive controls, restore/reindex,
sub-label deletion and paging regressions. The final coverage class passed on
Memory (22 run, 3 skipped) and RocksDB (22 run, no skips); complete RocksDB
Vertex/Edge/Coverage ran 460 with 29 skipped and zero failures/errors. Java 17
clean compile passed all 38 modules. This follows existing visibility; it does
not introduce a concurrent-DDL snapshot.
##########
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:
Fixed in 7f5a9173. The age=28 and city=Hangzhou cleanup queries now
explicitly use hasLabel("person"). I ran the actual shipped
Example1.loadSchema() and testLeftIndexProcess() methods on the RocksDB test
graph with Java assertions enabled through an external harness; one test
passed, including the method's empty-result checks and Curry's final
age=30/city=Shanghai. The harness and temporary source copies were removed from
the test checkout afterward. The example module also passed the Java 17
all-module clean compile.
--
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]
