Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18675 )

Change subject: IMPALA-11368: Iceberg time-travel error message should show 
timestamp in local timezone
......................................................................


Patch Set 1:

(2 comments)

Thanks for working on this!

http://gerrit.cloudera.org:8080/#/c/18675/1/fe/src/main/java/org/apache/impala/util/IcebergUtil.java
File fe/src/main/java/org/apache/impala/util/IcebergUtil.java:

http://gerrit.cloudera.org:8080/#/c/18675/1/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@100
PS1, Line 100: "
Can we include the timezone as well? As this will output the timestamp in the 
local timezone of the impala server. But users might be in a different 
timezone, and set their local timezone in the query option TIMEZONE. Which 
means that the timestamp in the FOR SYSTEM_TIME AS OF <ts> is in the local 
timezone of the user.

We can also keep the timezone in the error message in UTC if we include the 
timezone information. If we do this, then we must set the query option TIMEZONE 
(to an arbitrary timezone other than UTC) in the test, otherwise the behavior 
will depend on where the test is being executed.


http://gerrit.cloudera.org:8080/#/c/18675/1/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@572
PS1, Line 572: IllegalArgumentException
This has the assumption that the IllegalArgumentException is always because of 
this type of error. Maybe it's not too bad because the SQL11 standard says that 
time travel queries referring to future dates should return the current state. 
I.e. the only problem with the parameter is that if it is older than the first 
snapshot.

But still, maybe it would make sense to also try to fix the error message in 
the Iceberg repo? If the Iceberg community is not open to fix the error message 
we can still fix in Impala.



--
To view, visit http://gerrit.cloudera.org:8080/18675
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iba5d5eb65133f11cc4eb2fc15a19f7b25c14cc46
Gerrit-Change-Number: 18675
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Tamas Mate <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Wed, 29 Jun 2022 09:06:51 +0000
Gerrit-HasComments: Yes

Reply via email to