Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring back the ability to leverage the primary key in secondary indices #13324

Closed
wants to merge 1 commit into from

Conversation

ltamasi
Copy link
Contributor

@ltamasi ltamasi commented Jan 22, 2025

Summary: There are actually some use cases which would benefit from the ability to use the primary key when forming the secondary key prefix or value. One such use case, which is demonstrated using a unit test, is building a secondary index on non-initial part(s) of the primary key. The patch adds back this ability, which was was removed in #13207, with a twist: the earlier GetSecondaryKeyPrefix is essentially split into two parts, with GetSecondaryKeyPrefix now being responsible only for computing whatever the secondary index is built on (let's call this "index function result") and a new FinalizeSecondaryKeyPrefix method having the responsibility of dealing with serialization concerns like adding a length indicator for disambiguation. This also means a slight change for the SecondaryIndexIterator class: it now treats its Seek argument as an "index function result" and thus only calls the new FinalizeSecondaryKeyPrefix on it (but not GetSecondaryKeyPrefix).

Differential Revision: D68514201

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D68514201

…es (facebook#13324)

Summary:

There are actually some use cases which would benefit from the ability to use the primary key when forming the secondary key prefix or value. One such use case, which is demonstrated using a unit test, is building a secondary index on non-initial part(s) of the primary key. The patch adds back this ability, which was was removed in facebook#13207, with a twist: the earlier `GetSecondaryKeyPrefix` is essentially split into two parts, with `GetSecondaryKeyPrefix` now being responsible only for computing whatever the secondary index is built on (let's call this "index function result") and a new `FinalizeSecondaryKeyPrefix` method having the responsibility of dealing with serialization concerns like adding a length indicator for disambiguation. This also means a slight change for the `SecondaryIndexIterator` class: it now treats its `Seek` argument as an "index function result" and thus only calls the new `FinalizeSecondaryKeyPrefix` on it (but not `GetSecondaryKeyPrefix`).

Differential Revision: D68514201
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D68514201

Copy link
Contributor

@jaykorean jaykorean left a comment

Choose a reason for hiding this comment

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

Thanks!

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 35a27d8.

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

Successfully merging this pull request may close these issues.

3 participants