imbajin commented on code in PR #3243:
URL: https://github.com/apache/hugegraph/pull/3243#discussion_r4292849680
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java:
##########
@@ -1523,23 +1523,35 @@ private <R> QueryList<R> optimizeQueries(Query query,
return queries;
}
- boolean supportIn =
this.storeFeatures().supportsQueryWithInCondition();
- for (ConditionQuery cq : ConditionQueryFlatten.flatten(
- (ConditionQuery) query, supportIn)) {
- // Optimize by sysprop
- Query q = this.optimizeQuery(cq);
- /*
- * NOTE: There are two possibilities for the returned q:
- * 1.sysprop-query, which would not be empty.
- * 2.index-query result(ids after optimization), which may be
empty.
- */
- if (q == null) {
- queries.add(this.indexQuery(cq), this.batchSize);
- } else if (!q.empty()) {
- queries.add(q);
+ try {
+ boolean supportIn =
this.storeFeatures().supportsQueryWithInCondition();
+ for (ConditionQuery cq : ConditionQueryFlatten.flatten(
+ (ConditionQuery) query, supportIn)) {
+ // Optimize by sysprop
+ Query q = this.optimizeQuery(cq);
+ /*
+ * NOTE: There are two possibilities for the returned q:
+ * 1.sysprop-query, which would not be empty.
+ * 2.index-query result(ids after optimization), which may be
empty.
+ */
+ if (q == null) {
+ queries.add(this.indexQuery(cq), this.batchSize);
Review Comment:
⚠️ Important. Blocking: no. Summary: The new catch closes only branches
already in `queries`; if `indexQuery(cq)` opens a `BatchIdHolder` for one label
and throws while planning a later label, Java never calls `queries.add`, so the
current branch's holder list is unreachable. Close partially built holders in
`queryByUserprop()`/joint query construction and preserve the original error.
Evidence: `doIndexQueryBatch()` eagerly evaluates
`super.query(query).iterator()` while `IdHolderList` has no `close()` method.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphIndexTransaction.java:
##########
@@ -650,15 +680,14 @@ private PageIds doIndexQueryOnce(IndexLabel indexLabel,
}
}
- @Watched(prefix = "index")
- private Set<MatchedIndex> collectMatchedIndexes(ConditionQuery query) {
+ private List<? extends SchemaLabel> querySchemaLabels(ConditionQuery
query) {
ISchemaTransaction schema = this.params().schemaTransaction();
Set<Object> labels = query.conditionValues(HugeKeys.LABEL);
List<? extends SchemaLabel> schemaLabels;
if (query.containsConditionValues(HugeKeys.LABEL) && labels.isEmpty())
{
// An empty intersection is not an unrestricted label query.
- return Collections.emptySet();
+ return Collections.emptyList();
}
if (labels.size() == 1) {
Review Comment:
⚠️ Important. Blocking: no. Summary: `querySchemaLabels()` narrows this
query to `subA`, then `collectMatchedIndex()` sees only indexes attached
directly to that sub-label. With only `parentEdgeByName`,
`E().hasLabel("subA").has("coverageName", "same")` throws `NoIndexException`
despite `updateEdgeIndex()` storing subA records in the parent index; include
the father index in the plan and add a labeled regression. Evidence:
`SchemaLabel.indexLabels()` exposes only directly attached indexes;
`updateEdgeIndex()` indexes sub-label edges through the father.
--
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]
