friendlymatthew opened a new pull request, #7871: URL: https://github.com/apache/arrow-rs/pull/7871
# Rationale for this change I was investigating https://github.com/apache/arrow-rs/issues/7869, when I found we were calling `validate()` twice. For example: `try_get_impl` is aimed to perform shallow validation https://github.com/apache/arrow-rs/blob/13d79b35884bf1fb2b761dc8e70b39bb24ae6c6b/parquet-variant/src/variant/list.rs#L272-L280 However, `Variant::try_new_with_metadata` _will_ perform deep validation https://github.com/apache/arrow-rs/blob/13d79b35884bf1fb2b761dc8e70b39bb24ae6c6b/parquet-variant/src/variant.rs#L322-L327 This means fallible versions like `try_get` and `iter_try` will call (1) `validate` through `try_get_impl` -> `Variant::try_new_with_metadata` and then (2) manually call `validate` again https://github.com/apache/arrow-rs/blob/13d79b35884bf1fb2b761dc8e70b39bb24ae6c6b/parquet-variant/src/variant/list.rs#L241-L249 I personally found the `_impl` convention a bit hard to reason about. From what I understand, `_impl` functions should only perform shallow validation. Here are my proposed name changes: - `iter_try` -> `try_iter` to follow other `try_..` methods - `_impl` -> `with_shallow_validation` to make it clear to the reader that this function does basic validation - `validate` -> `with_deep_validation`, the builder method will perform linear validation - `is_validated` -> `is_fully_validated`, both shallow and deep validation has been done -- 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]
