Copilot commented on code in PR #2856: URL: https://github.com/apache/incubator-hugegraph/pull/2856#discussion_r2378864295
########## hugegraph-server/hugegraph-vector/src/main/java/org/apache/hugegraph/backend/store/jvector/VectorSessions.java: ########## @@ -0,0 +1,183 @@ +/* + * 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.store.jvector; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +import org.apache.hugegraph.backend.store.BackendEntry; +import org.apache.hugegraph.backend.store.BackendSession.AbstractBackendSession; +import org.apache.hugegraph.backend.store.BackendSessionPool; +import org.apache.hugegraph.config.HugeConfig; +import org.apache.hugegraph.type.HugeType; +import org.apache.hugegraph.util.Log; +import org.slf4j.Logger; + +/** + * Vector sessions for managing vector index operations + */ +public class VectorSessions extends BackendSessionPool { + + private static final Logger LOG = Log.logger(VectorSessions.class); + + private final String dataPath; + private final HugeConfig config; + private final Map<HugeType, VectorTable> tables; + private boolean closed; + + public VectorSessions(String dataPath, HugeConfig config) { + super(config, dataPath); + this.dataPath = dataPath; + this.config = config; + this.tables = new ConcurrentHashMap<>(); + this.closed = false; + } + + + public boolean close() { + this.closed = true; + // Close all tables + for (VectorTable table : this.tables.values()) { + try { + table.close(); + } catch (Exception e) { + LOG.warn("Failed to close vector table: {}", e.getMessage()); + } + } + this.tables.clear(); + return false; Review Comment: The method always returns false despite successfully closing tables. Consider returning true on successful closure or changing the return type to void if the boolean return value is not needed. ```suggestion return true; ``` ########## hugegraph-server/hugegraph-vector/src/main/java/org/apache/hugegraph/backend/store/jvector/VectorSessions.java: ########## @@ -0,0 +1,183 @@ +/* + * 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.store.jvector; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +import org.apache.hugegraph.backend.store.BackendEntry; +import org.apache.hugegraph.backend.store.BackendSession.AbstractBackendSession; +import org.apache.hugegraph.backend.store.BackendSessionPool; +import org.apache.hugegraph.config.HugeConfig; +import org.apache.hugegraph.type.HugeType; +import org.apache.hugegraph.util.Log; +import org.slf4j.Logger; + +/** + * Vector sessions for managing vector index operations + */ +public class VectorSessions extends BackendSessionPool { + + private static final Logger LOG = Log.logger(VectorSessions.class); + + private final String dataPath; + private final HugeConfig config; + private final Map<HugeType, VectorTable> tables; + private boolean closed; + + public VectorSessions(String dataPath, HugeConfig config) { + super(config, dataPath); + this.dataPath = dataPath; + this.config = config; + this.tables = new ConcurrentHashMap<>(); + this.closed = false; + } + + + public boolean close() { + this.closed = true; + // Close all tables + for (VectorTable table : this.tables.values()) { + try { + table.close(); + } catch (Exception e) { + LOG.warn("Failed to close vector table: {}", e.getMessage()); + } + } + this.tables.clear(); + return false; + } + + @Override + public void open() throws Exception { + + } + + @Override + protected boolean opened() { + return false; Review Comment: The opened() method always returns false, which contradicts the opened field being set to true in the Session.open() method. This should return the actual state of the session. ```suggestion return !this.closed; ``` ########## hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/serializer/VectorBackendEntry.java: ########## @@ -0,0 +1,197 @@ +/* + * 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.serializer; + +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; + +import org.apache.hugegraph.backend.id.Id; +import org.apache.hugegraph.backend.store.BackendEntry; +import org.apache.hugegraph.type.HugeType; + +/** + * Vector backend entry for handling vector index data + * This entry is specifically designed for vector operations and jvector integration + */ +public class VectorBackendEntry implements BackendEntry { + + // 基础字段（实现BackendEntry接口） + private final HugeType type; // VECTOR_INDEX + private final Id id; // 索引ID + private final Id subId; // 顶点ID + + // 向量核心字段 + private final String vectorId; // 向量唯一标识 + private final float[] vector; // 向量数据 + private final String metricType; // 度量类型 (L2, COSINE, DOT) + private final Integer dimension; // 向量维度 Review Comment: Comments should be in English to maintain consistency with the rest of the codebase. Consider translating these Chinese comments to English. ########## hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/schema/builder/IndexLabelBuilder.java: ########## @@ -526,6 +531,21 @@ private void checkFields(Set<Id> propertyIds) { "Search index can only build on text property, " + "but got %s(%s)", dataType, field); } + + // Vector index must build on float list + if(this.indexType.isVector()){ Review Comment: Missing space after 'if' keyword. Should be 'if (this.indexType.isVector()) {' to follow Java coding conventions. ```suggestion if (this.indexType.isVector()) { ``` ########## hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphIndexTransaction.java: ########## @@ -122,6 +123,15 @@ protected void asyncRemoveIndexLeft(ConditionQuery query, this.params().submitEphemeralJob(job); } + /** + * Get vector serializer for vector index operations + */ + private VectorSerializer getVectorSerializer() { + // Create VectorSerializer using the same pattern as the main serializer + HugeConfig config = this.params().configuration(); + return new VectorSerializer(config); + } Review Comment: Creating a new VectorSerializer instance on every call is inefficient. Consider caching the VectorSerializer instance as a class member to avoid repeated object creation. ########## hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/serializer/VectorSerializer.java: ########## @@ -0,0 +1,306 @@ +/* + * 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.serializer; + +import java.util.List; + +import org.apache.hugegraph.HugeGraph; +import org.apache.hugegraph.backend.id.Id; +import org.apache.hugegraph.backend.query.ConditionQuery; +import org.apache.hugegraph.backend.query.Query; +import org.apache.hugegraph.backend.store.BackendEntry; +import org.apache.hugegraph.config.HugeConfig; +import org.apache.hugegraph.schema.EdgeLabel; +import org.apache.hugegraph.schema.IndexLabel; +import org.apache.hugegraph.schema.PropertyKey; +import org.apache.hugegraph.schema.VertexLabel; +import org.apache.hugegraph.structure.HugeEdge; +import org.apache.hugegraph.structure.HugeEdgeProperty; +import org.apache.hugegraph.structure.HugeIndex; +import org.apache.hugegraph.structure.HugeVertex; +import org.apache.hugegraph.structure.HugeVertexProperty; +import org.apache.hugegraph.type.HugeType; +import org.apache.hugegraph.util.Log; +import org.slf4j.Logger; + +/** + * Vector serializer for handling vector index serialization + * This serializer only handles vector index operations, not general graph data + */ +public class VectorSerializer extends AbstractSerializer { + + private static final Logger LOG = Log.logger(VectorSerializer.class); + + public VectorSerializer() { + super(); + } + Review Comment: The no-argument constructor calls super() unnecessarily since it's implicitly called. Consider removing this constructor if it's not needed or documenting its purpose. ```suggestion ``` ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/graph/VertexAPI.java: ########## @@ -462,4 +541,34 @@ public String toString() { this.label, this.properties); } } + + // ANN search request class + private static class AnnSearchRequest { Review Comment: The AnnSearchRequest class should be public or moved to a separate public class to allow proper API documentation generation and client code usage. ```suggestion public static class AnnSearchRequest { ``` ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/graph/VertexAPI.java: ########## @@ -462,4 +541,34 @@ public String toString() { this.label, this.properties); } } + + // ANN search request class + private static class AnnSearchRequest { + @JsonProperty("vertex_label") + public String vertex_label; + @JsonProperty("properties") + public String properties; + @JsonProperty("user_vector") + public float[] user_vector; + @JsonProperty("metric") + public String metric; + @JsonProperty("dimension") + public Integer dimension; + + private static void checkRequest(AnnSearchRequest req) { + E.checkArgumentNotNull(req, "AnnSearchRequest can't be null"); + E.checkArgumentNotNull(req.vertex_label, "Parameter 'vertex_label' can't be null"); + E.checkArgumentNotNull(req.properties, "Parameter 'properties' can't be null"); + E.checkArgumentNotNull(req.user_vector, "Parameter 'user_vector' can't be null"); + E.checkArgument(req.user_vector.length > 0, "Parameter 'user_vector' can't be empty"); + E.checkArgumentNotNull(req.metric, "Parameter 'metric' can't be null"); + E.checkArgumentNotNull(req.dimension, "Parameter 'dimension' can't be null"); + } + + @Override + public String toString() { + return String.format("AnnSearchRequest{vertex_label=%s, properties=%s, user_vector=%s, metric=%s, dimension=%s}", + vertex_label, properties, Arrays.toString(user_vector), metric, dimension); Review Comment: Missing import for Arrays class. Add 'import java.util.Arrays;' to the imports section. -- 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]
