Fix issue #50: SASLib.open hangs for large files - #84
Merged
Conversation
Two bugs caused open() to scan the entire file when reading metadata from large SAS7BDAT files (e.g. 2 GB, 4000+ columns): 1. read_file_metadata ignored the return value of _process_page_meta, so the metadata loop never stopped at the first data page. Fixed by breaking out of the loop when _process_page_meta returns true. 2. _process_columnattributes_subheader used empty!+append! (replace semantics) instead of accumulate semantics. For wide files whose column attributes span multiple metadata pages, only the last page's attributes survived while column_symbols accumulated everything, causing a BoundsError in populate_column_indices. Fixed by switching to append!-only with a guard (using Base.length to avoid shadowing by the length parameter) that skips re-processing once all column attributes are loaded. Also adds test/test_large_sas.jl, a manual regression test against the Census AHS 2013 public-use file (~2 GB). Includes download instructions and is intentionally excluded from the standard Pkg.test() suite.
This file contains hidden or 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
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.
Problem
SASLib.openandSASLib.readboth fail on large SAS7BDAT files (e.g. a 2 GB file with 4,000+ columns). Fixes #50.Confirmed behaviour on
masteragainst the Census AHS 2013 public-use file (2.0 GB, 70,044 rows x 4,041 columns):open()completes but is ~2x slower than necessary (1.476s vs 0.785s)read()crashes withBoundsError: attempt to access 1355-element Vector{UInt8} at index [1356]inpopulate_column_indicesRoot cause
Two bugs in the metadata-reading path:
1.
read_file_metadataignored_process_page_meta's return value_process_page_metareturnstruewhen it has finished reading metadata (i.e. it has reached the first data page). The loop was discarding that signal and continuing to scan every page in the file — for a wide file this means reading through thousands of data pages unnecessarily.2.
_process_columnattributes_subheaderused replace semantics instead of accumulateThe function called
empty!beforeappend!, so for files whose column attributes span multiple metadata pages (wide files with 4,000+ columns), only the last page's chunk of attributes survived. Meanwhilecolumn_symbolsaccumulated all column names across pages, producing a length mismatch that caused theBoundsErrorinpopulate_column_indices.A secondary issue: the guard condition used
length(handler.column_types)wherelengthis also a parameter name, causing aMethodError: objects of type Int64 are not callable. Fixed by qualifying asBase.length.Fix
read_file_metadata: break out of the loop when_process_page_metareturnstrue_process_columnattributes_subheader: switch toappend!-only (accumulate across pages); add aBase.lengthguard to skip re-processing once all column attributes are loadedTesting
open()read(1000)read()full fileA new manual regression script
test/test_large_sas.jlis included with download instructions for the test file. It is intentionally excluded from the standardPkg.test()suite since the file must be downloaded separately.