pitrou commented on a change in pull request #9294:
URL: https://github.com/apache/arrow/pull/9294#discussion_r573750885
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc
##########
@@ -637,5 +637,77 @@ TYPED_TEST(TestBinaryArithmeticFloating, Mul) {
this->AssertBinop(Multiply, "[null, 2.0]", this->MakeNullScalar(), "[null,
null]");
}
+TEST(TestBinaryArithmetic, DispatchBest) {
+ for (std::string name : {"add", "subtract", "multiply", "divide"}) {
+ for (std::string suffix : {"", "_checked"}) {
+ name += suffix;
+
+ CheckDispatchBest(name, {int32(), int32()}, {int32(), int32()});
+ CheckDispatchBest(name, {int32(), null()}, {int32(), int32()});
+ CheckDispatchBest(name, {null(), int32()}, {int32(), int32()});
+
+ CheckDispatchBest(name, {int32(), int8()}, {int32(), int32()});
+ CheckDispatchBest(name, {int32(), int16()}, {int32(), int32()});
+ CheckDispatchBest(name, {int32(), int32()}, {int32(), int32()});
+ CheckDispatchBest(name, {int32(), int64()}, {int64(), int64()});
+
+ CheckDispatchBest(name, {int32(), uint8()}, {int32(), int32()});
+ CheckDispatchBest(name, {int32(), uint16()}, {int32(), int32()});
+ CheckDispatchBest(name, {int32(), uint32()}, {int64(), int64()});
+ CheckDispatchBest(name, {int32(), uint64()}, {int64(), int64()});
+
+ CheckDispatchBest(name, {uint8(), uint8()}, {uint8(), uint8()});
+ CheckDispatchBest(name, {uint8(), uint16()}, {uint16(), uint16()});
+
+ CheckDispatchBest(name, {int32(), float32()}, {float32(), float32()});
+ CheckDispatchBest(name, {float32(), int64()}, {float32(), float32()});
+ CheckDispatchBest(name, {float64(), int32()}, {float64(), float64()});
+
+ CheckDispatchBest(name, {dictionary(int8(), float64()), float64()},
+ {float64(), float64()});
+ CheckDispatchBest(name, {dictionary(int8(), float64()), int16()},
+ {float64(), float64()});
+ }
+ }
+}
+
+TEST(TestBinaryArithmetic, AddWithImplicitCasts) {
+ CheckScalarBinary("add", ArrayFromJSON(int32(), "[0, 1, 2, null]"),
+ ArrayFromJSON(float64(), "[0.25, 0.5, 0.75, 1.0]"),
+ ArrayFromJSON(float64(), "[0.25, 1.5, 2.75, null]"));
+
+ CheckScalarBinary("add", ArrayFromJSON(int8(), "[-16, 0, 16, null]"),
+ ArrayFromJSON(uint32(), "[3, 4, 5, 7]"),
+ ArrayFromJSON(int64(), "[-13, 4, 21, null]"));
+
+ CheckScalarBinary("add",
+ ArrayFromJSON(dictionary(int32(), int32()), "[8, 6, 3,
null, 2]"),
+ ArrayFromJSON(uint32(), "[3, 4, 5, 7, 0]"),
+ ArrayFromJSON(int64(), "[11, 10, 8, null, 2]"));
+
+ CheckScalarBinary("add", ArrayFromJSON(int32(), "[0, 1, 2, null]"),
+ std::make_shared<NullArray>(4),
+ ArrayFromJSON(int32(), "[null, null, null, null]"));
+
+ CheckScalarBinary("add", ArrayFromJSON(dictionary(int32(), int8()), "[0, 1,
2, null]"),
+ ArrayFromJSON(uint32(), "[3, 4, 5, 7]"),
+ ArrayFromJSON(int64(), "[3, 5, 7, null]"));
+}
+
+TEST(TestBinaryArithmetic, AddWithImplicitCastsUint64EdgeCase) {
+ // int64 is as wide as we can promote
+ CheckDispatchBest("add", {int8(), uint64()}, {int64(), int64()});
+
+ // this works sometimes
+ CheckScalarBinary("add", ArrayFromJSON(int8(), "[-1]"),
ArrayFromJSON(uint64(), "[0]"),
+ ArrayFromJSON(int64(), "[-1]"));
+
+ // ... but it can result in impossible implicit casts in the presence of
uint64, since
+ // some uint64 values cannot be cast to int64:
Review comment:
Nice! It's a bit weird that this happens for "add" rather than
"add_checked", but we'll live with it :-)
##########
File path: cpp/src/arrow/compute/cast.h
##########
@@ -157,5 +155,17 @@ Result<Datum> Cast(const Datum& value,
std::shared_ptr<DataType> to_type,
const CastOptions& options = CastOptions::Safe(),
ExecContext* ctx = NULLPTR);
+/// \brief Cast several values simultaneously. Safe cast options are used.
+/// \param[in] values datums to cast
+/// \param[in] descrs ValueDescrs to cast to
+/// \param[in] ctx the function execution context, optional
+/// \return the resulting datums
+///
+/// \since 1.0.0
Review comment:
The "since" isn't right here. We generally don't bother adding it, but
if we do we should certainly get it right :-)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
