Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22293 )
Change subject: IMPALA-13627: Handle legacy Hive timezone conversion ...................................................................... Patch Set 14: Code-Review+1 (4 comments) http://gerrit.cloudera.org:8080/#/c/22293/14//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22293/14//COMMIT_MSG@35 PS14, Line 35: Adds a new CLI and query option - use_legacy_hive_timestamp_conversion - : to select what conversion method to use when an old Hive file is : detected. This only applies the third case in the previous paragraph, right? Can you make this more explicit? http://gerrit.cloudera.org:8080/#/c/22293/14/be/src/exec/parquet/hdfs-parquet-scanner.cc File be/src/exec/parquet/hdfs-parquet-scanner.cc: http://gerrit.cloudera.org:8080/#/c/22293/14/be/src/exec/parquet/hdfs-parquet-scanner.cc@3185 PS14, Line 3185: writer_time_zone = kv.value; Probably not in this patch, but it could be useful to detect the case when the writer_time_zone is different than the current timezone in Impala, e.g. write a log line at info level or add this info to the profile. http://gerrit.cloudera.org:8080/#/c/22293/14/fe/src/main/java/org/apache/impala/service/JniFrontend.java File fe/src/main/java/org/apache/impala/service/JniFrontend.java: http://gerrit.cloudera.org:8080/#/c/22293/14/fe/src/main/java/org/apache/impala/service/JniFrontend.java@822 PS14, Line 822: // Defaults to GMT if timezone is not recognized. Shouldn't we return an error in this case? http://gerrit.cloudera.org:8080/#/c/22293/14/testdata/workloads/functional-query/queries/QueryTest/timestamp-conversion-hive-313.test File testdata/workloads/functional-query/queries/QueryTest/timestamp-conversion-hive-313.test: http://gerrit.cloudera.org:8080/#/c/22293/14/testdata/workloads/functional-query/queries/QueryTest/timestamp-conversion-hive-313.test@24 PS14, Line 24: SET timezone=PST; Can you check in one of the test files for the timezone=UTC case, as that will lead to a different code path? -- To view, visit http://gerrit.cloudera.org:8080/22293 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1271ed1da0b74366ab8315e7ec2d4ee47111e067 Gerrit-Change-Number: 22293 Gerrit-PatchSet: 14 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Wed, 12 Feb 2025 15:04:27 +0000 Gerrit-HasComments: Yes
