imbajin commented on code in PR #2994:
URL: https://github.com/apache/hugegraph/pull/2994#discussion_r3567581934
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphIndexTransaction.java:
##########
@@ -773,7 +783,8 @@ private Set<MatchedIndex>
collectMatchedIndexes(ConditionQuery query) {
}
schemaLabels = ImmutableList.of(schemaLabel);
} else {
- // Query doesn't have LABEL condition
+ // Query doesn't have LABEL condition or it doesn't resolve
+ // to a single label, so keep the conservative fallback.
if (query.resultType().isVertex()) {
schemaLabels = schema.getVertexLabels();
Review Comment:
⚠️ Multi-label `IN` falls back to every schema label here instead of the
resolved label IDs. On backends that preserve `IN` (notably Cassandra/MySQL),
`collectMatchedIndex()` can then build a plan for only the requested labels
that have a matching property index. For `LABEL IN [A, B]` where A is indexed
and B is not, matching B elements can be silently omitted; an unrelated indexed
label C can also suppress the expected no-index outcome. Please restrict
planning to all resolved label IDs and require complete coverage (or flatten
before planning / explicitly fall back or reject), then add a runtime
regression with A indexed, B unindexed but matching, and unrelated C indexed.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java:
##########
@@ -453,6 +459,22 @@ private static boolean canExtractHasContainers(HugeGraph
graph,
return true;
}
+ private static boolean isOnlyNonEqInLabelPredicate(HasContainer has) {
+ if (!has.getKey().equals(T.label.getAccessor())) {
+ return false;
+ }
+
+ List<P<Object>> predicates = new ArrayList<>();
+ collectPredicates(predicates, ImmutableList.of(has.getPredicate()));
+ for (P<Object> predicate : predicates) {
+ BiPredicate<?, ?> bp = predicate.getBiPredicate();
+ if (bp == Compare.eq || bp == Contains.within) {
Review Comment:
⚠️ This returns as soon as any connective leaf is `EQ`/`WITHIN`, so a mixed
predicate such as `P.or(P.eq("person"), P.neq("author"))` is treated as safe to
extract. The resulting top-level connective LABEL condition is not resolved by
`conditionValues()`, which only handles top-level EQ/IN, so it can enter the
wrong query/index path instead of remaining a TinkerPop filter. Please permit
extraction only when the entire connective predicate is safely expressible as
EQ/IN (or retain any predicate containing NEQ/WITHOUT/other leaves), and add
mixed OR/AND vertex and edge regressions.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/serializer/TextSerializer.java:
##########
@@ -516,6 +516,17 @@ private Query writeQueryEdgePrefixCondition(ConditionQuery
cq) {
return null;
}
+ private Object edgeIdConditionValue(ConditionQuery cq, HugeKeys key) {
+ if (key == HugeKeys.LABEL) {
+ /*
+ * LABEL may still be represented by multiple top-level EQ/IN
+ * relations before strict edge-id serialization.
+ */
+ return cq.conditionValue(key);
Review Comment:
⚠️ `conditionValue()` throws when LABEL retains multiple `IN` values. MySQL
declares `supportsQueryWithInCondition() == true`, so `GraphTransaction`
intentionally leaves a multi-label edge query unflattened; the query can then
reach this edge prefix/range fast path and fail before MySQL handles the
supported `IN`, unlike backends that flatten it first. Please always flatten
LABEL `IN` before edge-id serialization or bypass the single-edge-id fast path
when multiple labels remain, and add a MySQL-profile regression for
`outE(labelA, labelB)` with property/sort-key conditions.
--
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]
