Skip the COPY header when scanning for row (1.5)#537
Merged
Conversation
This is a backport of the PR duckdb#532 to `v1.5-variegata` stable branch. PostgresBinaryFileReader::FindLastCompleteRow started scanning at byte zero of the first buffer, so it read the 19 byte binary COPY file header as if it were a tuple: 'PG' became a field count of 20551 and 'COPY' became a field length of 1129270361. No row could ever be completed, the scan returned 0, and the reader threw Postgres binary file contains a row larger than the read buffer for any file that did not fit in a single buffer. With the default 32MB buffer that rejected every postgres binary file larger than 32MB, regardless of how small its rows were. Skip the header while scanning for row boundaries on the first fill, but keep it in the buffer so CheckHeader can still validate it. When no complete row fits alongside the header, hand the parser just the header and carry the partial row over to the next fill, where the whole buffer is available for it. Otherwise a row larger than buffer_size - 19 but no larger than buffer_size would still be rejected even though it fits. CheckHeader's bound becomes '>' so that a buffer holding exactly the header is accepted. Files that fit in one buffer were unaffected, which is why the existing tests - all of which use tiny fixtures - did not catch this.
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
1 participant
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 is a backport of the PR #532 to
v1.5-variegatastable branch.PostgresBinaryFileReader::FindLastCompleteRow started scanning at byte zero of the first buffer, so it read the 19 byte binary COPY file header as if it were a tuple: 'PG' became a field count of 20551 and 'COPY' became a field length of 1129270361. No row could ever be completed, the scan returned 0, and the reader threw
Postgres binary file contains a row larger than the read buffer
for any file that did not fit in a single buffer. With the default 32MB buffer that rejected every postgres binary file larger than 32MB, regardless of how small its rows were.
Skip the header while scanning for row boundaries on the first fill, but keep it in the buffer so CheckHeader can still validate it.
When no complete row fits alongside the header, hand the parser just the header and carry the partial row over to the next fill, where the whole buffer is available for it. Otherwise a row larger than buffer_size - 19 but no larger than buffer_size would still be rejected even though it fits. CheckHeader's bound becomes '>' so that a buffer holding exactly the header is accepted.
Files that fit in one buffer were unaffected, which is why the existing tests - all of which use tiny fixtures - did not catch this.