fix: avoid LZ4 overflow when compressing large dictionary values

[beinan]

Dictionary values are compressed with a single LZ4 call, but the default compression for that buffer is hardcoded to lz4 with no size check (primitive.rs:108). Once the dictionary exceeds LZ4_MAX_INPUT_SIZE (0x7E000000), the write fails with a bare Compression input too long from the lz4 crate.

Why this is reachable

This is not a synthetic edge case. A column whose values are duplicate-heavy enough to be dictionary encoded builds a dictionary bounded only by 0.8 * data_size, so a sufficiently large page can produce an oversized dictionary buffer.

The path is: should_dictionary_encode accepts the block → build_dict_values_compressor_field synthesizes a field carrying lance-encoding:compression = "lz4" → that hits the user-requested branch of try_general_compression, bypassing the MIN_BLOCK_SIZE_FOR_GENERAL_COMPRESSION size test entirely → CompressedBufferEncoder::compress flattens the dictionary and makes one LZ4 call.

Worth noting the automatic branch of try_general_compression is not implicated: it selects CompressedBufferEncoder::default(), which is zstd whenever the zstd feature is on.

I reproduced this with a list<large_binary> column of low-entropy values, which fails on a 2.25 GB dictionary buffer.

The fix

Fall back to zstd for the default when the dictionary values buffer is too large for LZ4. Zstd has no comparable input limit. An explicitly requested scheme (field metadata or env var) is still honored, so a user's choice is never silently changed — they get the error instead.

Also validate the input length in Lz4BufferCompressor::compress. The lz4 crate's error names neither the actual size nor the limit, which makes this hard to diagnose; the error now reports both and points at zstd.

Note the LZ4 buffers here prepend the uncompressed length as a u32, so this format cannot represent larger inputs regardless of compress_bound.

On the ratio tradeoff

Zstd is not a regression for this buffer. Measured at 256 MB with the crate versions in-tree, zstd level 0 (the default used elsewhere in lance-encoding):

data	zstd:0	lz4
dictionary buffer (repro shape)	5823x @ 1280 MB/s	249x @ 11594 MB/s
low-entropy tensor bytes	1.46x @ 113 MB/s	1.00x @ 1488 MB/s
random (control)	1.00x	1.00x

Zstd does not lose on ratio anywhere and wins substantially on the buffer in question. Since the fallback only triggers above 2 GiB — where LZ4 cannot run at all — the alternative is a failed write.

Tests

• test_lz4_input_size_limit — the size guard, written against a pure function so it does not allocate multiple GiB.
• test_resolve_dict_values_compression_metadata_large_falls_back_to_zstd — oversized falls back; exactly at the limit still uses LZ4.
• test_resolve_dict_values_compression_metadata_large_respects_explicit_request — an explicit lz4 request is preserved.

Verified end-to-end that the original 6.29 GiB list<large_binary> repro now writes successfully. cargo test -p lance-encoding (557 passed) and cargo test -p lance-file (155 passed) are green; cargo fmt --all and cargo clippy --tests -- -D warnings are clean.

The end-to-end repro itself is not included as a test since it allocates ~7 GiB, which is not appropriate for CI.

🤖 Generated with Claude Code

[lance-gatekeeper]

❌ Gate recommendation: request changes.

The default fallback is the right format-compatible direction, but codec selection must use the exact serialized block length and choose only a codec compiled into the current build. A complete revision can preserve explicit choices, use LZ4 only when its actual input fits, then use zstd—or uncompressed encoding when zstd is unavailable—for the implicit fallback.

[lance-gatekeeper]

dictionary_data.data_size() is smaller than the buffer that this compressor actually passes to LZ4 for variable-width dictionaries. VariableEncoder adds 8 bytes for 32-bit offsets or 16 bytes for 64-bit offsets, so a dictionary with data_size() == LZ4_MAX_INPUT_SIZE selects LZ4 here but reaches check_input_size at MAX + 8 / MAX + 16 and still fails the write. Select from the exact serialized block length (or serialize once before selection), and test the fixed/u32/u64 boundaries.

Bounded reproducer

The full writer case needs a multi-GiB allocation, so I ran this no-allocation boundary reproduction using the current head's selector and framing formulas:

