Repository navigation
[pull] main from apache:main - #267
Merged
Merged
Conversation
## Which issue does this PR close? - Closes #22796 ## Rationale for this change `approx_distinct` over-counted distinct values for `Utf8View` columns when the same short string appeared across batches with different layouts. Arrow stores strings ≤ 12 bytes inline in the 128-bit view integer. The fast path (no data buffers) hashed these as raw `u128`. But when a batch also had a long string, it fell into a different branch that hashed **all** strings as `&str` — including the short inline ones. The same string hashed differently in different batches, so HyperLogLog counted it twice. ## What changes are included in this PR? - **`StringViewHLLAccumulator::update_batch`** and **`Utf8ViewHasher`**: in mixed batches (data buffers present), short strings (≤ 12 bytes) are still hashed as the raw `u128` view; only long strings hash as `&str`. This keeps hashing consistent regardless of batch layout. - **Two regression tests**: - `utf8view_acc_split_batches_match_single_mixed_batch` — scalar accumulator - `utf8view_groups_short_string_hashed_consistently_across_batches` — group accumulator ## Are these changes tested? Yes, two new regression tests cover the exact failure mode. ## Are there any user-facing changes? Yes. `approx_distinct` on `Utf8View` / `VARCHAR VIEW` columns now returns correct (lower) counts. Results may differ from the previously incorrect values. --------- Co-authored-by: Yongting You <2010youy01@gmail.com>
## Which issue does this PR close? - Closes #22421. ## Rationale for this change CaseExpr should own its protobuf serialization and deserialization through the PhysicalExpr try_to_proto / try_from_proto hooks, matching the migration pattern used by other physical expressions and avoiding the central downcast chain. ## What changes are included in this PR? - Adds CaseExpr::try_to_proto in the PhysicalExpr impl. - Adds CaseExpr::try_from_proto as an inherent proto constructor. - Routes physical expression decoding for CaseExpr through the new hook. - Removes the old CaseExpr serialization arm and its helper from datafusion-proto. - Adds direct hook tests covering encode, decode, wrong node variant, missing when/then children, and child encode/decode error propagation. ## Are these changes tested? Yes: - cargo fmt --all - cargo test -p datafusion-physical-expr --features proto case::tests::try_ - cargo test -p datafusion-proto --test proto_integration - cargo clippy -p datafusion-physical-expr --features proto --tests -- -D warnings - cargo clippy -p datafusion-proto --test proto_integration -- -D warnings ## Are there any user-facing changes? No. This is an internal proto serialization refactor with the same wire format. --------- Co-authored-by: chakkk309 <chakkk309@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )