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.

Sign up for GitHub

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

Jump to bottom

Record parsing is too low level #881

Open
penberg opened this issue Feb 4, 2025 · 1 comment
Open

Record parsing is too low level #881

penberg opened this issue Feb 4, 2025 · 1 comment
Labels
enhancement New feature or request performance
Milestone
0.1.0

Comments

@penberg
Copy link
Collaborator

penberg commented Feb 4, 2025

We parse records in read_record() which means we parse every row in full even if we never use them. Let's make record parsing more lazy to avoid work.
@penberg penberg added enhancement New feature or request performance labels Feb 4, 2025
@penberg penberg added this to the 0.1.0 milestone Feb 4, 2025
@tribeiro
Copy link
Contributor

tribeiro commented Feb 7, 2025

I am really curious about this feature.

I was looking at how this is handled and it is not clear to me how you would implement lazy parsing without some major changes to Record.

The first change I see is converting values to private and adding methods to retrieve values to Record. Once this is done, and all the code is refactored to account for this change, I think read_record could be made to parse the header and store a lazy representation of values, that the retrieval methods could be modified to use.

Is this anywhere remotely close to what you had in mind?

@tribeiro tribeiro mentioned this issue Feb 10, 2025
Merged
penberg added a commit that referenced this issue Feb 10, 2025
@penberg
Merge 'Make record values private' from Tiago
604ca40 
This is an attempt to move towards #881. I am not sure this is the
direction you want to take. In any case, I thought I would take a crack
at converting `values` from `Record` to private and see how bad it would
be.
In the end, as you can see, it is not so bad. I think performance-wise
it shouldn't be a bad hit with Rust's zero-cost abstraction. Also,
during the process I noticed a couple improvements that could be made
here and there but I honestly wanted to start with something small
enough that wouldn't be too hard to review.
Anyway, let me know if this is really how you would like to proceed.

Closes #962
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request performance
Projects
None yet
Milestone
0.1.0
Development

No branches or pull requests
2 participants
@penberg @tribeiro