python3 -B -c 'lz4_max=0x7E000000
dict_data_size=lz4_max
selected="zstd" if dict_data_size > lz4_max else "lz4"
for offset_bits, framing in ((32, 8), (64, 16)):
    serialized_size=dict_data_size+framing
    print(offset_bits, selected, serialized_size, serialized_size > lz4_max)
    assert selected == "lz4" and serialized_size > lz4_max'

Observed:

32 lz4 2113929224 True
64 lz4 2113929232 True

[lance-gatekeeper]

This selects zstd even when the zstd feature is not compiled. The supported lz4,bitpacking feature configuration builds successfully, but an oversized implicit dictionary then asks GeneralBufferCompressor for zstd and gets package was not built with zstd support, so the original write failure remains. Make the implicit fallback feature-aware (zstd when available, otherwise none); explicit codec requests should continue to fail rather than being silently changed.

Reproducer

I ran a disposable crate with this dependency:

lance-encoding = { path = "/home/repo/rust/lance-encoding", default-features = false, features = ["lz4", "bitpacking"] }

use lance_encoding::encodings::physical::block::{
    CompressionConfig, CompressionScheme, GeneralBufferCompressor,
};

fn main() {
    let error = GeneralBufferCompressor::get_compressor(CompressionConfig::new(
        CompressionScheme::Zstd, None,
    ))
    .unwrap_err();
    println!("{error}");
    assert!(error.to_string().contains("not built with zstd support"));
}

Command: CARGO_TARGET_DIR=/home/agent/tmp/gate8359-feature-repro-target cargo run --quiet

Observed: Invalid user input: package was not built with zstd support

[beinan]

Pushed 8d8dafd addressing both review points, plus real-world numbers from the reporter.

Review feedback

Exact serialized length. Confirmed — the check compared LZ4's limit against DataBlock::data_size(), but CompressedBufferEncoder compresses the serialized form, which VariableEncoder prepends a header to (16 bytes for 64-bit offsets). My original repro shows the gap directly: data_size=2250400008 vs actual lz4 input_len=2250400024. A dictionary just under the limit would have serialized past it and still overflowed. Added variable_encoded_size() in binary.rs, next to the compress impl it mirrors, and the selection now checks against that. Covered by test_dict_values_serialized_size_accounts_for_header.

Codec availability. Also confirmed — falling back to "zstd" unconditionally fails with package was not built with zstd support on a build without that feature. The fallback is now zstd when compiled in, otherwise "none" (uncompressed) so the write still succeeds rather than trading one failure for another. The oversized test asserts against cfg!(feature = "zstd").

Note the library builds clean with --no-default-features --features lz4,bitpacking. The --tests build for that feature set fails on block.rs:679 importing block::zstd, but I confirmed that break pre-exists on main and is unrelated to this PR — left it alone to keep this focused.

Real-world numbers

The reporter verified the workaround on production data and measured it — single row, 2.431 GiB payload, 2600 chunks:

	lz4	zstd	delta
on disk	278.6 MiB	182.1 MiB	−35%
ratio	8.94×	13.67×	+53%
write	2.94 s / 848 MiB/s	4.28 s / 582 MiB/s	1.46× slower
read	0.85 s / 2915 MiB/s	1.32 s / 1889 MiB/s	1.54× slower

This is a better picture than my synthetic benchmark: 35% smaller on disk for ~1.5× slower write and read. Worth noting that tradeoff only applies where both codecs actually work — the fallback here triggers only above the LZ4 limit, where the alternative is a failed write, so nothing that succeeds today changes behavior.

Verification

cargo test -p lance-encoding --lib (558 passed), cargo test -p lance-file (155 passed), cargo fmt --all and cargo clippy -p lance-encoding --tests -- -D warnings clean. Re-confirmed the 6.29 GiB list<large_binary> repro still writes successfully after the rework.

[lance-gatekeeper]

✅ Gate recommendation: approve.

The revision now selects from the exact serialized dictionary-buffer length, preserves explicit codec choices, and uses an available existing encoding when LZ4 cannot accept the buffer. The focused boundary, precedence, codec-guard, feature-build, and dictionary round-trip checks found no blocker.

[Xuanwo]

