Asthenia0412 commented on PR #1376:
URL: https://github.com/apache/arrow-go/pull/1376#issuecomment-6176413510
Thanks for the thorough review @zeroshade! I have addressed all three points:
1. **Level documentation**: Updated `WithZstdLevel` doc comment to
explicitly map Zstandard levels 1-22 to the four encoder speed presets
(`SpeedFastest`, `SpeedDefault`, `SpeedBetterCompression`,
`SpeedBestCompression`) and document that `level <= 0` falls back to the
default.
2. **Implicit codec selection**: `WithZstdLevel` now sets `cfg.codec =
flatbuf.CompressionTypeZSTD` so it works standalone without requiring a
separate `WithZstd()` call.
3. **Tests**: Added `TestWithZstdLevel` covering:
- NewWriter roundtrip with level
- NewFileWriter roundtrip with level
- Different levels (1 vs 6) producing different compressed sizes
(demonstrating the encoder preset mapping)
- Non-positive level falling back to default
Regarding the 9-parameter `newRecordEncoder` signature — I recognize this is
unwieldy. A compression config struct would be cleaner, but given the scope of
this change (adding one int parameter threaded through an existing pipeline), I
kept the signature change minimal to avoid introducing broader refactoring.
Happy to bundle this as a follow-up if you think it warrants it.
All IPC tests pass locally (`go test ./arrow/ipc/... -count=1`). Let me know
if anything else needs adjustment!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
