You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Youwei Wang (Code Review)" <ge...@cloudera.org> on 2016/09/21 02:21:55 UTC
[Impala-ASF-CR] IMPALA-3504: function for current timestamp in UTC, i.e. utc timestamp()
Youwei Wang has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4490
Change subject: IMPALA-3504: function for current timestamp in UTC, i.e. utc_timestamp()
......................................................................
IMPALA-3504: function for current timestamp in UTC, i.e. utc_timestamp()
Implement a UDF to generate current UTC timestamp.
Generated timestamp has been verified using online UTC service
in term of both epoch value and human-readable time.
Usage: select utc_timestamp();
Example result: 2016-09-21 02:14:37;
Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
---
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M common/function-registry/impala_functions.py
3 files changed, 10 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4490/1
--
To view, visit http://gerrit.cloudera.org:8080/4490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC
Posted by "Youwei Wang (Code Review)" <ge...@cloudera.org>.
Youwei Wang has posted comments on this change.
Change subject: IMPALA-3504: UDF for current timestamp in UTC
......................................................................
Patch Set 8:
> Carrying the previous +1 so it can be reviewed for a +2.
Greetings, dear Matthew.
Thank you so much for this encouraging evaluation and all great efforts you have spent! :)
--
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: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3504: function for current timestamp in UTC, i.e. utc timestamp()
Posted by "Youwei Wang (Code Review)" <ge...@cloudera.org>.
Youwei Wang has uploaded a new patch set (#2).
Change subject: IMPALA-3504: function for current timestamp in UTC, i.e. utc_timestamp()
......................................................................
IMPALA-3504: function for current timestamp in UTC, i.e. utc_timestamp()
Implement a UDF to generate current UTC timestamp.
Generated timestamp has been verified using online UTC service
in term of both epoch value and human-readable time.
Usage: select utc_timestamp();
Example result: 2016-09-21 02:14:37;
Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
---
M be/src/exprs/expr-test.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.h
M be/src/service/impala-server.cc
M common/function-registry/impala_functions.py
M common/thrift/ImpalaInternalService.thrift
9 files changed, 34 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4490/2
--
To view, visit http://gerrit.cloudera.org:8080/4490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
[Impala-ASF-CR] IMPALA-3504: function for current timestamp in UTC, i.e. utc timestamp()
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.
Change subject: IMPALA-3504: function for current timestamp in UTC, i.e. utc_timestamp()
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4490/3/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
PS3, Line 842: typedef boost::date_time::c_local_adjustor<ptime> local_adj;
: const ptime& utime = boost::posix_time::microsec_clock::universal_time();
: const ptime& ltime = local_adj::utc_to_local(utime);
: query_ctx->__set_now_string(TimestampValue(ltime).DebugString());
: query_ctx->__set_now_utc_string(TimestampValue(utime).DebugString());
> This is a bit tricky because we're trying to get 1 timestamp and do the con
As a user, I would be surprised if now() and utc_timestamp() were off by 8 hours + 1 microsecond, rather than 8 hours exactly or 5.5 hours exactly.
--
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-3504: UDF for current timestamp in UTC
......................................................................
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4490/8/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
PS8, Line 842: query_ctx->__set_utc_timestamp_string(TimestampValue::UtcTime().DebugString());
: query_ctx->__set_now_string(TimestampValue::LocalTime().DebugString());
it would be better if NOW() and UTC_TIMESTAMP() return the same instance in time (represented in different timezones). The current implementation will return slightly different instances in time.
--
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: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has abandoned this change.
Change subject: IMPALA-3504: UDF for current timestamp in UTC
......................................................................
Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/4490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: abandon
Gerrit-Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC
Posted by "Youwei Wang (Code Review)" <ge...@cloudera.org>.
Youwei Wang has uploaded a new patch set (#7).
Change subject: IMPALA-3504: UDF for current timestamp in UTC
......................................................................
IMPALA-3504: UDF for current timestamp in UTC
Purpose: Returns the current date and time (in the local time zone)
as a TIMESTAMP value. Generated timestamp has been verified using
online time services.
Return type: timestamp
Example:
select utc_timestamp();
+-------------------------------+
| utc_timestamp() |
+-------------------------------+
| 2016-09-23 09:42:42.931447000 |
+-------------------------------+
Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
---
M be/src/exprs/expr-test.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.h
M be/src/service/impala-server.cc
M common/function-registry/impala_functions.py
M common/thrift/ImpalaInternalService.thrift
9 files changed, 51 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4490/7
--
To view, visit http://gerrit.cloudera.org:8080/4490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC
Posted by "Youwei Wang (Code Review)" <ge...@cloudera.org>.
Youwei Wang has uploaded a new patch set (#4).
Change subject: IMPALA-3504: UDF for current timestamp in UTC
......................................................................
IMPALA-3504: UDF for current timestamp in UTC
Purpose: Returns the current date and time (in the local time zone)
as a TIMESTAMP value. Generated timestamp has been verified using
online time services.
Return type: timestamp
Example:
select utc_timestamp();
+-------------------------------+
| utc_timestamp() |
+-------------------------------+
| 2016-09-23 09:42:42.931447000 |
+-------------------------------+
Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
---
M be/src/exprs/expr-test.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.h
M be/src/service/impala-server.cc
M common/function-registry/impala_functions.py
M common/thrift/ImpalaInternalService.thrift
9 files changed, 60 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4490/4
--
To view, visit http://gerrit.cloudera.org:8080/4490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-3504: UDF for current timestamp in UTC
......................................................................
Patch Set 8: Code-Review+1
Carrying the previous +1 so it can be reviewed for a +2.
--
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: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3504: function for current timestamp in UTC, i.e. utc timestamp()
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-3504: function for current timestamp in UTC, i.e. utc_timestamp()
......................................................................
Patch Set 1:
> > 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
--
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: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-3504: UDF for current timestamp in UTC
......................................................................
Patch Set 7: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4490/7/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
PS7, Line 4137: DCHECK
missed this before, but this should be EXPECT_TRUE
--
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: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3504: function for current timestamp in UTC, i.e. utc timestamp()
Posted by "Youwei Wang (Code Review)" <ge...@cloudera.org>.
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 <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-3504: UDF for current timestamp in UTC
......................................................................
Patch Set 6:
(6 comments)
Thanks for bearing with us, Youwei!
http://gerrit.cloudera.org:8080/#/c/4490/6/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
PS6, Line 4120: forward-lapsing time
forward-lapsing?
PS6, Line 4132: ut1, ut2;
can you name these to reference the values they're holding, e.g. unixtime_utc, unixtime_local?
PS6, Line 4136: const bool res1
This should be true unless there wasn't a value in the TimestampValue, which shouldn't happen here. You don't need to define res1/res2 and check after, just DCHECK(now_utc.ToUnixTime(...));
PS6, Line 4137: const bool res2
same
http://gerrit.cloudera.org:8080/#/c/4490/6/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
PS6, Line 843: boost::date_time::c_local_adjustor<ptime>::utc_to_local
I'd still vote to not bother with the conversion, but it seems fine.
PS6, Line 845: const time_duration td_delta = universal_time - local_time;
: DCHECK(td_delta >= time_duration(-12, 0, 0) && td_delta <= time_duration(12, 0, 0));
I'm not sure we should check this. While this seems intuitive, there might be weird corner cases (e.g. leap seconds/minutes/whatever?). Who knows. I'm not sure we need to DCHECK this
--
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: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3504: function for current timestamp in UTC, i.e. utc timestamp()
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-3504: function for current timestamp in UTC, i.e. utc_timestamp()
......................................................................
Patch Set 1:
Hi Youwei,
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().
--
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: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC
Posted by "Youwei Wang (Code Review)" <ge...@cloudera.org>.
Hello Matthew Jacobs,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4490
to look at the new patch set (#8).
Change subject: IMPALA-3504: UDF for current timestamp in UTC
......................................................................
IMPALA-3504: UDF for current timestamp in UTC
Purpose: Returns the current date and time (in the local time zone)
as a TIMESTAMP value. Generated timestamp has been verified using
online time services.
Return type: timestamp
Example:
select utc_timestamp();
+-------------------------------+
| utc_timestamp() |
+-------------------------------+
| 2016-09-23 09:42:42.931447000 |
+-------------------------------+
Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
---
M be/src/exprs/expr-test.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.h
M be/src/service/impala-server.cc
M common/function-registry/impala_functions.py
M common/thrift/ImpalaInternalService.thrift
9 files changed, 51 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4490/8
--
To view, visit http://gerrit.cloudera.org:8080/4490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
[Impala-ASF-CR] IMPALA-3504: function for current timestamp in UTC, i.e. utc timestamp()
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.
Change subject: IMPALA-3504: function for current timestamp in UTC, i.e. utc_timestamp()
......................................................................
Patch Set 3:
(15 comments)
http://gerrit.cloudera.org:8080/#/c/4490/3//COMMIT_MSG
Commit Message:
Line 10: Generated timestamp has been verified using online UTC service
Please try to break paragraphs close the red line, and put two line breaks at the end of a paragraph.
PS3, Line 11: term of both epoch
What do you mean by this?
http://gerrit.cloudera.org:8080/#/c/4490/3/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
Line 4117: timestamp_result = ConvertValue<TimestampValue>(GetValue("utc_timestamp()",
Please check that this relates to now() in the way that you expect.
http://gerrit.cloudera.org:8080/#/c/4490/3/be/src/exprs/timestamp-functions-ir.cc
File be/src/exprs/timestamp-functions-ir.cc:
Line 317: TimestampVal TimestampFunctions::Now(FunctionContext* context) {
It looks to me like this is now only used one place. Can you inline it there, please, and take it out of the .h file?
http://gerrit.cloudera.org:8080/#/c/4490/3/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:
PS3, Line 121: now_utc
'utc_timestamp', here and below
http://gerrit.cloudera.org:8080/#/c/4490/3/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:
PS3, Line 113: universal time
'Coordinated Universal Time ("UTC")'
PS3, Line 115: a daylight savings
Does DST on the local machine change the universal time?
PS3, Line 117: UniversalTime
'UtcTime'
http://gerrit.cloudera.org:8080/#/c/4490/3/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
Line 842: typedef boost::date_time::c_local_adjustor<ptime> local_adj;
It's not a good use of a line to define a typedef that's used exactly once.
PS3, Line 843: utime
'universal_time'
PS3, Line 843: &
Why are you using references, here and below?
PS3, Line 844: ltime
'local_time'
Line 845: query_ctx->__set_now_string(TimestampValue(ltime).DebugString());
DCHECK that ltime and utime are close - with 48 hours of each other, for instance.
Line 846: query_ctx->__set_now_utc_string(TimestampValue(utime).DebugString());
If you set utime before ltime, either:
1. set the utime in query_ctx before the ltime in query_ctx; or
2. Set utime, then set utc_string in query_ctx, then ltime, then now_string in query_ctx.
http://gerrit.cloudera.org:8080/#/c/4490/3/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:
PS3, Line 298: now_utc_string
'utc_timestamp_string'
--
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3504: function for current timestamp in UTC, i.e. utc timestamp()
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.
Change subject: IMPALA-3504: function for current timestamp in UTC, i.e. utc_timestamp()
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4490/3/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
PS3, Line 842: typedef boost::date_time::c_local_adjustor<ptime> local_adj;
: const ptime& utime = boost::posix_time::microsec_clock::universal_time();
: const ptime& ltime = local_adj::utc_to_local(utime);
: query_ctx->__set_now_string(TimestampValue(ltime).DebugString());
: query_ctx->__set_now_utc_string(TimestampValue(utime).DebugString());
> I don't think it matters since (AFAIK) the functions don't both promise to
I see your point. I personally don't trust any clock or any timezone db. :-)
Youwei, can you check what postgresql and mysql return for queries like
select now(), utc_timestamp();
--
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC
Posted by "Youwei Wang (Code Review)" <ge...@cloudera.org>.
Youwei Wang has posted comments on this change.
Change subject: IMPALA-3504: UDF for current timestamp in UTC
......................................................................
Patch Set 8:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/4490/6/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
PS6, Line 4120: cTime();
> forward-lapsing?
Greeting, Matthew.
As a commone sense, time never rewinds, time never goes back. Since I am not a native English speaker, would you please share me a more elegant and native way to experss this?
Thank you for any idea or hint. :)
PS6, Line 4132: .UtcToLoc
> can you name these to reference the values they're holding, e.g. unixtime_u
Done
PS6, Line 4136: EXPECT_TRUE(now
> This should be true unless there wasn't a value in the TimestampValue, whic
Done
PS6, Line 4137: EXPECT_TRUE(std
> same
Done
http://gerrit.cloudera.org:8080/#/c/4490/7/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
PS7, Line 4137: EXPECT
> missed this before, but this should be EXPECT_TRUE
Done
http://gerrit.cloudera.org:8080/#/c/4490/6/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
PS6, Line 843: g(TimestampValue::LocalTime().DebugString());
> I'd still vote to not bother with the conversion, but it seems fine.
As you wish, Sir. :)
PS6, Line 845:
: // Creating a random_generator every time is not free, but
> I'm not sure we should check this. While this seems intuitive, there might
Done
--
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: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3504: function for current timestamp in UTC, i.e. utc timestamp()
Posted by "Youwei Wang (Code Review)" <ge...@cloudera.org>.
Youwei Wang has posted comments on this change.
Change subject: IMPALA-3504: function for current timestamp in UTC, i.e. utc_timestamp()
......................................................................
Patch Set 1:
> 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.
I have got your idea. Please give me a little time to figure out how to meet your requirement. Thank you.
--
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: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC
Posted by "Youwei Wang (Code Review)" <ge...@cloudera.org>.
Youwei Wang has posted comments on this change.
Change subject: IMPALA-3504: UDF for current timestamp in UTC
......................................................................
Patch Set 8:
Greetings everyone. The review status of this patch has paused for a few days. If possible, would you please move this a bit forward? Thank you. :)
--
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: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC
Posted by "Youwei Wang (Code Review)" <ge...@cloudera.org>.
Youwei Wang has posted comments on this change.
Change subject: IMPALA-3504: UDF for current timestamp in UTC
......................................................................
Patch Set 4:
(16 comments)
http://gerrit.cloudera.org:8080/#/c/4490/3//COMMIT_MSG
Commit Message:
Line 10: as a TIMESTAMP value. Generated timestamp has been verified using
> Please try to break paragraphs close the red line, and put two line breaks
Done
PS3, Line 11: ine time services.
> What do you mean by this?
Done
http://gerrit.cloudera.org:8080/#/c/4490/3/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
Line 4117: TYPE_TIMESTAMP));
> Please check that this relates to now() in the way that you expect.
Done
http://gerrit.cloudera.org:8080/#/c/4490/3/be/src/exprs/timestamp-functions-ir.cc
File be/src/exprs/timestamp-functions-ir.cc:
Line 317: TimestampVal TimestampFunctions::Now(FunctionContext* context) {
> It looks to me like this is now only used one place. Can you inline it ther
Greetings, Jim.
Please allow me to clarify your intention. I guess you want me to remove both the declaration and implementation of this TimestampFunctions::Now from timestamp-functions.h and timestamp-functions-ir.cc and then embed such code:
const TimestampValue* now = context->impl()->state()->now();
TimestampVal return_val;
now->ToTimestampVal(&return_val);
return return_val;
to the place which you think is the only caller of this Now() function?
If so, I am afraid there is another reason forbidding me to do so: since there exist one implicit caller in the impala_functions.py file:
[['now', 'current_timestamp'], 'TIMESTAMP', [], '_ZN6impala18TimestampFunctions3NowEPN10impala_udf15FunctionContextE'],
Basically, this code is the entry of the front-end and it connects the front-end to the back-end using the function signature as you see here. If we take it out of the .h file, this will be broken and queries like "select now()/current_timestamp();" will stop working.
Based on the above reason, maybe we should not touch this code.
Please feel free to point out my mistake if you think I am wrong. Thank you.
http://gerrit.cloudera.org:8080/#/c/4490/3/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:
PS3, Line 121: utc_tim
> 'utc_timestamp', here and below
Done
http://gerrit.cloudera.org:8080/#/c/4490/3/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:
PS3, Line 113: dinated Univer
> 'Coordinated Universal Time ("UTC")'
Done
PS3, Line 115: ck such as a manua
> Does DST on the local machine change the universal time?
Greetings, Jim.
The answer to this question is NO. I have corrected this comment.
PS3, Line 117: UtcTime() {
> 'UtcTime'
Done
http://gerrit.cloudera.org:8080/#/c/4490/3/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
Line 842: //typedef boost::date_time::c_local_adjustor<ptime> local_adj;
> It's not a good use of a line to define a typedef that's used exactly once.
Done
PS3, Line 843: &
> Why are you using references, here and below?
Greetings, Jim.
My motivation is to avoid the repeated object constructions when they are assigned back and forth. It is the same idea as Java reference. So your concern is maybe this will expose some memory risks by doing so?
PS3, Line 843: unive
> 'universal_time'
Done
PS3, Line 844: local
> 'local_time'
Done
Line 845: universal_time);
> DCHECK that ltime and utime are close - with 48 hours of each other, for in
Greetings, Jim.
This DCHECK task is easy to do. And do you think the threshold of 12 hours would be okay? I believe the delta range between UTC and local time would not be bigger than 12 hours in term of the absolute value.
PS3, Line 842: //typedef boost::date_time::c_local_adjustor<ptime> local_adj;
: const ptime& universal_time = boost::posix_time::microsec_clock::universal_time();
: const ptime& local_time = boost::date_time::c_local_adjustor<ptime>::utc_to_local(
: universal_time);
: const time_duration td_delta = universal_time - local_time;
> I see your point. I personally don't trust any clock or any timezone db. :-
Greetings, Gentlemen.
The following is the output of the postgres:
Version: PostgreSQL 9.4.8 on x86_64-unknown-linux-gnu, compiled by gcc (Debian 4.9.2-10) 4.9.2, 64-bit
postgres=# select now(), current_timestamp at time zone 'UTC';
now | timezone
-------------------------------+----------------------------
2016-09-23 11:01:45.907497+08 | 2016-09-23 03:01:45.907497
(1 row)
===========================================================
The following is the output of the mysql:
mysql Ver 14.14 Distrib 5.5.49, for debian-linux-gnu (x86_64) using readline 6.3
mysql> select now(), utc_timestamp();
+---------------------+---------------------+
| now() | utc_timestamp() |
+---------------------+---------------------+
| 2016-09-23 15:02:42 | 2016-09-23 07:02:42 |
+---------------------+---------------------+
1 row in set (0.16 sec)
Thanks. :)
PS3, Line 842: //typedef boost::date_time::c_local_adjustor<ptime> local_adj;
: const ptime& universal_time = boost::posix_time::microsec_clock::universal_time();
: const ptime& local_time = boost::date_time::c_local_adjustor<ptime>::utc_to_local(
: universal_time);
: const time_duration td_delta = universal_time - local_time;
> I don't think it matters since (AFAIK) the functions don't both promise to
Greetings, Gentlemen.
Yes, Jim has proposed a more strict restriction to eliminate errors between UTC and local time. That is also why I do such modification from patch set 2 to patch set 3.
And Matthew's idea also makes much sense here for if such functions are executed fast enough, there would be no such error actually. That is what I have observed in my experiment. (I have repeated this sql "select now(), utc_timestamp();" for many times, no significant error is observed. And I know such error does ocurr under certain circumstances.)
Considering it is not possible to take both solutions, so which solution do you think is more applicable in our scenario? :)
http://gerrit.cloudera.org:8080/#/c/4490/3/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:
PS3, Line 298: utc_timestamp_
> 'utc_timestamp_string'
Done
--
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3504: function for current timestamp in UTC, i.e. utc timestamp()
Posted by "Youwei Wang (Code Review)" <ge...@cloudera.org>.
Youwei Wang has posted comments on this change.
Change subject: IMPALA-3504: function for current timestamp in UTC, i.e. utc_timestamp()
......................................................................
Patch Set 1:
> 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 \u2192 timezone
utc_timestamp + timezone \u2192 local_timestamp
local_timestamp + timezone \u2192 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. :)
--
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: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3504: function for current timestamp in UTC, i.e. utc timestamp()
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-3504: function for current timestamp in UTC, i.e. utc_timestamp()
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4490/3/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
PS3, Line 842: typedef boost::date_time::c_local_adjustor<ptime> local_adj;
: const ptime& utime = boost::posix_time::microsec_clock::universal_time();
: const ptime& ltime = local_adj::utc_to_local(utime);
: query_ctx->__set_now_string(TimestampValue(ltime).DebugString());
: query_ctx->__set_now_utc_string(TimestampValue(utime).DebugString());
> As a user, I would be surprised if now() and utc_timestamp() were off by 8
I don't think it matters since (AFAIK) the functions don't both promise to be at some specific point in time. Some docs say "the current date and time", whatever that means. If it were free and they were exactly the same, that'd be great, but I'd vote for less complexity. Are we even confident the conversion will result in it being the exact X hrs offset anyway? I don't trust any functions related to timestamps/timezones, etc., so IMO less touching/converting is more :)
--
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3504: function for current timestamp in UTC, i.e. utc timestamp()
Posted by "Youwei Wang (Code Review)" <ge...@cloudera.org>.
Youwei Wang has uploaded a new patch set (#3).
Change subject: IMPALA-3504: function for current timestamp in UTC, i.e. utc_timestamp()
......................................................................
IMPALA-3504: function for current timestamp in UTC, i.e. utc_timestamp()
Implement a UDF to generate current UTC timestamp.
Generated timestamp has been verified using online UTC service
in term of both epoch value and human-readable time.
Usage: select utc_timestamp();
Example result: 2016-09-21 02:14:37;
Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
---
M be/src/exprs/expr-test.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.h
M be/src/service/impala-server.cc
M common/function-registry/impala_functions.py
M common/thrift/ImpalaInternalService.thrift
9 files changed, 39 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4490/3
--
To view, visit http://gerrit.cloudera.org:8080/4490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC
Posted by "Youwei Wang (Code Review)" <ge...@cloudera.org>.
Youwei Wang has uploaded a new patch set (#6).
Change subject: IMPALA-3504: UDF for current timestamp in UTC
......................................................................
IMPALA-3504: UDF for current timestamp in UTC
Purpose: Returns the current date and time (in the local time zone)
as a TIMESTAMP value. Generated timestamp has been verified using
online time services.
Return type: timestamp
Example:
select utc_timestamp();
+-------------------------------+
| utc_timestamp() |
+-------------------------------+
| 2016-09-23 09:42:42.931447000 |
+-------------------------------+
Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
---
M be/src/exprs/expr-test.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.h
M be/src/service/impala-server.cc
M common/function-registry/impala_functions.py
M common/thrift/ImpalaInternalService.thrift
9 files changed, 58 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4490/6
--
To view, visit http://gerrit.cloudera.org:8080/4490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
[Impala-ASF-CR] IMPALA-3504: function for current timestamp in UTC, i.e. utc timestamp()
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-3504: function for current timestamp in UTC, i.e. utc_timestamp()
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4490/3/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
PS3, Line 842: typedef boost::date_time::c_local_adjustor<ptime> local_adj;
: const ptime& utime = boost::posix_time::microsec_clock::universal_time();
: const ptime& ltime = local_adj::utc_to_local(utime);
: query_ctx->__set_now_string(TimestampValue(ltime).DebugString());
: query_ctx->__set_now_utc_string(TimestampValue(utime).DebugString());
This is a bit tricky because we're trying to get 1 timestamp and do the conversion. I think it'd just be easier to get the utc timestamp (boost::posix_time::microsec_clock::universal_time()) and then separately get the local time timestamp (TimstampVal::LocalTime()). I don't think it's a big deal that they're slightly different readings, the functions don't guarantee anything about them being equivalent. The important thing about capturing them here is that we don't have to worry about getting different values on different machines at different times, i.e. that the udf returns different values depending on when the code happens to run.
--
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC, i.e. utc timestamp()
Posted by "Youwei Wang (Code Review)" <ge...@cloudera.org>.
Youwei Wang has uploaded a new patch set (#5).
Change subject: IMPALA-3504: UDF for current timestamp in UTC, i.e. utc_timestamp()
......................................................................
IMPALA-3504: UDF for current timestamp in UTC, i.e. utc_timestamp()
Purpose: Returns the current date and time (in the local time zone)
as a TIMESTAMP value. Generated timestamp has been verified using
online time services.
Return type: timestamp
Example:
select utc_timestamp();
+-------------------------------+
| utc_timestamp() |
+-------------------------------+
| 2016-09-23 09:42:42.931447000 |
+-------------------------------+
Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
---
M be/src/exprs/expr-test.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/timestamp-value.h
M be/src/service/impala-server.cc
M common/function-registry/impala_functions.py
M common/thrift/ImpalaInternalService.thrift
9 files changed, 59 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4490/5
--
To view, visit http://gerrit.cloudera.org:8080/4490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5ee6ee192aa469f77c711f27ad324696a42004d1
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>