Add support for from_utc_timestamp in Hive->Trino translation #109
Conversation
|
@wmoustafa, @shenodaguirguis PTAL when you have a chance |
coral-trino/src/test/java/com/linkedin/coral/trino/rel2trino/TestUtils.java
Outdated
Show resolved
Hide resolved
| { "test", "get_json_object_view", "SELECT \"json_extract\"(\"b\".\"b1\", '$.name')\nFROM \"test\".\"tablea\"" }, | ||
|
|
||
| { "test", "view_from_utc_timestamp", "SELECT " | ||
| + "CAST(\"at_timezone\"(\"from_unixtime_nanos\"(CAST(\"a_tinyint\" AS BIGINT) * 1000000), 'America/Los_Angeles') AS TIMESTAMP), " |
There was a problem hiding this comment.
TIMESTAMP is just an alias to timestamp(3), so:
AS TIMESTAMP -> AS timestamp(3)
plus, add "TODO trinodb/trino#6295 support high-precision timestamp"
| + "CAST(\"at_timezone\"(\"from_unixtime_nanos\"(CAST(\"a_tinyint\" AS BIGINT) * 1000000), 'America/Los_Angeles') AS TIMESTAMP), " | ||
| + "CAST(\"at_timezone\"(\"from_unixtime_nanos\"(CAST(\"a_smallint\" AS BIGINT) * 1000000), 'America/Los_Angeles') AS TIMESTAMP), " | ||
| + "CAST(\"at_timezone\"(\"from_unixtime_nanos\"(CAST(\"a_integer\" AS BIGINT) * 1000000), 'America/Los_Angeles') AS TIMESTAMP), " | ||
| + "CAST(\"at_timezone\"(\"from_unixtime_nanos\"(CAST(\"a_bigint\" AS BIGINT) * 1000000), 'America/Los_Angeles') AS TIMESTAMP), " |
There was a problem hiding this comment.
Did you consider using from_unixtime (without _nanos), to stay with lower precision timestamp where possible?
There was a problem hiding this comment.
Makes sense. I think I itiially though that from_unixtime only allows for second precision.
I updated the case for fractional types to use from_unixtime. For bigint/integer/smallint/tinyint I left from_unixtime_nanos to avoid unnecessary to_double conversion and division.
| if (call.getOperator().getName().equals("from_utc_timestamp")) { | ||
| Optional<RexNode> modifiedCall = visitFromUtcTimestampCall(call); | ||
| if (modifiedCall.isPresent()) { | ||
| return modifiedCall.get(); |
There was a problem hiding this comment.
Was it possible to handle this with a UDFTransformer?
There was a problem hiding this comment.
No, it is very limited right now. It feels extending it would be painful. Basically what we would need to do is to allow expressing logic I have here coded in Java in JSON we pass, to setup parameter translation. I strongly belive java code is better tool for the job.
|
|
||
| List<RexNode> convertedOperands = visitList(call.getOperands(), (boolean[]) null); | ||
| RexNode sourceValue = convertedOperands.get(0); | ||
| RexNode timezone = convertedOperands.get(1); |
There was a problem hiding this comment.
Wrap timezone into a canonicalize function call ($canonicalize_hive_timezone_id), so that we can in the future extend support to other Hive timezone identifiers which are not supported by Trino today.
This function would be identity for now.
losipiuk
force-pushed
the
lo/trino-from-utc-timestamp
branch
from
b7f16e4 to
02aa47a
Compare
|
@wmoustafa can you please take a look? |
Matching Trino PR: trinodb/trino#8502
@wmoustafa @hashhar @findepi PTAL