Skip to content

Conversation

@brancz
Copy link
Contributor

@brancz brancz commented Nov 10, 2024

Which issue does this PR close?

Closes #6710

Rationale for this change

See issue.

What changes are included in this PR?

Fix writing of IPC files where dict IDs are not preserved.

Are there any user-facing changes?

No user-facing changes, only a bug fix.

@tustvold @alamb

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @brancz . Nice find!

}

#[test]
fn test_roundtrip_nested_dict_no_preserve_dict_id() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified that this test covers the bugfix by reverting the code changes and running the tests:

called `Result::unwrap()` on an `Err` value: InvalidArgumentError("dictionary id 0 not found in schema")
thread 'reader::tests::test_roundtrip_nested_dict_no_preserve_dict_id' panicked at arrow-ipc/src/reader.rs:1735:79:
called `Result::unwrap()` on an `Err` value: InvalidArgumentError("dictionary id 0 not found in schema")

@alamb alamb merged commit 5a86db3 into apache:master Nov 15, 2024
alamb pushed a commit that referenced this pull request Jul 18, 2025
# Which issue does this PR close?

Does not yet close, but contributes towards:

- #6356 
- #5981 
- #1206

# Rationale for this change

See the above issues. And this is a follow up to

* #6711
* #6873

This was also split out from:
#7929

# What changes are included in this PR?

This removes the API to allow preserving `dict_id` set in the `Schema`'s
`Field` within arrow-ipc and arrow-flight. This is in an effort to
remove the `dict_id` field entirely and make it an IPC/flight-only
concern.

# Are these changes tested?

Yes, all existing tests continue to pass.

# Are there any user-facing changes?

Yes, these previously (in 54.0.0) deprecated functions/fields are
removed:

* `arrow_ipc::DictionaryTracker.set_dict_id`
* `arrow_ipc::DictionaryTracker::new_with_preserve_dict_id`
* `arrow_ipc::IpcWriteOptions.with_preserve_dict_id`
* `arrow_ipc::IpcWriteOptions.preserve_dict_id` (function and field)
* `arrow_ipc::schema_to_fb`
* `arrow_ipc::schema_to_bytes`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrow Changes to the arrow crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IPC file writer produces incorrect footer when not preserving dict ID

2 participants