You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "zanmato1984 (via GitHub)" <gi...@apache.org> on 2026/07/24 00:50:28 UTC
[PR] GH-50624: [C++][Compute] Tighten case_when exact dispatch for parameterized types [arrow]
zanmato1984 opened a new pull request, #50625:
URL: https://github.com/apache/arrow/pull/50625
### Rationale for this change
`case_when` exact dispatch was too permissive for parameterized value types. When the value arguments were registered by type id only, `DispatchExact` could incorrectly accept incompatible concrete types and return successful but corrupted results instead of failing or falling back to `DispatchBest`.
This fixes #50624.
### What changes are included in this PR?
- rename the case_when exact-match helper to `AllValueTypesMatchConstraint()` to make its intent explicit
- apply the same exact-match constraint across case_when value kernels so exact dispatch only accepts identical value `DataType`s
- add regression coverage for mismatched parameterized value types, including `fixed_size_binary`, `list`, `fixed_size_list`, `struct`, and dictionary exact-dispatch mismatches
### Are these changes tested?
Yes.
This PR extends `TestCaseWhen.DispatchExact` and adds `TestCaseWhen.ParameterizedValueTypeMismatch`. I also ran targeted `arrow-compute-scalar-if-else-test` coverage for:
- `TestCaseWhen.DispatchExact`
- `TestCaseWhen.DispatchBest`
- `TestCaseWhen.ParameterizedValueTypeMismatch`
### Are there any user-facing changes?
Yes.
`case_when` now rejects incompatible parameterized value types that previously could be incorrectly exact-dispatched, which could lead to corrupted results.
**This PR contains a "Critical Fix".** It fixes a bug where `case_when` could return successful but corrupted results for incompatible parameterized value types.
* GitHub Issue: #50624
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50624: [C++][Compute] Tighten case_when exact dispatch for parameterized types [arrow]
Posted by "zanmato1984 (via GitHub)" <gi...@apache.org>.
zanmato1984 commented on PR #50625:
URL: https://github.com/apache/arrow/pull/50625#issuecomment-5094804514
I'll merge this after CI turns green. Thanks for reviewing @pitrou !
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50624: [C++][Compute] Tighten case_when exact dispatch for parameterized types [arrow]
Posted by "zanmato1984 (via GitHub)" <gi...@apache.org>.
zanmato1984 merged PR #50625:
URL: https://github.com/apache/arrow/pull/50625
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50624: [C++][Compute] Tighten case_when exact dispatch for parameterized types [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #50625:
URL: https://github.com/apache/arrow/pull/50625#issuecomment-5098780801
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 974c3fc1a772eccc85bd3500bad8573a57ff5089.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/90154000564) has more details. It also includes information about 9 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50624: [C++][Compute] Tighten case_when exact dispatch for parameterized types [arrow]
Posted by "tadeja (via GitHub)" <gi...@apache.org>.
tadeja commented on PR #50625:
URL: https://github.com/apache/arrow/pull/50625#issuecomment-5941028434
(@zanmato1984 there's this [strange branch](https://github.com/apache/arrow/tree/codex/case-when-parameterized-exact-dispatch) you'd likely want to remove:)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50624: [C++][Compute] Tighten case_when exact dispatch for parameterized types [arrow]
Posted by "zanmato1984 (via GitHub)" <gi...@apache.org>.
zanmato1984 commented on code in PR #50625:
URL: https://github.com/apache/arrow/pull/50625#discussion_r3659604987
##########
cpp/src/arrow/compute/kernels/scalar_if_else.cc:
##########
@@ -2756,38 +2754,42 @@ void AddCaseWhenKernel(const std::shared_ptr<CaseWhenFunction>& scalar_function,
}
void AddPrimitiveCaseWhenKernels(const std::shared_ptr<CaseWhenFunction>& scalar_function,
- const std::vector<std::shared_ptr<DataType>>& types) {
+ const std::vector<std::shared_ptr<DataType>>& types,
+ std::shared_ptr<MatchConstraint> constraint) {
for (auto&& type : types) {
auto exec = GenerateTypeAgnosticPrimitive<CaseWhenFunctor>(*type);
- AddCaseWhenKernel(scalar_function, type, std::move(exec));
+ AddCaseWhenKernel(scalar_function, type, std::move(exec), constraint);
Review Comment:
This is in a loop so I don't think `std::move` is proper here. I've changed the parameter type to const ref to eliminate the ambiguity. Thanks.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50624: [C++][Compute] Tighten case_when exact dispatch for parameterized types [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #50625:
URL: https://github.com/apache/arrow/pull/50625#discussion_r3655683155
##########
cpp/src/arrow/compute/kernels/scalar_if_else.cc:
##########
@@ -2756,38 +2754,42 @@ void AddCaseWhenKernel(const std::shared_ptr<CaseWhenFunction>& scalar_function,
}
void AddPrimitiveCaseWhenKernels(const std::shared_ptr<CaseWhenFunction>& scalar_function,
- const std::vector<std::shared_ptr<DataType>>& types) {
+ const std::vector<std::shared_ptr<DataType>>& types,
+ std::shared_ptr<MatchConstraint> constraint) {
for (auto&& type : types) {
auto exec = GenerateTypeAgnosticPrimitive<CaseWhenFunctor>(*type);
- AddCaseWhenKernel(scalar_function, type, std::move(exec));
+ AddCaseWhenKernel(scalar_function, type, std::move(exec), constraint);
Review Comment:
Nit: `std::move`
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50624: [C++][Compute] Tighten case_when exact dispatch for parameterized types [arrow]
Posted by "zanmato1984 (via GitHub)" <gi...@apache.org>.
zanmato1984 commented on PR #50625:
URL: https://github.com/apache/arrow/pull/50625#issuecomment-5945158765
Thanks for pointing this out! That branch was pushed to upstream by mistake. I've deleted it now.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org