Youwei Wang has posted comments on this change. Change subject: IMPALA-3504: function for current timestamp in UTC, i.e. utc_timestamp() ......................................................................
Patch Set 2: > > > As I mentioned in your previous review and/or the JIRA, this > > needs > > > to use the query-wide timestamp, i.e. what we do for now(). > > > > Greetings, Matthew. > > If you don't mind, I want to propose an idea about this task. > > Considering three fators of utc_timestamp, local_timestamp and > > timezone, I believe we have following deductions: > > utc_timestamp + local_timestamp ? timezone > > utc_timestamp + timezone ? local_timestamp > > local_timestamp + timezone ? utc_timestamp > > PS: So far as I know, the UTC time is essentially the time > without > > any timezone offset. If you don't mind, I want to take the > Beijing > > time as example. Since Beijing timezone is of +8h offset, I can > get > > corresponding UTC time by minusing 8 from current Beijing time. I > > have done some calculation and calibration using online time > > service. And this approach works. If you find some mistakes in my > > approach, please feel free to point them out. Thank you. :) > > > > And I have found the following code: > > query_ctx->__set_now_string(TimestampValue::LocalTime().DebugString()); > > in be/src/service/impala-server.cc:841. > > So please allow me to assume we have local_timestamp of the host > > where Impala service runs. In this case, we can: > > 1. directly store the utc_timestamp in the context data > structure. > > Then utc_timestamp() can be accessed like the function now(). > > 2. save the timezone/offset in the context data structure. Then > we > > can use the now() and such timezone/offset to calculate the > > utc_timestamp. > > > > If possbile, would you please tell which solution you prefer? > > Thank you for reading this long post. :) > > Hi Youwei, > > Good question, thanks for following up. I think #1 is better > because #2 involves some additional computation on every invocation > of utc_timestamp() that may get expensive, e.g. "select > utc_timestamp() from table_with_1billion_rows;". The advantage I > see to #2 is if we wanted the timezone stored separately for some > other reason, but I think right now that would be premature. > > Thanks Greetings, Matthew. Thank you for your suggestion and comment - I totally agree with you. Storing the timezone in the runtime-state structure may save us a few bytes, but it does cost some overheads since we should convert the time repeatedly. My ideas are: 1. I choose storing the utc_timestamp in the runtime-state so we can get corresponding time zone offset using now() and utc_timestamp(). 2. Actually, I am afraid there exist some other issues about the conversion using local now() and a timezone. That is the JIRA IMPALA-3169, which I am working on now. Thank you again and any code review comment is highly welcomed. :) -- To view, visit http://gerrit.cloudera.org:8080/4490 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Youwei Wang <youwei.a.w...@intel.com> Gerrit-Reviewer: Jim Apple <jbap...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Youwei Wang <youwei.a.w...@intel.com> Gerrit-HasComments: No
