Repository navigation
Buffer read-only zip streams: one HTTP request per block, not per minizip read - #115
Open
scottynomad wants to merge 1 commit into
Open
scottynomad wants to merge 1 commit into
scottynomad wants to merge 1 commit into
Conversation
…izip read minizip reads a zip in small pieces (local headers, central directory entries, ~32 KiB inflate inputs), and mz_stream_duckdb_read passed each one straight to FileHandle::Read. On DuckDB 2.0's httpfs every such read is its own ranged GET: read_xlsx of a 40 KB remote workbook made 2,535 GETs (21-24s vs 0.08s on 1.5.6), and larger workbooks on a cloud bucket didn't finish within minutes (duckdb#114). A stream opened read-only on a seekable handle now keeps its own position and file size, serves reads from a 1 MiB block that it refills with one positional Read at the current position, and reads 1 MiB or more in place. Seek and tell touch no I/O. Write, append and create modes keep the existing unbuffered path. On 2.0.0 + httpfs 77392a7207: - the 40 KB public file takes 9 requests, 0.23s; - 0.6 / 2.7 / 8.2 MB workbooks on GCS take 8 / 16 / 22 requests and 0.5 / 1.2 / 1.5s; - output is identical (md5 over all rows) to local reads. Adds test/sql/excel/xlsx/read_xlsx_remote_requests.test (requires httpfs): 2999 rows in fewer than 50 HTTP requests. Fixes duckdb#114 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Fixes #114.
Problem
minizip reads a zip in small pieces: local headers, central directory entries, and ~32 KiB inflate inputs.
mz_stream_duckdb_readpassed each one straight toFileHandle::Read. On DuckDB 2.0's httpfs, every such read is its own ranged GET (some are 2 bytes).read_xlsxof a 40 KB remote workbook made 2,535 GETs, and larger workbooks on a cloud bucket didn't finish within minutes.read_csvandread_parquetaren't affected.Change (
src/excel/xlsx/zip_file.cpp)A stream opened read-only on a seekable handle (
CanSeek()) now:Readat the current position;seekandtellwithout I/O.Write, append and create modes keep the existing unbuffered path.
Results
On DuckDB 2.0.0 (
d54cbbe3df) with httpfs77392a7207, extension built from this branch:test/data/xlsx/2x3000.xlsx)Output is identical, by md5 over all rows, to reading the same files from local disk with both the unpatched 2.0.0 extension and 1.5.6.
Tests
make release+test/unittest "test/*": all tests passed (483 assertions in 34 test cases).test/sql/excel/xlsx/read_xlsx_remote_requests.test(requires httpfs): reads the public 2,999-row workbook and asserts fewer than 50 HTTP requests viaduckdb_logs_parsed('HTTP'). Run through the Python package with httpfs, it passes on this build (9 requests) and fails on the unpatched one (2,536).clang-format18 reports no changes in the new code. (The repo pins 11.0.1, which has no arm64 build.)🤖 Generated with Claude Code