diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ba27c934..9e16ff7b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -135,7 +135,22 @@ jobs: token: ${{ secrets.CODECOV_TOKEN }} files: ./TestResults/*.cobertura.xml flags: csharp - fail_ci_if_error: false + fail_ci_if_error: true + + # The codecov/project checks are required on master, and Codecov posts them only for a commit it + # has heard about; when nothing here ran, an empty upload passes them on carried-forward coverage. + - name: Checkout for empty Codecov upload + if: env.RUN != 'true' && matrix.os == 'ubuntu-latest' + uses: actions/checkout@v7 + + - name: Empty Codecov upload + if: env.RUN != 'true' && matrix.os == 'ubuntu-latest' + uses: codecov/codecov-action@v7 + with: + token: ${{ secrets.CODECOV_TOKEN }} + run_command: empty-upload + force: true + fail_ci_if_error: true isa-fallback: name: Test ISA fallback (${{ matrix.isa }}) diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index d0dd6254..c17d96a5 100644 --- a/.github/workflows/fuzz.yml +++ b/.github/workflows/fuzz.yml @@ -53,7 +53,7 @@ jobs: strategy: fail-fast: false matrix: - target: [xlsx, xlsx-memory, xlsx-differential, xlsx-pipeline, xlsb, xlsb-memory, xlsb-differential, xlsb-pipeline, xls, encrypted, csv, csv-sniff, csv-parallel, inflate] + target: [xlsx, xlsx-memory, xlsx-differential, xlsx-pipeline, xlsb, xlsb-memory, xlsb-differential, xlsb-pipeline, xls, encrypted, csv, csv-sniff, csv-parallel, inflate, deflate, deflate-segments] steps: - name: Checkout uses: actions/checkout@v7 @@ -111,11 +111,12 @@ jobs: encrypted) dict=encrypted ;; csv*) dict=csv ;; inflate) dict=inflate ;; + deflate*) dict= ;; esac ./libfuzzer-dotnet \ --target_path=fuzz-out/ExcelReader.Fuzz \ corpus/${{ matrix.target }} \ - -dict="tests/ExcelReader.Fuzz/dictionaries/$dict.dict" \ + ${dict:+-dict="tests/ExcelReader.Fuzz/dictionaries/$dict.dict"} \ -max_total_time=${{ github.event.inputs.seconds || '600' }} \ -max_len=131072 \ -timeout=25 \ diff --git a/.github/workflows/native-bindings.yml b/.github/workflows/native-bindings.yml index 7cdea891..1052889f 100644 --- a/.github/workflows/native-bindings.yml +++ b/.github/workflows/native-bindings.yml @@ -351,3 +351,24 @@ jobs: - name: Run the consumer run: ctest --test-dir build/consumer --output-on-failure -C Release + + - name: Test with coverage and sanitizers (Linux) + if: runner.os == 'Linux' + env: + EXCELREADER_NATIVE_LIB: ${{ steps.lib.outputs.path }} + ASAN_OPTIONS: detect_leaks=0 + UBSAN_OPTIONS: print_stacktrace=1 + run: | + cmake -S cpp -B build/cpp-coverage -DEXCELREADER_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Debug "-DCMAKE_CXX_FLAGS=--coverage -fprofile-update=atomic -fsanitize=address,undefined -fno-sanitize-recover=all -fno-omit-frame-pointer" + cmake --build build/cpp-coverage + ctest --test-dir build/cpp-coverage --output-on-failure + pipx run gcovr --root . --filter 'src/ExcelReader.Native/include/' --exclude-throw-branches --merge-lines --cobertura coverage-cpp.xml --txt + + - name: Upload C++ coverage to Codecov + if: runner.os == 'Linux' + uses: codecov/codecov-action@v7 + with: + token: ${{ secrets.CODECOV_TOKEN }} + files: ./coverage-cpp.xml + flags: cpp + fail_ci_if_error: false diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 09653c1f..bacdfde0 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -175,15 +175,25 @@ the seam. A wrong start also shifts every field boundary, so on a source with qu discarded pass can hand the callback values that appear nowhere in the file. For an accumulator both are invisible: the discarded instance is thrown away. -A guess is only wrong when the offset falls inside a quoted field. `CsvChunkWorker.GuessStart` first -reads the byte before the chunk start and returns the chunk start unchanged when it already sits on a -record boundary, because `CsvBoundaryResolver.FindRecordStart` only ever scans past a terminator and +A guess can only be wrong when the offset falls inside a quoted field, so `CsvChunkWorker.GuessStart` +first decides whether it does. `CsvBoundaryResolver.InferParity` reads the runs of quotes in a window +of up to 64 KB from the chunk start, reaching back over a run the start cuts and looking past the +chunk's end when it has to. Under RFC 4180 an odd run that opens a field follows a delimiter or a line +start, and one that closes a field precedes a delimiter, a line end or the end of the source, so the +first run that fits only one of the two decides. When a run fits both, the worker does not guess at +all: the chunk is left to the merge, which reads it from its confirmed start. + +When the chunk starts outside quotes and the byte before it is a terminator, `GuessStart` returns the +chunk start unchanged, because `CsvBoundaryResolver.FindRecordStart` only ever scans past a terminator and would otherwise overshoot by a whole record on every chunk that happens to be aligned — which, at a -1 MiB minimum chunk size, is every chunk of a fixed-width file. That leaves quote-free sources exact, -and the repeat documented as a caller obligation for the rest rather than designed out. Removing it -there too needs a quote-parity prefix pass over the bytes before any parsing starts; that pass is a -barrier and would cost roughly half the dop-16 runtime unless pipelined, which is more machinery than -the hazard currently warrants. +1 MiB minimum chunk size, is every chunk of a fixed-width file. + +`ForEachAsync` is therefore exactly once on a CSV that follows RFC 4180 quoting with no quoted field +longer than 64 KB. A stray quote inside an unquoted field, text after a closing quote or a longer quoted +field can still be misread, and those sources keep the repeat as a documented caller obligation. +Removing it there too would need a quote-parity prefix pass over the bytes before any parsing starts. +That pass is a barrier and would cost roughly half the dop-16 runtime unless pipelined, which is more +machinery than sources that break RFC 4180 warrant. ## Shared plumbing diff --git a/Directory.Build.props b/Directory.Build.props index 0e6a30ac..94839ae4 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -27,7 +27,7 @@ runtime; build; native; contentfiles; analyzers; buildtransitive all - + runtime; build; native; contentfiles; analyzers; buildtransitive all diff --git a/README.md b/README.md index cb3bd6e6..313ac3de 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,12 @@ High-performance Excel reading and writing for .NET 10. Reads `.xlsx`, `.xlsb`, ExcelReader is built for streaming spreadsheet workloads where low allocations matter. It reads worksheet rows as lightweight `ref struct` values, resolves shared strings, recognizes date styles, and handles sparse cells. Workbooks open from a path, a `Stream`, or a caller-owned `ReadOnlyMemory` — the in-memory path decompresses parts directly, without a `ZipArchive` or an intermediate stream. +It moves data, not presentation. Reading gives each cell's value and type, with a formula cell read as +its last calculated result, plus sheet names, visibility and Excel tables. Formula text, merged cells, +comments, hyperlinks, charts, images and conditional formatting are not read. Writing covers values, +column widths and cell styles limited to bold, italic and a number format. To edit an existing workbook +or produce formatted reports, use a library built for document manipulation. + ## Install ```bash @@ -45,7 +51,7 @@ foreach (Row row in workbook.FirstSheet) - [Reading untrusted files](docs/guide/untrusted-input.md) — the resource limits that bound what one hostile file can cost, and how to reject one cleanly. - [Migrating to v7](docs/guide/migrating-to-v7.md) — porting `CsvParallel` code to `SheetParallel`, which now reads XLSX and XLSB on several threads too. - [Migrating to v6](docs/guide/migrating-to-v6.md) — porting v5 reader code to workbooks and sheets, and reading sheets in parallel. -- [Benchmarks](docs/performance/benchmarks.md) — throughput and allocation against Sep, Sylvan, OfficeIMO and SpreadCheetah. [Live results](https://gabrielmarquezmatte.github.io/ExcelReader/dev/bench/). +- [Benchmarks](docs/performance/benchmarks.md) — throughput and allocation against Sep, Sylvan, ExcelDataReader, OfficeIMO and SpreadCheetah. [Live results](https://gabrielmarquezmatte.github.io/ExcelReader/dev/bench/). ## Command line diff --git a/codecov.yml b/codecov.yml index 203f0652..acbd864c 100644 --- a/codecov.yml +++ b/codecov.yml @@ -1,3 +1,30 @@ +coverage: + status: + project: + csharp: + target: 88% + flags: + - csharp + python: + target: 88% + flags: + - python + rust: + target: 88% + flags: + - rust + cpp: + target: 88% + flags: + - cpp + patch: + default: + target: 88% + +# gcov reports the C++ headers through the cpp/include/xl symlink; git tracks them at their real path. +fixes: + - "cpp/include/xl/::src/ExcelReader.Native/include/" + # Each language uploads under its own flag from a path-filtered job, so a flag a commit did not run keeps its last report. flag_management: default_rules: @@ -12,3 +39,6 @@ flag_management: - name: python paths: - python/ + - name: cpp + paths: + - src/ExcelReader.Native/include/ diff --git a/cpp/README.md b/cpp/README.md index a9c7ecc6..33aa3951 100644 --- a/cpp/README.md +++ b/cpp/README.md @@ -106,8 +106,23 @@ for (xl::Sheet sheet : *workbook->sheets()) { std::println("{}", sheet.name().va for (const auto& column : *workbook->sheet(1).infer_schema(1, 100)) { // column.name is nullopt when the column must be resolved by column.index instead. } + +// With no row type, parse the sheet through that schema. The columns are the xl::ColumnRef +// write_columns takes, so the table writes back as it was read. +auto schema = workbook->sheet(1).infer_schema(); +auto table = xl::parse(workbook->sheet(1), *schema); +for (const xl::ColumnRef& column : table->columns()) { /* column.type, column.values, ... */ } +auto written = xl::write_columns("copy.xlsx", table->columns()); + +// Or a batch at a time: an xl::TableReader, whose batches are xl::Tables. +auto reader = xl::parse_batches(workbook->sheet(1), *schema, {.batch_size = 10'000}); +for (auto& batch : *reader) { if (!batch) { break; } /* batch->columns() */ } ``` +A column matched by index has no name, and writing needs every column named or none. The schema is +a guess over a sample: a column inferred as `XL_T_I64` that holds a fraction further down fails the +read, unless the column is `nullable`. + Every entry point returns `std::expected` — this header throws nothing. A workbook hands out sheets. An `xl::Sheet` is the workbook handle and an index: it owns nothing, @@ -299,6 +314,18 @@ while (true) Same `header_row`/`batch_size` meaning as `xl::parse_batches` above. +With no row type, both take the columns at run time instead, such as the ones `infer_schema` +returned. A column with a name is matched by it, one without by its index: + +```cpp +auto schema = workbook->sheet(0).infer_schema(); +auto table = xl::to_arrow(workbook->sheet(0), *schema); +auto stream = xl::to_arrow_stream(workbook->sheet(0), *schema, {.batch_size = 10'000}); +``` + +The schema is a guess over a sample: a column inferred as `XL_T_I64` that holds a fraction further +down fails the read, unless the column is `nullable`. + ## Writing Two layers, mirroring the two on the reading side. diff --git a/cpp/tests/aggregate.cpp b/cpp/tests/aggregate.cpp index 366da363..76a1e9e9 100644 --- a/cpp/tests/aggregate.cpp +++ b/cpp/tests/aggregate.cpp @@ -473,6 +473,34 @@ static int test_sheet_aggregate_reports_a_throwing_accumulate() return 0; } +static int test_sheet_aggregate_fails_on_a_sheet_past_the_end() +{ + const auto bytes = generate(XL_FORMAT_XLSX, 10); + auto workbook = open_generated(bytes, XL_FORMAT_XLSX); + CHECK(workbook.has_value(), "open the generated workbook"); + auto result = workbook->sheet(99).aggregate(RowCounter{}, 1, 1); + CHECK(!result.has_value() && result.error().code < 0, "a sheet past the end fails with the library's status"); + return 0; +} + +static int test_aggregate_reports_a_seed_copy_that_throws() +{ + struct ThrowsOnCopy + { + ThrowsOnCopy() = default; + ThrowsOnCopy(const ThrowsOnCopy &) { throw std::runtime_error("no copies"); } + ThrowsOnCopy(ThrowsOnCopy &&) noexcept = default; + int32_t accumulate(xl::RowView) noexcept { return XL_OK; } + int32_t combine(ThrowsOnCopy &) noexcept { return XL_OK; } + }; + + std::span data(reinterpret_cast(kCsv.data()), kCsv.size()); + auto result = xl::aggregate_csv(data, ThrowsOnCopy{}); + CHECK(!result.has_value() && std::string_view(result.error().message).find("threw") != std::string_view::npos, + "a seed copy that throws must be reported as a callback that threw"); + return 0; +} + static int test_existing_csv_accumulator_works_unchanged() { const auto bytes = generate(XL_FORMAT_CSV, 10'000); @@ -508,6 +536,14 @@ int main() { return failed; } + if (int failed = test_sheet_aggregate_fails_on_a_sheet_past_the_end()) + { + return failed; + } + if (int failed = test_aggregate_reports_a_seed_copy_that_throws()) + { + return failed; + } if (int failed = test_aggregate_csv_memory()) { return failed; diff --git a/cpp/tests/arrow.cpp b/cpp/tests/arrow.cpp index 73639841..58c10e26 100644 --- a/cpp/tests/arrow.cpp +++ b/cpp/tests/arrow.cpp @@ -93,6 +93,20 @@ int main() auto second = xl::to_arrow_stream(stream_workbook->sheet(0), {.batch_size = 8}); CHECK(second.has_value(), "a second stream on one workbook opens"); + + xl::ArrowStream moved = std::move(*second); + CHECK(!second->schema().has_value(), "a moved-from stream has no schema"); + CHECK(!second->next().has_value(), "a moved-from stream reads nothing"); + CHECK(moved.schema().has_value(), "the moved-to stream still reports its schema"); + + const xl::InferredColumn columns[] = {{"Coluna3", 0, XL_T_I64, false}}; + CHECK(!xl::to_arrow_stream(stream_workbook->sheet(99)).has_value(), + "a stream over a sheet past the end fails to open"); + CHECK(!xl::to_arrow_stream(stream_workbook->sheet(99), columns).has_value(), + "a runtime-schema stream over a sheet past the end fails to open"); + auto zero_batch = xl::to_arrow_stream(stream_workbook->sheet(0), columns, {.batch_size = 0}); + CHECK(!zero_batch.has_value() && zero_batch.error().code == XL_INVALID_ARGUMENT, + "a runtime-schema stream rejects a batch size below 1"); } { @@ -199,6 +213,43 @@ int main() CHECK(reader.has_value(), "a typed reader opens while an Arrow stream is live"); } + { + auto wb = xl::Workbook::open(EXCELREADER_FIXTURE_PATH, options); + CHECK(wb.has_value(), "opening a workbook for the inferred-schema test must succeed"); + auto schema = wb->sheet(0).infer_schema(); + CHECK(schema.has_value() && !schema->empty(), "infer_schema must find columns"); + + auto inferred = xl::to_arrow(wb->sheet(0), *schema); + CHECK(inferred.has_value(), "to_arrow over an inferred schema must succeed"); + CHECK(inferred->schema.n_children == static_cast(schema->size()), "one Arrow column per inferred column"); + CHECK(inferred->array.length == 100, "the inferred read holds every data row"); + CHECK(std::strcmp(inferred->schema.children[0]->name, "Coluna1") == 0, "columns keep their header names"); + + auto by_index = wb->sheet(0).infer_schema(0); + CHECK(by_index.has_value() && !(*by_index)[0].name.has_value(), "a schema without a header has no names"); + auto unnamed = xl::to_arrow(wb->sheet(0), *by_index, {.header_row = 0}); + CHECK(unnamed.has_value(), "to_arrow resolves unnamed columns by index"); + CHECK(unnamed->array.length == 101, "with no header the header row is data"); + + auto empty = xl::to_arrow(wb->sheet(0), std::span{}); + CHECK(!empty.has_value() && empty.error().code == XL_INVALID_ARGUMENT, "an empty schema is rejected"); + + auto stream = xl::to_arrow_stream(wb->sheet(0), *schema, {.batch_size = 8}); + CHECK(stream.has_value(), "to_arrow_stream over an inferred schema must open"); + int64_t rows = 0; + while (true) + { + auto batch = stream->next(); + CHECK(batch.has_value(), "an inferred-schema batch must read without error"); + if (!batch->has_value()) + { + break; + } + rows += (*batch)->array.length; + } + CHECK(rows == 100, "the inferred stream delivers every data row"); + } + std::printf("OK: C++ arrow test passed\n"); return 0; } diff --git a/cpp/tests/batch.cpp b/cpp/tests/batch.cpp index 67e2a6f7..049d6b0d 100644 --- a/cpp/tests/batch.cpp +++ b/cpp/tests/batch.cpp @@ -189,6 +189,194 @@ namespace auto after = reader->next(); check(after.has_value() && after->has_value(), "the reader keeps reading after a whole-sheet read"); } + + const xl::ColumnRef *find_column(const xl::Table &table, std::string_view name) + { + for (const xl::ColumnRef &column : table.columns()) + { + if (column.name == name) + { + return &column; + } + } + return nullptr; + } + + void test_runtime_schema_matches_the_typed_parse(const std::vector &expected) + { + auto workbook = xl::Workbook::open(EXCELREADER_FIXTURE_PATH); + check(workbook.has_value(), "open the fixture for the runtime-schema parse"); + if (!workbook.has_value()) + { + return; + } + auto schema = workbook->sheet(0).infer_schema(); + check(schema.has_value() && !schema->empty(), "infer_schema finds columns"); + if (!schema.has_value()) + { + return; + } + + auto table = xl::parse(workbook->sheet(0), *schema); + check(table.has_value(), "parse over an inferred schema succeeds"); + if (!table.has_value()) + { + return; + } + check(table->size() == static_cast(expected.size()), "the runtime-schema parse reads every row"); + check(table->columns().size() == schema->size(), "one column per inferred column"); + + const xl::ColumnRef *coluna1 = find_column(*table, "Coluna1"); + const xl::ColumnRef *coluna3 = find_column(*table, "Coluna3"); + check(coluna1 != nullptr && coluna1->type == XL_T_STRING, "Coluna1 is a string column"); + check(coluna3 != nullptr && coluna3->type == XL_T_I64, "Coluna3 is an i64 column"); + if (coluna1 == nullptr || coluna3 == nullptr || coluna1->type != XL_T_STRING || coluna3->type != XL_T_I64) + { + return; + } + const auto *offsets = static_cast(coluna1->values); + const auto *text = reinterpret_cast(coluna1->data); + const auto *numbers = static_cast(coluna3->values); + bool same = true; + for (size_t row = 0; row < expected.size(); ++row) + { + const std::string_view cell(text + offsets[row], static_cast(offsets[row + 1] - offsets[row])); + same = same && cell == expected[row].first && numbers[row] == expected[row].second; + } + check(same, "the runtime-schema parse matches the typed parse cell for cell"); + + std::vector bytes; + check(xl::write_columns(bytes, table->columns(), {.format = XL_FORMAT_XLSX}).has_value(), + "a runtime-schema table writes back as it was read"); + auto reopened = xl::Workbook::open(bytes); + check(reopened.has_value(), "the written workbook opens"); + if (!reopened.has_value()) + { + return; + } + auto again = xl::parse(reopened->sheet(0), *schema); + check(again.has_value() && again->size() == table->size(), "the written workbook parses with the same schema"); + + auto by_index = workbook->sheet(0).infer_schema(0); + check(by_index.has_value() && !(*by_index)[0].name.has_value(), "a schema without a header has no names"); + if (by_index.has_value()) + { + auto unnamed = xl::parse(workbook->sheet(0), *by_index, {.header_row = 0}); + check(unnamed.has_value() && unnamed->size() == static_cast(expected.size()) + 1, + "unnamed columns resolve by index, and with no header the header row is data"); + } + + auto empty = xl::parse(workbook->sheet(0), std::span{}); + check(!empty.has_value() && empty.error().code == XL_INVALID_ARGUMENT, "an empty schema is rejected"); + + xl::Table moved = std::move(*table); + check(find_column(moved, "Coluna3") != nullptr, "a moved table keeps its column names"); + + auto replacement = xl::parse(workbook->sheet(0), *schema); + check(replacement.has_value(), "parse a second table to move-assign"); + if (replacement.has_value()) + { + moved = std::move(*replacement); + check(moved.size() == static_cast(expected.size()) && find_column(moved, "Coluna1") != nullptr, + "a move-assigned table holds the other table's rows and names"); + } + } + + void test_move_assigned_view_holds_the_other_rows(const std::vector &expected) + { + auto workbook = xl::Workbook::open(EXCELREADER_FIXTURE_PATH); + check(workbook.has_value(), "open the fixture for the move-assigned view"); + if (!workbook.has_value()) + { + return; + } + auto whole = xl::parse(workbook->sheet(0)); + auto reader = xl::parse_batches(workbook->sheet(0), {.batch_size = 8}); + check(whole.has_value() && reader.has_value(), "parse a view and open a reader to move-assign from"); + if (!whole.has_value() || !reader.has_value()) + { + return; + } + auto batch = reader->next(); + check(batch.has_value() && batch->has_value(), "read a batch to move-assign over the whole view"); + if (!batch.has_value() || !batch->has_value()) + { + return; + } + *whole = std::move(**batch); + check(whole->size() == 8 && std::string((*whole)[0].Coluna1) == expected[0].first, + "a move-assigned view holds the batch's rows"); + } + + void test_a_sheet_past_the_end_fails_every_read() + { + auto workbook = xl::Workbook::open(EXCELREADER_FIXTURE_PATH); + check(workbook.has_value(), "open the fixture for the past-the-end sheet"); + if (!workbook.has_value()) + { + return; + } + const xl::Sheet missing = workbook->sheet(99); + const xl::InferredColumn schema[] = {{"Coluna3", 0, XL_T_I64, false}}; + check(!xl::parse(missing).has_value(), "parse fails on a sheet past the end"); + check(!xl::parse_batches(missing).has_value(), "parse_batches fails on a sheet past the end"); + check(!xl::parse_batches(missing, schema).has_value(), "a runtime-schema parse_batches fails on a sheet past the end"); + } + + void test_runtime_schema_batches_equal_whole_sheet(const std::vector &expected) + { + auto workbook = xl::Workbook::open(EXCELREADER_FIXTURE_PATH); + check(workbook.has_value(), "open the fixture for the runtime-schema batched read"); + if (!workbook.has_value()) + { + return; + } + auto schema = workbook->sheet(0).infer_schema(); + check(schema.has_value(), "infer_schema succeeds for the batched read"); + if (!schema.has_value()) + { + return; + } + + auto reader = xl::parse_batches(workbook->sheet(0), *schema, {.batch_size = 8}); + check(reader.has_value(), "parse_batches over an inferred schema opens"); + if (!reader.has_value()) + { + return; + } + schema->clear(); + + std::vector coluna3; + int batches = 0; + for (auto &batch : *reader) + { + check(batch.has_value(), "a runtime-schema batch reads without error"); + if (!batch.has_value()) + { + return; + } + ++batches; + check(batch->size() <= 8, "no batch exceeds the requested batch size"); + const xl::ColumnRef *column = find_column(*batch, "Coluna3"); + check(column != nullptr && column->type == XL_T_I64, "every batch keeps the schema's names and types"); + if (column == nullptr || column->type != XL_T_I64) + { + return; + } + const auto *values = static_cast(column->values); + coluna3.insert(coluna3.end(), values, values + column->length); + } + check(batches == 13, "100 rows at batch size 8 is 13 batches"); + bool same = coluna3.size() == expected.size(); + for (size_t row = 0; same && row < expected.size(); ++row) + { + same = coluna3[row] == expected[row].second; + } + check(same, "the batches concatenate to the whole-sheet read"); + + auto rejected = xl::parse_batches(workbook->sheet(0), *schema, {.batch_size = 0}); + check(!rejected.has_value() && rejected.error().code == XL_INVALID_ARGUMENT, "a batch size below 1 is rejected"); + } } int main() @@ -200,6 +388,10 @@ int main() test_second_reader_is_allowed(); test_non_positive_batch_size_is_rejected(); test_foreign_read_leaves_the_reader_alone(); + test_runtime_schema_matches_the_typed_parse(expected); + test_runtime_schema_batches_equal_whole_sheet(expected); + test_move_assigned_view_holds_the_other_rows(expected); + test_a_sheet_past_the_end_fails_every_read(); if (failures != 0) { diff --git a/cpp/tests/numbers.cpp b/cpp/tests/numbers.cpp index 152e2123..1750915d 100644 --- a/cpp/tests/numbers.cpp +++ b/cpp/tests/numbers.cpp @@ -8,6 +8,7 @@ #include #include +#include #include namespace @@ -61,6 +62,8 @@ int main() check(xl::CellView{0, xl::CellType::Number, {}, 0.0125}.text() == "0.0125", "a fraction places its point"); check(xl::CellView{0, xl::CellType::Number, {}, -2.5}.text() == "-2.5", "a negative keeps its sign"); check(xl::CellView{0, xl::CellType::Number, {}, 0.0}.text() == "0", "zero is 0"); + check(xl::CellView{0, xl::CellType::Number, {}, std::numeric_limits::infinity()}.text() == "inf", + "infinity has no exponent to place"); round_trips(0.1); round_trips(1.0 / 3.0); round_trips(1e21); diff --git a/cpp/tests/sheets.cpp b/cpp/tests/sheets.cpp index 9942daef..f4d682f7 100644 --- a/cpp/tests/sheets.cpp +++ b/cpp/tests/sheets.cpp @@ -113,6 +113,25 @@ int main() check(decoded.has_value() && decoded->size() == 3, "read_all reads the whole named sheet"); auto schema = workbook->sheet(1).infer_schema(0, 10); check(schema.has_value() && schema->size() == 1, "infer_schema samples the named sheet"); + + const xl::Sheet beyond = workbook->sheet(2); + check(!beyond.name().has_value(), "a sheet past the end has no name"); + check(!beyond.visibility().has_value(), "a sheet past the end has no visibility"); + check(!beyond.read_all().has_value(), "read_all fails on a sheet past the end"); + check(!beyond.infer_schema().has_value(), "infer_schema fails on a sheet past the end"); + } + } + + { + auto workbook = xl::Workbook::open(two); + check(workbook.has_value(), "open for the moved-from workbook"); + if (workbook.has_value()) + { + xl::Workbook moved = std::move(*workbook); + check(!workbook->sheets().has_value(), "a moved-from workbook lists no sheets"); + check(!workbook->sheet_by_name("sheet0").has_value(), "a moved-from workbook finds no sheet by name"); + check(!workbook->is_date1904().has_value(), "a moved-from workbook has no date system"); + check(moved.is_date1904().has_value(), "the moved-to workbook still answers"); } } diff --git a/cpp/tests/sources.cpp b/cpp/tests/sources.cpp index 76c265d9..11024e1b 100644 --- a/cpp/tests/sources.cpp +++ b/cpp/tests/sources.cpp @@ -1,4 +1,5 @@ #include +#include #include #include @@ -7,12 +8,25 @@ #include #include #include +#include #include +#include #include #include #include #include +struct Quantity +{ + int64_t qty; +}; + +template <> +struct xl::ExcelMapper +{ + static constexpr auto get_bindings() { return std::make_tuple(xl::make_field("qty", &Quantity::qty)); } +}; + namespace { int failures = 0; @@ -98,6 +112,50 @@ namespace Counters &counters_; }; + class OpaqueThrowingSource final : public xl::Source + { + public: + std::uint64_t size() const override { return 1024; } + std::expected read_at(std::uint64_t, std::span) const override + { + throw 42; + } + }; + + class HugeSource final : public xl::Source + { + public: + std::uint64_t size() const override { return std::numeric_limits::max(); } + std::expected read_at(std::uint64_t, std::span) const override + { + return std::size_t{0}; + } + }; + + /// Serves the first `trip` bytes and fails every read past them, so a read fails partway. + class TrippingSource final : public xl::Source + { + public: + TrippingSource(std::vector bytes, std::uint64_t trip) : bytes_(std::move(bytes)), trip_(trip) {} + + std::uint64_t size() const override { return bytes_.size(); } + + std::expected read_at(std::uint64_t offset, std::span buffer) const override + { + if (offset >= trip_) + { + return std::unexpected(std::string("disk failed mid-read")); + } + const std::size_t count = std::min(buffer.size(), bytes_.size() - offset); + std::memcpy(buffer.data(), bytes_.data() + offset, count); + return count; + } + + private: + std::vector bytes_; + std::uint64_t trip_; + }; + class ErrorStream final : public xl::InputStream { public: @@ -172,6 +230,108 @@ namespace auto decoded = workbook.sheet(0).read_all(); return decoded.has_value() ? decoded->size() : 0; } + + bool mentions(const xl::Error &error, std::string_view text) + { + return error.message.find(text) != std::string::npos; + } + + std::expected open_tripping() + { + std::string csv = "qty\n"; + for (int i = 0; i < 50'000; ++i) + { + csv += std::to_string(i) + "\n"; + } + std::vector bytes(csv.size()); + std::memcpy(bytes.data(), csv.data(), csv.size()); + const std::uint64_t trip = bytes.size() / 2; + return xl::Workbook::open(std::make_unique(std::move(bytes), trip), + {.format = XL_FORMAT_CSV, .source_block_size = -1}); + } + + /// Drains `next()` and returns the error it stopped on, or nothing if the sheet ended cleanly. + template + std::optional first_error(Reader &reader) + { + while (true) + { + auto batch = reader.next(); + if (!batch.has_value()) + { + return batch.error(); + } + if (!batch->has_value()) + { + return std::nullopt; + } + } + } + + const xl::InferredColumn kQtySchema[] = {{"qty", 0, XL_T_I64, false}}; + + void test_a_typed_range_ends_on_a_failed_read() + { + auto workbook = open_tripping(); + check(workbook.has_value(), "a tripping source opens"); + if (!workbook.has_value()) + { + return; + } + auto reader = xl::parse_batches(workbook->sheet(0), {.batch_size = 100}); + check(reader.has_value(), "a typed reader opens before the failure"); + if (!reader.has_value()) + { + return; + } + int64_t rows = 0; + std::optional last; + for (auto &batch : *reader) + { + if (!batch.has_value()) + { + last = batch.error(); + break; + } + rows += batch->size(); + } + check(rows > 0, "batches before the failure arrive"); + check(last.has_value() && mentions(*last, "disk failed mid-read"), "the range ends on the source's error"); + } + + void test_a_table_reader_reports_a_failed_read() + { + auto workbook = open_tripping(); + if (!workbook.has_value()) + { + return; + } + auto reader = xl::parse_batches(workbook->sheet(0), kQtySchema, {.batch_size = 100}); + check(reader.has_value(), "a table reader opens before the failure"); + if (!reader.has_value()) + { + return; + } + const std::optional error = first_error(*reader); + check(error.has_value() && mentions(*error, "disk failed mid-read"), "a table reader reports the source's error"); + } + + void test_an_arrow_stream_reports_a_failed_read() + { + auto workbook = open_tripping(); + if (!workbook.has_value()) + { + return; + } + auto stream = xl::to_arrow_stream(workbook->sheet(0), kQtySchema, {.batch_size = 100}); + check(stream.has_value(), "an Arrow stream opens before the failure"); + if (!stream.has_value()) + { + return; + } + const std::optional error = first_error(*stream); + check(error.has_value() && mentions(*error, "disk failed mid-read"), "an Arrow stream reports the source's error"); + } } int main() @@ -215,6 +375,22 @@ int main() "an exception from the source becomes an error"); check(throwing.destroyed.load() == 1, "a source that threw is destroyed exactly once"); } + { + auto workbook = xl::Workbook::open(std::make_unique(), {.format = XL_FORMAT_XLSX}); + check(!workbook.has_value() && mentions(workbook.error(), "the source threw an exception"), + "a throw that is not a std::exception becomes an error"); + } + { + auto null_source = xl::Workbook::open(std::unique_ptr{}); + check(!null_source.has_value() && null_source.error().code == XL_INVALID_ARGUMENT, "a null source is rejected"); + auto null_stream = xl::Workbook::open(std::unique_ptr{}); + check(!null_stream.has_value() && null_stream.error().code == XL_INVALID_ARGUMENT, "a null stream is rejected"); + auto huge = xl::Workbook::open(std::make_unique()); + check(!huge.has_value() && huge.error().code == XL_INVALID_ARGUMENT, "a source larger than INT64_MAX is rejected"); + } + test_a_typed_range_ends_on_a_failed_read(); + test_a_table_reader_reports_a_failed_read(); + test_an_arrow_stream_reports_a_failed_read(); { auto workbook = xl::Workbook::open(std::make_unique(), {.format = XL_FORMAT_XLSX}); check(!workbook.has_value() && workbook.error().message.find("pipe broke") != std::string::npos, diff --git a/cpp/tests/write.cpp b/cpp/tests/write.cpp index b1d8d0b9..3db2cf95 100644 --- a/cpp/tests/write.cpp +++ b/cpp/tests/write.cpp @@ -45,6 +45,43 @@ struct xl::ExcelMapper } }; +struct NarrowRow +{ + uint8_t small; +}; + +template <> +struct xl::ExcelMapper +{ + static constexpr auto get_bindings() { return std::make_tuple(xl::make_field("small", &NarrowRow::small)); } +}; + +struct WideRow +{ + uint64_t big; +}; + +template <> +struct xl::ExcelMapper +{ + static constexpr auto get_bindings() { return std::make_tuple(xl::make_field("big", &WideRow::big)); } +}; + +struct NamedRow +{ + std::string name = "fallback"; + std::string_view view = "fallback"; +}; + +template <> +struct xl::ExcelMapper +{ + static constexpr auto get_bindings() + { + return std::make_tuple(xl::make_field("name", &NamedRow::name), xl::make_field("view", &NamedRow::view)); + } +}; + static std::filesystem::path temp_path(std::string_view name) { return std::filesystem::temp_directory_path() / @@ -302,6 +339,7 @@ static int test_format_from_extension() } CHECK(!xl::write_columns(temp_path("no-extension").string(), columns).has_value(), "a path without an extension must be rejected"); + CHECK(!xl::write_columns("ab", columns).has_value(), "a name shorter than any extension must be rejected"); return 0; } @@ -711,8 +749,102 @@ static int test_writer_handle_class_rejects_auto_for_memory() return 0; } +static std::expected, xl::Error> parse_narrow(const std::vector &values, + std::vector &bytes) +{ + std::array columns{xl::i64_column("small", values)}; + if (!xl::write_columns(bytes, columns, {.format = XL_FORMAT_XLSX}).has_value()) + { + return std::unexpected(xl::Error{XL_ERROR, "write_columns failed"}); + } + auto workbook = xl::Workbook::open(std::span(bytes)); + if (!workbook.has_value()) + { + return std::unexpected(workbook.error()); + } + auto sheets = workbook->sheets(); + if (!sheets.has_value()) + { + return std::unexpected(sheets.error()); + } + return xl::parse(sheets->front()); +} + +static int test_narrow_field_rejects_a_value_it_cannot_hold() +{ + std::vector bytes; + auto too_big = parse_narrow({1, 300}, bytes); + CHECK(!too_big.has_value(), "300 must not be read into a uint8_t"); + CHECK(too_big.error().code == XL_INVALID_ARGUMENT, "an out-of-range value is an invalid argument"); + CHECK(too_big.error().message.find("small") != std::string::npos, "the error must name the column"); + + auto negative = parse_narrow({-1}, bytes); + CHECK(!negative.has_value(), "-1 must not be read into a uint8_t"); + + auto limits = parse_narrow({0, 255}, bytes); + CHECK(limits.has_value(), "values at the uint8_t limits must parse"); + CHECK((*limits)[0].small == 0 && (*limits)[1].small == 255, "values at the limits must round-trip"); + + (void)parse_narrow({300}, bytes); + auto workbook = xl::Workbook::open(std::span(bytes)); + CHECK(workbook.has_value(), "the written bytes must open"); + auto batches = xl::parse_batches(workbook->sheets()->front()); + CHECK(batches.has_value(), "parse_batches must open"); + auto batch = batches->next(); + CHECK(!batch.has_value(), "a batch holding 300 must fail for a uint8_t field"); + return 0; +} + +static int test_unsigned_values_past_int64_are_refused_on_write() +{ + std::vector bytes; + const std::vector rows{{static_cast(INT64_MAX) + 1}}; + auto written = xl::write_sheet(bytes, rows, {.format = XL_FORMAT_XLSX}); + CHECK(!written.has_value(), "a uint64_t above INT64_MAX must not be written as a negative number"); + + const std::vector limit{{static_cast(INT64_MAX)}}; + CHECK(xl::write_sheet(bytes, limit, {.format = XL_FORMAT_XLSX}).has_value(), "INT64_MAX must be written"); + auto workbook = xl::Workbook::open(std::span(bytes)); + CHECK(workbook.has_value(), "the written bytes must open"); + auto table = xl::parse(workbook->sheets()->front()); + CHECK(table.has_value() && (*table)[0].big == static_cast(INT64_MAX), "INT64_MAX must round-trip"); + + std::vector streamed; + auto handle = xl::WriterHandle::open(streamed, {.format = XL_FORMAT_XLSX}); + CHECK(handle.has_value(), "the writer handle must open"); + CHECK(handle->start_sheet("S").has_value() && handle->start_row().has_value(), "the sheet and row must start"); + CHECK(!handle->write(UINT64_MAX).has_value(), "WriterHandle::write must refuse a uint64_t above INT64_MAX"); + return 0; +} + +static int test_a_valid_empty_string_replaces_the_default() +{ + const int32_t offsets[] = {0, 0}; + xl_column columns[] = { + xl_column{XL_T_STRING, 1, offsets, nullptr, nullptr, 0}, + xl_column{XL_T_STRING, 1, offsets, nullptr, nullptr, 0}, + }; + const xl_table table{2, 1, columns}; + const NamedRow row = xl::detail::row_at(table, 0); + CHECK(row.name.empty(), "an empty std::string must overwrite the default"); + CHECK(row.view.empty(), "an empty std::string_view must overwrite the default"); + return 0; +} + int main() { + if (test_narrow_field_rejects_a_value_it_cannot_hold() != 0) + { + return 1; + } + if (test_unsigned_values_past_int64_are_refused_on_write() != 0) + { + return 1; + } + if (test_a_valid_empty_string_replaces_the_default() != 0) + { + return 1; + } if (test_write_options() != 0) { return 1; diff --git a/docs/guide/reading.md b/docs/guide/reading.md index c02d2186..4a9b038c 100644 --- a/docs/guide/reading.md +++ b/docs/guide/reading.md @@ -121,11 +121,12 @@ These cases read on one thread with the same results and no error: - **`ParseAsync` and `AggregateAsync`** are exact on every format. `ParseAsync` yields the same items in the same order as the sequential parse; `AggregateAsync` equals its own single-threaded run. - **XLSB** delivers every row exactly once, in `ForEachAsync` too. - **XLSX** delivers exactly once for a well-formed sheet. The pipeline cuts the inflated sheet at what looks like a row start; on a sheet where those bytes sit inside cell content (a CDATA run, a comment) or the file is truncated, a worker detects it, and the read recovers by reading the rest sequentially. `ParseAsync` and `AggregateAsync` stay exact. In `ForEachAsync`, workers may already have called the callback for chunks that are then dropped, so on such a sheet rows can be delivered twice, or be rows the sequential reader would not produce. -- **CSV** is at least once in `ForEachAsync`, unchanged from v6: the callback may be invoked more than once for the same record. A partition whose start offset was guessed wrongly is read again from the confirmed offset, and the discarded pass may already have delivered a whole partition's worth of records, 1 MiB to 64 MiB of them, not just a few near the seam. On a source with quoted fields those records may also be misparsed, carrying values that appear nowhere in the file, and an exception the callback threw during a discarded pass is discarded with it. A start is only guessed wrongly inside a quoted field, so a source in which the quote character never appears delivers every record exactly once. For that guarantee on any source, use `AggregateAsync`, where the discarded partition's accumulator is thrown away, and apply side effects once it returns. +- **CSV** delivers exactly once when it follows RFC 4180 quoting (every quote opens or closes a quoted field, or is doubled inside one) and no quoted field is longer than 64 KB. A partition's start is read off the quotes around it, and a partition whose quotes could open as well as close a field waits for its confirmed start instead of being guessed. +- **CSV outside those rules** (a stray quote inside an unquoted field, text after a closing quote, a quoted field past 64 KB) is at least once in `ForEachAsync`: the callback may be invoked more than once for the same record. A partition whose start offset was guessed wrongly is read again from the confirmed offset, and the discarded pass may already have delivered a whole partition's worth of records, 1 MiB to 64 MiB of them, not just a few near the seam. Those records may also be misparsed, carrying values that appear nowhere in the file, and an exception the callback threw during a discarded pass is discarded with it. For exactly once on any source, use `AggregateAsync`, where the discarded partition's accumulator is thrown away, and apply side effects once it returns. -A callback that writes to a database or sends messages must be idempotent on CSV, and on XLSX if the sheet may be malformed, and must also tolerate records that do not exist: deduplicating on a key does not filter those out. +A callback that writes to a database or sends messages must be idempotent on a CSV that may break those rules, and on XLSX if the sheet may be malformed, and must also tolerate records that do not exist: deduplicating on a key does not filter those out. -[`ExcelParseException.Row`](xref:ExcelReader.Core.Parser.ExcelParseException.Row) on `ParseAsync` counts the rows emitted so far. On `ForEachAsync` and `AggregateAsync` it is the exact row when the read is sequential and `0` when it was partitioned. +[`ExcelParseException.Row`](xref:ExcelReader.Core.Parser.ExcelParseException.Row) on `ParseAsync` counts the rows emitted so far. On `ForEachAsync` and `AggregateAsync` it is the row a sequential read reports, whether or not the read was partitioned. See the [parallel sheet benchmarks](../performance/benchmarks.md#parallel-sheet-reading) and the [parallel CSV benchmarks](../performance/benchmarks.md#parallel-csv) for what this buys. On a real-data XLSX the sheet stage drops from 28 ms (prefetched, sequential) to about 17 ms, and on XLSB from 17 ms to about 13 ms; the floor is inflate, which runs on one thread. CSV has no such floor: its `AggregateAsync` at dop 4 is ~1.8x faster than the typed path while allocating ~450x less, with zero garbage collections. diff --git a/python/src/excelreader/reader.py b/python/src/excelreader/reader.py index bb07f645..161f2602 100644 --- a/python/src/excelreader/reader.py +++ b/python/src/excelreader/reader.py @@ -249,7 +249,7 @@ def rows(self) -> Iterator[list[Cell]]: buffer = ctypes.create_string_buffer(capacity) continue _check(status) - yield _decode_row(buffer.raw, written.value, date1904) + yield _decode_row(buffer[: written.value], written.value, date1904) finally: self._close_rows(cursor) diff --git a/python/src/excelreader/writer.py b/python/src/excelreader/writer.py index 57d27c60..32324c3e 100644 --- a/python/src/excelreader/writer.py +++ b/python/src/excelreader/writer.py @@ -41,54 +41,81 @@ def _resolve_write_format(name: str | None, path: Path) -> int: ) from None -def _buffer_of(values: Any) -> tuple[Any, int]: - """Returns (ctypes-addressable buffer, byte length) for one column's values. - - NumPy arrays, `array.array` and `bytes` all expose the buffer protocol, so one `memoryview` reads - every input shape the same way. The bytes are then COPIED into a ctypes block rather than - borrowed: the copy is owned by this module and cannot be moved or freed out from under the native - side while the call runs. The caller must keep the returned object alive for as long as the - pointer is in use. +_SIGNED_FORMATS = frozenset("bhilqn") +_ELEMENTS = { + ColumnType.I64: (_SIGNED_FORMATS, 8), + ColumnType.TIME: (_SIGNED_FORMATS, 8), + ColumnType.TIMESTAMP: (_SIGNED_FORMATS, 8), + ColumnType.F64: (frozenset("d"), 8), + ColumnType.BOOL: (frozenset("bB?"), 1), + ColumnType.DATE: (_SIGNED_FORMATS, 4), +} + + +def _checked_view(values: Any, formats: frozenset[str], itemsize: int, count: int, what: str) -> memoryview: + """The native side trusts `length` and the column type, and cannot see how big a Python + allocation really is, so a buffer of the wrong shape must be refused here, before any pointer + to it exists. """ - if isinstance(values, (bytes, bytearray)): - raw: Any = values - elif isinstance(values, array): - raw = values - else: - raw = getattr(values, "data", values) - blob = memoryview(raw).tobytes() - return (ctypes.c_char * len(blob)).from_buffer_copy(blob), len(blob) + view = memoryview(getattr(values, "data", values)) + code = view.format.lstrip("@=") + if view.ndim != 1 or view.itemsize != itemsize or code not in formats: + raise TypeError( + f"{what} must be a 1-D buffer of {itemsize}-byte elements with format one of {sorted(formats)}; " + f"got format {view.format!r} with {view.itemsize}-byte elements" + ) + if len(view) != count: + raise ValueError(f"{what} has {len(view)} elements but {count} are needed") + return view + + +def _copy(view: memoryview) -> Any: + """COPIES the bytes into a ctypes block rather than borrowing them: the copy is owned by this + module and cannot be moved or freed out from under the native side while the call runs. The + caller must keep the returned object alive for as long as the pointer is in use. + """ + blob = view.tobytes() + return (ctypes.c_char * len(blob)).from_buffer_copy(blob) def _native_column(column: Any, column_type: ColumnType, validity: bytes | None, row_count: int, keepalive: list[Any]) -> _native.NativeColumn: if isinstance(column, StringColumn): - offsets, _ = _buffer_of(column.offsets) - data, data_len = _buffer_of(column.data) + offsets = _copy(_checked_view(column.offsets, _SIGNED_FORMATS, 4, row_count + 1, "a STRING column's offsets")) + data_view = memoryview(column.data) + data_len = data_view.nbytes + data = _copy(data_view) keepalive += [offsets, data] return _native.NativeColumn( type=int(ColumnType.STRING), length=row_count, values=ctypes.cast(offsets, ctypes.c_void_p), - validity=_validity_pointer(validity, keepalive), + validity=_validity_pointer(validity, row_count, keepalive), data=ctypes.cast(data, ctypes.POINTER(ctypes.c_uint8)) if data_len else None, data_len=data_len, ) - values, _ = _buffer_of(column) + if column_type is ColumnType.STRING: + raise TypeError("a STRING column must be a StringColumn") + formats, itemsize = _ELEMENTS[column_type] + values = _copy(_checked_view(column, formats, itemsize, row_count, f"a {column_type.name} column")) keepalive.append(values) return _native.NativeColumn( type=int(column_type), length=row_count, values=ctypes.cast(values, ctypes.c_void_p), - validity=_validity_pointer(validity, keepalive), + validity=_validity_pointer(validity, row_count, keepalive), data=None, data_len=0, ) -def _validity_pointer(validity: bytes | None, keepalive: list[Any]) -> Any: +def _validity_pointer(validity: bytes | None, row_count: int, keepalive: list[Any]) -> Any: if validity is None: return None + if len(validity) < (row_count + 7) // 8: + raise ValueError( + f"the validity bitmap is {len(validity)} bytes, but {row_count} rows need {(row_count + 7) // 8}" + ) buffer = (ctypes.c_uint8 * len(validity)).from_buffer_copy(validity) keepalive.append(buffer) return ctypes.cast(buffer, ctypes.POINTER(ctypes.c_uint8)) @@ -150,7 +177,8 @@ def write_workbook( `format` is one of xlsx/xlsb/xls/csv. For a path, None infers it from the extension; an object with `write()` has no extension, so it needs `format`. The whole workbook is built in memory and - handed to `write()` in one call, at the object's current position; the object is not closed. + handed to `write()` at the object's current position, again for the rest whenever `write()` + returns that it took only part; the object is not closed. The column names become the header row. `options` sets the sheet name and CSV dialect; see `WriteOptions`. @@ -174,7 +202,24 @@ def write_workbook( raise ValueError( f"format= is required when writing to an object with write(); one of {sorted(_FORMAT_NAMES)}" ) - destination.write(_write_memory(table, types, format, options)) + _write_all(destination, _write_memory(table, types, format, options)) + + +def _write_all(destination: IO[bytes], data: bytes) -> None: + """Calls `write()` until it has taken all of `data`. A `write()` that returns None is taken to + have written everything, as files that predate the `int` return convention do. + """ + view = memoryview(data) + offset = 0 + while offset < len(data): + written = destination.write(data if offset == 0 else view[offset:]) + if written is None: + return + if not 0 < written <= len(data) - offset: + raise OSError( + f"write() made no progress: it returned {written} with {len(data) - offset} bytes left to write" + ) + offset += written def _write_file( @@ -371,7 +416,7 @@ def encrypt_package( `package` is a path to a complete file, or its bytes (`bytes`, `bytearray`, `memoryview`, or an `io.BytesIO` read whole). `destination` is a path, or a binary object with `write()` that - receives the encrypted workbook in one call and is not closed. Encryption parameters are fixed + receives the encrypted workbook, as `write_workbook` hands it over, and is not closed. Encryption parameters are fixed at Excel's own defaults; there are no options. """ password_encoded = password.encode("utf-8") @@ -414,4 +459,4 @@ def encrypt_package( if isinstance(destination, (str, os.PathLike)): Path(destination).write_bytes(encrypted) else: - destination.write(encrypted) + _write_all(destination, encrypted) diff --git a/python/src/excelreader/writer_handle.py b/python/src/excelreader/writer_handle.py index 0356d13b..1891f765 100644 --- a/python/src/excelreader/writer_handle.py +++ b/python/src/excelreader/writer_handle.py @@ -16,6 +16,7 @@ from excelreader import _native from excelreader.reader import _check from excelreader.types import ColumnType, WriteOptions +from excelreader.writer import _write_all _EPOCH_DATE = datetime.date(1970, 1, 1) _EPOCH_DATETIME = datetime.datetime(1970, 1, 1) @@ -131,8 +132,8 @@ def _finished_bytes(self, handle: ctypes.c_void_p) -> bytes: def close(self) -> None: """Finalizes and releases the writer. Safe to call more than once. - For a destination with `write()`, the finished workbook is written to it in one call, even - when sheets or rows were left open. + For a destination with `write()`, the finished workbook is written to it, even when sheets + or rows were left open; a `write()` that takes only part of it is called again for the rest. """ if self._handle is None: return @@ -140,7 +141,7 @@ def close(self) -> None: destination, self._destination = self._destination, None try: if destination is not None: - destination.write(self._finished_bytes(handle)) + _write_all(destination, self._finished_bytes(handle)) finally: _check(self._lib.xl_close_write_handle(handle)) diff --git a/python/tests/test_reader.py b/python/tests/test_reader.py index e5f0be9a..b286b8fe 100644 --- a/python/tests/test_reader.py +++ b/python/tests/test_reader.py @@ -361,6 +361,16 @@ def test_open_options_rejects_an_out_of_range_value(csv_path): open_workbook(csv_path, format="csv", options=OpenOptions(csv_delimiter=999)) +def test_rows_after_one_that_grew_the_buffer_decode_only_their_own_bytes(tmp_path): + path = tmp_path / "grown.csv" + path.write_text("value\n" + ("x" * 200_000) + "\nação\n\n7\n", encoding="utf-8") + + with open_workbook(path, format="csv") as workbook: + rows = [[cell.value for cell in row] for row in workbook.sheets[0].rows()] + + assert rows[2:] == [["ação"], [""], ["7"]] + + def test_open_options_limit_actually_aborts_an_oversized_read(tmp_path): path = tmp_path / "wide-cell.csv" path.write_text("value\n" + ("x" * 200_000) + "\n", encoding="utf-8") diff --git a/python/tests/test_writer.py b/python/tests/test_writer.py index 8c10c697..2e226bd8 100644 --- a/python/tests/test_writer.py +++ b/python/tests/test_writer.py @@ -1,4 +1,5 @@ import io +from array import array import pytest from excelreader import ( @@ -6,6 +7,8 @@ ColumnType, ExcelReaderError, PasswordIncorrectError, + StringColumn, + TypedTable, WriteOptions, encrypt_package, open_workbook, @@ -390,3 +393,97 @@ def test_write_workbook_rejects_a_destination_it_cannot_write(source_csv): with pytest.raises(TypeError, match="destination"): write_workbook(42, table, _TYPES, format="xlsx") + + +class _TrickleWriter: + def __init__(self, step): + self.step = step + self.data = bytearray() + + def write(self, chunk): + taken = bytes(chunk[: self.step]) + self.data += taken + return len(taken) + + +def test_write_workbook_finishes_a_partial_write(source_csv): + with open_workbook(source_csv) as workbook: + table = workbook.sheets[0].parse(_SCHEMA) + destination = _TrickleWriter(4) + + write_workbook(destination, table, _TYPES, format="xlsx") + + assert bytes(destination.data) == _to_bytes(table) + + +def test_write_workbook_fails_when_write_makes_no_progress(source_csv): + with open_workbook(source_csv) as workbook: + table = workbook.sheets[0].parse(_SCHEMA) + + with pytest.raises(OSError, match="no progress"): + write_workbook(_TrickleWriter(0), table, _TYPES, format="xlsx") + + +def test_encrypt_package_finishes_a_partial_write(source_csv): + with open_workbook(source_csv) as workbook: + table = workbook.sheets[0].parse(_SCHEMA) + destination = _TrickleWriter(7) + + encrypt_package(_to_bytes(table), destination, "hunter2") + + result = _parse_bytes(bytes(destination.data), "xlsx", password="hunter2") + assert list(result.columns[0]) == ["widget", "gadget"] + + +def _one_column_table(column, rows=1, validity=None): + return TypedTable(row_count=rows, names=["n"], columns=[column], validity=[validity]) + + +@pytest.mark.parametrize( + ("column", "column_type", "rows", "message"), + [ + (array("q", [7]), ColumnType.I64, 10, "1 elements but 10"), + (array("i", [1, 2]), ColumnType.I64, 1, "8-byte"), + (array("d", [1.0]), ColumnType.I64, 1, "format"), + (array("Q", [1]), ColumnType.I64, 1, "format"), + (array("q", [1]), ColumnType.DATE, 1, "4-byte"), + (StringColumn(array("i", [0]), b""), ColumnType.STRING, 1, "offsets"), + (StringColumn(array("q", [0, 1]), b"a"), ColumnType.STRING, 1, "offsets"), + ], +) +def test_write_workbook_rejects_a_column_buffer_that_does_not_match(column, column_type, rows, message): + with pytest.raises((TypeError, ValueError), match=message): + write_workbook(io.BytesIO(), _one_column_table(column, rows), [column_type], format="xlsx") + + +def test_write_workbook_rejects_a_short_validity_bitmap(): + table = _one_column_table(array("q", range(9)), rows=9, validity=b"\xff") + + with pytest.raises(ValueError, match="validity"): + write_workbook(io.BytesIO(), table, [ColumnType.I64], format="xlsx") + + +def test_write_workbook_takes_numpy_columns(): + numpy = pytest.importorskip("numpy") + table = TypedTable( + row_count=2, + names=["qty", "price", "day", "ok"], + columns=[ + numpy.array([3, 7], dtype=numpy.int64), + numpy.array([1.5, 2.5]), + numpy.array([0, 1], dtype=numpy.int32), + numpy.array([True, False]), + ], + validity=[None, None, None, None], + ) + types = [ColumnType.I64, ColumnType.F64, ColumnType.DATE, ColumnType.BOOL] + buffer = io.BytesIO() + + write_workbook(buffer, table, types, format="xlsx") + + schema = [ColumnSpec(t, name=n) for n, t in zip(table.names, types)] + with open_workbook(buffer.getvalue(), format="xlsx") as workbook: + result = workbook.sheets[0].parse(schema) + assert list(result.columns[0]) == [3, 7] + assert list(result.columns[1]) == [1.5, 2.5] + assert list(result.columns[3]) == [1, 0] diff --git a/python/tests/test_writer_handle.py b/python/tests/test_writer_handle.py index cbb95378..4078ad99 100644 --- a/python/tests/test_writer_handle.py +++ b/python/tests/test_writer_handle.py @@ -210,6 +210,36 @@ def test_a_writer_over_a_bytesio_finishes_a_sheet_left_open(): assert [row[0].value for row in workbook.sheets[0].rows()] == ["a"] +def test_a_writer_over_a_stream_finishes_a_partial_write(): + class Trickle: + def __init__(self): + self.data = bytearray() + + def write(self, chunk): + self.data += bytes(chunk[:5]) + return min(5, len(chunk)) + + destination = Trickle() + with open_writer(destination, "xlsx") as writer: + writer.start_sheet("Data") + writer.write_row(["widget", 7]) + + with open_workbook(bytes(destination.data)) as workbook: + assert [cell.value for cell in next(workbook.sheets[0].rows())] == ["widget", "7"] + + +def test_a_writer_over_a_stream_that_stalls_raises_and_still_closes(): + class Stalled: + def write(self, chunk): + return 0 + + writer = open_writer(Stalled(), "xlsx") + writer.start_sheet("Data") + with pytest.raises(OSError, match="no progress"): + writer.close() + writer.close() + + def test_a_writer_over_a_stream_needs_a_format_and_a_write_method(): import io diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 926a59f3..559f84c3 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -307,7 +307,7 @@ dependencies = [ "log", "quick-xml", "serde", - "zip", + "zip 8.6.0", ] [[package]] @@ -318,9 +318,9 @@ checksum = "37b2a672a2cb129a2e41c10b1224bb368f9f37a2b16b612598138befd7b37eb5" [[package]] name = "cc" -version = "1.4.4" +version = "1.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ad534f4357a5264cce5019c989cf66a4f0dc4e0d1b1d15f8aacec0ff7360273" +checksum = "6651c9ed80effdc7db0ff72512157f901af5e3549e341e24b1dd4887d836d838" dependencies = [ "find-msvc-tools", "shlex", @@ -332,6 +332,12 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" +[[package]] +name = "cfg_aliases" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f079e83a288787bcd14a6aea84cee5c87a67c5a3e660c30f557a3d24761b3527" + [[package]] name = "chrono" version = "0.4.45" @@ -553,7 +559,7 @@ version = "0.0.0" dependencies = [ "proc-macro2", "quote", - "syn 3.0.3", + "syn 3.0.9", ] [[package]] @@ -564,9 +570,9 @@ checksum = "c6e8948ce679d00a02a94739ea185595dca7118ed04feb991127e443bd3d761f" [[package]] name = "find-msvc-tools" -version = "0.1.11" +version = "0.1.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d45db016d36b838f563236e9193d0ee6ce38f3f68b6c94e914b4929c96bbb890" +checksum = "1c66915cc5391097de7a2b0b972e8505aa33c0df401fd3a6811dbdb32881ba48" [[package]] name = "flate2" @@ -800,6 +806,18 @@ dependencies = [ "simd-adler32", ] +[[package]] +name = "nix" +version = "0.31.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf20d2fde8ff38632c426f1165ed7436270b44f199fc55284c38276f9db47c3d" +dependencies = [ + "bitflags", + "cfg-if", + "cfg_aliases", + "libc", +] + [[package]] name = "num-bigint" version = "0.5.1" @@ -979,11 +997,11 @@ checksum = "d6f6ff9a378485b298a5286656da665ba74413d36db0979633275d2e708145d4" [[package]] name = "rust_xlsxwriter" -version = "0.99.1" +version = "0.100.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "34f2d2cda7223168a14fb86bc7411dbc3379bd7d49749ecc53b91ed98585b6b0" +checksum = "c44904ffb6c57b148ffcc8ce3011c28bc49b97e1a8343b2d632283cd566e1393" dependencies = [ - "zip", + "zip 9.0.5", ] [[package]] @@ -1034,7 +1052,7 @@ checksum = "e7a5d71263a5a7d47b41f6b3f06ba276f10cc18b0931f1799f710578e2309348" dependencies = [ "proc-macro2", "quote", - "syn 3.0.3", + "syn 3.0.9", ] [[package]] @@ -1090,9 +1108,9 @@ dependencies = [ [[package]] name = "syn" -version = "3.0.3" +version = "3.0.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "53e9bae58849f64dfa4f5d5ae372c8341f7305f82a3868709269343628b659a3" +checksum = "d78c8dee4c7bf0e14673097256fed6142ce9d3b85a408189d07482442145823b" dependencies = [ "proc-macro2", "quote", @@ -1100,10 +1118,10 @@ dependencies = [ ] [[package]] -name = "target-triple" -version = "1.0.1" +name = "target-tuple" +version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c3a6bfce3d99adfa72d24750a61f782f3036a81e7f86d8841ee1326deaebd171" +checksum = "876fef147edbcbddc8ac5cbbba92c7b86519e314e86638596c09673b2ed01e7f" [[package]] name = "termcolor" @@ -1174,15 +1192,15 @@ checksum = "7d56353a2a665ad0f41a421187180aab746c8c325620617ad883a99a1cbe66d2" [[package]] name = "trybuild" -version = "1.0.120" +version = "1.0.122" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e605bf6b39357663d8ba4e984f8be8da8df6bb32e81031d6889024ea8fd68e4" +checksum = "62db9c92d704393fbf2132041720cc80b689f2d3f28521015c2ac866223c11b8" dependencies = [ "glob", "serde", "serde_derive", "serde_json", - "target-triple", + "target-tuple", "termcolor", "toml", ] @@ -1430,6 +1448,21 @@ dependencies = [ "zopfli", ] +[[package]] +name = "zip" +version = "9.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e04976da629d3b04204ba8105b396a193af8ad2583b2c79bfb3ab8af580a11e" +dependencies = [ + "crc32fast", + "flate2", + "indexmap", + "memchr", + "nix", + "typed-path", + "zopfli", +] + [[package]] name = "zlib-rs" version = "0.6.7" diff --git a/rust/excelreader/Cargo.toml b/rust/excelreader/Cargo.toml index f8c8b5fb..bf0e9942 100644 --- a/rust/excelreader/Cargo.toml +++ b/rust/excelreader/Cargo.toml @@ -26,7 +26,7 @@ cc = "1" trybuild = "1" criterion = "0.8" calamine = { version = "0.36.1", features = ["chrono"] } -rust_xlsxwriter = "0.99.1" +rust_xlsxwriter = "0.100.0" [[bench]] name = "parse_bench" diff --git a/rust/excelreader/README.md b/rust/excelreader/README.md index 7d88134d..4dc6833c 100644 --- a/rust/excelreader/README.md +++ b/rust/excelreader/README.md @@ -103,6 +103,34 @@ for column in sheet.infer_schema(1, 100, false)? { `infer_schema`'s last argument, `parse_text`, also types text cells (every CSV field, or numbers stored as text): integers, decimals, `true`/`false` and ISO dates. +With no row type, `parse_with_schema` reads the sheet through that schema into a `Table`. Its +columns are the `Column`s `write_columns` takes, so the table writes back as it was read: + +```rust,no_run +use excelreader::{write_columns, ColumnData, ReadOptions, Workbook, WriteOptions}; + +let workbook = Workbook::open("book.xlsx")?; +let sheet = workbook.sheet(0)?; +let schema = sheet.infer_schema(1, 100, false)?; +let table = sheet.parse_with_schema(&schema, &ReadOptions::new())?; +for column in table.columns() { + if let ColumnData::I64(values) = column.data { + println!("{:?}: {} integers", column.name, values.len()); + } +} +write_columns("copy.xlsx", &table.columns(), &WriteOptions::new())?; + +// Or a batch at a time: TableBatches yields one Table per batch. +for batch in sheet.parse_batches_with_schema(&schema, &ReadOptions::new())? { + println!("{} rows", batch?.len()); +} +# Ok::<(), excelreader::Error>(()) +``` + +A column matched by index has no name, and writing needs every column named or none. The schema is +a guess over a sample: a column inferred as `I64` that holds a fraction further down fails the read, +unless the column is `nullable`. + `Workbook` is `Send + Sync`, so sheets can be read in parallel over one open workbook, which loads the shared-string table once: @@ -249,6 +277,28 @@ Same `batch_size` semantics as `parse_batches` below. Unlike `TypedBatches`, `ArrowBatches` does not fuse after an error — it follows arrow-rs's own semantics — so break on the first `Err` rather than continuing the loop. +With no row type, `to_arrow_with_schema` and `to_arrow_stream_with_schema` take the columns at run +time instead, such as the ones `infer_schema` returned. A column with a name is matched by it, one +without by its index: + +```rust,no_run +# #[cfg(feature = "arrow")] +# fn main() -> Result<(), excelreader::Error> { +use excelreader::{Workbook, ReadOptions}; + +let workbook = Workbook::open("book.xlsx")?; +let sheet = workbook.sheet(0)?; +let schema = sheet.infer_schema(1, 100, false)?; +let batch = sheet.to_arrow_with_schema(&schema, &ReadOptions::new())?; +# Ok(()) +# } +# #[cfg(not(feature = "arrow"))] +# fn main() {} +``` + +The schema is a guess over a sample: a column inferred as `I64` that holds a fraction further down +fails the read, unless the column is `nullable`. + ## Writing `#[derive(ExcelMapper)]` generates both halves, so the same struct reads and writes - the field diff --git a/rust/excelreader/src/arrow.rs b/rust/excelreader/src/arrow.rs index cbf54060..c1c9b668 100644 --- a/rust/excelreader/src/arrow.rs +++ b/rust/excelreader/src/arrow.rs @@ -13,16 +13,43 @@ use arrow::ffi::{from_ffi, FFI_ArrowArray, FFI_ArrowSchema}; use arrow::ffi_stream::{ArrowArrayStreamReader, FFI_ArrowArrayStream}; use arrow::record_batch::RecordBatchReader; -use crate::ffi; -use crate::workbook::{build_specs, check}; -use crate::{Error, ErrorKind, ExcelMapper, ReadOptions, Sheet, Workbook}; +use crate::ffi::{self, XlColumnSpec}; +use crate::workbook::{build_specs, check, schema_specs}; +use crate::{Error, ErrorKind, ExcelMapper, InferredColumn, ReadOptions, Sheet, Workbook}; impl<'w> Sheet<'w> { /// [`parse`](Self::parse) into one Arrow [`RecordBatch`], with the same `#[derive(ExcelMapper)]` /// mapping and [`ReadOptions`]. pub fn to_arrow(&self, options: &ReadOptions) -> Result { - let arena = build_specs::(); + self.arrow_from_specs(&build_specs::().specs, options) + } + + /// [`to_arrow`](Self::to_arrow) over a schema chosen at run time, such as one + /// [`infer_schema`](Self::infer_schema) returned. A column with a name is matched by it, one + /// without by its index; an empty schema is an error. + pub fn to_arrow_with_schema( + &self, + schema: &[InferredColumn], + options: &ReadOptions, + ) -> Result { + self.arrow_from_specs(&schema_specs(schema).specs, options) + } + + /// [`to_arrow_stream`](Self::to_arrow_stream) over a schema chosen at run time, as in + /// [`to_arrow_with_schema`](Self::to_arrow_with_schema). + pub fn to_arrow_stream_with_schema( + &self, + schema: &[InferredColumn], + options: &ReadOptions, + ) -> Result, Error> { + self.arrow_stream_from_specs(&schema_specs(schema).specs, options) + } + fn arrow_from_specs( + &self, + specs: &[XlColumnSpec], + options: &ReadOptions, + ) -> Result { let mut array = FFI_ArrowArray::empty(); let mut schema = FFI_ArrowSchema::empty(); @@ -30,8 +57,8 @@ impl<'w> Sheet<'w> { ffi::xl_parse_arrow( self.handle(), self.index(), - arena.specs.as_ptr(), - arena.specs.len() as i32, + specs.as_ptr(), + specs.len() as i32, options.header_row, options.parallelism, &mut array as *mut FFI_ArrowArray as *mut c_void, @@ -55,16 +82,22 @@ impl<'w> Sheet<'w> { &self, options: &ReadOptions, ) -> Result, Error> { - let arena = build_specs::(); + self.arrow_stream_from_specs(&build_specs::().specs, options) + } + fn arrow_stream_from_specs( + &self, + specs: &[XlColumnSpec], + options: &ReadOptions, + ) -> Result, Error> { let mut stream = FFI_ArrowArrayStream::empty(); check(unsafe { ffi::xl_parse_arrow_stream( self.handle(), self.index(), - arena.specs.as_ptr(), - arena.specs.len() as i32, + specs.as_ptr(), + specs.len() as i32, options.header_row, options.batch_size, &mut stream as *mut FFI_ArrowArrayStream as *mut c_void, diff --git a/rust/excelreader/src/ffi.rs b/rust/excelreader/src/ffi.rs index 9b5c8a43..80a922e9 100644 --- a/rust/excelreader/src/ffi.rs +++ b/rust/excelreader/src/ffi.rs @@ -148,8 +148,8 @@ pub struct XlColumnSpec { pub nullable: i32, } -/// One column of a native table. Reachable from `__private` because the derive's generated -/// bindings read it through the `column_*` accessors. +/// One column of a native table. Crate-private: the derive's generated bindings only ever see it +/// wrapped in a `ColumnRef`, which nothing outside this crate can construct. #[repr(C)] pub struct XlColumn { pub r#type: i32, diff --git a/rust/excelreader/src/lib.rs b/rust/excelreader/src/lib.rs index d7804774..fed98411 100644 --- a/rust/excelreader/src/lib.rs +++ b/rust/excelreader/src/lib.rs @@ -26,8 +26,8 @@ pub use rows::{AllRows, CellIter, CellRef, CellType, RowCursor, RowRef}; pub use source::Source; pub use temporal::{Date, Time, Timestamp}; pub use workbook::{ - ColumnType, ExcelMapper, InferredColumn, Sheet, SheetVisibility, TableView, TableViewIter, - TypedBatches, Workbook, + ColumnType, ExcelMapper, InferredColumn, Sheet, SheetVisibility, Table, TableBatches, + TableView, TableViewIter, TypedBatches, Workbook, }; pub use writer::{ encrypt_package, write_columns, write_columns_to, write_sheet, write_sheet_to, Column, @@ -42,10 +42,9 @@ pub use self::arrow::ArrowBatches; /// follows semver. #[doc(hidden)] pub mod __private { - pub use crate::ffi::XlColumn; pub use crate::workbook::{ column_bool, column_date, column_f64, column_i64, column_str, column_time, - column_timestamp, ColumnBinding, + column_timestamp, ColumnBinding, ColumnRef, }; pub use crate::writer::{push_str, set_valid, OwnedColumn, OwnedColumnData}; diff --git a/rust/excelreader/src/workbook.rs b/rust/excelreader/src/workbook.rs index 4071abf6..6148fb51 100644 --- a/rust/excelreader/src/workbook.rs +++ b/rust/excelreader/src/workbook.rs @@ -3,7 +3,8 @@ use crate::ffi::{ XL_BUFFER_TOO_SMALL, XL_EOF, XL_OK, }; use crate::{ - AllRows, Date, Error, ErrorKind, OpenOptions, ReadOptions, RowCursor, RowRef, Time, Timestamp, + AllRows, Column, ColumnData, Date, Error, ErrorKind, OpenOptions, ReadOptions, RowCursor, + RowRef, Time, Timestamp, }; use std::marker::PhantomData; use std::path::Path; @@ -410,6 +411,59 @@ impl<'w> Sheet<'w> { TableView::new(table, arena.bindings, "xl_parse_typed") } + /// [`parse`](Self::parse) over a schema chosen at run time, such as one + /// [`infer_schema`](Self::infer_schema) returned. A column with a name is matched by it, one + /// without by its index; an empty schema is an error. + pub fn parse_with_schema( + &self, + schema: &[InferredColumn], + options: &ReadOptions, + ) -> Result { + let specs = schema_specs(schema); + let mut table = empty_table(); + check(unsafe { + ffi::xl_parse_typed( + self.workbook.handle, + self.index, + specs.specs.as_ptr(), + specs.specs.len() as i32, + options.header_row, + options.parallelism, + &mut table, + ) + })?; + Table::new(table, schema, "xl_parse_typed") + } + + /// [`parse_with_schema`](Self::parse_with_schema) delivered [`ReadOptions`]'s batch size rows + /// at a time, as [`parse_batches`](Self::parse_batches) does. `schema` is copied, so it need not + /// outlive the batches. + pub fn parse_batches_with_schema( + &self, + schema: &[InferredColumn], + options: &ReadOptions, + ) -> Result, Error> { + let specs = schema_specs(schema); + let mut reader: *mut ffi::XlTypedReader = std::ptr::null_mut(); + check(unsafe { + ffi::xl_typed_reader_open( + self.workbook.handle, + self.index, + specs.specs.as_ptr(), + specs.specs.len() as i32, + options.header_row, + options.batch_size, + &mut reader, + ) + })?; + Ok(TableBatches { + reader, + schema: schema.to_vec(), + done: false, + _workbook: PhantomData, + }) + } + /// [`parse`](Self::parse) delivered [`ReadOptions`]'s batch size rows at a time; a negative /// batch size is an error. The header row is consumed once, here, not re-read per batch. pub fn parse_batches( @@ -614,7 +668,7 @@ pub struct ColumnBinding { /// Candidate header names, in priority order — the first one present in the header row wins. pub names: &'static [&'static str], pub column_type: ColumnType, - pub assign: fn(&mut T, &XlColumn, i64), + pub assign: fn(&mut T, ColumnRef<'_>, i64), } // Hand-written: `#[derive(Clone)]` would add a `T: Clone` bound the user's row struct need not meet. @@ -635,6 +689,21 @@ pub trait ExcelMapper: Sized { pub use excelreader_derive::ExcelMapper; +/// A column of a table the native library returned. Only this crate can build one, which is what +/// lets the safe `column_*` accessors trust its pointers: a hand-built `XlColumn` never reaches them. +/// +/// ```compile_fail +/// #![forbid(unsafe_code)] +/// let column = excelreader::__private::XlColumn { +/// r#type: 1, length: 1, +/// values: std::ptr::null(), validity: std::ptr::null(), +/// data: std::ptr::null(), data_len: 0, +/// }; +/// excelreader::__private::column_i64(excelreader::__private::ColumnRef(&column), 0); +/// ``` +#[derive(Clone, Copy)] +pub struct ColumnRef<'a>(&'a XlColumn); + fn is_valid(col: &XlColumn, row: i64) -> bool { if col.validity.is_null() { return true; @@ -674,7 +743,8 @@ fn check_access(col: &XlColumn, row: i64, expected_type: i32, accessor: &str) { /// # Panics /// If `col` is not a string column, if `row` is out of bounds, or if the native library returned /// bytes that are not valid UTF-8 (an ABI contract violation). -pub fn column_str(col: &XlColumn, row: i64) -> &str { +pub fn column_str<'a>(col: ColumnRef<'a>, row: i64) -> &'a str { + let col = col.0; check_access(col, row, ffi::XL_T_STRING, "column_str"); unsafe { let offsets = col.values as *const i32; @@ -689,28 +759,32 @@ pub fn column_str(col: &XlColumn, row: i64) -> &str { /// # Panics /// If `col` is not an `i64` column, or `row` is out of bounds. -pub fn column_i64(col: &XlColumn, row: i64) -> i64 { +pub fn column_i64(col: ColumnRef<'_>, row: i64) -> i64 { + let col = col.0; check_access(col, row, ffi::XL_T_I64, "column_i64"); unsafe { *(col.values as *const i64).offset(row as isize) } } /// # Panics /// If `col` is not an `f64` column, or `row` is out of bounds. -pub fn column_f64(col: &XlColumn, row: i64) -> f64 { +pub fn column_f64(col: ColumnRef<'_>, row: i64) -> f64 { + let col = col.0; check_access(col, row, ffi::XL_T_F64, "column_f64"); unsafe { *(col.values as *const f64).offset(row as isize) } } /// # Panics /// If `col` is not a bool column, or `row` is out of bounds. -pub fn column_bool(col: &XlColumn, row: i64) -> bool { +pub fn column_bool(col: ColumnRef<'_>, row: i64) -> bool { + let col = col.0; check_access(col, row, ffi::XL_T_BOOL, "column_bool"); unsafe { *(col.values as *const u8).offset(row as isize) != 0 } } /// # Panics /// If `col` is not a date column, or `row` is out of bounds. -pub fn column_date(col: &XlColumn, row: i64) -> Date { +pub fn column_date(col: ColumnRef<'_>, row: i64) -> Date { + let col = col.0; check_access(col, row, ffi::XL_T_DATE, "column_date"); Date { days_since_epoch: unsafe { *(col.values as *const i32).offset(row as isize) }, @@ -719,7 +793,8 @@ pub fn column_date(col: &XlColumn, row: i64) -> Date { /// # Panics /// If `col` is not a time column, or `row` is out of bounds. -pub fn column_time(col: &XlColumn, row: i64) -> Time { +pub fn column_time(col: ColumnRef<'_>, row: i64) -> Time { + let col = col.0; check_access(col, row, ffi::XL_T_TIME, "column_time"); Time { micros_since_midnight: unsafe { *(col.values as *const i64).offset(row as isize) }, @@ -728,7 +803,8 @@ pub fn column_time(col: &XlColumn, row: i64) -> Time { /// # Panics /// If `col` is not a timestamp column, or `row` is out of bounds. -pub fn column_timestamp(col: &XlColumn, row: i64) -> Timestamp { +pub fn column_timestamp(col: ColumnRef<'_>, row: i64) -> Timestamp { + let col = col.0; check_access(col, row, ffi::XL_T_TIMESTAMP, "column_timestamp"); Timestamp { micros_since_epoch: unsafe { *(col.values as *const i64).offset(row as isize) }, @@ -803,7 +879,7 @@ impl TableView { }; for (col, binding) in columns.iter().zip(self.bindings.iter()) { if is_valid(col, row) { - (binding.assign)(&mut instance, col, row); + (binding.assign)(&mut instance, ColumnRef(col), row); } } Some(instance) @@ -841,6 +917,126 @@ impl std::fmt::Debug for TableView { } } +/// Owns a [`Sheet::parse_with_schema`] result. Frees the native table on `Drop`. +/// +/// Its columns are the [`Column`]s [`write_columns`](crate::write_columns) takes, borrowed from +/// this table. A column matched by index has no name, and writing needs every column named or none. +pub struct Table { + table: XlTable, + names: Vec>, +} + +impl Table { + /// Takes ownership of `table`, freeing it and failing if it does not have one column of a known + /// type and of `row_count` rows per `schema` entry. + fn new(mut table: XlTable, schema: &[InferredColumn], source: &str) -> Result { + let columns = if table.columns.is_null() { + &[][..] + } else { + unsafe { std::slice::from_raw_parts(table.columns, table.column_count.max(0) as usize) } + }; + let consistent = table.row_count >= 0 + && columns.len() == schema.len() + && columns + .iter() + .all(|c| c.length == table.row_count && ColumnType::from_raw(c.r#type).is_some()); + if !consistent { + unsafe { ffi::xl_free_table(&mut table) }; + return Err(Error::new( + ErrorKind::Other, + format!( + "{source} returned a table that does not match its {} specs", + schema.len() + ), + )); + } + Ok(Table { + table, + names: schema.iter().map(|c| c.name.clone()).collect(), + }) + } + + pub fn len(&self) -> i64 { + self.table.row_count + } + + pub fn is_empty(&self) -> bool { + self.table.row_count == 0 + } + + pub fn num_columns(&self) -> usize { + self.names.len() + } + + /// Column `index`, or `None` past the last one. + pub fn column(&self, index: usize) -> Option> { + let name = self.names.get(index)?; + let raw = unsafe { &*self.table.columns.add(index) }; + Some(Column { + name: name.as_deref(), + data: unsafe { column_data(raw) }, + validity: (!raw.validity.is_null()) + .then(|| unsafe { native_slice(raw.validity, (raw.length as usize).div_ceil(8)) }), + }) + } + + /// Every column, in schema order. + pub fn columns(&self) -> Vec> { + (0..self.num_columns()) + .filter_map(|i| self.column(i)) + .collect() + } +} + +/// # Safety +/// `ptr` must be null or point to `len` live `E`s that outlive `'a`. +unsafe fn native_slice<'a, E>(ptr: *const E, len: usize) -> &'a [E] { + if ptr.is_null() || len == 0 { + return &[]; + } + std::slice::from_raw_parts(ptr, len) +} + +/// # Safety +/// `raw` must be a column of a native table whose type [`Table::new`] checked. +unsafe fn column_data(raw: &XlColumn) -> ColumnData<'_> { + let rows = raw.length as usize; + match ColumnType::from_raw(raw.r#type) { + Some(ColumnType::String) => ColumnData::Str { + offsets: native_slice(raw.values.cast::(), rows + 1), + data: native_slice(raw.data, raw.data_len as usize), + }, + Some(ColumnType::I64) => ColumnData::I64(native_slice(raw.values.cast(), rows)), + Some(ColumnType::F64) => ColumnData::F64(native_slice(raw.values.cast(), rows)), + Some(ColumnType::Bool) => ColumnData::Bool(native_slice(raw.values.cast(), rows)), + Some(ColumnType::Date) => ColumnData::Date(native_slice(raw.values.cast(), rows)), + Some(ColumnType::Time) => ColumnData::Time(native_slice(raw.values.cast(), rows)), + Some(ColumnType::Timestamp) => ColumnData::Timestamp(native_slice(raw.values.cast(), rows)), + None => unreachable!("Table::new rejects unknown column types"), + } +} + +impl Drop for Table { + fn drop(&mut self) { + unsafe { + ffi::xl_free_table(&mut self.table); + } + } +} + +// SAFETY: as for TableView. +unsafe impl Send for Table {} +unsafe impl Sync for Table {} + +impl std::fmt::Debug for Table { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Table") + .field("rows", &self.table.row_count) + .field("columns", &self.names) + .finish_non_exhaustive() + } +} + /// Iterator over the batches of one chunked typed read. Closes the native reader on `Drop`. pub struct TypedBatches<'a, T: ExcelMapper> { reader: *mut ffi::XlTypedReader, @@ -895,6 +1091,57 @@ impl std::fmt::Debug for TypedBatches<'_, T> { } } +/// [`TypedBatches`]' counterpart for [`Sheet::parse_batches_with_schema`], yielding [`Table`]s. +pub struct TableBatches<'a> { + reader: *mut ffi::XlTypedReader, + schema: Vec, + done: bool, + _workbook: PhantomData<&'a Workbook>, +} + +// SAFETY: as for TypedBatches. +unsafe impl Send for TableBatches<'_> {} + +impl Iterator for TableBatches<'_> { + type Item = Result; + + fn next(&mut self) -> Option { + if self.done { + return None; + } + + let mut table = empty_table(); + let status = unsafe { ffi::xl_typed_reader_next(self.reader, &mut table) }; + if status == XL_EOF { + self.done = true; + return None; + } + if status != XL_OK { + self.done = true; + return Some(Err(last_error(status))); + } + + let batch = Table::new(table, &self.schema, "xl_typed_reader_next"); + self.done = batch.is_err(); + Some(batch) + } +} + +impl Drop for TableBatches<'_> { + fn drop(&mut self) { + unsafe { ffi::xl_typed_reader_close(self.reader) }; + } +} + +impl std::fmt::Debug for TableBatches<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("TableBatches") + .field("columns", &self.schema.len()) + .field("done", &self.done) + .finish_non_exhaustive() + } +} + pub struct TableViewIter<'a, T: ExcelMapper> { view: &'a TableView, row: i64, @@ -962,3 +1209,43 @@ pub(crate) fn build_specs() -> SpecArena { _name_lens: name_lens, } } + +/// [`SpecArena`] for a schema chosen at run time: `specs` points into `schema`'s names and into the +/// two `_name_*` vectors, so all three must outlive the native call. +pub(crate) struct SchemaSpecs<'a> { + pub(crate) specs: Vec, + _name_ptrs: Vec<*const u8>, + _name_lens: Vec, + _schema: PhantomData<&'a [InferredColumn]>, +} + +/// Lowers a runtime schema into `xl_column_spec`s: a column with a name is matched by it, one +/// without by its index. +pub(crate) fn schema_specs(schema: &[InferredColumn]) -> SchemaSpecs<'_> { + let name_ptrs: Vec<*const u8> = schema + .iter() + .map(|c| c.name.as_deref().map_or(std::ptr::null(), str::as_ptr)) + .collect(); + let name_lens: Vec = schema + .iter() + .map(|c| c.name.as_deref().map_or(0, |n| n.len() as i32)) + .collect(); + let specs: Vec = schema + .iter() + .enumerate() + .map(|(i, c)| XlColumnSpec { + names: &name_ptrs[i], + name_lens: &name_lens[i], + name_count: i32::from(c.name.is_some()), + index: c.index, + r#type: c.column_type.raw(), + nullable: i32::from(c.nullable), + }) + .collect(); + SchemaSpecs { + specs, + _name_ptrs: name_ptrs, + _name_lens: name_lens, + _schema: PhantomData, + } +} diff --git a/rust/excelreader/tests/derive_ui/pass/forbid_unsafe.rs b/rust/excelreader/tests/derive_ui/pass/forbid_unsafe.rs new file mode 100644 index 00000000..f7043913 --- /dev/null +++ b/rust/excelreader/tests/derive_ui/pass/forbid_unsafe.rs @@ -0,0 +1,21 @@ +#![forbid(unsafe_code)] + +use excelreader::{Date, ExcelMapper, Time, Timestamp}; + +#[derive(Default, ExcelMapper)] +struct Row { + #[excel(name = "Nome")] + nome: String, + #[excel(name = "Idade")] + idade: Option, + #[excel(name = "Dia")] + dia: Date, + #[excel(name = "Hora")] + hora: Time, + #[excel(name = "Instante")] + instante: Option, +} + +fn main() { + assert_eq!(Row::bindings().len(), 5); +} diff --git a/rust/excelreader/tests/parse_arrow.rs b/rust/excelreader/tests/parse_arrow.rs index 698d0b13..77af7f48 100644 --- a/rust/excelreader/tests/parse_arrow.rs +++ b/rust/excelreader/tests/parse_arrow.rs @@ -174,3 +174,71 @@ fn parallel_parse_arrow_matches_the_sequential_row_count() { drop(workbook); let _ = std::fs::remove_file(path); } + +#[test] +fn an_inferred_schema_reads_into_arrow_without_a_row_type() { + let workbook = Workbook::open(fixture_path()).expect("open must succeed"); + let sheet = workbook.sheet(0).expect("sheet 0"); + let schema = sheet + .infer_schema(1, 100, false) + .expect("infer_schema must succeed"); + + let batch = sheet + .to_arrow_with_schema(&schema, &ReadOptions::new()) + .expect("to_arrow_with_schema must succeed"); + + assert_eq!(batch.num_columns(), schema.len()); + assert_eq!(batch.num_rows(), 100); + assert_eq!(batch.schema().field(0).name(), "Coluna1"); + let coluna3 = batch.column_by_name("Coluna3").expect("Coluna3 is read"); + assert!(coluna3.as_any().downcast_ref::().is_some()); +} + +#[test] +fn a_schema_without_names_resolves_columns_by_index() { + let workbook = Workbook::open(fixture_path()).expect("open must succeed"); + let sheet = workbook.sheet(0).expect("sheet 0"); + let schema = sheet + .infer_schema(0, 100, false) + .expect("infer_schema must succeed"); + assert!(schema.iter().all(|c| c.name.is_none())); + + let batch = sheet + .to_arrow_with_schema(&schema, &ReadOptions::new().header_row(0)) + .expect("to_arrow_with_schema must succeed"); + + assert_eq!(batch.num_columns(), schema.len()); + assert_eq!( + batch.num_rows(), + 101, + "with no header the header row is data" + ); +} + +#[test] +fn an_empty_schema_is_rejected() { + let workbook = Workbook::open(fixture_path()).expect("open must succeed"); + assert!(workbook + .sheet(0) + .expect("sheet 0") + .to_arrow_with_schema(&[], &ReadOptions::new()) + .is_err()); +} + +#[test] +fn an_inferred_schema_streams_every_row() { + let workbook = Workbook::open(fixture_path()).expect("open must succeed"); + let sheet = workbook.sheet(0).expect("sheet 0"); + let schema = sheet + .infer_schema(1, 100, false) + .expect("infer_schema must succeed"); + + let stream = sheet + .to_arrow_stream_with_schema(&schema, &ReadOptions::new().batch_size(8)) + .expect("the stream must open"); + assert_eq!(stream.schema().fields().len(), schema.len()); + let rows: usize = stream + .map(|batch| batch.expect("a batch must read").num_rows()) + .sum(); + assert_eq!(rows, 100); +} diff --git a/rust/excelreader/tests/parse_typed.rs b/rust/excelreader/tests/parse_typed.rs index 00032d05..33c0381c 100644 --- a/rust/excelreader/tests/parse_typed.rs +++ b/rust/excelreader/tests/parse_typed.rs @@ -1,4 +1,4 @@ -use excelreader::ReadOptions; +use excelreader::{write_columns_to, ColumnData, ReadOptions, Table, WriteOptions}; use excelreader::{ColumnType, Date, ExcelMapper, Format, OpenOptions, Workbook}; #[derive(Default, ExcelMapper)] @@ -355,3 +355,101 @@ fn parallel_parse_matches_the_sequential_parse() { drop(workbook); let _ = std::fs::remove_file(path); } + +fn column_by_name<'t>(table: &'t Table, name: &str) -> ColumnData<'t> { + table + .columns() + .into_iter() + .find(|c| c.name == Some(name)) + .unwrap_or_else(|| panic!("{name} must be read")) + .data +} + +#[test] +fn an_inferred_schema_parses_without_a_row_type() { + let workbook = open_fixture(); + let sheet = workbook.sheet(0).expect("sheet 0"); + let schema = sheet + .infer_schema(1, 100, false) + .expect("infer_schema must succeed"); + + let table = sheet + .parse_with_schema(&schema, &ReadOptions::new()) + .expect("parse_with_schema must succeed"); + + assert_eq!(table.len(), 100); + assert_eq!(table.num_columns(), schema.len()); + assert!(table.column(schema.len()).is_none()); + let ColumnData::Str { offsets, data } = column_by_name(&table, "Coluna1") else { + panic!("Coluna1 must be a string column"); + }; + assert_eq!(&data[offsets[0] as usize..offsets[1] as usize], b"Valor1"); + let ColumnData::I64(coluna3) = column_by_name(&table, "Coluna3") else { + panic!("Coluna3 must be an i64 column"); + }; + assert_eq!(coluna3.len(), 100); + assert_eq!(coluna3[0], 1); +} + +#[test] +fn a_schema_without_names_parses_by_index() { + let workbook = open_fixture(); + let sheet = workbook.sheet(0).expect("sheet 0"); + let schema = sheet + .infer_schema(0, 100, false) + .expect("infer_schema must succeed"); + + let table = sheet + .parse_with_schema(&schema, &ReadOptions::new().header_row(0)) + .expect("parse_with_schema must succeed"); + + assert_eq!(table.len(), 101, "with no header the header row is data"); + assert!(table.columns().iter().all(|c| c.name.is_none())); +} + +#[test] +fn parse_with_schema_rejects_an_empty_schema() { + let workbook = open_fixture(); + assert!(workbook + .sheet(0) + .expect("sheet 0") + .parse_with_schema(&[], &ReadOptions::new()) + .is_err()); +} + +#[test] +fn a_parsed_table_writes_back_unchanged() { + let workbook = open_fixture(); + let sheet = workbook.sheet(0).expect("sheet 0"); + let schema = sheet + .infer_schema(1, 100, false) + .expect("infer_schema must succeed"); + let table = sheet + .parse_with_schema(&schema, &ReadOptions::new()) + .expect("parse_with_schema must succeed"); + + let mut bytes = Vec::new(); + write_columns_to( + &mut bytes, + &table.columns(), + &WriteOptions::new().format(Format::Xlsx), + ) + .expect("a parsed table must write"); + + let reopened = OpenOptions::new() + .open_bytes(&bytes) + .expect("the written workbook must open"); + let again = reopened + .sheet(0) + .expect("sheet 0") + .parse_with_schema(&schema, &ReadOptions::new()) + .expect("the written workbook must parse with the same schema"); + assert_eq!(again.len(), table.len()); + let (ColumnData::I64(before), ColumnData::I64(after)) = ( + column_by_name(&table, "Coluna3"), + column_by_name(&again, "Coluna3"), + ) else { + panic!("Coluna3 must stay an i64 column"); + }; + assert_eq!(before, after); +} diff --git a/rust/excelreader/tests/typed_reader.rs b/rust/excelreader/tests/typed_reader.rs index 0a340a29..c0962bb5 100644 --- a/rust/excelreader/tests/typed_reader.rs +++ b/rust/excelreader/tests/typed_reader.rs @@ -1,5 +1,5 @@ use excelreader::ReadOptions; -use excelreader::{ErrorKind, ExcelMapper, TypedBatches, Workbook}; +use excelreader::{ColumnData, ErrorKind, ExcelMapper, TableBatches, TypedBatches, Workbook}; #[derive(Default, ExcelMapper)] struct Record { @@ -123,3 +123,58 @@ fn two_readers_on_one_workbook_each_yield_every_row() { assert_eq!(count(one), 100); assert_eq!(count(two), 100); } + +fn schema_batches(workbook: &Workbook, batch_size: i64) -> TableBatches<'_> { + let sheet = workbook.sheet(0).expect("sheet 0"); + let schema = sheet + .infer_schema(1, 100, false) + .expect("infer_schema must succeed"); + sheet + .parse_batches_with_schema(&schema, &ReadOptions::new().batch_size(batch_size)) + .expect("the batches must open") +} + +#[test] +fn schema_batches_equal_the_whole_sheet() { + let expected: Vec = whole_sheet().into_iter().map(|(_, n)| n).collect(); + for batch_size in [1, 7, 8, 9, 1000] { + let workbook = open(); + let mut coluna3 = Vec::new(); + for batch in schema_batches(&workbook, batch_size) { + let batch = batch.expect("a batch must read"); + assert!(batch.len() <= batch_size); + let column = batch + .columns() + .into_iter() + .find(|c| c.name == Some("Coluna3")) + .expect("every batch keeps the schema's names"); + let ColumnData::I64(values) = column.data else { + panic!("Coluna3 must stay an i64 column"); + }; + coluna3.extend_from_slice(values); + } + assert_eq!(coluna3, expected, "batch size {batch_size}"); + } +} + +#[test] +fn schema_batches_with_batch_size_zero_yield_one_batch() { + let workbook = open(); + let batches: Vec<_> = schema_batches(&workbook, 0) + .map(|b| b.expect("a batch must read")) + .collect(); + assert_eq!(batches.len(), 1); + assert_eq!(batches[0].len(), 100); +} + +#[test] +fn schema_batches_reject_a_negative_batch_size() { + let workbook = open(); + let sheet = workbook.sheet(0).expect("sheet 0"); + let schema = sheet + .infer_schema(1, 100, false) + .expect("infer_schema must succeed"); + assert!(sheet + .parse_batches_with_schema(&schema, &ReadOptions::new().batch_size(-1)) + .is_err()); +} diff --git a/src/ExcelReader.Core/Parser/Internal/ProjectionRules.cs b/src/ExcelReader.Core/Parser/Internal/ProjectionRules.cs index 3bc6b544..3ea05966 100644 --- a/src/ExcelReader.Core/Parser/Internal/ProjectionRules.cs +++ b/src/ExcelReader.Core/Parser/Internal/ProjectionRules.cs @@ -20,5 +20,12 @@ internal static ExcelParseException MissingRequiredValue(string name, int row) { return new ExcelParseException(row, name); } + + internal static ExcelParseException AtRow(ExcelParseException original, long row) + { + return original.RawValue.Length == 0 + ? MissingRequiredValue(original.ColumnName, (int)row) + : new ExcelParseException((int)row, original.ColumnName, original.RawValue); + } } } diff --git a/src/ExcelReader.Core/Parser/ParallelCsv/CsvBoundaryResolver.cs b/src/ExcelReader.Core/Parser/ParallelCsv/CsvBoundaryResolver.cs index 577f03c9..56f0d199 100644 --- a/src/ExcelReader.Core/Parser/ParallelCsv/CsvBoundaryResolver.cs +++ b/src/ExcelReader.Core/Parser/ParallelCsv/CsvBoundaryResolver.cs @@ -20,6 +20,68 @@ internal static bool StartsRecord(ReadOnlySpan pair) return pair[0] == Cr && pair[1] != Lf; } + // Reads the quote state at window[start] off the first odd run of quotes there, starting from the whole of + // a run that start cuts; even runs leave the parity alone. Under RFC 4180 a run that opens a field follows a + // delimiter or a line start, and one that closes a field precedes a delimiter, a line end or the end of the + // source, so a run that fits only one of the two decides. A run that fits both is null, as is one the + // window cuts off: only the confirmed start can place that chunk. No run, or one that fits neither, is + // guessed outside, and the confirmation after the parse still checks it. + internal static CsvQuoteParity? InferParity( + ReadOnlySpan window, int start, bool startsAtSource, bool endsAtEof, byte quote, byte delimiter) + { + int runStart = start; + while (runStart > 0 && window[runStart - 1] == quote) + { + runStart--; + } + if (runStart == 0 && start > 0 && !startsAtSource) + { + return null; + } + bool flipped = ((start - runStart) & 1) == 1; + int i = runStart; + if (runStart == start) + { + int found = window[start..].IndexOf(quote); + i = found < 0 ? -1 : start + found; + } + while (i >= 0) + { + int end = i + 1; + while (end < window.Length && window[end] == quote) + { + end++; + } + if (((end - i) & 1) == 0) + { + int next = window[end..].IndexOf(quote); + i = next < 0 ? -1 : end + next; + continue; + } + if (end == window.Length && !endsAtEof) + { + return flipped ? null : CsvQuoteParity.Outside; + } + if (i == 0 && !startsAtSource) + { + return null; + } + bool closes = end == window.Length || EndsField(window[end], delimiter); + bool opens = i == 0 || EndsField(window[i - 1], delimiter); + if (closes && opens) + { + return null; + } + return closes != flipped ? CsvQuoteParity.Inside : CsvQuoteParity.Outside; + } + return flipped ? null : CsvQuoteParity.Outside; + } + + private static bool EndsField(byte b, byte delimiter) + { + return b == delimiter || b == Lf || b == Cr; + } + internal static int FindRecordStart(ReadOnlySpan window, byte quote, CsvQuoteParity parity) { bool inside = parity == CsvQuoteParity.Inside; diff --git a/src/ExcelReader.Core/Parser/ParallelCsv/CsvChunkWorker.cs b/src/ExcelReader.Core/Parser/ParallelCsv/CsvChunkWorker.cs index 347cb8a5..1e6c768a 100644 --- a/src/ExcelReader.Core/Parser/ParallelCsv/CsvChunkWorker.cs +++ b/src/ExcelReader.Core/Parser/ParallelCsv/CsvChunkWorker.cs @@ -105,6 +105,8 @@ internal CsvChunkResult(int index, List models, long actualStart, long resolv internal static class CsvChunkWorker { private const int InitialBoundaryWindow = 4 * 1024; + private const int InferenceWindow = 64 * 1024; + private const int InferenceLookBehind = 64; private const int BoundaryWindowGrowth = 8; internal static ValueTask> ParseAsync( @@ -119,7 +121,7 @@ internal static ValueTask> ParseAsync( Utf8StringCache? contentCache, CancellationToken ct) { - long start = confirmedStart ?? GuessStart(source, chunk, readerOptions.Quote); + long start = confirmedStart ?? GuessStart(source, chunk, readerOptions.Quote, readerOptions.Delimiter); if (start >= source.Length) { return new ValueTask>(new CsvChunkResult(chunk.Index, models, start, long.MaxValue)); @@ -213,15 +215,16 @@ private static async ValueTask> ParseFromAsync( return (resolvedNextStart, failure, records); } - internal static long GuessStart(CsvChunkSource source, CsvChunk chunk, byte quote) + // long.MaxValue when the chunk holds no record start, or when its quote state cannot be told from its + // own bytes; either way the merge reads it from the confirmed start. + internal static long GuessStart(CsvChunkSource source, CsvChunk chunk, byte quote, byte delimiter) { long chunkLength = chunk.End - chunk.Start; - if (chunkLength <= 0) + if (chunkLength <= 0 || InferParity(source, chunk.Start, quote, delimiter) is not CsvQuoteParity parity) { return long.MaxValue; } - - if (chunk.Start > 0 && AlreadyAtRecordStart(source, chunk.Start)) + if (parity == CsvQuoteParity.Outside && AlreadyAtRecordStart(source, chunk.Start)) { return chunk.Start; } @@ -229,8 +232,7 @@ internal static long GuessStart(CsvChunkSource source, CsvChunk chunk, byte quot if (source.IsMemory) { int cap = (int)Math.Min(chunkLength, int.MaxValue - 1); - int found = CsvBoundaryResolver.FindRecordStart( - source.WindowSpan(chunk.Start, cap), quote, CsvQuoteParity.Outside); + int found = CsvBoundaryResolver.FindRecordStart(source.WindowSpan(chunk.Start, cap), quote, parity); return found < 0 ? long.MaxValue : chunk.Start + found; } @@ -247,8 +249,7 @@ internal static long GuessStart(CsvChunkSource source, CsvChunk chunk, byte quot { return long.MaxValue; } - int found = CsvBoundaryResolver.FindRecordStart( - buffer.AsSpan(0, read), quote, CsvQuoteParity.Outside); + int found = CsvBoundaryResolver.FindRecordStart(buffer.AsSpan(0, read), quote, parity); if (found >= 0) { return chunk.Start + found; @@ -266,8 +267,42 @@ internal static long GuessStart(CsvChunkSource source, CsvChunk chunk, byte quot } } + // The window reaches back over a run of quotes the start may cut, and past the chunk's end when it has to: + // it only serves to find the first run of quotes. + private static CsvQuoteParity? InferParity(CsvChunkSource source, long start, byte quote, byte delimiter) + { + if (start <= 0) + { + return CsvQuoteParity.Outside; + } + int back = (int)Math.Min(InferenceLookBehind, start); + int size = back + (int)Math.Min(InferenceWindow, source.Length - start); + bool startsAtSource = start == back; + if (source.IsMemory) + { + ReadOnlySpan window = source.WindowSpan(start - back, size)[..size]; + return CsvBoundaryResolver.InferParity(window, back, startsAtSource, start - back + size == source.Length, quote, delimiter); + } + byte[] buffer = ArrayPool.Shared.Rent(size); + try + { + int read = source.ReadWindow(start - back, buffer.AsSpan(0, size)); + return read <= back + ? CsvQuoteParity.Outside + : CsvBoundaryResolver.InferParity(buffer.AsSpan(0, read), back, startsAtSource, start - back + read == source.Length, quote, delimiter); + } + finally + { + ArrayPool.Shared.Return(buffer); + } + } + private static bool AlreadyAtRecordStart(CsvChunkSource source, long start) { + if (start <= 0) + { + return true; + } Span pair = stackalloc byte[2]; return source.ReadWindow(start - 1, pair) == pair.Length && CsvBoundaryResolver.StartsRecord(pair); } diff --git a/src/ExcelReader.Core/Parser/ParallelCsv/ParallelCsvEnumerable.cs b/src/ExcelReader.Core/Parser/ParallelCsv/ParallelCsvEnumerable.cs index e6b6d84b..dc1d54e6 100644 --- a/src/ExcelReader.Core/Parser/ParallelCsv/ParallelCsvEnumerable.cs +++ b/src/ExcelReader.Core/Parser/ParallelCsv/ParallelCsvEnumerable.cs @@ -101,7 +101,7 @@ private long AdvanceOrThrow(CsvChunkResult result, long recordsBefore) { if (result.Failure is not null) { - throw RenumberFailure(result.Failure, _config.HeaderRow + recordsBefore + result.Records + 1); + throw ProjectionRules.AtRow(result.Failure, _config.HeaderRow + recordsBefore + result.Records + 1); } return result.ResolvedNextStart; } @@ -214,14 +214,5 @@ private static async Task WaitForChunkAsync(Task chunkReady, Task allWorke } await chunkReady.ConfigureAwait(false); } - - private static ExcelParseException RenumberFailure(ExcelParseException original, long globalRow) - { - if (original.RawValue.Length == 0) - { - return ProjectionRules.MissingRequiredValue(original.ColumnName, (int)globalRow); - } - return new ExcelParseException((int)globalRow, original.ColumnName, original.RawValue); - } } } diff --git a/src/ExcelReader.Core/Parser/ParallelCsv/ParallelCsvProcessor.cs b/src/ExcelReader.Core/Parser/ParallelCsv/ParallelCsvProcessor.cs index fe054d8a..0c6e3a89 100644 --- a/src/ExcelReader.Core/Parser/ParallelCsv/ParallelCsvProcessor.cs +++ b/src/ExcelReader.Core/Parser/ParallelCsv/ParallelCsvProcessor.cs @@ -1,4 +1,5 @@ using System.Runtime.ExceptionServices; +using ExcelReader.Core.Parser.Internal; using ExcelReader.Core.Reader; using ExcelReader.Core.Reader.Csv; @@ -11,15 +12,43 @@ internal static class ParallelCsvProcessor private const int MaxChunkBytes = 64 * 1024 * 1024; private const int RowsPerCancellationCheck = 4096; - private sealed class ChunkOutcome(long actualStart, TState state) + private sealed class ChunkOutcome(long actualStart, TState state, FailureGate? gate) { public readonly long ActualStart = actualStart; + public readonly FailureGate? Gate = gate; public long ResolvedNextStart = long.MaxValue; public TState State = state; public long Delivered; + public bool Abandoned; public ExceptionDispatchInfo? Failure; } + // The lowest index of a chunk that failed. Chunks past it stop: the merge throws at that failure before + // reaching them, unless the failing chunk was misguessed and is re-read clean, and then it reads them itself. + private sealed class FailureGate + { + private int _first = int.MaxValue; + + internal void Fail(int index) + { + int seen = Volatile.Read(ref _first); + while (index < seen) + { + int prior = Interlocked.CompareExchange(ref _first, index, seen); + if (prior == seen) + { + return; + } + seen = prior; + } + } + + internal bool Past(int index) + { + return index > Volatile.Read(ref _first); + } + } + internal static Task RunWithChunkSizeAsync( ReadOnlyMemory data, RowAggregation aggregation, RowActionFactory? factory, CsvReaderOptions reader, int headerRow, int degreeOfParallelism, int chunkSize, CancellationToken ct) @@ -122,47 +151,62 @@ private static async Task PartitionedAsync( : (int)Math.Clamp(dataLength / ((long)dop * ChunksPerWorker), MinChunkBytes, MaxChunkBytes); CsvChunkPlan plan = CsvChunkPlan.Create(dataStart, dataLength, dop, chunkSize); - var outcomes = new ChunkOutcome[plan.Count]; + var outcomes = new ChunkOutcome?[plan.Count]; + var gate = new FailureGate(); var workers = new Task[Math.Min(dop, plan.Count)]; for (int w = 0; w < workers.Length; w++) { - workers[w] = Task.Run(() => DrainPlanAsync(source, plan, dataStart, reader, aggregation, outcomes, ct), ct); + workers[w] = Task.Run(() => DrainPlanAsync(source, plan, dataStart, reader, aggregation, outcomes, gate, ct), ct); } await Task.WhenAll(workers).ConfigureAwait(false); TState merged = default!; long confirmedStart = dataStart; + long recordsBefore = headerRow; for (int i = 0; i < plan.Count && confirmedStart < long.MaxValue; i++) { - ChunkOutcome outcome = outcomes[i]; - if (outcome.ActualStart != confirmedStart) + ChunkOutcome? outcome = outcomes[i]; + if (outcome is null || outcome.Abandoned || outcome.ActualStart != confirmedStart) { - outcome = await ParseChunkAsync(source, plan[i], confirmedStart, reader, aggregation, ct).ConfigureAwait(false); + outcome = await ParseChunkAsync(source, plan[i], confirmedStart, reader, aggregation, gate: null, ct).ConfigureAwait(false); + } + if (outcome.Failure is not null) + { + // A partitioned run parses without row numbers; the failing record is the chunk's last delivered one. + if (outcome.Failure.SourceException is ExcelParseException { Row: 0 } parse) + { + throw ProjectionRules.AtRow(parse, recordsBefore + outcome.Delivered); + } + outcome.Failure.Throw(); } - outcome.Failure?.Throw(); merged = i == 0 ? outcome.State : aggregation.Combine(merged, outcome.State); confirmedStart = outcome.ResolvedNextStart; + recordsBefore += outcome.Delivered; } return merged; } private static async Task DrainPlanAsync( CsvChunkSource source, CsvChunkPlan plan, long dataStart, CsvReaderOptions reader, - RowAggregation aggregation, ChunkOutcome[] outcomes, CancellationToken ct) + RowAggregation aggregation, ChunkOutcome?[] outcomes, FailureGate gate, CancellationToken ct) { while (plan.TryTakeNext(out CsvChunk chunk)) { ct.ThrowIfCancellationRequested(); - long start = chunk.Index == 0 ? dataStart : CsvChunkWorker.GuessStart(source, chunk, reader.Quote); - outcomes[chunk.Index] = await ParseChunkAsync(source, chunk, start, reader, aggregation, ct).ConfigureAwait(false); + if (gate.Past(chunk.Index)) + { + continue; + } + long start = chunk.Index == 0 ? dataStart : CsvChunkWorker.GuessStart(source, chunk, reader.Quote, reader.Delimiter); + outcomes[chunk.Index] = await ParseChunkAsync(source, chunk, start, reader, aggregation, gate, ct).ConfigureAwait(false); } } private static ValueTask> ParseChunkAsync( CsvChunkSource source, CsvChunk chunk, long start, CsvReaderOptions reader, - RowAggregation aggregation, CancellationToken ct) + RowAggregation aggregation, FailureGate? gate, CancellationToken ct) { - var outcome = new ChunkOutcome(start, aggregation.Seed()); + var outcome = new ChunkOutcome(start, aggregation.Seed(), gate); if (start >= source.Length) { return new ValueTask>(outcome); @@ -193,7 +237,7 @@ private static void ParseMemoryChunk( } catch (Exception ex) when (ex is not OperationCanceledException) { - outcome.Failure = ExceptionDispatchInfo.Capture(ex); + Fail(outcome, chunk, ex); } finally { @@ -223,13 +267,19 @@ private static async ValueTask> ParseFileChunkAsync } catch (Exception ex) when (ex is not OperationCanceledException) { - outcome.Failure = ExceptionDispatchInfo.Capture(ex); + Fail(outcome, chunk, ex); } } } return outcome; } + private static void Fail(ChunkOutcome outcome, CsvChunk chunk, Exception ex) + { + outcome.Failure = ExceptionDispatchInfo.Capture(ex); + outcome.Gate?.Fail(chunk.Index); + } + private static bool DeliverWithinChunk( CsvReader.Enumerator rows, CsvChunk chunk, ChunkOutcome outcome, RowAction accumulate, CancellationToken ct) { @@ -239,6 +289,11 @@ private static bool DeliverWithinChunk( outcome.ResolvedNextStart = absolute; return false; } + if (outcome.Gate is not null && outcome.Gate.Past(chunk.Index)) + { + outcome.Abandoned = true; + return false; + } if (++outcome.Delivered % RowsPerCancellationCheck == 0) { ct.ThrowIfCancellationRequested(); diff --git a/src/ExcelReader.Core/Parser/SheetPipeline/SheetPipelineRunner.cs b/src/ExcelReader.Core/Parser/SheetPipeline/SheetPipelineRunner.cs index a2262549..b53ca460 100644 --- a/src/ExcelReader.Core/Parser/SheetPipeline/SheetPipelineRunner.cs +++ b/src/ExcelReader.Core/Parser/SheetPipeline/SheetPipelineRunner.cs @@ -68,7 +68,7 @@ internal static async IAsyncEnumerable RunAsync( ChunkOutcome outcome = await pending.WaitAsync(ct).ConfigureAwait(false); if (outcome.Failure is not null) { - ThrowRenumbered(outcome.Failure, headerRow + rows); + ThrowRenumbered(outcome.Failure, headerRow + rows, outcome.Rows); } rows += outcome.Rows; yield return outcome.State; @@ -92,16 +92,12 @@ internal static async IAsyncEnumerable RunAsync( if (recover) { - TState tail = default!; - try + ChunkOutcome tail = source.ParseTail(headerRow, rows, aggregation, cancellationToken); + if (tail.Failure is not null) { - tail = source.ParseTail(headerRow, rows, aggregation, cancellationToken); + ThrowRenumbered(tail.Failure, headerRow + rows, tail.Rows); } - catch (ExcelParseException ex) - { - ThrowRenumbered(ExceptionDispatchInfo.Capture(ex), headerRow + rows); - } - yield return tail; + yield return tail.State; } } finally @@ -168,14 +164,13 @@ private static async Task WorkAsync( } } - private static void ThrowRenumbered(ExceptionDispatchInfo failure, long rowsBefore) + // A chunk numbers its rows from 1 when it parses into models; a callback or accumulator run leaves Row + // at 0, and then the chunk's count of rows read, which includes the failing one, places it. + private static void ThrowRenumbered(ExceptionDispatchInfo failure, long rowsBefore, long rowsInChunk) { - if (failure.SourceException is ExcelParseException { Row: > 0 } parse) + if (failure.SourceException is ExcelParseException parse) { - int row = (int)(rowsBefore + parse.Row); - throw parse.RawValue.Length == 0 - ? ProjectionRules.MissingRequiredValue(parse.ColumnName, row) - : new ExcelParseException(row, parse.ColumnName, parse.RawValue); + throw ProjectionRules.AtRow(parse, rowsBefore + (parse.Row > 0 ? parse.Row : rowsInChunk)); } failure.Throw(); } diff --git a/src/ExcelReader.Core/Parser/SheetPipeline/SheetPipelineSource.cs b/src/ExcelReader.Core/Parser/SheetPipeline/SheetPipelineSource.cs index bf096983..8300a1be 100644 --- a/src/ExcelReader.Core/Parser/SheetPipeline/SheetPipelineSource.cs +++ b/src/ExcelReader.Core/Parser/SheetPipeline/SheetPipelineSource.cs @@ -43,7 +43,7 @@ internal abstract ChunkOutcome ParseChunk( ReadOnlyMemory chunk, bool first, RowAggregation aggregation, CancellationToken ct); // Reads the sheet again from its start: skipRows counts data rows, after the header. - internal abstract TState ParseTail(int headerRow, long skipRows, RowAggregation aggregation, CancellationToken ct); + internal abstract ChunkOutcome ParseTail(int headerRow, long skipRows, RowAggregation aggregation, CancellationToken ct); internal static SheetPipelineSource? Create(IExcelSheet sheet) { diff --git a/src/ExcelReader.Core/Parser/SheetPipeline/XlsbPipelineSource.cs b/src/ExcelReader.Core/Parser/SheetPipeline/XlsbPipelineSource.cs index 217efc3b..444b8f97 100644 --- a/src/ExcelReader.Core/Parser/SheetPipeline/XlsbPipelineSource.cs +++ b/src/ExcelReader.Core/Parser/SheetPipeline/XlsbPipelineSource.cs @@ -82,7 +82,7 @@ internal override ChunkOutcome ParseChunk( return outcome; } - internal override TState ParseTail(int headerRow, long skipRows, RowAggregation aggregation, CancellationToken ct) + internal override ChunkOutcome ParseTail(int headerRow, long skipRows, RowAggregation aggregation, CancellationToken ct) { throw new System.Diagnostics.UnreachableException("An XLSB chunk always ends on a record boundary."); } diff --git a/src/ExcelReader.Core/Parser/SheetPipeline/XlsxPipelineSource.cs b/src/ExcelReader.Core/Parser/SheetPipeline/XlsxPipelineSource.cs index ef852c05..f6b66167 100644 --- a/src/ExcelReader.Core/Parser/SheetPipeline/XlsxPipelineSource.cs +++ b/src/ExcelReader.Core/Parser/SheetPipeline/XlsxPipelineSource.cs @@ -80,28 +80,34 @@ internal override ChunkOutcome ParseChunk( return outcome; } - internal override TState ParseTail(int headerRow, long skipRows, RowAggregation aggregation, CancellationToken ct) + internal override ChunkOutcome ParseTail(int headerRow, long skipRows, RowAggregation aggregation, CancellationToken ct) { - TState state = aggregation.Seed(); + var outcome = new ChunkOutcome(aggregation.Seed()); using XlsxWorkbook.Enumerator rows = workbook.OpenSheetWithoutPrefetch(index); for (long skipped = 0; skipped < headerRow + skipRows; skipped++) { if (!rows.MoveNext()) { - return state; + return outcome; } } RowAction accumulate = aggregation.Accumulate; - long delivered = 0; - while (rows.MoveNext()) + try { - if (++delivered % RowsPerCancellationCheck == 0) + while (rows.MoveNext()) { - ct.ThrowIfCancellationRequested(); + if (++outcome.Rows % RowsPerCancellationCheck == 0) + { + ct.ThrowIfCancellationRequested(); + } + accumulate(ref outcome.State, rows.Current); } - accumulate(ref state, rows.Current); } - return state; + catch (Exception ex) when (ex is not OperationCanceledException) + { + outcome.Failure = ExceptionDispatchInfo.Capture(ex); + } + return outcome; } } } diff --git a/src/ExcelReader.Core/Reader/IRowAccumulator.cs b/src/ExcelReader.Core/Reader/IRowAccumulator.cs index ec30d5b2..78f8a439 100644 --- a/src/ExcelReader.Core/Reader/IRowAccumulator.cs +++ b/src/ExcelReader.Core/Reader/IRowAccumulator.cs @@ -9,8 +9,9 @@ namespace ExcelReader.Core.Reader /// /// /// runs concurrently across instances on worker threads, and must mutate only the - /// instance it is called on. For CSV, a partition that started at a misguessed row boundary is read again - /// into a new instance and the first one is discarded, so an instance may see rows that are later + /// instance it is called on. For CSV, a partition that started at a misguessed row boundary, which takes a + /// quote outside RFC 4180 rules or a quoted field longer than 64 KB, is read again into a new instance and + /// the first one is discarded, so an instance may see rows that are later /// thrown away — including rows whose parse or threw, and rows misparsed from /// the wrong start that appear nowhere in the sheet. A side effect outside the instance is not undone. /// XLSX and XLSB never re-read a partition. diff --git a/src/ExcelReader.Core/Reader/Schema/SchemaInference.cs b/src/ExcelReader.Core/Reader/Schema/SchemaInference.cs index b5978362..2b2ca687 100644 --- a/src/ExcelReader.Core/Reader/Schema/SchemaInference.cs +++ b/src/ExcelReader.Core/Reader/Schema/SchemaInference.cs @@ -159,7 +159,9 @@ internal void Observe(in Cell cell, bool isDate1904, bool parseText) SawNonBinaryNumber |= !IsBinaryDigit(cell.Value); break; case CellType.Date: - SawDate = true; + bool hasTime = cell.TryGetDateTime(isDate1904, out DateTime date) && date.TimeOfDay != TimeSpan.Zero; + SawTimestamp |= hasTime; + SawDate |= !hasTime; break; case CellType.Boolean: SawBool = true; diff --git a/src/ExcelReader.Core/Reader/SheetDataReader.cs b/src/ExcelReader.Core/Reader/SheetDataReader.cs index 95cfee9c..d42127db 100644 --- a/src/ExcelReader.Core/Reader/SheetDataReader.cs +++ b/src/ExcelReader.Core/Reader/SheetDataReader.cs @@ -110,6 +110,13 @@ private static Dictionary BuildOrdinals(string?[] names) map[name] = i; } } + for (int i = 0; i < names.Length; i++) + { + if (names[i] is null) + { + map.TryAdd($"Column{i}", i); + } + } return map; } diff --git a/src/ExcelReader.Core/Reader/SheetParallel.cs b/src/ExcelReader.Core/Reader/SheetParallel.cs index 0dfd53b2..52eaaee8 100644 --- a/src/ExcelReader.Core/Reader/SheetParallel.cs +++ b/src/ExcelReader.Core/Reader/SheetParallel.cs @@ -23,16 +23,16 @@ namespace ExcelReader.Core.Reader /// or a non-UTF-8 encoding). /// /// - /// Delivery guarantees. The general contract is the weaker CSV one: ForEachAsync is at-least-once, - /// and a partition whose start was guessed wrongly is discarded and read again. XLSB delivers every row - /// exactly once, in ForEachAsync too. XLSX delivers exactly once for a well-formed sheet; a - /// malformed one can make ForEachAsync deliver a row twice. ParseAsync yields the same items in + /// Delivery guarantees. XLSB delivers every row exactly once, in ForEachAsync too. XLSX does for a + /// well-formed sheet, and CSV for one that follows RFC 4180 quoting with no quoted field longer than 64 KB; + /// outside those, ForEachAsync is at-least-once, since a partition whose start was guessed wrongly + /// is discarded and read again. ParseAsync yields the same items in /// the same order as the sequential on every format, and /// AggregateAsync and ForEachAsync equal their own single-thread run. /// /// /// : ParseAsync renumbers it from the count emitted at the - /// merge; ForEachAsync and AggregateAsync report the exact row when sequential and 0 when partitioned. + /// merge; ForEachAsync and AggregateAsync report the row a sequential read reports, partitioned or not. /// /// public static class SheetParallel @@ -54,9 +54,10 @@ public static class SheetParallel /// Parsing and type conversion run in parallel; whatever the caller does per row does not. A caller /// whose per-row work dominates will see little gain from raising . /// For CSV, yields substantially less here than sequentially, - /// since each partition keeps its own dedup cache. A partition read from a wrongly guessed start is - /// discarded before any of its rows are yielded, but its cells have already passed through the parser's - /// converters and property setters, which must therefore be free of side effects. + /// since each partition keeps its own dedup cache. A partition read from a wrongly guessed start, which + /// takes a CSV outside the exactly-once conditions of ForEachAsync, is discarded before any of its + /// rows are yielded, but its cells have already passed through the parser's converters and property + /// setters, which must therefore be free of side effects. /// public static IAsyncEnumerable ParseAsync( IExcelSheet sheet, ExcelParser parser, SheetParallelOptions? options = null, CancellationToken ct = default) @@ -91,23 +92,32 @@ public static IAsyncEnumerable ParseAsync( /// several at once; the caller owns any synchronization. Records arrive in no particular order. /// /// - /// Delivery is at least once in general: may be invoked more than once for the + /// XLSB delivers every row exactly once. CSV does when every quote opens or closes a quoted field or is + /// doubled inside one, as RFC 4180 has it, and no quoted field runs past 64 KB: a partition's start is then + /// read off the quotes around it, and one whose quotes could open or close a field waits for its confirmed + /// start. XLSX delivers exactly once for a well-formed sheet. + /// + /// + /// Outside those, delivery is at least once: may be invoked more than once for the /// same record. On CSV, a partition whose start was guessed wrongly is read again from the confirmed /// offset, and the discarded speculative pass may already have delivered a whole partition's worth of /// records, which may also be misparsed: a wrong start shifts every field boundary. An exception - /// throws during a discarded pass is discarded with it. A start is only guessed - /// wrongly inside a quoted field, so a CSV in which the quote character never appears delivers every - /// record exactly once. XLSB delivers every row exactly once. XLSX does too for a well-formed sheet; on - /// a malformed one, workers may already have called for rows that are then - /// dropped, so a row can be delivered twice or be one the sequential reader would not produce. + /// throws during a discarded pass is discarded with it. On a malformed XLSX sheet, + /// workers may already have called for rows that are then dropped, so a row can be + /// delivered twice or be one the sequential reader would not produce. /// /// - /// A with side effects must therefore be idempotent and tolerate records that - /// do not exist. For exactly-once on any source, use + /// A with side effects on such a source must therefore be idempotent and tolerate + /// records that do not exist. For exactly-once on any source, use /// , where the /// discarded partition's accumulator is thrown away, and apply the side effects once it returns. /// /// + /// When or a parse throws, partitions further into the sheet stop: on CSV at their + /// next record, on XLSX and XLSB within the few chunks already in flight. Partitions before it run to the + /// end, so every record a sequential read would deliver before the failure is delivered. + /// + /// /// A column points into a pooled buffer the worker reuses once /// returns. Copy anything that must outlive the call. /// @@ -145,8 +155,8 @@ public static Task ForEachAsync( /// value throws ; empty rows are skipped. /// /// - /// is exact when the sheet is read sequentially and - /// 0 when it is partitioned, since a partition does not know how many rows precede it. + /// is the row a sequential read reports, partitioned or not: the + /// partitions merge in sheet order, so the first failure in that order is the one thrown. /// /// public static Task ForEachAsync( @@ -212,8 +222,8 @@ public static Task AggregateAsync( /// [ExcelRequired] column or value throws ; empty rows are skipped. /// /// - /// is exact when the sheet is read sequentially and 0 when it is - /// partitioned, since a partition does not know how many rows precede it. + /// is the row a sequential read reports, partitioned or not: the + /// partitions merge in sheet order, so the first failure in that order is the one thrown. /// /// Otherwise carries the same contract and fallbacks as the overload. /// diff --git a/src/ExcelReader.Core/Reader/Xlsb/XlsbWorkbookParts.cs b/src/ExcelReader.Core/Reader/Xlsb/XlsbWorkbookParts.cs index 12c3eeea..a84c49d3 100644 --- a/src/ExcelReader.Core/Reader/Xlsb/XlsbWorkbookParts.cs +++ b/src/ExcelReader.Core/Reader/Xlsb/XlsbWorkbookParts.cs @@ -9,18 +9,19 @@ internal static (string Name, string Path, ExcelSheetVisibility Visibility)[] Pa Dictionary rels = XlsxXml.ParseRelationships(relsBytes); List<(string, string, ExcelSheetVisibility)> sheets = []; var reader = new Biff12RecordReader(workbookBin); + var alternateLookup = rels.GetAlternateLookup>(); while (reader.TryReadRecord(out int id, out ReadOnlySpan payload)) { if (id != Brt.BundleSh) { continue; } - AddSheet(payload, rels, sheets); + AddSheet(payload, alternateLookup, sheets); } return [.. sheets]; } - private static void AddSheet(ReadOnlySpan payload, Dictionary rels, List<(string, string, ExcelSheetVisibility)> sheets) + private static void AddSheet(ReadOnlySpan payload, Dictionary.AlternateLookup> rels, List<(string, string, ExcelSheetVisibility)> sheets) { if (payload.Length < 8) { @@ -34,10 +35,11 @@ private static void AddSheet(ReadOnlySpan payload, Dictionary #include #include #include @@ -1502,6 +1503,10 @@ namespace xl return (col.validity[byte_idx] & (1 << bit_idx)) != 0; } + /// The standard integer type `std::in_range` accepts in place of a character type. + template + using standard_integer_t = std::conditional_t, std::make_signed_t, std::make_unsigned_t>; + template void assign_field(Class &instance, const xl_column &col, int64_t row, const FieldBinding &binding) { @@ -1526,11 +1531,13 @@ namespace xl const int32_t *offsets = static_cast(col.values); int32_t start = offsets[row]; int32_t end = offsets[row + 1]; - if (end > start && col.data != nullptr) + if (end <= start || col.data == nullptr) { - const char *str_data = reinterpret_cast(col.data) + start; - instance.*(binding.member) = T(str_data, static_cast(end - start)); + instance.*(binding.member) = T{}; + return; } + const char *str_data = reinterpret_cast(col.data) + start; + instance.*(binding.member) = T(str_data, static_cast(end - start)); } else if constexpr (std::is_same_v) { @@ -1588,6 +1595,43 @@ namespace xl return instance; } + template + std::optional check_range(const xl_column &col, int64_t rows, const FieldBinding &binding) + { + using Field = unwrap_optional_t; + if constexpr (std::is_integral_v && !std::is_same_v) + { + const int64_t *values = static_cast(col.values); + for (int64_t row = 0; row < rows; ++row) + { + if (is_valid(col, row) && !std::in_range>(values[row])) + { + return Error{XL_INVALID_ARGUMENT, "column '" + std::string(binding.column_names[0]) + "' holds " + + std::to_string(values[row]) + " at row " + std::to_string(row) + + ", which its field's type cannot represent"}; + } + } + } + return std::nullopt; + } + + template + std::optional check_ranges(const xl_table &table, const Tuple &bindings, std::index_sequence) + { + std::optional problem; + (void)(... || (problem = check_range(table.columns[Is], table.row_count, std::get(bindings))).has_value()); + return problem; + } + + /// TableView converts on access and returns `T`, so a value the row type cannot hold is + /// refused once, when the table is handed over, instead of wrapping on every read. + template + std::optional check_ranges(const xl_table &table) + { + static constexpr auto bindings = ExcelMapper::get_bindings(); + return check_ranges(table, bindings, std::make_index_sequence>{}); + } + template int32_t with_specs(const Sheet &sheet, Call &&call) { @@ -1599,6 +1643,27 @@ namespace xl return call(Access::handle(sheet), sheet.index(), specs.data(), static_cast(num_fields)); } + template + int32_t with_specs(const Sheet &sheet, std::span columns, Call &&call) + { + const size_t count = columns.size(); + std::vector name_slots(count); + std::vector name_lens(count); + std::vector specs(count); + for (size_t i = 0; i < count; ++i) + { + const InferredColumn &column = columns[i]; + if (column.name.has_value()) + { + name_slots[i] = reinterpret_cast(column.name->data()); + name_lens[i] = static_cast(column.name->size()); + } + specs[i] = xl_column_spec{&name_slots[i], &name_lens[i], column.name.has_value() ? 1 : 0, + column.index, column.type, column.nullable ? 1 : 0}; + } + return call(Access::handle(sheet), sheet.index(), specs.data(), static_cast(count)); + } + inline std::optional check_batch_size(int64_t batch_size) { if (batch_size > 0) @@ -1783,66 +1848,91 @@ namespace xl { return std::unexpected(detail::make_error(status)); } - return detail::Access::make>(table); + TableView view = detail::Access::make>(table); + if (std::optional problem = detail::check_ranges(table)) + { + return std::unexpected(std::move(*problem)); + } + return view; } - - /// @brief parse delivered a batch at a time. + /// @brief The columns of a parse over a runtime schema, held in native columnar memory. /// - /// An input range of `std::expected, Error>` that stops after the first error. Each - /// batch owns its memory and outlives the reader. Use from one thread at a time. - /// @tparam T Row type, mapped by `ExcelMapper`. - template - class TypedReader + /// Owns that memory. Its columns are the ColumnRefs write_columns takes, borrowed from this + /// table. A column matched by index has no name, and writing needs every column named or none. + class Table { public: - TypedReader(const TypedReader &) = delete; - TypedReader &operator=(const TypedReader &) = delete; + Table(const Table &) = delete; + Table &operator=(const Table &) = delete; - TypedReader(TypedReader &&other) noexcept : reader_(std::exchange(other.reader_, nullptr)) {} + Table(Table &&other) noexcept + : table_(std::exchange(other.table_, xl_table{})), names_(std::move(other.names_)), + columns_(std::move(other.columns_)) + { + } - TypedReader &operator=(TypedReader &&other) noexcept + Table &operator=(Table &&other) noexcept { if (this != &other) { - xl_typed_reader_close(reader_); - reader_ = std::exchange(other.reader_, nullptr); + xl_free_table(&table_); + table_ = std::exchange(other.table_, xl_table{}); + names_ = std::move(other.names_); + columns_ = std::move(other.columns_); } return *this; } - ~TypedReader() { xl_typed_reader_close(reader_); } + ~Table() { xl_free_table(&table_); } - /// @brief Reads the next batch. - /// @return The batch, or empty at the end of the sheet. - std::expected>, Error> next() + int64_t size() const noexcept { return table_.row_count; } + bool empty() const noexcept { return table_.row_count == 0; } + + /// @brief Every column, in schema order. + std::span columns() const noexcept { return columns_; } + + private: + friend struct detail::Access; + + // A moved vector keeps its elements in place, so the names columns_ views stay valid. + Table(xl_table table, std::span schema) : table_(table) { - xl_table table{}; - int32_t status = xl_typed_reader_next(reader_, &table); - if (status == XL_EOF) + names_.reserve(schema.size()); + columns_.reserve(schema.size()); + for (size_t i = 0; i < schema.size(); ++i) { - return std::optional>{}; + const std::string &name = names_.emplace_back(schema[i].name.value_or(std::string{})); + const xl_column &raw = table_.columns[i]; + const int64_t validity_len = raw.validity == nullptr ? 0 : (raw.length + 7) / 8; + columns_.push_back(ColumnRef{name, raw.type, raw.length, raw.values, raw.validity, validity_len, + raw.data, raw.data_len}); } - if (status != XL_OK) - { - return std::unexpected(detail::make_error(status)); - } - return std::optional>(TableView(table)); } - class iterator + xl_table table_{}; + std::vector names_; + std::vector columns_; + }; + + namespace detail + { + /// The input iterator of a batched read: `Reader::next()` yields `Batch`es until it is empty + /// or fails, and the failure is the last element. + template + class BatchIterator { public: using iterator_category = std::input_iterator_tag; using iterator_concept = std::input_iterator_tag; - using value_type = std::expected, Error>; + using value_type = std::expected; using difference_type = std::ptrdiff_t; - iterator() = default; + BatchIterator() = default; value_type &operator*() { return *current_; } - iterator &operator++() + BatchIterator &operator++() { advance(); return *this; @@ -1850,15 +1940,15 @@ namespace xl void operator++(int) { advance(); } - friend bool operator==(const iterator &it, std::default_sentinel_t) noexcept + friend bool operator==(const BatchIterator &it, std::default_sentinel_t) noexcept { return !it.current_.has_value(); } private: - friend class TypedReader; + friend struct Access; - explicit iterator(TypedReader *reader) : reader_(reader) { advance(); } + explicit BatchIterator(Reader *reader) : reader_(reader) { advance(); } void advance() { @@ -1868,7 +1958,7 @@ namespace xl return; } current_.reset(); - std::expected>, Error> batch = reader_->next(); + std::expected, Error> batch = reader_->next(); if (!batch.has_value()) { current_ = value_type(std::unexpect, std::move(batch.error())); @@ -1884,11 +1974,97 @@ namespace xl current_ = value_type(std::move(**batch)); } - TypedReader *reader_ = nullptr; + Reader *reader_ = nullptr; std::optional current_{}; }; - iterator begin() { return iterator(this); } + inline std::expected adopt_table(xl_table table, std::span schema, + const char *source) + { + const bool consistent = table.row_count >= 0 && table.column_count >= 0 && + static_cast(table.column_count) == schema.size() && + std::all_of(table.columns, table.columns + table.column_count, + [&](const xl_column &c) { return c.length == table.row_count; }); + if (!consistent) + { + xl_free_table(&table); + return std::unexpected(Error{XL_ERROR, std::string(source) + " returned a table that does not match its specs"}); + } + return Access::make(table, schema); + } + } + + /// @brief parse over a schema chosen at run time, such as one Sheet::infer_schema returned. + /// @param sheet The sheet to read. + /// @param columns One per output column: matched by `name` when it has one, else by `index`. + /// Must not be empty. + /// @param options Header row and parallelism. + /// @return Every row after the header. + inline std::expected parse(Sheet sheet, std::span columns, + const ReadOptions &options = {}) + { + xl_table table{}; + const int32_t status = detail::with_specs( + sheet, columns, [&](xl_workbook *handle, int32_t index, const xl_column_spec *specs, int32_t count) + { return xl_parse_typed(handle, index, specs, count, options.header_row, options.parallelism, &table); }); + if (status != XL_OK) + { + return std::unexpected(detail::make_error(status)); + } + return detail::adopt_table(table, columns, "xl_parse_typed"); + } + + /// @brief parse delivered a batch at a time. + /// + /// An input range of `std::expected, Error>` that stops after the first error. Each + /// batch owns its memory and outlives the reader. Use from one thread at a time. + /// @tparam T Row type, mapped by `ExcelMapper`. + template + class TypedReader + { + public: + TypedReader(const TypedReader &) = delete; + TypedReader &operator=(const TypedReader &) = delete; + + TypedReader(TypedReader &&other) noexcept : reader_(std::exchange(other.reader_, nullptr)) {} + + TypedReader &operator=(TypedReader &&other) noexcept + { + if (this != &other) + { + xl_typed_reader_close(reader_); + reader_ = std::exchange(other.reader_, nullptr); + } + return *this; + } + + ~TypedReader() { xl_typed_reader_close(reader_); } + + /// @brief Reads the next batch. + /// @return The batch, or empty at the end of the sheet. + std::expected>, Error> next() + { + xl_table table{}; + int32_t status = xl_typed_reader_next(reader_, &table); + if (status == XL_EOF) + { + return std::optional>{}; + } + if (status != XL_OK) + { + return std::unexpected(detail::make_error(status)); + } + TableView view(table); + if (std::optional problem = detail::check_ranges(table)) + { + return std::unexpected(std::move(*problem)); + } + return std::optional>(std::move(view)); + } + + using iterator = detail::BatchIterator>; + + iterator begin() { return detail::Access::make(this); } std::default_sentinel_t end() const noexcept { return {}; } private: @@ -1921,6 +2097,94 @@ namespace xl return detail::Access::make>(reader); } + /// @brief parse over a runtime schema, delivered a batch at a time. + /// + /// TypedReader's counterpart, whose batches are Tables. + class TableReader + { + public: + TableReader(const TableReader &) = delete; + TableReader &operator=(const TableReader &) = delete; + + TableReader(TableReader &&other) noexcept + : reader_(std::exchange(other.reader_, nullptr)), schema_(std::move(other.schema_)) + { + } + + TableReader &operator=(TableReader &&other) noexcept + { + if (this != &other) + { + xl_typed_reader_close(reader_); + reader_ = std::exchange(other.reader_, nullptr); + schema_ = std::move(other.schema_); + } + return *this; + } + + ~TableReader() { xl_typed_reader_close(reader_); } + + /// @brief Reads the next batch. + /// @return The batch, or empty at the end of the sheet. + std::expected, Error> next() + { + xl_table table{}; + int32_t status = xl_typed_reader_next(reader_, &table); + if (status == XL_EOF) + { + return std::optional
{}; + } + if (status != XL_OK) + { + return std::unexpected(detail::make_error(status)); + } + std::expected batch = detail::adopt_table(table, schema_, "xl_typed_reader_next"); + if (!batch.has_value()) + { + return std::unexpected(std::move(batch.error())); + } + return std::optional
(std::move(*batch)); + } + + using iterator = detail::BatchIterator; + + iterator begin() { return detail::Access::make(this); } + std::default_sentinel_t end() const noexcept { return {}; } + + private: + friend struct detail::Access; + + TableReader(xl_typed_reader *reader, std::span schema) + : reader_(reader), schema_(schema.begin(), schema.end()) + { + } + + xl_typed_reader *reader_ = nullptr; + std::vector schema_; + }; + + /// @brief Opens a TableReader over a sheet. + /// @param sheet The sheet to read. + /// @param columns As for the runtime-schema parse; copied, so it need not outlive the reader. + /// @param options Header row and batch size. + inline std::expected parse_batches(Sheet sheet, std::span columns, + const ReadOptions &options = {}) + { + if (std::optional problem = detail::check_batch_size(options.batch_size)) + { + return std::unexpected(std::move(*problem)); + } + xl_typed_reader *reader = nullptr; + const int32_t status = detail::with_specs( + sheet, columns, [&](xl_workbook *handle, int32_t index, const xl_column_spec *specs, int32_t count) + { return xl_typed_reader_open(handle, index, specs, count, options.header_row, options.batch_size, &reader); }); + if (status != XL_OK) + { + return std::unexpected(detail::make_error(status)); + } + return detail::Access::make(reader, columns); + } + namespace detail { @@ -2110,6 +2374,7 @@ namespace xl typename ColumnStorage::type storage{}; std::vector validity{}; int64_t rows = 0; + bool out_of_range = false; void reserve(size_t count) { @@ -2212,6 +2477,7 @@ namespace xl } else if constexpr (std::is_integral_v) { + out_of_range |= !std::in_range(static_cast>(value)); storage.push_back(static_cast(value)); } else if constexpr (std::is_floating_point_v) @@ -2270,6 +2536,12 @@ namespace xl return (... || std::get(builders).overflowed()); } + template + bool any_out_of_range(const Builders &builders, std::index_sequence) + { + return (... || std::get(builders).out_of_range); + } + template std::array to_refs(const Builders &builders, const Tuple &bindings, std::index_sequence) @@ -2304,6 +2576,11 @@ namespace xl return std::unexpected(Error{XL_INVALID_ARGUMENT, "a string column exceeds 2 GiB, which int32 offsets cannot address."}); } + if (any_out_of_range(builders, indices)) + { + return std::unexpected(Error{XL_INVALID_ARGUMENT, "an integer field holds a value above INT64_MAX, " + "which an int64 column cannot represent."}); + } // refs borrows pointers into builders, so both must stay in the scope of the write call. const std::array refs = to_refs(builders, bindings, indices); @@ -2424,6 +2701,11 @@ namespace xl } else if constexpr (std::is_integral_v) { + if (!std::in_range(static_cast>(value))) + { + return std::unexpected(Error{XL_INVALID_ARGUMENT, "the value is above INT64_MAX, which an int64 " + "cell cannot represent."}); + } return status_result(xl_write_int64(handle_, static_cast(value))); } else if constexpr (std::is_floating_point_v) diff --git a/src/ExcelReader.Native/include/excelreader_arrow.hpp b/src/ExcelReader.Native/include/excelreader_arrow.hpp index 00a2fb0e..78448ee2 100644 --- a/src/ExcelReader.Native/include/excelreader_arrow.hpp +++ b/src/ExcelReader.Native/include/excelreader_arrow.hpp @@ -83,6 +83,26 @@ namespace xl return result; } + /// @brief to_arrow over a schema chosen at run time, such as one Sheet::infer_schema returned. + /// @param sheet The sheet to read. + /// @param columns One per output column: matched by `name` when it has one, else by `index`. + /// Must not be empty. + /// @param options Header row and parallelism. + inline std::expected to_arrow(Sheet sheet, std::span columns, + const ReadOptions &options = {}) + { + ArrowTable result; + const int32_t status = detail::with_specs( + sheet, columns, [&](xl_workbook *handle, int32_t index, const xl_column_spec *specs, int32_t count) + { return xl_parse_arrow(handle, index, specs, count, options.header_row, options.parallelism, + &result.array, &result.schema); }); + if (status != XL_OK) + { + return std::unexpected(detail::make_error(status)); + } + return result; + } + /// @brief parse_batches's Arrow counterpart: record batches pulled one at a time. /// /// Use from one thread at a time. @@ -199,6 +219,29 @@ namespace xl } return detail::Access::make(stream); } + + /// @brief to_arrow_stream over a schema chosen at run time, as for the to_arrow overload. + /// @param sheet The sheet to read. + /// @param columns One per output column: matched by `name` when it has one, else by `index`. + /// Must not be empty. + /// @param options Header row and batch size. + inline std::expected to_arrow_stream(Sheet sheet, std::span columns, + const ReadOptions &options = {}) + { + if (std::optional problem = detail::check_batch_size(options.batch_size)) + { + return std::unexpected(std::move(*problem)); + } + ArrowArrayStream stream{}; + const int32_t status = detail::with_specs( + sheet, columns, [&](xl_workbook *handle, int32_t index, const xl_column_spec *specs, int32_t count) + { return xl_parse_arrow_stream(handle, index, specs, count, options.header_row, options.batch_size, &stream); }); + if (status != XL_OK) + { + return std::unexpected(detail::make_error(status)); + } + return detail::Access::make(stream); + } } #endif /* XL_EXCELREADER_ARROW_HPP */ diff --git a/tests/ExcelReader.Benchmarks/ExcelReader.Benchmarks.csproj b/tests/ExcelReader.Benchmarks/ExcelReader.Benchmarks.csproj index 00e45d00..c45e5851 100644 --- a/tests/ExcelReader.Benchmarks/ExcelReader.Benchmarks.csproj +++ b/tests/ExcelReader.Benchmarks/ExcelReader.Benchmarks.csproj @@ -25,7 +25,8 @@ - + + diff --git a/tests/ExcelReader.Benchmarks/Read/ReadBenchmark.cs b/tests/ExcelReader.Benchmarks/Read/ReadBenchmark.cs index f17e01f1..d865c667 100644 --- a/tests/ExcelReader.Benchmarks/Read/ReadBenchmark.cs +++ b/tests/ExcelReader.Benchmarks/Read/ReadBenchmark.cs @@ -1,3 +1,4 @@ +using System.Text; using BenchmarkDotNet.Attributes; using ExcelReader.Core.Reader; using OfficeIMO.Excel; @@ -18,6 +19,7 @@ public class ReadBenchmark [GlobalSetup] public async Task SetupAsync() { + Encoding.RegisterProvider(CodePagesEncodingProvider.Instance); _workbook = await WorkbookGenerator.BuildAsync(Rows); _xlsbWorkbook = await WorkbookGenerator.BuildXlsbAsync(Rows); } @@ -82,6 +84,22 @@ public long Sylvan() return AccumulateSylvanExcel(reader); } + [Benchmark] + public long ExcelDataReader() + { + using var ms = new MemoryStream(_workbook, writable: false); + using var reader = global::ExcelDataReader.ExcelReaderFactory.CreateReader(ms); + return AccumulateDataReader(reader); + } + + [Benchmark] + public long ExcelDataReaderXlsb() + { + using var ms = new MemoryStream(_xlsbWorkbook, writable: false); + using var reader = global::ExcelDataReader.ExcelReaderFactory.CreateReader(ms); + return AccumulateDataReader(reader); + } + [Benchmark] public long OfficeIMO() { diff --git a/tests/ExcelReader.Benchmarks/Read/XlsReadBenchmark.cs b/tests/ExcelReader.Benchmarks/Read/XlsReadBenchmark.cs index f45f7902..c141ded6 100644 --- a/tests/ExcelReader.Benchmarks/Read/XlsReadBenchmark.cs +++ b/tests/ExcelReader.Benchmarks/Read/XlsReadBenchmark.cs @@ -51,6 +51,14 @@ public long Sylvan() return AccumulateSylvanExcel(reader); } + [Benchmark] + public long ExcelDataReader() + { + using var ms = new MemoryStream(_workbook, writable: false); + using var reader = global::ExcelDataReader.ExcelReaderFactory.CreateReader(ms); + return AccumulateDataReader(reader); + } + [Benchmark] public long OfficeIMO() { diff --git a/tests/ExcelReader.Tests/Parser/ParallelCsv/AggregateCsvModelTests.cs b/tests/ExcelReader.Tests/Parser/ParallelCsv/AggregateCsvModelTests.cs index 96a14747..a476e06b 100644 --- a/tests/ExcelReader.Tests/Parser/ParallelCsv/AggregateCsvModelTests.cs +++ b/tests/ExcelReader.Tests/Parser/ParallelCsv/AggregateCsvModelTests.cs @@ -271,18 +271,22 @@ public async Task SkipsEmptyRecords() } [Fact] - public async Task ReportsTheExactRowSequentiallyAndZeroWhenPartitioned() + public async Task ReportsTheSequentialRowWhenPartitioned() { - byte[] csv = "Name,Id,Note\nada,1,n\nbob,2,n\ncid,oops,n\ndan,4,n\n"u8.ToArray(); + byte[] csv = "Name,Id,Note\nada,1,\"x\ny\"\n\nbob,2,n\ncid,oops,n\ndan,4,n\neve,bad,n\n"u8.ToArray(); ExcelParser map = ExcelParser.Generated(new ExcelParserOptions { ThrowOnParseFailure = true }); using CsvReader reader = Excel.FromCsv(csv); ExcelParseException sequential = await Assert.ThrowsAsync(() => SheetParallel.AggregateAsync( reader.FirstSheet, map, new SheetParallelOptions { DegreeOfParallelism = 1, HeaderRow = 1 }, TestContext.Current.CancellationToken)); - Assert.Equal(4, sequential.Row); + Assert.Equal(5, sequential.Row); - ExcelParseException partitioned = await Assert.ThrowsAsync(() => Chunked(csv, map, dop: 4, chunkSize: 8, headerRow: 1)); - Assert.Equal(0, partitioned.Row); + for (int chunkSize = 1; chunkSize <= csv.Length; chunkSize++) + { + ExcelParseException partitioned = await Assert.ThrowsAsync(() => Chunked(csv, map, dop: 4, chunkSize, headerRow: 1)); + Assert.Equal(sequential.Row, partitioned.Row); + Assert.Equal(sequential.ColumnName, partitioned.ColumnName); + } } [Fact] diff --git a/tests/ExcelReader.Tests/Parser/ParallelCsv/AggregateCsvParallelTests.cs b/tests/ExcelReader.Tests/Parser/ParallelCsv/AggregateCsvParallelTests.cs index 144f8c3c..bcf6156b 100644 --- a/tests/ExcelReader.Tests/Parser/ParallelCsv/AggregateCsvParallelTests.cs +++ b/tests/ExcelReader.Tests/Parser/ParallelCsv/AggregateCsvParallelTests.cs @@ -158,7 +158,8 @@ public async Task SweepsAChunkBoundaryAcrossEveryByteOffsetOfAQuotedNewlineFixtu [Fact] public async Task DiscardsExceptionsThrownWhileReadingAMisguessedPartition() { - byte[] csv = Encoding.UTF8.GetBytes("ok,\"one\nBOOM,x\"\nok,two\nok,\"three\nBOOM,y\"\nok,four\n"); + // Text after each closing quote keeps the start guess from seeing that a chunk begins inside the field. + byte[] csv = Encoding.UTF8.GetBytes("ok,\"one\nBOOM,x\"z\nok,two\nok,\"three\nBOOM,y\"z\nok,four\n"); List expected = Sequential(csv); int misguessedDeliveries = 0; var throwOnBoom = new RowAggregation> diff --git a/tests/ExcelReader.Tests/Parser/ParallelCsv/CsvBoundaryResolverTests.cs b/tests/ExcelReader.Tests/Parser/ParallelCsv/CsvBoundaryResolverTests.cs index f9d8824d..2319d7d6 100644 --- a/tests/ExcelReader.Tests/Parser/ParallelCsv/CsvBoundaryResolverTests.cs +++ b/tests/ExcelReader.Tests/Parser/ParallelCsv/CsvBoundaryResolverTests.cs @@ -1,3 +1,4 @@ +using System.Text; using ExcelReader.Core.Parser.ParallelCsv; namespace ExcelReader.Tests.Parser.ParallelCsv @@ -5,6 +6,7 @@ namespace ExcelReader.Tests.Parser.ParallelCsv public class CsvBoundaryResolverTests { private const byte Quote = (byte)'"'; + private const byte Delimiter = (byte)','; [Fact] public void FindsTheByteAfterAPlainNewline() @@ -95,5 +97,93 @@ public void FindsABoundaryThatLandsOnTheVeryLastByte() Assert.Equal(3, start); } + + private static CsvQuoteParity? Infer(string window, char before = 'x', bool endsAtEof = false, byte delimiter = Delimiter) + { + return InferAfter(before.ToString(), window, endsAtEof, delimiter); + } + + private static CsvQuoteParity? InferAfter(string behind, string window, bool endsAtEof = false, byte delimiter = Delimiter) + { + byte[] bytes = Encoding.UTF8.GetBytes(behind + window); + return CsvBoundaryResolver.InferParity(bytes, Encoding.UTF8.GetByteCount(behind), startsAtSource: false, endsAtEof, Quote, delimiter); + } + + [Theory] + [InlineData("th\nree\",4\n")] + [InlineData("ree\"\n")] + [InlineData("ree\"\r\n")] + [InlineData("a\"\"b\",4\n")] + [InlineData("ab\"\"\",4\n")] + public void ARunThatClosesAFieldMeansTheWindowStartsInside(string window) + { + Assert.Equal(CsvQuoteParity.Inside, Infer(window)); + } + + [Theory] + [InlineData("4,\"three\",5\n")] + [InlineData(",4\n\"x\ny\",5\n")] + [InlineData("4,\"\",5\n")] + [InlineData("4,\"\"\"x\",5\n")] + [InlineData("plain,4\nplain,5\n")] + [InlineData("")] + [InlineData("ab\"")] + public void ARunThatOpensAFieldOrNoRunMeansOutside(string window) + { + Assert.Equal(CsvQuoteParity.Outside, Infer(window)); + } + + [Theory] + [InlineData("a,b\"c,4\n")] + [InlineData("a\"x\",4\n")] + public void ARunThatBreaksTheQuotingRulesIsGuessedOutside(string window) + { + Assert.Equal(CsvQuoteParity.Outside, Infer(window)); + } + + [Theory] + [InlineData("x,\",b\",4\n")] + [InlineData("x\n\",b\",4\n")] + [InlineData("x,\"\n\",4\n")] + [InlineData("1,\"\"\",x\",2\n")] + public void ARunThatCouldOpenOrCloseIsUnknown(string window) + { + Assert.Null(Infer(window)); + } + + [Fact] + public void ARunAtTheWindowStartIsReadAgainstTheByteBeforeIt() + { + Assert.Equal(CsvQuoteParity.Outside, Infer("\"x\",4\n", before: ',')); + Assert.Equal(CsvQuoteParity.Inside, Infer("\",4\n", before: 'a')); + Assert.Null(Infer("\",4\n", before: ',')); + Assert.Null(Infer("\"x\",4\n", before: '"')); + Assert.Null(InferAfter("", "\"x\",4\n")); + } + + [Fact] + public void AStartInsideARunCountsTheQuotesBeforeIt() + { + Assert.Equal(CsvQuoteParity.Inside, InferAfter("x,\"", "\",4,\"abc\"\n")); + Assert.Equal(CsvQuoteParity.Outside, InferAfter("x,\"said \"", "\"hi\"\"\",5\n")); + Assert.Equal(CsvQuoteParity.Inside, InferAfter("x\n\"", "\"\"quoted\"\" start\",5\n")); + Assert.Null(InferAfter("\"", "\"x\",4\n")); + Assert.Null(InferAfter("x,\"", "\"\"")); + Assert.Null(InferAfter("a\"", "\"b,4\n")); + } + + [Fact] + public void ARunAtTheEndOfTheSourceCloses() + { + Assert.Equal(CsvQuoteParity.Inside, Infer("ree\"", endsAtEof: true)); + Assert.Equal(CsvQuoteParity.Outside, Infer("ree\"", endsAtEof: false)); + } + + [Fact] + public void HonoursTheDelimiter() + { + Assert.Equal(CsvQuoteParity.Inside, Infer("ree\";4\n", delimiter: (byte)';')); + Assert.Equal(CsvQuoteParity.Outside, Infer("ree\";4\n")); + } } } diff --git a/tests/ExcelReader.Tests/Parser/ParallelCsv/CsvChunkWorkerTests.cs b/tests/ExcelReader.Tests/Parser/ParallelCsv/CsvChunkWorkerTests.cs index 04a46369..053736b7 100644 --- a/tests/ExcelReader.Tests/Parser/ParallelCsv/CsvChunkWorkerTests.cs +++ b/tests/ExcelReader.Tests/Parser/ParallelCsv/CsvChunkWorkerTests.cs @@ -35,6 +35,16 @@ private static async Task> ParseAsync(byte[] csv, long start CancellationToken.None); } + [Fact] + public void AChunkAtTheSourceStartIsGuessedToStartThere() + { + byte[] csv = "Ada,0001\nBob,0002\n"u8.ToArray(); + + long start = CsvChunkWorker.GuessStart(new CsvChunkSource(csv.AsMemory()), new CsvChunk(0, 0, csv.Length), (byte)'"', (byte)','); + + Assert.Equal(0, start); + } + [Fact] public async Task ParsesOnlyTheRecordsStartingInsideItsChunk() { @@ -92,13 +102,24 @@ public async Task GuessesItsOwnStartWhenNoConfirmedStartIsGiven() } [Fact] - public async Task GuessesWrongWhenANewlineHidesInsideAQuotedField() + public async Task SkipsANewlineHiddenInsideTheQuotedFieldItStartsIn() { byte[] csv = "Name,Age\n\"a\nb\",0001\nCid,0003\n"u8.ToArray(); - CsvChunkResult result = await ParseAsync(csv, start: 10, end: 20); + CsvChunkResult result = await ParseAsync(csv, start: 10, end: 22); - Assert.Equal(12, result.ActualStart); + Assert.Equal(20, result.ActualStart); + } + + [Fact] + public async Task LeavesAChunkWhoseFirstQuoteCouldOpenOrCloseToTheMerge() + { + byte[] csv = "Name,Age\nAda,0001\n\",b\",0002\nCid,0003\n"u8.ToArray(); + + CsvChunkResult result = await ParseAsync(csv, start: 10, end: 30); + + Assert.Equal(long.MaxValue, result.ActualStart); + Assert.Empty(result.Models); } [Fact] diff --git a/tests/ExcelReader.Tests/Parser/ParallelCsv/ForEachCsvParallelTests.cs b/tests/ExcelReader.Tests/Parser/ParallelCsv/ForEachCsvParallelTests.cs index 75358416..9c399824 100644 --- a/tests/ExcelReader.Tests/Parser/ParallelCsv/ForEachCsvParallelTests.cs +++ b/tests/ExcelReader.Tests/Parser/ParallelCsv/ForEachCsvParallelTests.cs @@ -1,4 +1,5 @@ using System.Collections.Concurrent; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Text; using ExcelReader.Core.Parser; @@ -160,6 +161,39 @@ await Assert.ThrowsAsync( TestContext.Current.CancellationToken)); } + [Fact] + public async Task ForEachRecord_BodyThrows_LaterChunksStopDelivering() + { + const int rows = 20_000; + byte[] csv = SalesCsv(rows); + int failed = 0; + int afterFailure = 0; + + await Assert.ThrowsAsync(() => ChunkedRecord( + csv, + sale => + { + if (Volatile.Read(ref failed) == 1) + { + if (Interlocked.Increment(ref afterFailure) == 1) + { + Thread.Sleep(100); + } + return; + } + if (sale.Units == 1) + { + Volatile.Write(ref failed, 1); + throw new InvalidOperationException("boom"); + } + }, + new ChunkProbe(), + dop: 2, + chunkSize: 2048)); + + Assert.True(afterFailure <= 1, $"{afterFailure} records were delivered after the first one failed"); + } + [Fact] public async Task ForEachRecord_Cancelled_Throws() { @@ -506,15 +540,107 @@ await ChunkedMapped( } } + [Theory] + [InlineData("\"x\ny\",{0}\n")] + [InlineData("\"x\",{0},\"y\"\n")] + [InlineData("\"said \"\"hi\"\"\",{0}\n")] + [InlineData("\"\"\"quoted\"\" start\",{0}\n")] + public async Task ForEachRecord_Chunked_QuotedFields_ReadsEveryChunkAndRecordOnce(string row) + { + const int rows = 4000; + const string header = "Name,Units\n"; + var sb = new StringBuilder(); + sb.Append(header); + for (int i = 1; i <= rows; i++) + { + sb.AppendFormat(CultureInfo.InvariantCulture, row, i); + } + byte[] csv = Encoding.UTF8.GetBytes(sb.ToString()); + var probe = new ChunkProbe(); + var calls = new ConcurrentDictionary(); + + await ChunkedRecord(csv, sale => calls.AddOrUpdate(sale.Units, 1, static (_, n) => n + 1), probe, dop: 4, chunkSize: 2048); + + int planned = CsvChunkPlan.Create(header.Length, csv.Length - header.Length, 4, 2048).Count; + Assert.True(planned > 1, $"expected the source to partition, planned {planned} chunk(s)"); + Assert.Equal(planned, probe.Chunks); + Assert.Equal(rows, calls.Count); + Assert.All(calls, pair => Assert.Equal(1, pair.Value)); + } + + [SuppressMessage("Security", "CA5394:Do not use insecure randomness", + Justification = "A seeded CSV generator, not randomness protecting anything.")] + private static string RandomRfcField(Random random) + { + const string alphabet = "ab ,\n\r\""; + var field = new StringBuilder(); + for (int n = random.Next(0, 12); n > 0; n--) + { + field.Append(alphabet[random.Next(alphabet.Length)]); + } + string text = field.ToString(); + bool needsQuotes = text.AsSpan().IndexOfAny(",\n\r\"") >= 0; + return needsQuotes || random.Next(3) == 0 ? "\"" + text.Replace("\"", "\"\"", StringComparison.Ordinal) + "\"" : text; + } + + [Fact] + [SuppressMessage("Security", "CA5394:Do not use insecure randomness", + Justification = "A seeded CSV generator, not randomness protecting anything.")] + public async Task ForEachRecord_Chunked_RandomRfcCsv_DeliversEveryRecordExactlyOnce() + { + const int rows = 300; + var random = new Random(20261020); + for (int file = 0; file < 25; file++) + { + var sb = new StringBuilder("Name,Units\n"); + for (int i = 1; i <= rows; i++) + { + sb.Append(RandomRfcField(random)).Append(',').Append(i.ToString(CultureInfo.InvariantCulture)).Append(random.Next(4) == 0 ? "\r\n" : "\n"); + } + byte[] csv = Encoding.UTF8.GetBytes(sb.ToString()); + foreach (int chunkSize in new[] { 7, 31, 64, 200, 1000 }) + { + var calls = new ConcurrentDictionary(); + await ChunkedRecord(csv, sale => calls.AddOrUpdate(sale.Units, 1, static (_, n) => n + 1), new ChunkProbe(), dop: 4, chunkSize); + + Assert.Equal(rows, calls.Count); + Assert.All(calls, pair => Assert.Equal(1, pair.Value)); + } + } + } + + // Quoted fields that start with the delimiter give runs that could open or close a field, so those + // chunks are left to the merge, which reads them from their confirmed start. + [Fact] + public async Task ForEachRecord_Chunked_AmbiguousQuotes_DeliversEveryRecordOnce() + { + const int rows = 4000; + var sb = new StringBuilder(); + sb.Append("Name,Units\n"); + for (int i = 1; i <= rows; i++) + { + sb.Append(CultureInfo.InvariantCulture, $"\",x\ny\",{i}\n"); + } + byte[] csv = Encoding.UTF8.GetBytes(sb.ToString()); + var calls = new ConcurrentDictionary(); + + await ChunkedRecord(csv, sale => calls.AddOrUpdate(sale.Units, 1, static (_, n) => n + 1), new ChunkProbe(), dop: 4, chunkSize: 2048); + + Assert.Equal(rows, calls.Count); + Assert.All(calls, pair => Assert.Equal(1, pair.Value)); + } + + // Text after a closing quote breaks the quoting rules, which the parser reads leniently but the start + // guess cannot see, so a chunk that starts inside such a field is read twice. [Fact] - public async Task ForEachRecord_Chunked_QuotedNewlines_InvokesBodyTwiceForSomeRecords() + public async Task ForEachRecord_Chunked_TextAfterAClosingQuote_InvokesBodyTwiceForSomeRecords() { const int rows = 4000; var sb = new StringBuilder(); sb.Append("Name,Units\n"); for (int i = 1; i <= rows; i++) { - sb.Append(CultureInfo.InvariantCulture, $"\"x\ny\",{i}\n"); + sb.Append(CultureInfo.InvariantCulture, $"\"x\ny\"z,{i}\n"); } byte[] csv = Encoding.UTF8.GetBytes(sb.ToString()); var probe = new ChunkProbe(); diff --git a/tests/ExcelReader.Tests/Parser/SheetParallelXlsxTests.cs b/tests/ExcelReader.Tests/Parser/SheetParallelXlsxTests.cs index b4696b3b..a68904f8 100644 --- a/tests/ExcelReader.Tests/Parser/SheetParallelXlsxTests.cs +++ b/tests/ExcelReader.Tests/Parser/SheetParallelXlsxTests.cs @@ -2,6 +2,7 @@ using System.Globalization; using System.Text; using ExcelReader.Core.Parser; +using ExcelReader.Core.Parser.ParallelCsv; using ExcelReader.Core.Parser.SheetPipeline; using ExcelReader.Core.Reader; using ExcelReader.Core.Reader.Xlsx; @@ -189,6 +190,36 @@ public async Task AParseFailureAfterPipelineRecoveryReportsTheSheetRow() } } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task APartitionedAggregateFailureReportsTheSheetRow(bool needsRecovery) + { + CancellationToken ct = TestContext.Current.CancellationToken; + var sb = new StringBuilder("NameQty"); + for (int r = 2; r <= 300; r++) + { + string name = needsRecovery && r == 20 ? " ]]>" : string.Create(CultureInfo.InvariantCulture, $"n{r}"); + string qty = r == 211 ? "oops" : string.Create(CultureInfo.InvariantCulture, $"{r}"); + sb.Append(CultureInfo.InvariantCulture, $"{name}{qty}"); + } + using MemoryStream ms = WorkbookBuilder.Build(sb.ToString()); + using XlsxWorkbook workbook = Excel.FromXlsx(ms); + ExcelParser parser = ExcelParser.FromAttributes(new ExcelParserOptions { ThrowOnParseFailure = true }); + ExcelParseException sequential = await Assert.ThrowsAsync(() => SheetParallel.AggregateAsync( + workbook.FirstSheet, parser, new SheetParallelOptions { DegreeOfParallelism = 1, HeaderRow = 1 }, ct)); + Assert.Equal(211, sequential.Row); + SheetPipelineSource source = SheetPipelineSource.Create(workbook.FirstSheet)!; + + foreach (int chunkBytes in new[] { 64, 90, 96, 150, 400 }) + { + ExcelParseException parallel = await Assert.ThrowsAsync(() => SheetPipelineRunner.AggregateAsync( + source, MappedAggregation.Unbound, MappedAggregation.Binder(parser, parser.Info, false, 1), 1, 4, chunkBytes, ct)); + Assert.Equal(sequential.Row, parallel.Row); + Assert.Equal(sequential.ColumnName, parallel.ColumnName); + } + } + [Fact] public async Task ACallbackExceptionReachesTheCaller() { diff --git a/tests/ExcelReader.Tests/Parser/SheetPipeline/XlsxPipelineTests.cs b/tests/ExcelReader.Tests/Parser/SheetPipeline/XlsxPipelineTests.cs index e2621211..6e4f94ea 100644 --- a/tests/ExcelReader.Tests/Parser/SheetPipeline/XlsxPipelineTests.cs +++ b/tests/ExcelReader.Tests/Parser/SheetPipeline/XlsxPipelineTests.cs @@ -228,6 +228,19 @@ public async Task ARowLikeTagInsideARowMatchesSequential(string poison) } } + [Fact] + public async Task ALongReadAfterRecoveryMatchesSequential() + { + using MemoryStream ms = WorkbookBuilder.Build(Rows(5000, r => r <= 100 && r % 10 == 0 ? " ]]>" : "n")); + using XlsxWorkbook workbook = Excel.FromXlsx(ms); + List expected = Sequential(workbook, 0); + + foreach (int chunkBytes in new[] { 64, 90, 150, 400 }) + { + Assert.Equal(expected, await PipelineAsync(workbook, 0, 4, chunkBytes)); + } + } + [Fact] public async Task RowsAfterTheEndOfSheetDataAreIgnored() { diff --git a/tests/ExcelReader.Tests/Reader/DialectRobustnessTests.cs b/tests/ExcelReader.Tests/Reader/DialectRobustnessTests.cs index 9c1bd18d..4e19a725 100644 --- a/tests/ExcelReader.Tests/Reader/DialectRobustnessTests.cs +++ b/tests/ExcelReader.Tests/Reader/DialectRobustnessTests.cs @@ -82,6 +82,39 @@ public void SingleQuotedWorkbookAttributesAreRead() Assert.Equal("1", e.Current[0].GetString()); } + [Fact] + public void XlsxSheetWithUnresolvedRelationshipThrows() + { + var ms = new MemoryStream(); + using (var zip = new ZipArchive(ms, ZipArchiveMode.Create, leaveOpen: true)) + { + WriteEntry(zip, "xl/worksheets/sheet1.xml", + $""""""); + WriteEntry(zip, "xl/workbook.xml", + $""""""); + WriteEntry(zip, "xl/_rels/workbook.xml.rels", + $""""""); + } + ms.Position = 0; + + InvalidDataException ex = Assert.Throws(() => Excel.FromXlsx(ms)); + Assert.Contains("Lost", ex.Message, StringComparison.Ordinal); + } + + [Fact] + public void XlsbSheetWithUnresolvedRelationshipThrows() + { + byte[] workbookBin = + [ + .. B.Record(Brt.BundleSh, [.. B.U32(0), .. B.U32(0), .. B.WideString("rId1"), .. B.WideString("Data")]), + .. B.Record(Brt.BundleSh, [.. B.U32(0), .. B.U32(0), .. B.WideString("rId2"), .. B.WideString("Lost")]), + ]; + byte[] rels = Encoding.UTF8.GetBytes(""""""); + + InvalidDataException ex = Assert.Throws(() => XlsbWorkbookParts.ParseSheets(workbookBin, rels)); + Assert.Contains("Lost", ex.Message, StringComparison.Ordinal); + } + [Fact] public void SingleQuotedStyleAttributesDetectDate() { diff --git a/tests/ExcelReader.Tests/Reader/SchemaInferenceTests.cs b/tests/ExcelReader.Tests/Reader/SchemaInferenceTests.cs index d9ca2f9e..238019b8 100644 --- a/tests/ExcelReader.Tests/Reader/SchemaInferenceTests.cs +++ b/tests/ExcelReader.Tests/Reader/SchemaInferenceTests.cs @@ -2,6 +2,7 @@ using ExcelReader.Core.Reader.Csv; using ExcelReader.Core.Reader.Schema; using ExcelReader.Core.Reader.Xlsx; +using ExcelReader.Core.Writer.Xlsx; namespace ExcelReader.Tests.Reader { @@ -228,6 +229,66 @@ public void Should_GuessOneTypePerColumn_When_EverySampledCellAgrees() Assert.Equal(new ExcelColumnSchema { Index = 2, Name = "Ratio", Type = ExcelColumnType.Float64Column, IsNullable = false }, schema[2]); } + private static ExcelColumnType InferDateStyledColumn(bool date1904, params string[] values) + { + const string Styles = """"""; + string rows = string.Concat(values.Select((value, i) => $"""{value}""")); + using MemoryStream ms = WorkbookBuilder.Build(rows, styles: Styles, date1904: date1904); + using XlsxWorkbook reader = Excel.FromXlsx(ms.ToArray()); + return Excel.InferSchema(reader.FirstSheet, new SchemaInferenceOptions { HeaderRow = 0 })[0].Type; + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Should_KeepADateColumn_When_EverySampledDateIsAtMidnight(bool date1904) + { + Assert.Equal(ExcelColumnType.DateColumn, InferDateStyledColumn(date1904, "45293", "45294")); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Should_InferATimestamp_When_ASampledDateCarriesATimeOfDay(bool date1904) + { + Assert.Equal(ExcelColumnType.TimestampColumn, InferDateStyledColumn(date1904, "45293.520833333336")); + } + + [Fact] + public void Should_PromoteToTimestamp_When_DatesAndTimestampsMix() + { + Assert.Equal(ExcelColumnType.TimestampColumn, InferDateStyledColumn(false, "45293", "45293.5", "45294")); + } + + [Fact] + public void Should_KeepADateColumn_When_ADateStyledSerialIsOutOfRange() + { + Assert.Equal(ExcelColumnType.DateColumn, InferDateStyledColumn(false, "45293", "1E+300")); + } + + [Fact] + public void Should_InferATimestamp_When_ADateTimeIsWrittenThroughTheFacade() + { + using var ms = new MemoryStream(); + using (XlsxWorkbookWriter wb = XlsxWorkbookWriter.Create(ms, leaveOpen: true)) + { + XlsxSheetWriter sheet = wb.AddSheet("S"); + using (XlsxRowWriter row = sheet.StartRow()) + { + row.Write("When"); + } + using (XlsxRowWriter row = sheet.StartRow()) + { + row.Write(new DateTime(2024, 1, 2, 12, 30, 0)); + } + sheet.End(); + wb.End(); + } + using XlsxWorkbook reader = Excel.FromXlsx(ms.ToArray()); + + Assert.Equal(ExcelColumnType.TimestampColumn, Excel.InferSchema(reader.FirstSheet)[0].Type); + } + [Fact] public void Should_FallBackToString_When_SampledCellsMixKinds() { diff --git a/tests/ExcelReader.Tests/Reader/SheetDataReaderTests.cs b/tests/ExcelReader.Tests/Reader/SheetDataReaderTests.cs index e725b8f9..c9d6c178 100644 --- a/tests/ExcelReader.Tests/Reader/SheetDataReaderTests.cs +++ b/tests/ExcelReader.Tests/Reader/SheetDataReaderTests.cs @@ -89,6 +89,31 @@ public async Task HeaderRowBeyondSheetSizeIsAnEmptyResultSet() Assert.False(data.Read()); } + [Fact] + public async Task GetOrdinalResolvesTheNamesGetNameSynthesizes() + { + await using var ms = await TypedWorkbook.BuildAsync([7, "hello"]); + await using IExcelWorkbook reader = await Excel.OpenAsync(ms, ct: TestContext.Current.CancellationToken); + using var data = new SheetDataReader(reader.FirstSheet, new SheetDataReaderOptions { HeaderRow = 0 }); + + Assert.Equal(0, data.GetOrdinal(data.GetName(0))); + Assert.Equal(1, data.GetOrdinal("column1")); + } + + [Fact] + public async Task GetOrdinalPrefersARealHeaderOverASynthesizedNameItCollidesWith() + { + await using var ms = await TypedWorkbook.BuildAsync( + ["Id", null, "Column1", null, "Last"], + [1, 2, 3, 4, 5]); + await using IExcelWorkbook reader = await Excel.OpenAsync(ms, ct: TestContext.Current.CancellationToken); + using var data = new SheetDataReader(reader.FirstSheet); + + Assert.Equal("Column1", data.GetName(1)); + Assert.Equal(2, data.GetOrdinal("Column1")); + Assert.Equal(3, data.GetOrdinal(data.GetName(3))); + } + [Fact] public async Task GetOrdinalForUnknownNameThrows() {