Skip to content

ARROW-15919: [C++] Add function not commutative with timestamps & duration maths#12612

Closed
rok wants to merge 4 commits into
apache:masterfrom
rok:ARROW-15919
Closed

ARROW-15919: [C++] Add function not commutative with timestamps & duration maths#12612
rok wants to merge 4 commits into
apache:masterfrom
rok:ARROW-15919

Conversation

@rok

@rok rok commented Mar 11, 2022

Copy link
Copy Markdown
Member

This is to resolve ARROW-15919.

It adds commutativity to kernels:

  • add/add_checked(timestamp, duration)->timestamp
  • add/add_checked(time32/64, duration)->time32/64
  • add/add_checked(date32/64, duration)->date32/64

@github-actions

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@rok rok force-pushed the ARROW-15919 branch 2 times, most recently from a353e51 to b60d2aa Compare March 14, 2022 15:12
@rok

rok commented Mar 14, 2022

Copy link
Copy Markdown
Member Author

@pitrou could you please take a look at this?
I'm afraid it's not very pretty.

Comment thread cpp/src/arrow/compute/kernels/scalar_arithmetic.cc Outdated

@pitrou pitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks again @rok! Here are a couple more suggestions.

Comment thread cpp/src/arrow/compute/kernels/scalar_arithmetic.cc Outdated
Comment thread cpp/src/arrow/compute/kernels/scalar_arithmetic.cc Outdated
Comment thread cpp/src/arrow/compute/kernels/scalar_temporal_test.cc Outdated
@pitrou pitrou changed the title ARROW-15919: [C++] Add_kernel not commutative with timestamps & duration maths ARROW-15919: [C++] Add function not commutative with timestamps & duration maths Mar 17, 2022
@rok rok force-pushed the ARROW-15919 branch 2 times, most recently from 2bad7b9 to 5faf75e Compare March 17, 2022 20:52
@rok rok requested a review from pitrou March 18, 2022 17:13

@pitrou pitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you very much @rok. LGTM.

@pitrou pitrou closed this in 70b8a82 Mar 21, 2022
@ursabot

ursabot commented Mar 21, 2022

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = e258e1c and contender = 70b8a82. 70b8a82 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
[Finished ⬇️0.5% ⬆️0.08%] test-mac-arm
[Finished ⬇️0.36% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.68% ⬆️0.0%] 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

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.

3 participants