ARROW-11950: [C++][Compute] Add unary negative kernel

[edponce]

This pull request adds the Negate arithmetic compute kernel for integral and floating-point types. The NegateChecked version is not implemented for unsigned integral types and overflow behavior is consistent with equivalent Add/Subtract operations. The Negate kernels are registered as "negate" and "negate_checked".

This PR also extends support for unary arithmetic compute kernels and tests.

@bkietz please review

[github-actions]

https://issues.apache.org/jira/browse/ARROW-11950

[edponce]

Windows build is failing due to C4146 warning which is triggered when an unsigned integer is negated. This negation is done purposefully with the expected behavior of modulo arithmetic. An alternative is to negate unsigned integers using two's complement explicitly (see https://github.com/apache/arrow/blob/master/cpp/src/arrow/util/basic_decimal.cc#L375):

unsigned int x = 10;
unsigned int x_neg = ~x + 1;

@bkietz@pitrou please advise

[edponce]

After changing the negation operation for unsigned integers, Windows build passes successfully (i.e., no warning).

[cyb70289]

Same PR as #10016??

[edponce]

Resolving same issue as in PR #10016 but #10113 is the one to officially review. I initially had submitted draft #10016 but mistakenly submitted a new PR (i.e., #10113).

[bkietz]

This is looking good, thanks! A few comments

[bkietz]

This looks good, please add one test for NullType.

[bkietz]

Please add a test asserting the behavior when NullType is used. I expect this will emit a "no kernel found" error (acceptable), or leave the type unpromoted:

Suggested change

	CheckDispatchBest("negate", {null()}, {null()});

This would also be fine, but I'd like to see a trivial test case asserting we don't try to access absent buffers:

CheckScalarUnary("negate", NullArray(10), NullArray(10));

[edponce]

Using a NullType does trigger a NotImplemented error as expected, but I can't catch/assert the error from CheckDispatchBest() to pass the tests. Similar scenario for CheckScalarUnary. Any ideas?

[bkietz]

You could add a CheckDispatchFails(function_name, argument_types) assert function to go with CheckDispatchBest()

[edponce]

I added CheckDispatchFails() which checks if status is an error after invoking either DispatchBest() or DispatchExact. It seems the only error produced by Dispatch*() is NotImplemented. Nevertheless, I made the function description general enough and check for not OK status instead of only NotImplemented error, https://github.com/edponce/arrow/blob/ARROW-11950-Compute-Add-unary-negative-kernel/cpp/src/arrow/compute/kernels/test_util.h#L151-L152

[edponce]

I did not add CheckScalarUnary("negate", std::make_shared<NullArray>(10), std::make_shared<NullArray>(10)) test because it fails with NotImplemented kernel via a different path of functions. Specifically, CheckScalar() --> CallFunction() which is where the error is captured. I think adding a family of CheckScalar*Fails() function would be beneficial for testing but consider it outside the scope of this PR. Should I open an issue for extending these tests?

[cyb70289]

PR #10098 changed a bit the behaviour of kernel error handling. Errors are not populated through KernelContext any more.
You can reference latest code in this source file. E.g., https://github.com/apache/arrow/blob/master/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc#L91

[edponce]

Thanks for this notice.

[bkietz]

LGTM, thanks for doing this!