search

linkedin / coral

Coral is a translation, analysis, and query rewrite engine for SQL and other relational languages.
BSD 2-Clause "Simplified" License
783 stars 184 forks source link

[Coral-Trino] Avoid accidental translation of Trino `from_unixtime` SQL call #464

Closed findinpath closed 11 months ago

findinpath commented 11 months ago

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 https://github.com/linkedin/coral/issues/459#issuecomment-1762160693

Fixes https://github.com/linkedin/coral/issues/459

How was this patch tested?

Regular unit tests

wmoustafa commented 11 months ago

I think you would want to implement an operator in coral-trino similar to TrinoElementAtFunction and have the transformer convert to that one as opposed to creating a Hive function (operator). Operators currently part of "Hive Function Registry" will end up in Coral common as they capture Coral IR, and not a language specific set of operators.

findinpath commented 11 months ago

@wmoustafa

I created an alternative PR https://github.com/linkedin/coral/pull/465 to follow-up on your request

think you would want to implement an operator in coral-trino similar to TrinoElementAtFunction

However, I failed in getting it to work exactly because the function was not mentioned in StaticHiveFunctionRegistry.

StaticHiveFunctionRegistry contains already Trino (not Hive) specific content:

https://github.com/linkedin/coral/blob/a7e33e6972995eb275366878a0de3201525003af/coral-hive/src/main/java/com/linkedin/coral/hive/hive2rel/functions/StaticHiveFunctionRegistry.java#L306-L307

I believe that, in the current state of the coral library, my current PR is the right approach to fix the from_unixtime regresssion reported on https://github.com/linkedin/coral/issues/459.

findinpath commented 11 months ago

Superseded by https://github.com/linkedin/coral/pull/467