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

Column: Test and document behaviour for auto-incrementing IDs #64

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

webmaster128
Copy link
Member

I wrote this test to better understand how Column keys are supposed to work in case of gaps and removals. Turns out a key is never re-used. This limited the number of elements that can ever be created to the uint32 range, even if elements are consistently removed.

A typical use case for such a column would be logs. For all the things that happen, a new line is added. Some are cleared later on for cleanup purposes. I.e. there can be cases with significantly more row creations than number of elements over the lifetime of the container.

I don't think there is anything wrong with the above behaviour. It's nice and clean if well understood. But I am wondering if hardcoding a limit of 4 billion row creations is enough.

@webmaster128 webmaster128 requested a review from uint September 18, 2024 21:41
Copy link
Collaborator

@uint uint left a comment

Choose a reason for hiding this comment

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

The test looks good!

@uint
Copy link
Collaborator

uint commented Sep 20, 2024

I don't think there is anything wrong with the above behaviour. It's nice and clean if well understood. But I am wondering if hardcoding a limit of 4 billion row creations is enough.

Sure. I'm not very familiar with all the use cases and where the idea of columns came from, so I wasn't sure what I was solving when I implemented it. It's all up for change.

We can make the index generic (with a sane default), or something along those lines?

@webmaster128
Copy link
Member Author

We can make the index generic (with a sane default), or something along those lines?

Yeah, I like. Maybe fix it to u32 in cw-storey which should be a great default.

@webmaster128 webmaster128 merged commit 55f0234 into main Sep 20, 2024
2 checks passed
@webmaster128 webmaster128 deleted the column-remove-test branch September 20, 2024 20:45
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.

2 participants