fix: don't load NDJSON data into memory all at once #344
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mikix
commented
Jan 30, 2025
| def operational_errors(self) -> tuple[Exception]: | ||
| def operational_errors(self) -> tuple[type[Exception], ...]: |
There was a problem hiding this comment.
Unrelated, but I noticed that this method had incorrect typing (I think my fault).
mikix
commented
Jan 30, 2025
|
|
||
| return all_tables | ||
|
|
||
|
|
||
| def _handle_load_ndjson_dir(args: dict[str, str], backend: base.DatabaseBackend) -> None: |
There was a problem hiding this comment.
This method is mostly a refactor, with the only new functionality a progress bar for loading scanning the NDJSON for its schema. (Which in my 26G test folder takes four minutes.)
I was feeling like the --load-ndjson-dir logic was spread around enough if/elses that it was hard to reason about. So moving it all into one method helped, especially now that there is extra code for the progress bar.
This commit changes how --load-ndjson-dir works, telling DuckDB to load the data from files on disk itself, rather than us loading that data into memory all at once, then handing it to DuckDB. Which allows querying larger-than-memory data sets. But... the SQL itself can still be a memory bottleneck, if it requires loading too much data into memory.
mikix
force-pushed
the
mikix/streaming
branch
from
4a44670 to
b7ecea0
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit changes how --load-ndjson-dir works, telling DuckDB to load the data from files on disk itself, rather than us loading that data into memory all at once, then handing it to DuckDB.
Which allows querying larger-than-memory data sets.
But... the SQL itself can still be a memory bottleneck, if it requires loading too much data into memory.
Checklist
docs/needs to be updatedgenerate-mdcorestudy fields that not in US Core, update our list of those incore-study-details.mdmanifest.toml