diff --git a/CHANGELOG.md b/CHANGELOG.md index 15aa84a4..7037303d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- `WriteOptions.withZstd(true)` now throws `IllegalArgumentException` when `allowedCascading()` is `0` — Zstd only ever competes inside the cascade, so at the default cascading depth the flag was a silent no-op producing byte-identical output to `defaults()`. ([#366](https://github.com/dfa1/vortex-java/pull/366)) + ## [0.14.0] — 2026-09-03 ### Added diff --git a/docs/reference.md b/docs/reference.md index 7b1c4023..d7708064 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -144,7 +144,7 @@ Record: `(int chunkSize, boolean enableZoneMaps, double compressionRatioThreshol |--------|-------| | `withZoneMaps(boolean)` | Toggle per-chunk min/max/sum statistics | | `withGlobalDict(boolean)` | Toggle the shared cross-chunk dictionary | -| `withZstd(boolean)` | Add Zstandard to the cascade codec competition | +| `withZstd(boolean)` | Add Zstandard to the cascade codec competition. Requires `allowedCascading > 0` — Zstd only competes inside the cascade, so `withZstd(true)` throws `IllegalArgumentException` at depth 0; combine with `cascading(depth)` | | `withGlobalDictMaxRetainedBytes(long)` | Aggregate heap budget for buffered global-dict candidate columns | | `withEdition(Edition)` | Enable an [edition](#editions) for its family, replacing any edition already enabled for that family. No method disables the guard entirely — an `unstable`-family encoding is reached by enabling its edition explicitly, e.g. `withEdition(Editions.UNSTABLE_2025_05_0)` | diff --git a/writer/src/main/java/io/github/dfa1/vortex/writer/WriteOptions.java b/writer/src/main/java/io/github/dfa1/vortex/writer/WriteOptions.java index 8a19705d..b0cb8d15 100644 --- a/writer/src/main/java/io/github/dfa1/vortex/writer/WriteOptions.java +++ b/writer/src/main/java/io/github/dfa1/vortex/writer/WriteOptions.java @@ -21,6 +21,9 @@ /// file size by 10–15% on real-world datasets compared to ALP+bitpack alone. /// Trade-off: Zstd decompression is ~6× slower than ALP decode; /// prefer the default (`false`) for read-heavy workloads. +/// Requires `allowedCascading > 0`: Zstd only ever competes inside the +/// cascade, so `true` at depth 0 would be a silent no-op — the compact +/// constructor rejects that combination instead. /// @param globalDictMaxRetainedBytes aggregate byte budget for the buffered per-chunk code arrays all /// global-dictionary candidate columns may retain in the heap while /// waiting for `close()` (default 1 GB). A shared dictionary must see @@ -55,8 +58,20 @@ public record WriteOptions( MemorySize globalDictMaxRetainedBytes, Map editions ) { - /// Defensively copies `editions` into an immutable map. + /// Defensively copies `editions` into an immutable map and rejects a Zstd flag that could + /// never take effect. + /// + /// @throws IllegalArgumentException if `enableZstd` is `true` while `allowedCascading` is `0`: + /// Zstd only ever competes inside the cascade + /// (`VortexWriter` only builds a `CascadingCompressor`, which + /// is where the Zstd codec is added, when `allowedCascading > + /// 0`); at depth 0 the flag would be silently ignored. public WriteOptions { + if (enableZstd && allowedCascading == 0) { + throw new IllegalArgumentException( + "enableZstd requires allowedCascading > 0 (Zstd only competes inside the cascade); " + + "use WriteOptions.cascading(depth).withZstd(true)"); + } editions = Map.copyOf(editions); } @@ -120,8 +135,13 @@ public WriteOptions withGlobalDict(boolean enabled) { /// Trade-off: Zstd decompression is ~6× slower than ALP reconstruction or bitpack unpack. /// Use `false` (the default) for read-heavy or latency-sensitive workloads. /// + /// `enabled=true` requires `allowedCascading() > 0` — Zstd only ever competes inside the + /// cascade, so enabling it at depth 0 (e.g. straight off [#defaults()]) would be a silent + /// no-op. Combine with [#cascading(int)]: `WriteOptions.cascading(depth).withZstd(true)`. + /// /// @param enabled `true` to enable Zstd in the compression cascade /// @return a new `WriteOptions` with the Zstd flag updated + /// @throws IllegalArgumentException if `enabled` is `true` and `allowedCascading()` is `0` public WriteOptions withZstd(boolean enabled) { return new WriteOptions(chunkSize, enableZoneMaps, compressionRatioThreshold, allowedCascading, globalDict, enabled, globalDictMaxRetainedBytes, editions); diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/WriteOptionsTest.java b/writer/src/test/java/io/github/dfa1/vortex/writer/WriteOptionsTest.java index e1a66063..4274ff14 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/WriteOptionsTest.java +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/WriteOptionsTest.java @@ -5,6 +5,7 @@ import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; /// Unit tests for [WriteOptions] factories and copy-methods. class WriteOptionsTest { @@ -63,4 +64,25 @@ void withGlobalDictMaxRetainedBytes_returnsNewInstance() { assertThat(result).isNotSameAs(base); assertThat(base.globalDictMaxRetainedBytes()).isEqualTo(DEFAULT_BUDGET); } + + @Test + void withZstd_onDefaults_rejectedBecauseCascadingIsZero() { + // Given — defaults() has allowedCascading == 0, so Zstd could never be reached by + // VortexWriter (it is only added to the cascade codec list, which is only consulted when + // allowedCascading > 0); this must fail loudly rather than silently write plain files. + WriteOptions base = WriteOptions.defaults(); + + // When / Then + assertThatIllegalArgumentException().isThrownBy(() -> base.withZstd(true)); + } + + @Test + void cascading_withZstd_succeeds() { + // Given / When + WriteOptions result = WriteOptions.cascading(3).withZstd(true); + + // Then + assertThat(result.enableZstd()).isTrue(); + assertThat(result.allowedCascading()).isEqualTo(3); + } }