Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

New virtual index #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

alpe
Copy link
Collaborator

@alpe alpe commented Feb 12, 2020

Add a new index type on top of a natural key table.

Resolves #16 with a different approach

@alpe alpe mentioned this pull request Feb 12, 2020
@alpe alpe force-pushed the alex/virtual_index branch from e61e648 to ed5c03f Compare February 12, 2020 18:11
Copy link
Member

@aaronc aaronc left a comment

Choose a reason for hiding this comment

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

Could you comment on the pros and cons of this as opposed to the approach in #16?

One thing I would note is that I think #16 potentially simplifies key construction because it doesn't require manual concatenation, you just need to return two byte slices.

}

func newIndex(builder Indexable, prefix byte, indexer *Indexer) *MultiKeyIndex {
// NewVirtualIndex provides an index API on top of a natural key table. The use case is
// a combined natural key where the first element has a fixed size.
Copy link
Member

Choose a reason for hiding this comment

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

What would you do if the first element doesn't have a fixed size? Is there a way you could use the max 255 dynamic length codec with this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Without a fixed size we would easily get overlapping results in a prefix scan. We may be able to filter them out with a custom iterator on "simple natural" keys but this is a very special use case.

The one I wanted to cover was a natural key starting with the groupID so that we can have a typed index:
idx := AsUInt64Index(NewVirtualIndex(builder)) without extra costs.

@aaronc aaronc mentioned this pull request Feb 14, 2020
@alpe alpe force-pushed the alex/virtual_index branch from ed5c03f to add699f Compare March 3, 2020 10:25
@alpe alpe changed the base branch from orm_develop to new_master March 3, 2020 10:25
@alpe
Copy link
Collaborator Author

alpe commented Mar 3, 2020

Could you comment on the pros and cons of this as opposed to the approach in #16?

It is a bit hard for me to argue for the one or the other as #16 is not completely clear to me. I can see how it moves the concat one layer down into orm for simple create/get/has operations with all the key parts are available. How would a prefix scan look like? Is there any "special" logic involved in building the RowID from this like adding separater chars, etc?

This Virtual index is based on mostly existing components and logic. In combination with the UInt64Index it can replace a persistent index and save gas on IO. But the concept is probably not very intuitive and the use cases are limited.

@alpe alpe marked this pull request as ready for review March 3, 2020 11:00
@alpe alpe requested a review from aaronc March 3, 2020 16:46
@aaronc
Copy link
Member

aaronc commented Mar 5, 2020

@alpe let's chat about this live next time we get a chance. I want to understand this approach better and think about how it addresses the needs of say that GroupMember table.

@alpe alpe changed the base branch from new_master to master March 24, 2020 06:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants