subject:"\[Impala\-ASF\-CR\] IMPALA\-3504\: UDF for current timestamp in UTC"

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-07-06 Thread Matthew Jacobs (Code Review)

Matthew Jacobs has submitted this change and it was merged.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


IMPALA-3504: UDF for current timestamp in UTC

This change adds a UDF "utc_timestamp" which returns the current
date and time in UTC. Example query:

select utc_timestamp();

+---+
| utc_timestamp()   |
+---+
| 2017-06-15 17:36:39.290773000 |
+---+

Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Reviewed-on: http://gerrit.cloudera.org:8080/7203
Tested-by: Impala Public Jenkins
Reviewed-by: Matthew Jacobs 
---
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
M fe/src/test/java/org/apache/impala/testutil/TestUtils.java
10 files changed, 73 insertions(+), 5 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Matthew Jacobs: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-07-06 Thread Matthew Jacobs (Code Review)

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-07-06 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 7: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-07-06 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 7:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/826/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-30 Thread Matthew Jacobs (Code Review)

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 6:

Not sure why the aws download failed, but this could use a rebase anyway

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-29 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 6: Verified-1

Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/810/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-29 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 6:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/810/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-29 Thread Matthew Jacobs (Code Review)

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 6: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-29 Thread Bikramjeet Vig (Code Review)

Hello Impala Public Jenkins, Matthew Jacobs, Dan Hecht,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7203

to look at the new patch set (#6).

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..

IMPALA-3504: UDF for current timestamp in UTC

This change adds a UDF "utc_timestamp" which returns the current
date and time in UTC. Example query:

select utc_timestamp();

+---+
| utc_timestamp()   |
+---+
| 2017-06-15 17:36:39.290773000 |
+---+

Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
---
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
M fe/src/test/java/org/apache/impala/testutil/TestUtils.java
10 files changed, 73 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/7203/6
-- 
To view, visit http://gerrit.cloudera.org:8080/7203
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-28 Thread Matthew Jacobs (Code Review)

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 5: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-28 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 5:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/802/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-28 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 4: Verified-1

Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/799/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-28 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 4:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/799/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-27 Thread Dan Hecht (Code Review)

Dan Hecht has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-22 Thread Bikramjeet Vig (Code Review)

Bikramjeet Vig has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7203/3/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:

Line 343:   /// Use pointer to avoid inclusion of timestampvalue.h and avoid 
clang issues.
> Given the names are so different that it's not obvious (yet the names are o
Done


http://gerrit.cloudera.org:8080/#/c/7203/3/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:

Line 371:   // String containing a timestamp (in UTC) set at the query 
submission time.
> document same point in time as 'now_string'
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-22 Thread Bikramjeet Vig (Code Review)

Hello Matthew Jacobs,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7203

to look at the new patch set (#4).

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..

IMPALA-3504: UDF for current timestamp in UTC

This change adds a UDF "utc_timestamp" which returns the current
date and time in UTC. Example query:

select utc_timestamp();

+---+
| utc_timestamp()   |
+---+
| 2017-06-15 17:36:39.290773000 |
+---+

Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
---
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, 67 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/7203/4
-- 
To view, visit http://gerrit.cloudera.org:8080/7203
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-21 Thread Bikramjeet Vig (Code Review)

Bikramjeet Vig has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7203/1/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:

PS1, Line 371:  a
> as -> at
I did a second look at this while looking at Dan's latest comment. I think it 
should remain as "set as the query submission time" since this means that it 
represents the query submission time.
This would also remove the need to specify that it represents the same point in 
time as now_string


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-20 Thread Dan Hecht (Code Review)

Dan Hecht has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7203/3/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:

Line 343:   /// Use pointer to avoid inclusion of timestampvalue.h and avoid 
clang issues.
Given the names are so different that it's not obvious (yet the names are okay 
since they are consistent with the built-ins they implement), I think it would 
help to document that these are the same point in time but that now is in local 
time and utc_timestamp_ is in UTC.


http://gerrit.cloudera.org:8080/#/c/7203/3/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:

Line 124:   }
not your change and you shouldn't fix it in this commit, but all remaining uses 
of LocalTime() seem wrong. We should't be using TimestampValue as a general 
timer mechanism -- it should just be used for the TIMESTAMP datatype (and 
additionally some of the uses don't make much sense in local time).


http://gerrit.cloudera.org:8080/#/c/7203/3/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:

Line 371:   // String containing a timestamp (in UTC) set at the query 
submission time.
document same point in time as 'now_string'


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-16 Thread Matthew Jacobs (Code Review)

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 3: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-16 Thread Bikramjeet Vig (Code Review)

Bikramjeet Vig has uploaded a new patch set (#3).

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..

IMPALA-3504: UDF for current timestamp in UTC

This change adds a UDF "utc_timestamp" which returns the current
date and time in UTC. Example query:

select utc_timestamp();

+---+
| utc_timestamp()   |
+---+
| 2017-06-15 17:36:39.290773000 |
+---+

Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
---
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, 63 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/7203/3
-- 
To view, visit http://gerrit.cloudera.org:8080/7203
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-16 Thread Bikramjeet Vig (Code Review)

Bikramjeet Vig has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7203/2/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

PS2, Line 5192: "now()";
> this message isn't going to be very clear later
Done


PS2, Line 5193: utc_timestamp(
> same
Done


PS2, Line 5201: now
> local_time
Done


PS2, Line 5202: TimestampValue
> can you make this const, then make a separate TimestampValue which is to be
Done


PS2, Line 5187: const string stmt = "select now(), utc_timestamp()";
  :   vector result_types;
  :   Status status = executor_->Exec(stmt, _types);
  :   EXPECT_TRUE(status.ok()) << "stmt: " << stmt << "\nerror: " 
<< status.GetDetail();
  :   DCHECK(result_types.size() == 2);
  :   EXPECT_EQ(TypeToOdbcString(TYPE_TIMESTAMP), 
result_types[0].type) << "now()";
  :   EXPECT_EQ(TypeToOdbcString(TYPE_TIMESTAMP), 
result_types[1].type) << "utc_timestamp()";
  :   string result_row;
  :   status = executor_->FetchResult(_row);
  :   EXPECT_TRUE(status.ok()) << "stmt: " << stmt << "\nerror: " 
<< status.GetDetail();
  :   vector result_cols;
  :   boost::split(result_cols, result_row, boost::is_any_of("\t"));
  :   // To ensure this fails if columns are not tab separated
  :   DCHECK(result_cols.size() == 2);
  :   const TimestampValue now = 
ConvertValue(result_cols[0]);
  :   TimestampValue utc_timestamp = 
ConvertValue(result_cols[1]);
  :   utc_timestamp.UtcToLocal();
  :   EXPECT_EQ(utc_timestamp, now);
> can you wrap this in a set of braces to (1) make the code associated with t
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-16 Thread Matthew Jacobs (Code Review)

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7203/2/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

PS2, Line 5192: "now()";
this message isn't going to be very clear later


PS2, Line 5193: utc_timestamp(
same


PS2, Line 5201: now
local_time


PS2, Line 5202: TimestampValue
can you make this const, then make a separate TimestampValue which is to be 
converted (not const), with a name like "local_time_converted", e.g.

const TimestampValue utc_time...
TimestampValue local_time_converted(utc_time);
local_time_converted.UtcToLocal();
EXPECT_EQ(local_time, local_time_converted);

then it's more clear what we're checking


PS2, Line 5187: const string stmt = "select now(), utc_timestamp()";
  :   vector result_types;
  :   Status status = executor_->Exec(stmt, _types);
  :   EXPECT_TRUE(status.ok()) << "stmt: " << stmt << "\nerror: " 
<< status.GetDetail();
  :   DCHECK(result_types.size() == 2);
  :   EXPECT_EQ(TypeToOdbcString(TYPE_TIMESTAMP), 
result_types[0].type) << "now()";
  :   EXPECT_EQ(TypeToOdbcString(TYPE_TIMESTAMP), 
result_types[1].type) << "utc_timestamp()";
  :   string result_row;
  :   status = executor_->FetchResult(_row);
  :   EXPECT_TRUE(status.ok()) << "stmt: " << stmt << "\nerror: " 
<< status.GetDetail();
  :   vector result_cols;
  :   boost::split(result_cols, result_row, boost::is_any_of("\t"));
  :   // To ensure this fails if columns are not tab separated
  :   DCHECK(result_cols.size() == 2);
  :   const TimestampValue now = 
ConvertValue(result_cols[0]);
  :   TimestampValue utc_timestamp = 
ConvertValue(result_cols[1]);
  :   utc_timestamp.UtcToLocal();
  :   EXPECT_EQ(utc_timestamp, now);
can you wrap this in a set of braces to (1) make the code associated with this 
case easily identified and (2) scope some of the variables that won't be needed 
later but have generic names.

e.g.


{
  const string stmt...
}


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-16 Thread Bikramjeet Vig (Code Review)

Bikramjeet Vig has uploaded a new patch set (#2).

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..

IMPALA-3504: UDF for current timestamp in UTC

This change adds a UDF "utc_timestamp" which returns the current
date and time in UTC. Example query:

select utc_timestamp();

+---+
| utc_timestamp()   |
+---+
| 2017-06-15 17:36:39.290773000 |
+---+

Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
---
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(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/7203/2
-- 
To view, visit http://gerrit.cloudera.org:8080/7203
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-16 Thread Bikramjeet Vig (Code Review)

Bikramjeet Vig has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 1:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/7203/1/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

Line 259:   vector GetValues(const vector& exprs,
> This is overwrought for a function that gets used once with a single Column
This method was initially written to be as general as possible and closely 
resemble the GetValue method, so it can be re-used in the future. But since 
this is the only case where we need to get values simultaneously in one 
statement (since we need the time-stamps to represent the same point in time), 
I have instead made this specific to our case and moved inline with the test.


Line 290:   EXPECT_EQ(TypeToOdbcString(expr_type.type), 
result_types[index].type)
> Move this up before the FetchResult
Done


Line 5210:   TimestampValue utc_start_time = TimestampValue::UtcTime();
> const
Done


PS1, Line 5226: TimestampValue
> const
Done


Line 5229:   EXPECT_TRUE(utc_timestamp == now);
> EXPECT_EQ?
Done


http://gerrit.cloudera.org:8080/#/c/7203/1/be/src/runtime/runtime-state.cc
File be/src/runtime/runtime-state.cc:

Line 81: utc_timestamp_(new 
TimestampValue(TimestampValue::Parse(query_state->query_ctx().utc_timestamp_string))),
> long line, please wrap
Done


Line 81: utc_timestamp_(new 
TimestampValue(TimestampValue::Parse(query_state->query_ctx().utc_timestamp_string))),
> long line - brak at or before 90.
Done


http://gerrit.cloudera.org:8080/#/c/7203/1/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:

Line 126:   /// Returns Coordinated Universal Time ("UTC") with microsecond 
accuracy. This should
> Returns the current Coordinated Universal Time...
Done


http://gerrit.cloudera.org:8080/#/c/7203/1/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:

PS1, Line 321: as
> at
Done


PS1, Line 371: as
> as -> at
Done


PS1, Line 371: (i
> nit: add space
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-15 Thread Jim Apple (Code Review)

Jim Apple has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7203/1/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

PS1, Line 291: index++)
> at() throws an exception. Prefer [].
Sidebar: Will ASAN catch all out-of-bounds accesses with [] indexing? If not, 
should we prefer at() for tests?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-15 Thread Matthew Jacobs (Code Review)

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 1:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/7203/1/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

Line 259:   vector GetValues(const vector& exprs,
This warrants a comment explaining what this does, the arguments, and the 
return value. Please mention what happens in the case of an error.


PS1, Line 273:   EXPECT_TRUE(expect_error) << "stmt: " << stmt << "\nerror: 
"
 :   << status.GetDetail();
1 line? I don't think this will be too long


PS1, Line 283: EXPECT_TRUE(status.ok()) << "stmt: " << stmt << "\nerror: "
 : << status.GetDetail(
1 line?


PS1, Line 291: index++)
this is confusing here because it's not clear how this gets evaluated given the 
macro implementation. please do any potentially ambiguous var incrementing on a 
separate line.


http://gerrit.cloudera.org:8080/#/c/7203/1/be/src/runtime/runtime-state.cc
File be/src/runtime/runtime-state.cc:

Line 81: utc_timestamp_(new 
TimestampValue(TimestampValue::Parse(query_state->query_ctx().utc_timestamp_string))),
long line, please wrap


http://gerrit.cloudera.org:8080/#/c/7203/1/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:

Line 126:   /// Returns Coordinated Universal Time ("UTC") with microsecond 
accuracy. This should
Returns the current Coordinated Universal Time...


http://gerrit.cloudera.org:8080/#/c/7203/1/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:

PS1, Line 321: as
at


PS1, Line 371: as
as -> at


PS1, Line 371: (i
nit: add space


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-15 Thread Jim Apple (Code Review)

Jim Apple has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..


Patch Set 1:

(15 comments)

http://gerrit.cloudera.org:8080/#/c/7203/1/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

Line 259:   vector GetValues(const vector& exprs,
This is overwrought for a function that gets used once with a single 
ColumnType. Can you simplify it?


PS1, Line 259: GetValues
Give new methods comments explaining what they are for.


PS1, Line 260: false
is this just dead code - nobody calls this with expect_error = true, yes?


Line 261: DCHECK(exprs.size() == expr_types.size());
!empty(), too, right?


Line 264: for (const string expr : exprs) {
One must be very careful with for loops like this. THis copies the string. Try 
'const auto&'.


Line 267: string stmt = ss.str();
const


Line 268: stmt.erase(stmt.size() - 1);
Just don't write the comma in the first place.


Line 289: for (const ColumnType expr_type : expr_types) {
const reference unless you want the copy.


Line 290:   EXPECT_EQ(TypeToOdbcString(expr_type.type), 
result_types[index].type)
Move this up before the FetchResult


Line 291:   << exprs.at(index++);
If you use at for indexing, use it consistently (see line above)


Line 5210:   TimestampValue utc_start_time = TimestampValue::UtcTime();
const


PS1, Line 5226: TimestampValue
const


PS1, Line 5226: ConvertValue
This could be in GetValues if this is the only call.


Line 5229:   EXPECT_TRUE(utc_timestamp == now);
EXPECT_EQ?


http://gerrit.cloudera.org:8080/#/c/7203/1/be/src/runtime/runtime-state.cc
File be/src/runtime/runtime-state.cc:

Line 81: utc_timestamp_(new 
TimestampValue(TimestampValue::Parse(query_state->query_ctx().utc_timestamp_string))),
long line - brak at or before 90.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Jim Apple 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-06-15 Thread Bikramjeet Vig (Code Review)

Bikramjeet Vig has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/7203

Change subject: IMPALA-3504: UDF for current timestamp in UTC
..

IMPALA-3504: UDF for current timestamp in UTC

This change adds a UDF "utc_timestamp" which returns the current
date and time in UTC. Example query:

select utc_timestamp();

+---+
| utc_timestamp()   |
+---+
| 2017-06-15 17:36:39.290773000 |
+---+

Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
---
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, 81 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/7203/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7203
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2017-01-06 Thread Matthew Jacobs (Code Review)

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 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Youwei Wang

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2016-10-17 Thread Dan Hecht (Code Review)

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 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Youwei Wang 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2016-10-04 Thread Youwei Wang (Code Review)

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 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Youwei Wang 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2016-09-30 Thread Youwei Wang (Code Review)

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 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Youwei Wang 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2016-09-26 Thread Youwei Wang (Code Review)

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 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Youwei Wang

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2016-09-26 Thread Matthew Jacobs (Code Review)

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 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Youwei Wang 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2016-09-24 Thread Youwei Wang (Code Review)

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 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Youwei Wang

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2016-09-24 Thread Matthew Jacobs (Code Review)

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::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 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Youwei Wang 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC, i.e. utc timestamp()

2016-09-23 Thread Youwei Wang (Code Review)

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 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Youwei Wang

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2016-09-23 Thread Youwei Wang (Code Review)

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(_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 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 local_adj;
 :   const ptime& universal_time = 
boost::posix_time::microsec_clock::universal_time();
 :   const ptime& local_time = 
boost::date_time::c_local_adjustor::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 |

[Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC

2016-09-23 Thread Youwei Wang (Code Review)

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 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Youwei Wang

39 matches

Mail list logo