Skip to content

Conversation

@foskey51
Copy link
Contributor

Which issue does this PR close?

Rationale for this change

NA

What changes are included in this PR?

This PR introduces a temporal_duration_coercion fn, that coerces the Duration unit to Timestamp unit. I've tested it with the failing queries mentioned in the issue.....

> select '2024-02-23'::date::timestamp + arrow_cast(arrow_cast('03:12:44'::time, 'Time32(Second)')::int, 'Duration(Second)');
+---------------------------------------------------------------------------------------------------------------+
| Utf8("2024-02-23") + arrow_cast(arrow_cast(Utf8("03:12:44"),Utf8("Time32(Second)")),Utf8("Duration(Second)")) |
+---------------------------------------------------------------------------------------------------------------+
| 2024-02-23T03:12:44                                                                                           |
+---------------------------------------------------------------------------------------------------------------+
1 row(s) fetched. 
Elapsed 0.010 seconds.
> select '2024-02-23'::date::timestamp + arrow_cast(1234, 'Duration(Second)');
+-----------------------------------------------------------------------+
| Utf8("2024-02-23") + arrow_cast(Int64(1234),Utf8("Duration(Second)")) |
+-----------------------------------------------------------------------+
| 2024-02-23T00:20:34                                                   |
+-----------------------------------------------------------------------+
1 row(s) fetched. 
Elapsed 0.009 seconds.

Are these changes tested?

no, test cases are not covered for this pr.

Are there any user-facing changes?

no

@github-actions github-actions bot added the logical-expr Logical plan and expressions label Dec 22, 2025
@Omega359
Copy link
Contributor

FYI I have incorporated your changes into my updates for #19022 as I'm trying to condense all the code changes. I've also added a bunch of slt tests to cover the expected behaviour. I'll be sure to add attribution to you for this code

@Omega359
Copy link
Contributor

This has been incorporated into #19460

@alamb alamb marked this pull request as draft December 24, 2025 12:16
github-merge-queue bot pushed a commit that referenced this pull request Dec 28, 2025
## Which issue does this PR close?

<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax. For example
`Closes #123` indicates that this PR will close issue #123.
-->

- Closes #19022 
- Closes #19038 
- Closes #19024
- Closes #19457

## Rationale for this change

Improve coverage of date / time / interval arithmetic operations

## What changes are included in this PR?

type coercion improvements, numerous slt tests.

## Are these changes tested?

Yes

## Are there any user-facing changes?

Additional arithmetic support.

Thanks to @foskey51 for the timestamp + duration fix, @pepijnve for the
initial set of .slt tests.

---------

Co-authored-by: Pepijn Van Eeckhoudt <[email protected]>
Co-authored-by: Martin Grigorov <[email protected]>
Co-authored-by: Andrew Lamb <[email protected]>
@alamb alamb closed this in #19460 Dec 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

logical-expr Logical plan and expressions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Teach the type arithmetic code to support Timestamp + Duration(Second) -> timestamp

2 participants