Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20681 )
Change subject: IMPALA-12322: Support converting UTC timestamps read from Kudu to local time ...................................................................... Patch Set 12: Code-Review+1 (3 comments) Great work! Only minor comments. http://gerrit.cloudera.org:8080/#/c/20681/12/common/function-registry/impala_functions.py File common/function-registry/impala_functions.py: http://gerrit.cloudera.org:8080/#/c/20681/12/common/function-registry/impala_functions.py@257 PS12, Line 257: [['to_utc_timestamp'], 'TIMESTAMP', ['TIMESTAMP', 'STRING', 'BOOLEAN'], : "impala::TimestampFunctions::ToUtcUnambiguous"], Maybe this should rather go to the invisible functions list? I am not sure here - while I would prefer to not make this a supported functions, it can be useful for users in some cases. http://gerrit.cloudera.org:8080/#/c/20681/12/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java: http://gerrit.cloudera.org:8080/#/c/20681/12/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@744 PS12, Line 744: */ Can you mention that now this can return a list for timestamps? http://gerrit.cloudera.org:8080/#/c/20681/12/testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test File testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test: http://gerrit.cloudera.org:8080/#/c/20681/12/testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test@18 PS12, Line 18: order by id here and at other queries: "order by id" is not necessary - the rows in the results section + the actual results are ordered before comparison by default in EE tests otherwise most tests would need order by, as Impala has no guarantee for the order of returned rows if there are multiple files in a table -- To view, visit http://gerrit.cloudera.org:8080/20681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9a1e7a13e617cc18deef14289cf9b958588397d3 Gerrit-Change-Number: 20681 Gerrit-PatchSet: 12 Gerrit-Owner: Zihao Ye <eyiz...@163.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Reviewer: Zihao Ye <eyiz...@163.com> Gerrit-Comment-Date: Thu, 07 Dec 2023 22:39:57 +0000 Gerrit-HasComments: Yes
