wmalpica commented on a change in pull request #11161:
URL: https://github.com/apache/arrow/pull/11161#discussion_r709708121
##########
File path: cpp/src/arrow/compute/kernels/scalar_cast_test.cc
##########
@@ -1485,51 +1485,57 @@ TEST(Cast, StringToBoolean) {
TEST(Cast, StringToInt) {
for (auto string_type : {utf8(), large_utf8()}) {
for (auto signed_type : {int8(), int16(), int32(), int64()}) {
- CheckCast(ArrayFromJSON(string_type, R"(["0", null, "127", "-1", "0"])"),
- ArrayFromJSON(signed_type, "[0, null, 127, -1, 0]"));
+ CheckCast(
+ ArrayFromJSON(string_type, R"(["0", null, "127", "-1", "0", "0x0",
"0x7F"])"),
+ ArrayFromJSON(signed_type, "[0, null, 127, -1, 0, 0, 127]"));
Review comment:
Those cases are explicitly not allowed. Do you mean that I should add
them to `CheckCastFails`?
A negative before a hex does not make sense. If the value can be negative
(signed integer) then you should use the binary representation of that negative
number, for example for int8 0xFF is -1 and 0xF0 is -128.
For the case of multiple prefixed zeros, I opted for not allowing that to be
allowed, since allowing it would make the logic more complex (probably slower)
and it seems like it would be a very odd way for a hex string to be formatted.
--
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]
