hassaanch23 commented on PR #11144: URL: https://github.com/apache/arrow-rs/pull/11144#issuecomment-5926984055
@kita-renji Thank you for this. Running the matrix against the merge base as well is what made the dictionary gap visible. Everything is addressed in 67e56f4 and 315e6d1: **Dictionary value type.** Agreed that it's the same root cause. Instead of setting `value_type` inside `set_validate_utf8`, I removed the `value_type` field. `DictionaryDecoder::value_type()` now derives it from `validate_utf8`, so dictionary values are strings exactly when they are validated, and the two can't drift apart again. With only that change reverted, the new valid-data test fails with the `Child type mismatch for Dictionary(Int32, Utf8). Expected Utf8 but child data had Binary` you reported. I checked the opposite direction too, a `UTF8` column read into `Dictionary(_, Binary)`. The schema check already rejects it (`requested Dictionary(Int32, Binary) but found Utf8`), so a validated column can never need `Binary` values. **JSON.** Added `test_read_non_utf8_json`: a `JSON` column holding invalid UTF-8, read without a supplied schema. It fails on the merge base and returns `encountered non UTF-8 data` here. The description now mentions it. **Encodings.** The tests now write each column five ways: dictionary pages; dictionary pages that fall back to PLAIN after the first value; and PLAIN, DELTA_LENGTH_BYTE_ARRAY and DELTA_BYTE_ARRAY without a dictionary. The write helper asserts each chunk used the intended encodings, so a change in writer defaults can't quietly stop a decoder from being exercised. The invalid values now follow an empty value and a null, so in the fallback case they land in the PLAIN pages rather than the dictionary. They are read into all six targets (`Utf8`, `LargeUtf8`, `Utf8View`, and a dictionary of each). Valid data goes into the same six and is checked with `validate_full`. **Doc nit.** The trait doc now says that passing `false` does not turn validation off for a column annotated as a string. `cargo test -p parquet` (with and without `--all-features`) and clippy are clean. -- 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]
