dmitry-chirkov-dremio commented on code in PR #55171:
URL: https://github.com/apache/arrow/pull/55171#discussion_r4370375191
##########
cpp/src/arrow/compute/kernels/scalar_compare.cc:
##########
@@ -478,6 +499,10 @@ std::shared_ptr<ScalarFunction>
MakeFlippedCompare(std::string name,
std::make_shared<CompareFunction>(name, Arity::Binary(), std::move(doc));
for (const ScalarKernel* kernel : func.kernels()) {
ScalarKernel flipped_kernel = *kernel;
+ auto in_types = kernel->signature->in_types();
+ std::reverse(in_types.begin(), in_types.end());
+ flipped_kernel.signature =
Review Comment:
Can we replace this block with:
```cpp
const auto& in_types = kernel->signature->in_types();
if (in_types[0] != in_types[1]) {
flipped_kernel.signature = KernelSignature::Make(
{in_types[1], in_types[0]}, kernel->signature->out_type());
}
```
This keeps the decimal same-scale constraint on existing kernels. Without
it, a bound expression returns false for `1.00 < 9.0` because it compares the
stored integers 100 and 90.
I tested this change locally: all 144 focused decimal checks pass, versus 40
failures without it. Please also add an expression regression test for `<` and
`<=` with different decimal scales.
--
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]
