Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-15506: [C++][Compute] Support Null type in hash_sum/hash_product/hash_mean #12299

Closed
wants to merge 2 commits into from

Conversation

Crystrix
Copy link
Contributor

  • If min_count == 0 and skip_nulls == true hash_sum returns an int64 scalar of 0, otherwise return a int64 scalar of null
  • If min_count == 0 and skip_nulls == true hash_product returns an int64 scalar of 1, otherwise return a int64 scalar of null
  • If min_count == 0 and skip_nulls == true hash_mean returns an float64 scalar of 0, otherwise return a float64 scalar of null

@github-actions
Copy link

std::shared_ptr<DataType> out_type() const override { return float64(); }

void output_empty(const std::shared_ptr<Buffer>& data) override {
std::memset(data->mutable_data(), 0, num_groups_ * sizeof(double));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather use std::fill_n(double*, ...) here for clarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pitrou, memset is replaced with fill_n.

@pitrou pitrou requested a review from lidavidm January 31, 2022 13:55
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@lidavidm lidavidm closed this in 9e8ac56 Feb 2, 2022
@ursabot
Copy link

ursabot commented Feb 2, 2022

Benchmark runs are scheduled for baseline = 3f9daeb and contender = 9e8ac56. 9e8ac56 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.71% ⬆️2.14%] ursa-i9-9960x
[Finished ⬇️0.22% ⬆️0.0%] ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

vibhatha pushed a commit to vibhatha/arrow that referenced this pull request Feb 4, 2022
…t/hash_mean

- If min_count == 0 and skip_nulls == true `hash_sum` returns an int64 scalar of 0, otherwise return a int64 scalar of null
- If min_count == 0 and skip_nulls == true `hash_product` returns an int64 scalar of 1, otherwise return a int64 scalar of null
- If min_count == 0 and skip_nulls == true `hash_mean` returns an float64 scalar of 0, otherwise return a float64 scalar of null

Closes apache#12299 from Crystrix/arrow-15506

Authored-by: Chenxi Li <chenxi.li@live.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants