Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;

Expand Down Expand Up @@ -209,6 +210,10 @@ protected BackendColumnIterator queryByIds(RocksDBSessions.Session session,
return this.queryById(session, ids.iterator().next());
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

‼️ 这里把 getByIds() 放到 RocksDBTable 父类会改变非 Vertex/Edge 表的查询语义。父类 queryById() 仍然使用 session.scan(table, id.asBytes()),并且注释里说明 vertex/schema 目前还不能统一改成 point get;但这个分支会让所有未 override queryByIds() 的 RocksDB table 在多 id 查询时改走 exact multi-get。

这对 Vertex / Edge 是合理的,因为它们的 queryById() 已经是 getById();但 schema/index 等表原本依赖 prefix scan,放在父类可能导致多 id 查询查不到本应返回的列。建议父类保留旧的 scan-based 实现,把 multi-get 下沉到 Vertex / Edge 的 override 中。

Suggested change
}
// NOTE: this will lead to lazy create rocksdb iterator
return BackendColumnIterator.wrap(new FlatMapperIterator<>(
ids.iterator(), id -> this.queryById(session, id)
));

然后在 RocksDBTables.Vertex / RocksDBTables.Edge 中分别 override queryByIds(),仅在 !session.hasChanges() 时调用 getByIds()


if (!session.hasChanges()) {
return this.getByIds(session, ids);
}

// NOTE: this will lead to lazy create rocksdb iterator
return BackendColumnIterator.wrap(new FlatMapperIterator<>(
ids.iterator(), id -> this.queryById(session, id)
Expand All @@ -224,13 +229,15 @@ protected BackendColumnIterator getById(RocksDBSessions.Session session, Id id)
return BackendColumnIterator.iterator(col);
}

protected BackendColumnIterator getByIds(RocksDBSessions.Session session, Set<Id> ids) {
protected BackendColumnIterator getByIds(RocksDBSessions.Session session,
Collection<Id> ids) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

‼️ Set<Id>Collection<Id> 语义变化需注意

原方法接收 Set<Id>(天然去重),改为 Collection<Id> 后,传入 List 时若含重复 ID,RocksDB multiGet 会对同一 key 重复查询并返回重复结果。

测试 testVertexQueryByIdsWithDuplicateIds 验证了这个行为(id1 返回 2 次),但这与原 Set 语义不一致。需要确认上层 IdQuery 的 ids 是否可能含重复——如果含重复,行为变更可能导致上层重复处理数据。

建议:在 getByIds 入口去重以保持原语义,或者明确文档说明 Collection 含重复返回的行为变更。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added LinkedHashSet deduplication at the getByIds() entry point while preserving input order; skips if Set is already passed. Test updated to verify dedup behavior. The
backend deduplication does not affect final semantics since GraphTransaction reassembles results by the original input order.

if (ids.size() == 1) {
return this.getById(session, ids.iterator().next());
}

List<byte[]> keys = new ArrayList<>(ids.size());
for (Id id : ids) {
Collection<Id> uniqueIds = ids instanceof Set ? ids : new LinkedHashSet<>(ids);
List<byte[]> keys = new ArrayList<>(uniqueIds.size());
for (Id id : uniqueIds) {
keys.add(id.asBytes());
}
return session.get(this.table(), keys);
Expand Down Expand Up @@ -309,7 +316,7 @@ protected static BackendEntryIterator newEntryIterator(BackendColumnIterator col
}

protected static BackendEntryIterator newEntryIteratorOlap(
BackendColumnIterator cols, Query query, boolean isOlap) {
BackendColumnIterator cols, Query query, boolean isOlap) {
return new BinaryEntryIterator<>(cols, query, (entry, col) -> {
if (entry == null || !entry.belongToMe(col)) {
HugeType type = query.resultType();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.util.Collection;
import java.util.List;

import org.apache.hugegraph.backend.id.Id;
Expand Down Expand Up @@ -178,13 +177,6 @@ public Vertex(String database) {
protected BackendColumnIterator queryById(RocksDBSessions.Session session, Id id) {
return this.getById(session, id);
}

@Override
protected BackendColumnIterator queryByIds(RocksDBSessions.Session session,
Collection<Id> ids) {
// TODO: use getByIds() after batch version multi-get is ready
return super.queryByIds(session, ids);
}
}

public static class Edge extends RocksDBTable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import org.apache.hugegraph.unit.rocksdb.RocksDBCountersTest;
import org.apache.hugegraph.unit.rocksdb.RocksDBSessionTest;
import org.apache.hugegraph.unit.rocksdb.RocksDBSessionsTest;
import org.apache.hugegraph.unit.rocksdb.RocksDBTableQueryByIdsTest;
import org.apache.hugegraph.unit.serializer.BinaryBackendEntryTest;
import org.apache.hugegraph.unit.serializer.BinaryScatterSerializerTest;
import org.apache.hugegraph.unit.serializer.BinarySerializerTest;
Expand Down Expand Up @@ -141,6 +142,7 @@
RocksDBSessionsTest.class,
RocksDBSessionTest.class,
RocksDBCountersTest.class,
RocksDBTableQueryByIdsTest.class,

/* utils */
VersionTest.class,
Expand Down
Loading
Loading