GH-45362: [C++] Fix identity cast for time and list scalar #45370
Conversation
|
|
|
Windows CI failed seems to be related. |
mapleFU
force-pushed
the
GH-45362
branch
2 times, most recently
from
a615d7c to
22134a8
Compare
|
Also cc @pitrou since this is new year and @zanmato1984 might not have time for this |
pitrou
changed the title
github-actions
bot
added
awaiting committer review
| test_identity_cast_for_type(type); | ||
| } | ||
| /* | ||
| for (auto& type: { |
There was a problem hiding this comment.
decimal seems not have castTo
There was a problem hiding this comment.
Can we open issues for the missing casts?
There was a problem hiding this comment.
Added, but seems previous issue wants to deprecate them, I think these issues may also be mark as not planned, lol
cpp/src/arrow/scalar_test.cc
Outdated
| // arrow::map(arrow::binary(), arrow::int32()), | ||
| // struct_({field("float", arrow::float32())}), |
There was a problem hiding this comment.
struct and map seems not supports castTo non string types
mapleFU
changed the title
| test_identity_cast_for_type(type); | ||
| } | ||
| /* | ||
| for (auto& type: { |
There was a problem hiding this comment.
Can we open issues for the missing casts?
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit d9e7fa6. There were 8 benchmark results with an error:
There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 4 possible false positives for unstable benchmarks that are known to sometimes produce them. |
Rationale for this change
The PR #40237 introduced code:
However, the
Time64Type::TypeClassdoesn't exist, so SFINAE always failed.Are these changes tested?
Yes
Are there any user-facing changes?
No.
Time64ScalarandTime32Scalarfails #45362