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

arrowToRecord() produce wrong timestamp for some future dates #910

Closed
Yifeng-Sigma opened this issue Sep 7, 2023 · 8 comments
Closed

arrowToRecord() produce wrong timestamp for some future dates #910

Yifeng-Sigma opened this issue Sep 7, 2023 · 8 comments
Assignees
Labels
bug Erroneous or unexpected behaviour status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector.

Comments

@Yifeng-Sigma
Copy link
Contributor

Super easy to reproduce,

https://github.com/sigmacomputing/gosnowflake/blob/17fd0144860b6ee0164c398488ad9fa02fb65f36/converter_test.go#L850
change year to >= 2263 or <= 1677 and the the unit test will fail.
While per documentation it's within the range "Snowflake recommends using years between 1582 and 9999".

@Yifeng-Sigma Yifeng-Sigma added the bug Erroneous or unexpected behaviour label Sep 7, 2023
@Yifeng-Sigma Yifeng-Sigma changed the title arrowToRecord() produce wrong dates for some future dates arrowToRecord() produce wrong timestamp for some future dates Sep 7, 2023
@sfc-gh-dszmolka
Copy link
Contributor

hi and thank you for submitting this issue with us, we'll take a look

@sfc-gh-dszmolka
Copy link
Contributor

PR #918 with the fix in progress. Thank you for bearing with us while this is getting fixed !

@sfc-gh-dszmolka sfc-gh-dszmolka added status-pr_pending_merge A PR is made and is under review and removed status-in_progress Issue is worked on by the driver team labels Oct 2, 2023
@sfc-gh-dszmolka
Copy link
Contributor

PR merged and will be part of the October release, expected towards the end of October

@sfc-gh-dszmolka sfc-gh-dszmolka added status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. and removed status-pr_pending_merge A PR is made and is under review labels Oct 10, 2023
@sfc-gh-dszmolka
Copy link
Contributor

gosnowflake 1.7.0 released with the fix, thank you all for bearing with us !

@madisonchamberlain
Copy link
Contributor

Hello, I am wondering if there are any recommendations on how ArrowSnowflakeTimestampToTime should be used.

@sfc-gh-dszmolka
Copy link
Contributor

hi there - we seem to have some in converter_test.go; would that suffice as an example or should we provide something more ?

@Yifeng-Sigma
Copy link
Contributor Author

Yifeng-Sigma commented Jan 24, 2024

hi there - we seem to have some in converter_test.go; would that suffice as an example or should we provide something more ?

@sfc-gh-dszmolka Thank you, this is helpful. One more question:
Is it possible for you to provide timestamp in microsecond precision? microsecond should be able to fit in arrow.Timestamp without the need of using arrow.Struct.
In our use case, the connector passes the arrow record from Snowflake to another service for decoding. If we have microsecond precision, we can avoid the need to port the arrowSnowflakeTimestampToTime logic.

@sfc-gh-dszmolka
Copy link
Contributor

sfc-gh-dszmolka commented Jan 25, 2024

we'll check @Yifeng-Sigma , started a discussion on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Erroneous or unexpected behaviour status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector.
Projects
None yet
Development

No branches or pull requests

4 participants