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<string> GetValues(const vector<string>& 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 <[email protected]>
Gerrit-Reviewer: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Jim Apple <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-HasComments: Yes

Reply via email to