Youwei Wang has posted comments on this change.
Change subject: IMPALA-3504: function for current timestamp in UTC, i.e.
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
> > any timezone offset. If you don't mind, I want to take the
> > time as example. Since Beijing timezone is of +8h offset, I can
> > 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
> > Then utc_timestamp() can be accessed like the function now().
> > 2. save the timezone/offset in the context data structure. Then
> > 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.
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-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>