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

[Coral-Trino] Avoid accidental translation of Trino from_unixtime SQL call - SqlFunction implementation #467

Merged

Conversation

findinpath
Copy link
Contributor

@findinpath findinpath commented Oct 20, 2023

What changes are proposed in this pull request, and why are they necessary?

Avoid the accidental translation of the Trino from_unixtime introduced through #426

Context #459 (comment)

Fixes #459

Spin-off from #464 to showcase the solution based on TimestampFromUnixtime SqlOperator implementation.

How was this patch tested?

Integration tests

@findinpath findinpath force-pushed the findinpath/from_unixtime_operator branch 2 times, most recently from 061055f to 07ac623 Compare October 20, 2023 18:58
@findinpath findinpath changed the title Draft: [Coral-Trino] Avoid accidental translation of Trino from_unixtime SQL call Draft operator: [Coral-Trino] Avoid accidental translation of Trino from_unixtime SQL call Oct 20, 2023
@wmoustafa
Copy link
Contributor

I have made a small fix in TrinoSqlDialect that fixes 2 text cases. The third case fails because from_unixtime_nanos, which I think needs a similar fix?

@wmoustafa
Copy link
Contributor

Fixed the remaining test case.

…QL call

Co-authored-by: Walaa Eldin Moustafa <wmoustafa@linkedin.com>
@findinpath findinpath force-pushed the findinpath/from_unixtime_operator branch from cbe144d to e5d6ae6 Compare October 22, 2023 19:37
@findinpath findinpath marked this pull request as ready for review October 22, 2023 19:37
@findinpath findinpath changed the title Draft operator: [Coral-Trino] Avoid accidental translation of Trino from_unixtime SQL call [Coral-Trino] Avoid accidental translation of Trino from_unixtime SQL call Oct 22, 2023
@findinpath findinpath changed the title [Coral-Trino] Avoid accidental translation of Trino from_unixtime SQL call [Coral-Trino] Avoid accidental translation of Trino from_unixtime SQL call - SqlFunction implementation Oct 22, 2023
@wmoustafa wmoustafa merged commit d53af31 into linkedin:master Oct 23, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: FromUnixtimeOperatorTransformer possibly built on false supposisions.
2 participants