I don’t fully understand this issue. Are we talking about a dictionary larger than 2 GiB? If so, it feels like we should approach such data with a different strategy.

How large those large_binary? Should they be stored as blob?

[beinan]

@Xuanwo Good questions, and I think you're right on the design point. Let me answer directly.

Yes, this is a dictionary larger than 2 GiB, and nothing currently prevents that. The budget is purely proportional with no absolute term (primitive.rs:6047):

let max_encoded_size = (data_size as f64 * threshold_ratio) as u64;  // 0.8 * data_size

It is enforced during encoding (dict.rs:177-191 aborts cleanly if exceeded), but it scales without limit with page size. In my repro the dictionary was 2.25 GB against a 5.4 GB budget — comfortably within it. DEFAULT_DICT_MAX_CARDINALITY = 100_000 caps entries, not bytes.

How large are the values? ~0.96 MB each (2.431 GiB / 2600 chunks). Which leads to the more interesting problem:

There is no per-value size gate on dictionary encoding at all. The gates are block type, a minimum of 100 values, and sample uniqueness < 0.98. Meanwhile is_narrow uses MINIBLOCK_MAX_BYTE_LENGTH_PER_VALUE = 256 (primitive.rs:4793) to decide values are too wide for miniblock. But dictionary is tried first and short-circuits that judgment (primitive.rs:6544), so ~1 MB values get forced into miniblock anyway — a path the codebase's own heuristic says they don't belong in. Without dictionary encoding this data would take fullzip.

So I agree the deeper issue is that dictionary encoding is running on data it was never meant for.

On blob: yes, that's likely the right answer for this user, and it does work on a list child — the marker goes on the item field, as test_write_and_scan_struct_nested_list_blob_v2 (dataset/blob.rs:5670) shows for exactly this shape. The guide recommends it above ~1 MB, which is this data almost exactly. I'll relay that.

What I'd suggest for this PR: I think it's still worth landing as-is, but on narrower grounds than I originally framed it. Independent of whether the dictionary should be this large, create_block_compressor hands Lz4BufferCompressor a buffer it cannot encode, and the resulting error names neither the size nor the limit. That's a bug at that layer regardless of what we decide upstream of it. But you're right that it makes the giant dictionary encodable rather than preventing it.

For the root cause I see two options, and I'd rather you pick than guess:

1. A max-value-width gate in should_dictionary_encode, reusing the Stat::MaxLength that dict.rs:131 already fetches. Targets the actual defect — dictionary bypassing the narrowness judgment. Riskier: I haven't measured the compression-ratio impact on legitimately duplicate-heavy mid-size columns.
2. An absolute cap on max_encoded_size. Blunter but safer, and precedented — feat: abort dictionary encode if not useful #5055 established the abort pattern and refactor: use dict entries and encoded size instead of cardinality for dict decision #5891 deliberately made encoded size the knob.

Happy to do either in this PR or a follow-up. If you'd prefer I fold one in here and drop the codec fallback entirely, that also works — though I'd keep the check_input_size guard so this fails with a legible error rather than a bare Compression input too long if it's ever reached another way.

[beinan]

@Xuanwo here is the reproduction, so you can judge the "is this real" part independently of the design discussion.

Reproducer

Drop this in rust/lance-file/tests/ (needs arrow-buffer in dev-deps). It fails on main and passes with this PR. ~20s, allocates ~7 GiB.

#[test_log::test(tokio::test)]
#[ignore = "allocates ~7 GiB"]
async fn repro_compression_input_too_long() {
    const VALUE_SIZE: usize = 45_000;
    const NUM_DISTINCT: usize = 50_000;
    const NUM_VALUES: usize = 150_000;

    // Distinct values, but low byte entropy, and repeated in runs so the
    // 4096-element uniqueness sample sees duplicates and dictionary encoding wins.
    let distinct: Vec<Vec<u8>> = (0..NUM_DISTINCT).map(|i| {
        let mut v: Vec<u8> = (0..VALUE_SIZE).map(|j| (j % 16) as u8).collect();
        v[..8].copy_from_slice(&(i as u64).to_le_bytes());
        v
    }).collect();
    let values: Vec<&[u8]> = (0..NUM_VALUES)
        .map(|i| distinct[(i / 3) % NUM_DISTINCT].as_slice()).collect();

    let arr = LargeBinaryArray::from_iter_values(values);
    let item_field = Arc::new(Field::new("item", DataType::LargeBinary, true));
    let offsets = OffsetBuffer::new(vec![0i32, arr.len() as i32].into());
    let list = ListArray::new(item_field.clone(), offsets, Arc::new(arr), None);
    let schema = Arc::new(Schema::new(vec![
        Field::new("data", DataType::List(item_field), false)]));
    let batch = RecordBatch::try_new(schema.clone(), vec![Arc::new(list)]).unwrap();

    let fs = FsFixture::default();
    let reader = RecordBatchIterator::new(vec![Ok(batch)], schema);
    write_lance_file(reader, &fs, ConcreteFileVersion::V2_2,
        FileWriterOptions::default()).await;
}

