Respect timestamp coercion when coercing complex column #17900
Conversation
Use == for enum comparison
Get type directly from HiveColumnHandle instead of generating them from HiveType.
Praveen2112
requested review from
findepi,
huberty89,
kokosing,
krvikash and
vlad-lyutenko
Praveen2112
force-pushed
the
praveen/timestamp_on_coerced_complex_column
branch
3 times, most recently
from
93da480 to
0631344
Compare
Praveen2112
force-pushed
the
praveen/timestamp_on_coerced_complex_column
branch
from
0631344 to
7cc5d79
Compare
Praveen2112
changed the title
Praveen2112
force-pushed
the
praveen/timestamp_on_coerced_complex_column
branch
from
7cc5d79 to
fd5118e
Compare
There was a problem hiding this comment.
what is the point of si2i field?
There was a problem hiding this comment.
It is a non TIMESTAMP column which we are coercing so as to reproduce this issue
There was a problem hiding this comment.
But does the comment help here - in the next set of PRs we would be adding additional column like timestamp which will be coerced.
plugin/trino-hive/src/main/java/io/trino/plugin/hive/HivePageSourceProvider.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
maybe recommendTableName -> tableNamePrefix?
There was a problem hiding this comment.
I wanted to have a unified representation across this class.
There was a problem hiding this comment.
nit: uppercase data types?
There was a problem hiding this comment.
I was thinking on handling it as a follow-up once the changes related to coercion are implemented. WDYT ?
Previously MILLISECONDS precision was leaked when corecion applied on complex column even if coercion was not applied on timestamp column.
Praveen2112
force-pushed
the
praveen/timestamp_on_coerced_complex_column
branch
from
fd5118e to
a538773
Compare
|
@krvikash AC |
plugin/trino-hive/src/main/java/io/trino/plugin/hive/HivePageSourceProvider.java
Show resolved
Hide resolved
4 participants
Description
Previously MILLISECONDS precision was leaked when corecion applied on complex column even if coercion was not applied on timestamp column.
Additional context and related issues
Release notes
( ) This is not user-visible or docs only and no release notes are required.
(x) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: