Repository navigation
fix(storage): pad uncompressed flush buffer to allocated page range on disk - #1091
Open
Tyagiquamar wants to merge 2 commits into
Open
Tyagiquamar wants to merge 2 commits into
Tyagiquamar wants to merge 2 commits into
Conversation
adsharma
self-requested a review
October 4, 2026 17:14
adsharma
requested changes
Oct 4, 2026
adsharma
left a comment
Contributor
There was a problem hiding this comment.
Unused vars + clang-format + test coverage for ALP
Contributor
Author
|
Addressed maintainer feedback: removed unused startOffset, formatted touched files with clang-format, and added RelCopyMultiPageFloatUncheckpointedScan unit test coverage verifying ALP floating-point uncheckpointed scans. |
This branch has not been deployed
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
When batch-loading rel tables via
COPYwithout intermediate checkpoints, subsequent scans of flushed CSR node groups fail with a read-past-EOF assertion inLocalFileSystem::readFromFile(localFileInfo->getFileSize() >= position + numBytes).Root cause
ColumnChunkData::flushandColumn::flushDataallocate page ranges usingPageAllocator::allocatePageRange(preScanMetadata.getNumPages()). When flushing uncompressed chunks or ALP floating-point data,uncompressedFlushBufferpreviously wrote only the raw unpadded buffer size (buffer.size()) to the data file. For multi-page allocations where the buffer content did not align to whole page multiples, trailing pages within the allocated range were not written on disk, leaving the underlying physical file shorter than the logical page index space. Subsequent scans reading subsequent pages atpageIdx * LBUG_PAGE_SIZEencountered a truncated file size.Fix
uncompressedFlushBuffer, when in persistent mode (!dataFH->isInMemoryMode()) andbuffer.size_bytes() < entry.numPages * LBUG_PAGE_SIZE, pad the remaining allocated page range with zero bytes on disk.flushCompressedFloats, zero-pad any remaining data pages before the ALP exception pages when fewer data pages than allocated are used.RelCopyMultiPageUncheckpointedScanregression test intest/copy/copy_test.cppto verify that batch COPY-loaded rels spanning multiple pages can be queried without an intermediate checkpoint.Fixes #1059