ARROW-16807: [C++][R] count distinct incorrectly merges state

[drin]

This addresses a bug where the count_distinct function simply added counts when merging state. The correct logic would be to return the number of distinct elements after both states have been merged.

State for count_distinct is backed by a MemoTable, which is then backed by a HashTable. To properly merge state, this PR adds 2 functions to each MemoTable: MaybeInsert and MergeTable. The MaybeInsert function handles simplified logic for inserting an element into the MemoTable. The MergeTable function handles iteration over elements in the MemoTable to be merged.

This PR also adds an R test and a C++ test. The R test mirrors what was provided in ARROW-16807. The C++ test, AllChunkedArrayTypesWithNulls, mirrors another C++ test, AllArrayTypesWithNulls, but uses chunked arrays for test data.

[github-actions]

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

[drin]

I just realized that this introduces (or maybe just exposes) a bug when calling this function on scalar inputs. If the input is a scalar, non_nulls is incremented without changing state. To address this "correctly," the code path for scalar inputs should also update the state by using GetOrInsert as is done for the code path for vector inputs.

I am working on figuring this out, given that the compiler has type conversion issues when attempting to just call GetOrInsert with a scalar reference.

[drin]

@wesm @westonpace previous pair of commits makes this work for scalar inputs. I tried putting some of the conditional logic ("if type is decimal") in CountDistinctImpl but it ended up being difficult to satisfy the compiler.

the current change to UnboxScalar tries to essentially use is_decimal, but then I realized that there isn't an easy way to call the appriopriate view function unless I know which decimal type I have. Additionally, the UnboxScalar implementations for Decimal types gets the value, which I think would be difficult to downcast/change back to string_view.

all in all, this was the only approach I could get to work and it seemed semi reasonable. Feedback would be much appreciated though.

[wesm]

@drin I pushed a simpler solution for the UnboxScalar change and removed MaybeInsert. We should probably open a follow up Jira to improve error handling in hash table merges but I don't think it's necessary to do that in this PR

[drin]

I made a JIRA to track error handling improvements for hash table merge: ARROW-17074

I specifically made the priority minor, but feel free to change it (I don't know the protocol for priority assignment)

[cyb70289]

Looks great. Only some minor comments.
Thanks @drin !

[drin]

thanks for the reviews @cyb70289 and Wes!

[westonpace]

This looks good from my perspective. Thanks for adding the tests. I'll let others confirm before merging.

[cyb70289]

Oops, why suddently so many CI errors?

• R lint error and test failure
  https://github.com/apache/arrow/runs/7362285260?check_suite_focus=true#step:12:51
  https://github.com/apache/arrow/runs/7362285354?check_suite_focus=true#step:11:21344
  https://github.com/apache/arrow/runs/7361802264?check_suite_focus=true#step:5:14778
  Related to this PR? cc @nealrichardson

• protobuf symbol not found in flight and opentelemetry
  https://github.com/apache/arrow/runs/7361802181?check_suite_focus=true#step:10:188
  https://github.com/apache/arrow/runs/7361802975?check_suite_focus=true#step:9:1306
  cc @lidavidm

[wesm]

@cyb70289 these CI failures don't seem related — R has some lint failures and there are these otel protobuf linkage issues. I'll go ahead to merge this

[nealrichardson]

R issues are likely a combination of transient upstream issues and stuff that has been fixed in master. If things persist I'll follow up.

[ursabot]

Benchmark runs are scheduled for baseline = afc6840 and contender = af4db77. af4db77 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Failed ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️1.18% ⬆️0.03%] test-mac-arm
[Failed ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️1.36% ⬆️0.14%] ursa-thinkcentre-m75q
Buildkite builds:
[Failed] af4db773 ec2-t3-xlarge-us-east-2
[Failed] af4db773 test-mac-arm
[Failed] af4db773 ursa-i9-9960x
[Finished] af4db773 ursa-thinkcentre-m75q
[Finished] afc6840c ec2-t3-xlarge-us-east-2
[Failed] afc6840c test-mac-arm
[Failed] afc6840c ursa-i9-9960x
[Finished] afc6840c ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java