Skip to content

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.

Sign up for GitHub

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

Open
contrueCT wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix(query): clarify condition resolution semantics for label queries #2994

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7e95e5e
improve(query): clarify condition resolution semantics
contrueCT Apr 12, 2026
43d4541
test(core): avoid unnecessary data setup in label index regression
contrueCT Apr 13, 2026
88831e5
improve(query): consolidate unique label resolution
contrueCT Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
168 changes: 129 additions & 39 deletions ...erver/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/ConditionQuery.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,24 +256,27 @@ public boolean containsLabelOrUserpropRelation() {
return false;
}

/**
* Returns the legacy condition value of the specified key.
*
* This method keeps the historical behavior for existing callers:
* <ul>
* <li>returns {@code null} if no top-level EQ/IN relation exists</li>
* <li>returns {@code null} if top-level EQ/IN relations resolve to empty</li>
* <li>returns the single value if only one value is resolved</li>
* <li>returns the raw IN list if there is exactly one top-level IN relation</li>
* <li>throws if multiple values remain after resolving several relations</li>
* </ul>
*
* Prefer {@link #conditionValues(Object)}, {@link #uniqueConditionValue(Object)}
* or {@link #conditionValue(Object)} for new code that needs explicit
* semantics.
*/
@Watched
public <T> T condition(Object key) {
List<Object> valuesEQ = InsertionOrderUtil.newList();
List<Object> valuesIN = InsertionOrderUtil.newList();
for (Condition c : this.conditions) {
if (c.isRelation()) {
Condition.Relation r = (Condition.Relation) c;
if (r.key().equals(key)) {
if (r.relation() == RelationType.EQ) {
valuesEQ.add(r.value());
} else if (r.relation() == RelationType.IN) {
Object value = r.value();
assert value instanceof List;
valuesIN.add(value);
}
}
}
}
this.collectConditionValues(key, valuesEQ, valuesIN);
if (valuesEQ.isEmpty() && valuesIN.isEmpty()) {
return null;
}
Expand All @@ -288,29 +291,8 @@ public <T> T condition(Object key) {
return value;
}

boolean initialized = false;
Set<Object> intersectValues = InsertionOrderUtil.newSet();
for (Object value : valuesEQ) {
List<Object> valueAsList = ImmutableList.of(value);
if (!initialized) {
intersectValues.addAll(valueAsList);
initialized = true;
} else {
CollectionUtil.intersectWithModify(intersectValues,
valueAsList);
}
}
for (Object value : valuesIN) {
@SuppressWarnings("unchecked")
List<Object> valueAsList = (List<Object>) value;
if (!initialized) {
intersectValues.addAll(valueAsList);
initialized = true;
} else {
CollectionUtil.intersectWithModify(intersectValues,
valueAsList);
}
}
Set<Object> intersectValues = this.resolveConditionValues(valuesEQ,
valuesIN);

if (intersectValues.isEmpty()) {
return null;
Expand All @@ -323,20 +305,128 @@ public <T> T condition(Object key) {
return value;
}

/**
* Returns whether there is any top-level relation for the specified key.
*/
public boolean containsCondition(Object key) {
for (Condition c : this.conditions) {
if (c.isRelation()) {
Condition.Relation r = (Condition.Relation) c;
if (r.key().equals(key)) {
return true;
}
}
}
return false;
}

/**
* Returns the resolved candidate values of the specified key from
* top-level EQ/IN relations.
*
* Use {@link #containsCondition(Object)} to distinguish "no condition"
* from "conditions exist but resolve to an empty intersection".
*/
public Set<Object> conditionValues(Object key) {
List<Object> valuesEQ = InsertionOrderUtil.newList();
List<Object> valuesIN = InsertionOrderUtil.newList();
this.collectConditionValues(key, valuesEQ, valuesIN);
if (valuesEQ.isEmpty() && valuesIN.isEmpty()) {
return InsertionOrderUtil.newSet();
}
return this.resolveConditionValues(valuesEQ, valuesIN);
}

/**
* Returns the unique resolved value of the specified key from top-level
* EQ/IN relations.
*
* Returns {@code null} when the resolved candidate set is empty. Throws
* if multiple values remain after resolution.
*/
public <T> T conditionValue(Object key) {
Set<Object> values = this.conditionValues(key);
if (values.isEmpty()) {
return null;
}
E.checkState(values.size() == 1,
"Illegal key '%s' with more than one value: %s",
key, values);
@SuppressWarnings("unchecked")
T value = (T) values.iterator().next();
return value;
}

/**
* Returns the unique resolved value of the specified key from top-level
* EQ/IN relations, or {@code null} if the resolved candidate set doesn't
* contain exactly one value.
*
* Use this method when callers want "single-or-null" semantics instead of
* treating multiple remaining values as an error.
*/
public <T> T uniqueConditionValue(Object key) {
Set<Object> values = this.conditionValues(key);
if (values.size() != 1) {
return null;
}
@SuppressWarnings("unchecked")
T value = (T) values.iterator().next();
return value;
}

public void unsetCondition(Object key) {
this.conditions.removeIf(c -> c.isRelation() && ((Relation) c).key().equals(key));
}

public boolean containsCondition(HugeKeys key) {
return this.containsCondition((Object) key);
}

private void collectConditionValues(Object key, List<Object> valuesEQ,
List<Object> valuesIN) {
for (Condition c : this.conditions) {
if (c.isRelation()) {
Condition.Relation r = (Condition.Relation) c;
if (r.key().equals(key)) {
return true;
if (r.relation() == RelationType.EQ) {
valuesEQ.add(r.value());
} else if (r.relation() == RelationType.IN) {
Object value = r.value();
assert value instanceof List;
valuesIN.add(value);
}
}
}
}
return false;
}

private Set<Object> resolveConditionValues(List<Object> valuesEQ,
List<Object> valuesIN) {
boolean initialized = false;
Copy link
Copy Markdown
Member

@imbajin imbajin Apr 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ resolveConditionValues duplicates logic already in condition() — consider extracting the shared intersection

The new resolveConditionValues() method contains the same intersection logic as the existing condition() method. The only difference is condition() has extra fast-path returns and the final single-value check.

Since condition() now delegates collectConditionValues() to the new private method (good!), it could also delegate the intersection to resolveConditionValues() to fully eliminate the duplicated loop:

public <T> T condition(Object key) {
    List<Object> valuesEQ = InsertionOrderUtil.newList();
    List<Object> valuesIN = InsertionOrderUtil.newList();
    this.collectConditionValues(key, valuesEQ, valuesIN);
    if (valuesEQ.isEmpty() && valuesIN.isEmpty()) {
        return null;
    }
    // Keep legacy fast paths for backward compatibility
    if (valuesEQ.size() == 1 && valuesIN.isEmpty()) { ... }
    if (valuesEQ.isEmpty() && valuesIN.size() == 1) { ... }
    // Delegate to shared intersection instead of duplicating
    Set<Object> intersectValues = this.resolveConditionValues(valuesEQ, valuesIN);
    // ... rest of legacy checks
}

This would make condition() and conditionValues() share the same intersection code path, reducing divergence risk.

Set<Object> intersectValues = InsertionOrderUtil.newSet();
for (Object value : valuesEQ) {
List<Object> valueAsList = ImmutableList.of(value);
if (!initialized) {
intersectValues.addAll(valueAsList);
initialized = true;
} else {
CollectionUtil.intersectWithModify(intersectValues,
valueAsList);
}
}
for (Object value : valuesIN) {
@SuppressWarnings("unchecked")
List<Object> valueAsList = (List<Object>) value;
if (!initialized) {
intersectValues.addAll(valueAsList);
initialized = true;
} else {
CollectionUtil.intersectWithModify(intersectValues,
valueAsList);
}
}
return intersectValues;
}

public boolean containsCondition(Condition.RelationType type) {
Expand Down
15 changes: 13 additions & 2 deletions ...ugegraph-core/src/main/java/org/apache/hugegraph/backend/serializer/BinarySerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ private Query writeQueryEdgeRangeCondition(ConditionQuery cq) {
if (direction == null) {
direction = Directions.OUT;
}
Id label = cq.condition(HugeKeys.LABEL);
Id label = (Id) this.edgeIdConditionValue(cq, HugeKeys.LABEL);

BytesBuffer start = BytesBuffer.allocate(BytesBuffer.BUF_EDGE_ID);
writePartitionedId(HugeType.EDGE, vertex, start);
Expand Down Expand Up @@ -722,7 +722,7 @@ private Query writeQueryEdgePrefixCondition(ConditionQuery cq) {
int count = 0;
BytesBuffer buffer = BytesBuffer.allocate(BytesBuffer.BUF_EDGE_ID);
for (HugeKeys key : EdgeId.KEYS) {
Object value = cq.condition(key);
Object value = this.edgeIdConditionValue(cq, key);

if (value != null) {
count++;
Expand Down Expand Up @@ -763,6 +763,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);
}
return cq.condition(key);
}

@Override
protected Query writeQueryCondition(Query query) {
HugeType type = query.resultType();
Expand Down
15 changes: 13 additions & 2 deletions .../hugegraph-core/src/main/java/org/apache/hugegraph/backend/serializer/TextSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
}
return cq.condition(key);
}

@Override
protected Query writeQueryCondition(Query query) {
ConditionQuery result = (ConditionQuery) query;
Expand Down
4 changes: 2 additions & 2 deletions ...-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/store/ram/RamTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ public boolean matched(Query query) {
int conditionsSize = cq.conditionsSize();
Object owner = cq.condition(HugeKeys.OWNER_VERTEX);
Directions direction = cq.condition(HugeKeys.DIRECTION);
Id label = cq.condition(HugeKeys.LABEL);
Id label = cq.uniqueConditionValue(HugeKeys.LABEL);

if (direction == null && conditionsSize > 1) {
for (Condition cond : cq.conditions()) {
Expand Down Expand Up @@ -316,7 +316,7 @@ private Iterator<HugeEdge> query(ConditionQuery query) {
if (dir == null) {
dir = Directions.BOTH;
}
Id label = query.condition(HugeKeys.LABEL);
Id label = query.uniqueConditionValue(HugeKeys.LABEL);
if (label == null) {
label = IdGenerator.ZERO;
}
Expand Down
25 changes: 17 additions & 8 deletions ...r/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphIndexTransaction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,11 @@ private IdHolderList queryByLabel(ConditionQuery query) {
HugeType queryType = query.resultType();
IndexLabel il = IndexLabel.label(queryType);
validateIndexLabel(il);
Id label = query.condition(HugeKeys.LABEL);
assert label != null;
// Query-by-label builds a label index entry and requires one
// deterministically resolved label instead of best-effort fallback.
Id label = query.conditionValue(HugeKeys.LABEL);
E.checkState(label != null, "Expect one label value for query: %s",
query);

HugeType indexType;
SchemaLabel schemaLabel;
Expand Down Expand Up @@ -482,7 +485,7 @@ private IdHolderList queryByUserprop(ConditionQuery query) {
}
Set<MatchedIndex> indexes = this.collectMatchedIndexes(query);
if (indexes.isEmpty()) {
Id label = query.condition(HugeKeys.LABEL);
Id label = query.uniqueConditionValue(HugeKeys.LABEL);
throw noIndexException(this.graph(), query, label);
}

Expand Down Expand Up @@ -756,11 +759,16 @@ private PageIds doIndexQueryOnce(IndexLabel indexLabel,
@Watched(prefix = "index")
private Set<MatchedIndex> collectMatchedIndexes(ConditionQuery query) {
ISchemaTransaction schema = this.params().schemaTransaction();
Id label = query.condition(HugeKeys.LABEL);
boolean hasLabel = query.containsCondition(HugeKeys.LABEL);
Set<Object> labels = query.conditionValues(HugeKeys.LABEL);

List<? extends SchemaLabel> schemaLabels;
if (label != null) {
// Query has LABEL condition
if (hasLabel && labels.isEmpty()) {
return Collections.emptySet();
}
if (labels.size() == 1) {
Id label = (Id) labels.iterator().next();
// Query has one resolved LABEL condition
SchemaLabel schemaLabel;
if (query.resultType().isVertex()) {
schemaLabel = schema.getVertexLabel(label);
Expand All @@ -773,7 +781,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();
} else if (query.resultType().isEdge()) {
Expand Down Expand Up @@ -1781,7 +1790,7 @@ protected long removeIndexLeft(ConditionQuery query,
}

// Check label is matched
Id label = query.condition(HugeKeys.LABEL);
Id label = query.uniqueConditionValue(HugeKeys.LABEL);
// NOTE: original condition query may not have label condition,
// which means possibly label == null.
if (label != null && !element.schemaLabel().id().equals(label)) {
Expand Down
9 changes: 5 additions & 4 deletions ...server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1059,7 +1059,7 @@ protected Iterator<HugeEdge> queryEdgesFromBackend(Query query) {
ConditionQueryFlatten.flatten((ConditionQuery) query, supportIn).stream();

Stream<Iterator<HugeEdge>> edgeIterators = flattenedQueries.map(cq -> {
Id label = cq.condition(HugeKeys.LABEL);
Id label = cq.uniqueConditionValue(HugeKeys.LABEL);
if (this.storeFeatures().supportsFatherAndSubEdgeLabel() &&
label != null &&
graph().edgeLabel(label).isFather() &&
Expand Down Expand Up @@ -1389,7 +1389,7 @@ private static boolean matchEdgeSortKeys(ConditionQuery query,
boolean matchAll,
HugeGraph graph) {
assert query.resultType().isEdge();
Id label = query.condition(HugeKeys.LABEL);
Id label = query.uniqueConditionValue(HugeKeys.LABEL);
if (label == null) {
return false;
}
Expand Down Expand Up @@ -1522,7 +1522,7 @@ private Query optimizeQuery(ConditionQuery query) {
throw new HugeException("Not supported querying by id and conditions: %s", query);
}

Id label = query.condition(HugeKeys.LABEL);
Id label = query.uniqueConditionValue(HugeKeys.LABEL);

// Optimize vertex query
if (label != null && query.resultType().isVertex()) {
Expand Down Expand Up @@ -1914,7 +1914,8 @@ private boolean rightResultFromIndexQuery(Query query, HugeElement elem) {
}

ConditionQuery cq = (ConditionQuery) query;
if (cq.condition(HugeKeys.LABEL) != null && cq.resultType().isEdge()) {
if (cq.uniqueConditionValue(HugeKeys.LABEL) != null &&
cq.resultType().isEdge()) {
if (cq.conditions().size() == 1) {
// g.E().hasLabel(xxx)
return true;
Expand Down
Loading
Loading