Skip to content

feat: Use single DB query to iterate for index#354

Open
Peeja wants to merge 1 commit intomainfrom
feat/one-db-query-to-iterate-index
Open

feat: Use single DB query to iterate for index#354
Peeja wants to merge 1 commit intomainfrom
feat/one-db-query-to-iterate-index

Conversation

@Peeja
Copy link
Member

@Peeja Peeja commented Feb 23, 2026

PR Dependency Tree

This tree was auto-generated by Charcoal

Copy link
Member

@volmedo volmedo left a comment

Choose a reason for hiding this comment

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

LGTM, just a couple nits if you wanna take a look.

return shards, nil
}

func (r *Repo) ForEachNodeInIndex(ctx context.Context, indexID id.IndexID, yield func(shardDigest multihash.Multihash, nodeCID cid.Cid, nodeSize uint64, shardOffset uint64) error) error {
Copy link
Member

Choose a reason for hiding this comment

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

nit: what do you think of using the idiomatic iterator syntax?

We could create a new NodeInIndex type as:

type NodeInIndex struct {
    Cid                cid.Cid
    Size               uint64
    ShardDigest multihash.Multihash
    ShardOffset uint64

and then have this function be:

func (r *Repo) NodesInIndex(ctx context.Context, indexID id.IndexID) iter.Seq2[NodeInIndex, error]

and then the caller can range over the results and do whatever is needed with them.

}
var rows []row
err = repo.ForEachNodeInIndex(t.Context(), index.ID(), func(shardDigest multihash.Multihash, nodeCID cid.Cid, nodeSize uint64, shardOffset uint64) error {
rows = append(rows, row{shardDigest: shardDigest, nodeCID: nodeCID, nodeSize: nodeSize, shardOffset: shardOffset})
Copy link
Member

@volmedo volmedo Feb 23, 2026

Choose a reason for hiding this comment

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

nit: why not build the map here directly instead of adding to a list and then converting that to a map?

Also, to DRY the assertions afterwards, I'd build a map with the expected results indexed by CID and then do the requires here:

nodeInIndex := expectedNodesByCID[nodeCID.String()]
require.Equal(t, nodeInIndex.ShardDigest, []byte(shardDigest))
require.Equal(t, nodeInIndex.Size, nodeSize)
require.Equal(t, nodeInIndex.ShardOffset, shardOffset)

To ensure we are getting the expected 4 nodes, we can add a simple counter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants