Streaming writer handle, CLI tool, schema inference, and write/Arrow bindings for C++/Rust/Python - #93
Merged
Merged
Conversation
Adds excelreader_cpp_write_compare_benchmarks under the existing EXCELREADER_BUILD_BENCHMARKS_COMPARE flag: all four cases write the same 14-column, 65,535-row shape from the same in-memory rows, so the numbers differ by writer rather than by workload. xlsxio's write side is built as its own static library against the minizip compat shim the read side already set up. Also fixes benchmark_write.cpp's BM_WriteColumns, which wrote 4 columns against BM_WriteSheet's 7. That made the columnar path look ~2x faster when most of the gap was three fewer columns of work; with both at 7 the real difference is a few percent.
The Python README had no benchmark section at all; adds read and write tables from benchmarks/bench_read.py and bench_write.py, with the two DataFrame comparisons (pandas to_excel, polars write_excel) labelled as the only matched-work pairs and the Arrow-plus-pylist conversion the DataFrame helpers pay called out explicitly. The C++ standalone write table is the re-run after BM_WriteColumns was corrected to write all 7 columns.
libxlsxwriter is a pure-C streaming writer (same author as rust_xlsxwriter), the closest competitor class to ExcelReader's own native core - worth measuring next to xlnt and xlsxio. Its CMakeLists.txt is FetchContent-friendly except for one hard dependency: find_package(ZLIB REQUIRED), with nothing to redirect it to. madler/zlib has no Config-mode package and OVERRIDE_FIND_PACKAGE only covers Config mode, so this fetches zlib itself and prepends a small generated FindZLIB.cmake to CMAKE_MODULE_PATH that aliases the target already built - a target alias rather than a hardcoded library path, which is what makes it work under a multi-config generator (Visual Studio) where the real .lib only exists once a config is chosen at build time. Verified end-to-end: configures, builds, links, and runs on Windows/MSVC. One xlsxio run in this session logged an internal zip-creation error without failing the benchmark, so that row in the README is marked provisional pending a clean rerun - not something this change caused or can fix.
Adds BM_DuckDB_Xlsx_Full to the read comparison and BM_DuckDB_Write to
the write comparison, both under the existing
EXCELREADER_BUILD_BENCHMARKS_COMPARE flag.
DuckDB is fetched as a prebuilt library (libduckdb-{windows,osx,linux}-*.
zip from its GitHub Releases) rather than built from source - its
amalgamated source is enormous and would dwarf every other FetchContent
build in this file, and DuckDB already publishes prebuilt binaries per
platform the same way this project's own excelreader-native-*.{dll,so,
dylib} assets are consumed (see cmake/FetchNativeLib.cmake).
Read side runs a single SQL aggregate over read_xlsx() - the idiomatic
way to make a SQL engine touch every cell, not a workaround. Write side
loads rows via DuckDB's Appender API before the timed region, then times
only COPY ... TO ... WITH (FORMAT xlsx), matching the "transpose outside
the loop" treatment BM_ExcelReader_WriteColumns already gets.
Every API call (Connection::Query, MaterializedQueryResult::GetValue,
Appender::AppendRow, date_t's day-count constructor) was checked against
DuckDB's actual headers and test suite before writing this, not
recalled from memory - not yet build-verified end to end.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…th option structs
…ed and infer_schema
…mat tables from one source
…_memory/write_columns
The symbol was already exported from the NativeAOT library via [UnmanagedCallersOnly], but absent from the three hand-maintained .def copies that lib.exe/dlltool turn into the Windows import library, so C++ and Rust could not link against it. Adds a test asserting the three copies stay identical.
Wraps the existing xl_parse_arrow export. Kept in its own header so a caller who does not want the Arrow C Data Interface declarations never includes them, and deliberately free of any Apache Arrow C++ dependency: the C Data Interface is the interop currency, so the caller feeds the pair into whichever Arrow implementation they already link.
…arrow Both native entry points take an identical xl_column_spec array. Pulls the pointer bookkeeping (and the keepalive vectors the specs point into) into one SpecArena so the upcoming Arrow binding does not copy it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Imports the native xl_parse_arrow export through the Arrow C Data Interface and hands back an arrow::array::RecordBatch. Off by default, matching the chrono feature: arrow-rs is a large dependency the typed parse path needs none of. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- DefFileSyncTests: drop the cpp/include/xl symlink arm from the .def comparison (it resolves to the same file as the canonical copy, so it compared a file to itself and would break on Windows CI runners that don't enable core.symlinks). Only two .def copies are physically distinct; the remaining test still catches real drift. - workbook.rs: make SpecArena generic and carry T::bindings() alongside the flat spec array, so build_specs::<T>() computes bindings exactly once instead of parse_sheet<T> calling T::bindings() a second time. arrow.rs is unaffected and compiles unmodified. - Add C++ coverage for parse_arrow failing cleanly on an out-of-range header_row (cpp/tests/arrow.cpp), and strengthen the matching Rust test to prove the workbook is still usable after the failed call.
A thin shell over ExcelReader.Core's public API - nothing here parses a spreadsheet byte. ConsoleAppFramework generates the parsing, routing and --help (from XML doc comments, so help cannot drift from the signature); it is a source generator referenced with PrivateAssets, so nothing ships at runtime (verified via dotnet publish - only ExcelReader.Cli/.Core land in the output). The command bodies live in CliCommands as plain functions over explicit writers, keeping the tested surface clear of the framework's static output hooks. sheets lists a workbook's sheets; convert streams a sheet out as CSV (to a file or stdout, via the existing CsvWorkbookWriter); schema prints the inferred column schema via Excel.InferSchema. All three share Open() (sheet selection by index or name) and Execute() (exit 0/1, stderr on expected failures). Deviations from the plan text, forced by the concrete framework version (ConsoleAppFramework 5.7.13): - delimiter is 'char? = null' resolved to ',' in Commands.Convert, not 'char = \',\'' - the generator's own codegen mis-emits a char default literal that is itself a comma (var arg3 = (char),; - a real generator parsing bug), so the default lives in the adapter body instead. - Commands' methods stay instance methods, not static: ConsoleAppFramework's app.Add<T>() rejects a class with only static commands (CAF012). CA1822 and the CsvSheetWriter IDISP001 (End() is the real teardown, matching CsvWorkbookWriterTests.cs's own suppression) are scoped-NoWarn with comments in the csproj, not code changes. - All three commands landed in one pass instead of three separate tasks - mechanical, same files, no reason to split the commits by command. ExcelReader.Tests.csproj: the Cli ProjectReference and CliTests.cs are net10.0-only (Condition on TargetFramework / Compile Remove) - the CLI project is single-target and a dotnet tool has no net8.0 consumer. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Audited the CLI and native layer for bugs/vulnerabilities/simplifications
(agent-verified: parser memory safety, DoS, XXE all clean; three concrete
issues confirmed and fixed below).
CLI (src/ExcelReader.Cli/CliCommands.cs):
- convert wrote Number/Boolean cells through Write(string?) instead of the
typed overloads, so every xlsx/xlsb/xls conversion target lost its
numeric/boolean typing (Int64Column/Float64Column round-tripped back as
StringColumn). Boolean cells store a raw '0'/'1' byte across every
format (CellAccumulator.AddBool), not "True"/"False", so the fix reads
that byte directly rather than through bool.TryParse.
- ResolveFormat: an --output with no extension reported "unrecognized
extension '.'" (Path.GetExtension("noext") == ""), reading as if the
user typed a bare dot. Now names the actual problem.
- convert now rejects an --output that names an existing directory with
a clear message, instead of a directory-shaped path failing FileStream
with a misleading "Access to the path is denied".
Core writer (XlsRowWriter.cs, XlsbRowWriter.cs):
- Write(int?)/Write(long?)/Write(double?)/Write(decimal?)'s null branch
advanced _columnIndex directly instead of through Skip(1), bypassing
the column-count bounds check every other nullable overload in the
same class already goes through (Skip's own remarks describe fixing
this exact bug - just missed on these four numeric overloads).
Native FFI (Exports.cs, NativeApi.Open/Typed/Arrow/Schema.cs):
- xl_close had no try/catch, unlike every other NativeApi entry point -
an IOException from FileStream.Dispose (e.g. source volume gone) would
unwind straight through the UnmanagedCallersOnly frame instead of
returning XL_ERROR.
- xl_free_row/xl_free_rows/xl_free_table/xl_free_schema and both Arrow
release callbacks are void in the ABI with no status code to report
through; an exception escaping any of them is a fail-fast abort for
the native caller, uncatchable in C/C++/Rust/Python. All six now catch
and log to xl_last_error instead of propagating.
- FreeSchema dereferenced spec.Names without checking it for null first.
- BuildTable/BuildArrowSchema/BuildArrowArray leaked every already-built
column/child when a later one threw (OOM is the realistic trigger -
xl_parse_arrow briefly holds both the intermediate table and its Arrow
copy at once). Each now releases what it already built before rethrowing.
Also strengthened three CliTests.cs assertions that would have passed on
wrong data (sheets listing checked only the first line; the delimiter
test checked "contains one ';'" rather than every field; the format
round-trip test checked only the first row's column count, not row
count or cell values).
WriterStateGuard.ValidateSheetName: internal -> private, one caller in
the same file.
Build clean, 1141/1141 tests (net10.0), 1112/1112 (net8.0).
Benchmark ResultsMeasured on ExcelReader.Benchmarks.ColdStartBenchmark
ExcelReader.Benchmarks.CsvParseBenchmark
ExcelReader.Benchmarks.CsvReadBenchmark
ExcelReader.Benchmarks.CsvWriteBenchmark
ExcelReader.Benchmarks.ParseBenchmark
ExcelReader.Benchmarks.ReadBenchmark
ExcelReader.Benchmarks.RealDataReadBenchmark
ExcelReader.Benchmarks.RecordWriteBenchmark
ExcelReader.Benchmarks.WriteBenchmark
ExcelReader.Benchmarks.XlsReadBenchmark
ExcelReader.Benchmarks.XlsWriteBenchmark
|
added 2 commits
August 25, 2026 22:05
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #93 +/- ##
==========================================
- Coverage 86.56% 85.17% -1.40%
==========================================
Files 129 136 +7
Lines 9533 10041 +508
Branches 1789 1868 +79
==========================================
+ Hits 8252 8552 +300
- Misses 958 1159 +201
- Partials 323 330 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
2 participants
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.
Summary
Major feature batch since v2.1.3: a native streaming writer handle, a new
excelreaderdotnet CLI tool, schema inference, and write/Arrow support extended to the C++, Rust, and
Python bindings (previously read-only there).
Core (C#)
Excel.InferSchema+ExcelColumnSchema/ExcelColumnType— samples a sheet and returns ausable
ColumnSpec[], directly consumable byParseTyped. Currently unshipped API.convert, a column-count-limit bypass, and FFI handle leaks.Native ABI (C ABI,
ExcelReader.Native)xl_open_write_handle[_to_memory],xl_start_sheet/xl_start_row,scalar
xl_write_string/xl_write_int64/xl_write_float64/xl_write_bool/xl_write_date/xl_write_time/xl_write_timestamp/xl_write_null,xl_end_row/xl_end_sheet,xl_close_write_handle,xl_write_handle_bytes. Lets a caller build a workbook row-by-rowinstead of materializing a full columnar table first.
xl_write_typed_to_memory— in-memory typed write, mirroring the existing file-based path.xl_next_row_decoded,xl_free_row) —XL_ABI_VERSIONbumped 2 → 3; the equivalent per-row decode logicstays internal, backing
xl_read_all_decoded.CLI (new:
ExcelReader.Cli, packaged asExcelReader.NET.Cli)excelreader sheets|schema|convert, with an interactive Spectre.Console table/spinner on areal terminal and identical plain tab-separated output when piped/redirected.
release.ymlnow actually packs and pushesExcelReader.NET.Clito NuGet — previously onlyself-contained AOT binaries were attached to GitHub Releases, so the README's documented
dotnet tool install --global ExcelReader.NET.Cliwould have failed.C++ / Rust / Python bindings
xl::write_columns,xl::write_sheet<T>,xl::WriterHandle(RAII over the newstreaming ABI),
xl::parse_arrow/ArrowTable,nullable<T>field support.ExcelWriter+#[derive(ExcelMapper)]-generated writer,writer_handlemodule,parse_arrowbehind thearrowfeature, write benchmarks againstrust_xlsxwriter.bindings intentionally not added — ctypes' per-call FFI cost doesn't fit a row-at-a-time
API, and the existing columnar
write_workbook/write_pandas/write_polarsalready coverthe realistic Python write path.
Breaking changes
XL_ABI_VERSION2 → 3. Any direct C ABI consumer that doesn't go through the shippedbindings should re-check
xl_abi_version()before upgrading.PublicAPI.Shipped.txt— the new managed API (InferSchema,ExcelColumnSchema,ExcelColumnType) is still tracked as Unshipped and will promote onthe next tag.
Test plan
dotnet build ExcelReader.slnx(net8.0 + net10.0) — 0 warningsdotnet test tests/ExcelReader.Tests— full suite passingcargo test/cargo bench --no-run(rust/excelreader, rust/excelreader-derive)pytest python/testsdotnet pack src/ExcelReader.Cliverified locally — produces a valid dotnet-toolpackage (
DotnetToolSettings.xml+ Core/Cli/Spectre.Console DLLs undertools/)cpp/tests— all passing