Skip to content

Conversation

@comphead
Copy link
Contributor

Which issue does this PR close?

Closes #8336
Closes #7958

Rationale for this change

Declared limits for to_timestamp, to_timestamp_nanos functions to prevent overflows. Its been agreed in #8336 (comment) to use to_timestamp_seconds function for values between 1677-09-21T00:12:44.0 and 2262-04-11T23:47:16.0

What changes are included in this PR?

Are these changes tested?

No need

Are there any user-facing changes?

No

@github-actions github-actions bot added the physical-expr Changes to the physical-expr crates label Nov 29, 2023
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @comphead


/// to_timestamp_nanos SQL function
///
/// Note: `to_timestamp_nanos` returns `Timestamp(Nanosecond)`. The supported range for integer input is between `-9223372037` and `9223372036`.
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't seem right. I think the argument is treated as nanoseconds, so the range is really -i64::MIN to i64::MAX

For example, this works just fine

❯ select to_timestamp_nanos(92233720360000)
;
+-------------------------------------------+
| to_timestamp_nanos(Int64(92233720360000)) |
+-------------------------------------------+
| 1970-01-02T01:37:13.720360                |
+-------------------------------------------+
1 row in set. Query took 0.000 seconds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, you are right

to_timestamp_nanos(expression)
```

Note: `to_timestamp_nanos` returns `Timestamp(Nanosecond)`. The supported range for integer input is between `-9223372037` and `9223372036`.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks @comphead

@alamb alamb merged commit c19260d into apache:main Dec 1, 2023
@alamb
Copy link
Contributor

alamb commented Dec 1, 2023

🚀

appletreeisyellow pushed a commit to appletreeisyellow/datafusion that referenced this pull request Dec 14, 2023
* document timestamp input limis

* fix text

* prettier

* remove doc for nanoseconds

* Update datafusion/physical-expr/src/datetime_expressions.rs

Co-authored-by: Andrew Lamb <[email protected]>

---------

Co-authored-by: Andrew Lamb <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-expr Changes to the physical-expr crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Timestamp overflows for extreme low/high values Inconsistent Signedness Of Legacy Parquet Timestamps Written By Spark

2 participants