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

Add Insert::written_bytes method #191

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

Conversation

mmcgee-jump
Copy link

Summary

Add a function to retrieve written_bytes from and Insert, so users can create their own inserter logic.


/// Returns the number of serialized bytes that have been written because of
/// write operations.
pub fn written_bytes(&self) -> usize {
Copy link
Collaborator

@loyd loyd Dec 16, 2024

Choose a reason for hiding this comment

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

Add #[inline], please, it's supposed to be called on every row, so it's reasonable to allow over-crate inlining.

Also, please run benchmarks (the insert bench with `--features inserter) to ensure we don't introduce regression here. Run on master and this branch and share results.

@loyd
Copy link
Collaborator

loyd commented Dec 16, 2024

LGTM, thanks for your contribution!

@mmcgee-jump, can you share why the inserter feature is unsuitable for your use case?


/// Returns the number of serialized bytes that have been written because of
/// write operations.
pub fn written_bytes(&self) -> usize {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, it's questionable to use usize vs u64 here.

Formally, it's possible to write more than 4GiB on 32b systems inside one INSERT query.

So, I think we should use u64 here.

@loyd loyd changed the title api: add Insert::witten_bytes function Add Insert::written_bytes method Dec 16, 2024
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