-
Notifications
You must be signed in to change notification settings - Fork 619
fix(query): clarify condition resolution semantics for label queries #2994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
ac25ec5
c4cf3d5
ed3b788
a326f44
b10e3c2
a183571
ebc31c8
2df4802
939cc12
9300691
7c85cca
9e24432
3646a14
9eb75e8
212e89a
b16f5f1
18fe87f
057b390
2627903
c35b389
bc2a999
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -457,7 +457,7 @@ private Query writeQueryEdgeRangeCondition(ConditionQuery cq) { | |
| if (direction == null) { | ||
| direction = Directions.OUT; | ||
| } | ||
| Object label = cq.condition(HugeKeys.LABEL); | ||
| Object label = this.edgeIdConditionValue(cq, HugeKeys.LABEL); | ||
|
|
||
| List<String> start = new ArrayList<>(cq.conditionsSize()); | ||
| start.add(writeEntryId((Id) vertex)); | ||
|
|
@@ -491,7 +491,7 @@ private Query writeQueryEdgePrefixCondition(ConditionQuery cq) { | |
| List<String> condParts = new ArrayList<>(cq.conditionsSize()); | ||
|
|
||
| for (HugeKeys key : EdgeId.KEYS) { | ||
| Object value = cq.condition(key); | ||
| Object value = this.edgeIdConditionValue(cq, key); | ||
| if (value == null) { | ||
| break; | ||
| } | ||
|
|
@@ -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); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| return cq.condition(key); | ||
| } | ||
|
|
||
| @Override | ||
| protected Query writeQueryCondition(Query query) { | ||
| ConditionQuery result = (ConditionQuery) query; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Low · Clarify the empty-IN legacy contract
🔄 Contract mismatch
The new Javadoc says condition() returns null when top-level EQ/IN relations resolve to an empty set, but the sole-IN fast path at lines 288-291 returns the raw list before resolution. A single
LABEL IN []therefore returns an empty List, not null.Tip
Impact: The documented contract contradicts the preserved legacy behavior, which can mislead callers selecting among condition(), conditionValues(), and conditionValue(). The boundary is also absent from QueryTest.
🛠️ Suggested change
Document that a sole IN relation returns its raw list even when empty, and add a QueryTest assertion for
LABEL IN []to pin the legacy behavior.🤖 Codex review · GPT-5.6 Sol · effort: xhigh