On main (6.29 GiB payload):

IO { source: Custom { kind: InvalidInput, error: "Compression input too long." },
     location: block.rs:316 }

With instrumentation showing the chain:

dict gate: num_values=150000 threshold_card=75000 data_size=6751200008
           sample_unique_ratio=Some(Some(0.3359375))
dict path: dict data_size=2250400008
explicit scheme=lz4 data_size=2250400008
lz4 compress input_len=2250400024

The dictionary is 2.25 GB against a 5.4 GB budget — well within it. Nothing rejects it. The reporter hit this on production data with no unusual settings; LANCE_ENCODING_DICT_VALUES_COMPRESSION=zstd is what unblocked them.

So: real bug, reachable from an ordinary write, and the error names neither the size nor the limit.

On blob — I think it only partly fits

You are right that blob is the better home for ~1 MB values. But talking to the reporter, their column is not uniformly large: most values are small, a few are very large, and the distribution is skewed. Declaring the whole column a blob column to accommodate a minority of big values is a heavier change than the data warrants, and it changes the read API for every consumer of that column.

Blob v2 does handle skew better than I first credited — the tiering is per value, not per column (dataset/blob.rs:51-52):

const INLINE_MAX: usize = 64 * 1024;              // 64KB inline cutoff
const DEDICATED_THRESHOLD: usize = 4 * 1024 * 1024; // 4MB dedicated cutoff

So small values stay inline and only the heavy ones go to sidecars. That is a genuinely reasonable fit, and I will pass it along as a recommendation.

(Minor: docs/src/guide/blob.md:402 states 16 KiB and 2 MiB for these two thresholds, which disagrees with the code. Happy to file that separately.)

But I do not think it resolves the bug. Blob is opt-in per field, and the failure happens for users who have not opted in and have no signal that they should — the write simply fails with an error that does not suggest a remedy. Whatever we recommend for this schema, an ordinary list<large_binary> write should not hit an unencodable buffer.

That is why I would still land the codec-selection fix, and treat the "should a >2 GiB dictionary exist" question as the separate follow-up. I am happy to take that follow-up too — I just would not want the crash to stay in main while we settle the larger design question. Your call on whether to fold it in here.

[Xuanwo]

Thank you @beinan, I will take a think around this and maybe answer you on Monday.

[beinan]

Thank you @beinan, I will take a think around this and maybe answer you on Monday.

No rush, happy to chat any time

[lance-gatekeeper]

🟡 Gate recommendation: maintainer decision required.

The codec fix is correct for the demonstrated LZ4 boundary, but it also decides whether the default writer should admit a >2 GiB dictionary that readers must fetch, decode, and cache as one unit. The format defines that eager dictionary contract but no acceptable maximum, so the resource policy cannot be resolved from existing invariants.

Please choose between:

• preserving dictionary compression and using zstd or uncompressed storage when implicit LZ4 cannot encode the buffer, accepting multi-GiB dictionary cache state;
• imposing an absolute size or codec-capability cutoff for auto-generated dictionaries and falling back to the existing structural encoding, with raw storage or a descriptive error for source dictionary arrays.

Blob remains an opt-in schema choice with different read semantics, not a transparent fallback. The deciding criterion is whether compression ratio and successful generic inline writes outweigh bounded initialization and cache memory. A max-value-width gate is not equivalent: miniblocks contain dictionary indices, not those original values.