imbajin commented on code in PR #2864: URL: https://github.com/apache/hugegraph/pull/2864#discussion_r4274566241
########## hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/AdjacentEdgesQuery.java: ########## @@ -0,0 +1,396 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hugegraph.backend.query; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; + +import org.apache.hugegraph.backend.id.Id; +import org.apache.hugegraph.backend.query.Condition.Relation; +import org.apache.hugegraph.backend.query.Condition.RelationType; +import org.apache.hugegraph.structure.HugeEdge; +import org.apache.hugegraph.structure.HugeElement; +import org.apache.hugegraph.type.HugeType; +import org.apache.hugegraph.type.define.Directions; +import org.apache.hugegraph.type.define.HugeKeys; +import org.apache.hugegraph.util.CollectionUtil; +import org.apache.hugegraph.util.E; +import org.apache.hugegraph.util.InsertionOrderUtil; + +import com.google.common.collect.ImmutableList; + +public class AdjacentEdgesQuery extends ConditionQuery { + + private Id ownerVertex; + private Directions direction; + private Id[] edgeLabels; + + private List<Condition> selfConditions; + + private static final Id[] EMPTY = new Id[0]; + + public AdjacentEdgesQuery(Id ownerVertex, Directions direction) { + this(ownerVertex, direction, EMPTY); + } + + public AdjacentEdgesQuery(Id ownerVertex, Directions direction, + Id[] edgeLabels) { + super(HugeType.EDGE); + E.checkArgument(ownerVertex != null, + "The edge query must contain source vertex"); + E.checkArgument(direction != null, + "The edge query must contain direction"); + this.ownerVertex = ownerVertex; + this.direction = direction; + this.edgeLabels = edgeLabels; + this.selfConditions = null; + } + + @Override + public int conditionsSize() { + int size = super.conditionsSize() + 2; + if (this.edgeLabels.length > 0) { + size += 1; + } + return size; + } + + @Override + public ConditionQuery query(Condition condition) { + if (!condition.isRelation()) { + return super.query(condition); + } + + // Reset this.selfConditions cache + this.selfConditions = null; + + // Update condition fields + Relation relation = ((Condition.Relation) condition); + Object key = relation.key(); + RelationType relationType = relation.relation(); + + if (key == HugeKeys.OWNER_VERTEX) { + this.ownerVertex = (Id) relation.value(); + return this; + } + if (key == HugeKeys.DIRECTION) { + this.direction = (Directions) relation.value(); + return this; + } + if (key == HugeKeys.LABEL) { + Collection<Id> labels = null; + if (relationType == RelationType.EQ) { + labels = ImmutableList.of((Id) relation.value()); + } else if (relationType == RelationType.IN) { + @SuppressWarnings("unchecked") + Collection<Id> value = (Collection<Id>) relation.value(); + labels = value; + } else { + E.checkArgument(false, + "Unexpected relation type '%s' for '%s'", + relationType, key); + } + + if (this.edgeLabels.length == 0) { + this.edgeLabels = labels.toArray(new Id[0]); + } else { + Collection<Id> edgeLabels = CollectionUtil.intersect( + Arrays.asList(this.edgeLabels), + labels); + if (edgeLabels.isEmpty()) { + // Returns empty result if conditions are conflicting + this.limit(0L); + } + this.edgeLabels = edgeLabels.toArray(new Id[0]); + } + return this; + } + + return super.query(condition); + } + + @Override + public Collection<Condition> conditions() { + List<Condition> conds = this.selfConditions(); + if (super.conditionsSize() > 0) { + conds.addAll(super.conditions()); + } + return conds; Review Comment: [P1] Preserve generic condition-flattening semantics The combined condition getter appends into the cached mutable self-condition list; copy() shares that list, while resetConditions() leaves the field-backed direction and labels intact. For g.V(id).bothE("knows").has("since", 2020), with since as the first/only sort key, the generic flatten path retains the original DIRECTION IN relation and query() casts its list value to Directions, throwing ClassCastException. A Java 17 probe using the unchanged AdjacentEdgesQuery and ConditionQueryFlatten classes with reduced collaborators reproduces this; the no-property BOTH control produces two queries. This was not a full graph integration run. Please make condition views non-mutating and preserve independent copy/reset semantics, or materialize an ordinary ConditionQuery before generic flattening outside the fast path. Copying the cached list alone does not reset the field-backed IN condition. ########## hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/HugeVertexStep.java: ########## @@ -133,7 +133,7 @@ protected ConditionQuery constructEdgesQuery( Id vertex = (Id) traverser.get().id(); Directions direction = Directions.convert(this.getDirection()); - EdgeLabel[] els = graph.mapElName2El(this.getEdgeLabels()); + Id[] els = graph.mapElName2Id(this.getEdgeLabels()); Review Comment: [P1] Preserve father/sub-label routing for child edge labels The removed EdgeLabel overload emitted LABEL=fatherId and SUB_LABEL=childId for a child label. mapElName2Id() now passes only childId to AdjacentEdgesQuery, which emits LABEL=childId. HugeEdge.assignId() still stores inherited edges under fatherId followed by childId, and BinarySerializer uses those fields for the query prefix. Consequently g.V(id).outE(childLabel) targets the wrong prefix and misses existing child-label edges. Please retain the schema-aware father/sub-label translation and add a child-label adjacency regression test, with an ordinary label as control. This finding is based on the exact-head query/storage code paths, not a live backend run. -- 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]
