Visitar URL original
fix: write the Feather file version the Data Exporter asked for by MaxFreedomPollard · Pull Request #1158 · tconbeer/harlequin · GitHub
Skip to content

fix: write the Feather file version the Data Exporter asked for - #1158

Merged
tconbeer merged 1 commit into
tconbeer:mainfrom
MaxFreedomPollard:fix/feather-file-version
Sep 15, 2026
Merged

tconbeer merged 1 commit into
tconbeer:mainfrom
MaxFreedomPollard:fix/feather-file-version

Conversation

@MaxFreedomPollard

Copy link
Copy Markdown
Contributor

Exporting a query to Feather from the Data Exporter always writes a Feather V1 file, whatever the dialog's File Version says, and the Compression and Chunk Size options are silently dropped. The dialog defaults File Version to 2 and describes Version 1 as "the more limited legacy format", so the default export is the legacy format the dialog says it is not writing. The file on disk starts with FEA1 rather than ARROW1, and Uncompressed, LZ4 and zstd all produce the same file, byte for byte. File Version = 1 is the only setting that does what the dialog promises, and it does so by accident. parquet, orc, csv and json are unaffected, and so is hsql --format feather, which passes no file options at all and so gets pyarrow's own default.

There is no open issue for this. I found it reading export.py, so the CHANGELOG entry references none.

What are the key elements of this solution?

_export_feather (src/harlequin/export.py:288 on main) names compression, compression_level and chunksize and coerces the last two to ints, but never names version, so the dialog's value falls into **kwargs and reaches pyarrow.feather.write_feather untouched. The dialog's Select holds its choices as text (choices=[("2", "2"), ("1", "1")], default="2" in src/harlequin/copy_formats.py) and CopyOptionsMenu.current_options sends every declared option, touched or not, so what arrives is the string "2". pyarrow picks the format with an equality test that a string loses: if version == 2 in _feather.pyx selects V2 and anything else falls to the V1 branch. The Python wrapper's own version == 1 guard is skipped for the same reason, so the V2-only keywords are handed to a V1 writer that ignores them instead of refusing them.

The fix gives version a name, coerces it in the try block that already coerces compression_level and chunksize, and forwards it explicitly. It also maps the dialog's uncompressed to None when File Version = 1, because V1 holds no compression and pyarrow refuses the keyword there. Without that, the one setting that worked by accident would start erroring.

TestDeclaredOptions already describes this defect class in its docstring, an option the writer has no name for being a silent no-op, and its st_size > 0 assertion is the gap that let this one through: it sends the string "2" and a V1 file is not empty. The existing TestFeatherOptions::test_version passes {"version": int(version)}, an int the dialog cannot send, which is the other reason it went unnoticed. Both new tests in TestFeatherOptions are driven by a FEATHER_DIALOG mapping built from the declared options, so they stay pinned to what the dialog actually sends. TestFeatherOptions::test_the_version_the_dialog_sends_is_the_version_written asserts on the file's magic bytes; on main the [1-FEA1] case passes, because V1 is what main writes for everything, and [2-ARROW1] fails. TestFeatherOptions::test_the_compression_the_dialog_sends_changes_the_file asserts a compressed file differs from an uncompressed one; on main both [lz4] and [zstd] fail, because the bytes are identical.

Why did you design your solution this way? Did you assess any alternatives? Are there tradeoffs?

The alternative was to convert the value in the dialog, by giving SelectOption a typed choice. That changes harlequin.options, which every out-of-tree adapter builds against, for one option in one format. The writer already does this coercion for compression_level and chunksize, so version belongs beside them.

The tradeoff is that 12 of the 24 combinations the dialog can send, counting each of its two text boxes as empty or filled, now open an error modal where they used to write a V1 file. Each one is pyarrow refusing a genuine contradiction in its own words: V1 with LZ4 or zstd (8 of them), V1 with a chunk size (2), and Uncompressed with a compression level (2). The other 12 write and round-trip, 10 as ARROW1 and 2 as FEA1. Surfacing Arrow's refusals as an error modal is what this file already does, per the 2.8.1 entry about ORC and Feather options.

One combination stays quiet: File Version = 1 with a compression level typed still writes its FEA1 file and ignores the level, since V1 has no compression for a level to apply to. That is pyarrow's own behavior; I did not extend the special case to dropping a level the user typed.

The blast radius is the IDE's Data Exporter only. hsql builds file_options for csv and tsv alone, so feather gets {}, and the new version: str | int = 2 default keeps that path byte-identical.

Does this PR require a change to Harlequin's docs?

  • No.
  • Yes, and I have opened a PR at tconbeer/harlequin-web.
  • Yes; I haven't opened a PR, but the gist of the change is: ...

Did you add or update tests for this change?

  • Yes.
  • No, I believe tests aren't necessary.
  • No, I need help with testing this change.

Please complete the following checklist:

  • I have added an entry to CHANGELOG.md, under the [Unreleased] section heading. That entry references the issue closed by this PR.
  • I acknowledge Harlequin's MIT license. I do not own my contribution.

Testing performed on macOS 26.2 arm64, Python 3.10.15.

uv run pytest tests/unit_tests/test_export.py -m "not online" -n0 gives 121 passed. With src/harlequin/export.py reverted to main and the new tests left in place, the same command gives 3 failed, 118 passed, naming test_the_version_the_dialog_sends_is_the_version_written[2-ARROW1], test_the_compression_the_dialog_sends_changes_the_file[lz4] and [zstd].

uv run pytest -m "not online" gives 1 failed, 2028 passed, 6 skipped, with 148 snapshots passed. The failure is tests/adapter_tests/test_sqlite.py::test_load_extension, and it fails identically on unmodified main here: the committed hello0.dylib fixture is x86_64 and dlopen wants arm64e on this machine.

ruff format . --diff gives 199 files already formatted, ruff check . gives All checks passed!, mypy gives Success: no issues found in 151 source files, and lint-imports gives Contracts: 5 kept, 0 broken.

Outside pytest, calling write_file with exactly the mapping the dialog builds from its declared defaults writes a file whose first bytes are FEA1 on main and ARROW1 with this change, and on the test fixture's two-row table Uncompressed, LZ4 and zstd go from three identical 272-byte files to 666, 770 and 762 bytes. The same call with no file options, which is the hsql path, writes ARROW1 either way.

Exporting a query to Feather from the Data Exporter always wrote a Feather V1
file, whatever the dialog's File Version said, and the Compression and Chunk
Size options were silently dropped.

`_export_feather` never named `version`, so the copy dialog's value fell into
`**kwargs` and reached pyarrow as the string it was declared with. pyarrow
writes V2 only when `version` equals the integer 2 and the legacy V1 format for
anything else, so every export from the dialog wrote a V1 file and V1 quietly
ignored the Compression and Chunk Size keywords the Python wrapper had already
waved through.

The writer now takes `version` by name and coerces it to an int beside the two
options it already coerced. Feather V1 holds no compression at all, so the
dialog's `uncompressed` becomes `None` when V1 was asked for, which is how the
V1 writer spells the same thing.
@tconbeer
tconbeer merged commit 33b8c0e into tconbeer:main Sep 15, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants