thinkharderdev commented on PR #5971: URL: https://github.com/apache/arrow-rs/pull/5971#issuecomment-2197030833
> The code / change looks good to me. Thank you @thinkharderdev and @avantgardnerio > > Perhaps we can also update the documentation on https://docs.rs/arrow-flight/latest/arrow_flight/encode/enum.DictionaryHandling.html > > In terms of breaking API change, we are trying to release breaking changes less often (see docs https://github.com/apache/arrow-rs?tab=readme-ov-file#arrow-and-parquet-crates) > > > As currently formulated I think this constitutes a breaking API change, perhaps we could make this opt-in > > I personally suggest > > 1. Changing this PR so it doesn't have a breaking API change by disabling the feature by default but make it enableable via an API in `FlightDataEncoderBuilder` similar to https://docs.rs/arrow-flight/latest/arrow_flight/encode/struct.FlightDataEncoderBuilder.html#method.with_max_flight_data_size > 2. Merge this PR (to release next week) > 3. Create a new PR that changes the default of the flag (or maybe even removes the flag entirely) that we can merge once main opens for breaking changes Ok, based on suggestions I reformulated to make this optional. `IpcWriteOptions` now has a `preserve_dict_id` flag which when set to `false` will auto-assign dict IDs. By default it is set to `true`. This is still technically a breaking change since I added the flag to the constructor but if we'd like avoid that as well I can remove it and just require setting it through the builder method. -- 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]
