fix(arrow-ipc): return an error for a DictionaryBatch without its data

[PlenoraETL]

Which issue does this PR close?

Closes #11019.

Rationale for this change

DictionaryBatch.data is optional in the flatbuffer grammar and required by the format. get_dictionary_values opened it with unwrap, so a file that omits it panicked instead of producing an ArrowError.

This is the same class of field, in the same file, that the footer's custom metadata was just fixed for:

let (Some(key), Some(value)) = (kv.key(), kv.value()) else {
    return Err(ArrowError::ParseError(
        "Custom metadata in the IPC footer is missing a key or a value".to_string(),
    ));
};

The patch uses that shape. The proposal is essentially "finish the round".

A panic matters more than usual on this path: under libfuzzer-sys a caught panic still becomes abort() before unwinding, so a fuzz target reading untrusted IPC goes into quarantine rather than reporting a rejected input.

What changes are included in this PR?

One let ... else in get_dictionary_values, plus a regression test.

Measured before and after on main: the fixture panics without the change and returns Parser error: Dictionary batch is missing its data with it. cargo test -p arrow-ipc passes — 132, 7 and 11.

Are these changes tested?

Yes. The fixture is the original 2169-byte input from the fuzzing corpus of plenora-IO-tools, not a reduced one, added under arrow-ipc/test/data/.

Are there any user-facing changes?

A file that used to panic now returns an ArrowError::ParseError. No API change.

[Jefffrey]

Suggested change

	// `DictionaryBatch.data` is optional in the flatbuffer grammar and
	// required by the format, so a message that omits it panics here
	// rather than producing an error. This mirrors the check the footer
	// metadata already got: report it, don't unwrap it.

these LLM comments are really not necessary

[Jefffrey]

Suggested change

	/// A dictionary message that omits its `data` must report an error.
	///
	/// `DictionaryBatch.data` is optional in the flatbuffer grammar and required
	/// by the format, so a file that omits it is structurally decodable but
	/// semantically incomplete. `get_dictionary_values` used to open it with
	/// `unwrap`, which turned a malformed file into a panic.
	///
	/// The fixture is the original 2169-byte input from a fuzzing corpus, not a
	/// reduced one.

same here

[Jefffrey]

Suggested change

	fn dictionary_batch_without_data_is_an_error_not_a_panic() {
	fn test_invalid_dictionary_batch_without_data() {

[Jefffrey]

please move this test down; it shouldnt be above the imports

[Jefffrey]

could we refactor the test to somehow create this data in memory as part of the test? instead of commiting a separate file

[PlenoraETL]

Thanks for the review — all five points addressed, branch force-pushed.

• Dropped the comment above the fix and the doc comment on the test.
• Renamed to test_invalid_dictionary_batch_without_data and moved it down, next to test_missing_buffer_metadata_error.
• Removed the committed .arrow file: the test now builds the malformed DictionaryBatch in memory with FlatBufferBuilder, following the shape of test_missing_buffer_metadata_error. It calls read_dictionary directly, so there's no file and no byte-patching involved.

cargo test -p arrow-ipc passes (132 / 7 / 11) and clippy is clean.

One note in case it matters to you: I had to #[allow(deprecated)] on Field::new_dict, since read_dictionary looks the field up by dict id and that's the constructor that sets one. Happy to build the schema differently if there's a preferred way now that dict ids are on their way out.