Zihao Ye 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:

(3 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.

I've moved it to the invisible functions list. I feel it might not be intuitive 
as a function provided to users. If needed, we can introduce functions with 
clearer naming and parameters in a separate Jira ticket.


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?
Done


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

Thank you for the reminder, all those "order by id" have been removed.



--
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 <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Reviewer: Zihao Ye <[email protected]>
Gerrit-Comment-Date: Tue, 12 Dec 2023 09:59:01 +0000
Gerrit-HasComments: Yes

Reply via email to