You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "sandeep akinapelli (Code Review)" <ge...@cloudera.org> on 2017/06/28 04:43:43 UTC

[Impala-ASF-CR] IMPALA-5317: add DATE TRUNC() function

sandeep akinapelli has uploaded a new change for review.

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

Change subject: IMPALA-5317: add DATE_TRUNC() function
......................................................................

IMPALA-5317: add DATE_TRUNC() function

Added a UDF builtin function date_trunc.
Reuse many of the Trunc functions implemented already for trunc()
Added checks to ensure that truncation results that fall outside of
posix timestamp range are returned as NULL.
Added ctest for the date_trunc function.

Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
---
M be/src/exprs/expr-test.cc
M be/src/exprs/udf-builtins-ir.cc
M be/src/exprs/udf-builtins.cc
M be/src/exprs/udf-builtins.h
M common/function-registry/impala_functions.py
5 files changed, 389 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: sandeep akinapelli <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-5317: add DATE TRUNC() function

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5317: add DATE_TRUNC() function
......................................................................


Patch Set 2:

(15 comments)

Just minor comments, mainly cleanup. I'd also like someone else to look over the code, particularly the tests, to make sure we're not missing any edge cases (we've been burned in the past with subtle timestamp code).

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

Line 7392:   TestIsNull("date_trunc('MILLENNIUM', '1416-05-08 10:30:00')", TYPE_TIMESTAMP);
It would be good to test MILLENIUM with 1999-12-31 11:59:59 or similar since there's a special case in the code for 2000.


Line 7393:   TestIsNull("date_trunc('WEEK', '1400-01-01 00:00:00')", TYPE_TIMESTAMP);
I think we should also test the edge cases 1400-1-6 and 1400-1-7 to make sure there isn't an off-by-one error in the calculation.


http://gerrit.cloudera.org:8080/#/c/7313/1/be/src/exprs/udf-builtins.cc
File be/src/exprs/udf-builtins.cc:

Line 420: 
> Yes agree with you. MY thoughts initially were that if there are conflicts 
Yeah I agree, I went down the same line of reasoning but I think this works out better.


http://gerrit.cloudera.org:8080/#/c/7313/2/be/src/exprs/udf-builtins.cc
File be/src/exprs/udf-builtins.cc:

PS2, Line 52: untils
units?


Line 245:   // fractional seconds are nanoseconds as per the build configuration
This comment is a bit cryptic - not clear which build configuration it means. Maybe something like "Fractional seconds are nanoseconds because Boost is configured to use nanosecond precision".


Line 254:   // fractional seconds are nanoseconds as per the build configuration.
Same here.


Line 260: // used by both Trunc and DateTrunc functions to perform the truncation
Put doTrunc() in an anonymous namespace -  "namespace {" - to avoid avoid exporting the symbol from this module.


PS2, Line 261: doTrunc
nit: casing should be DoTrunc().


Line 267:   static const date week_limit_date(1400, 1, 6);
local static variables have weird semantics - it would be best to avoid them. How about just inlining the value on line 279 - the value is already mentioned in the comment.


PS2, Line 272:       // for millenium < 2000 year value goes to 1000
             :       // (outside impala supported range)
nit: comment fits on one line.


Line 275:       if (orig_date.is_special()) return TimestampVal::null();
Shouldn't we check whether the date is special before checking the year? I'm not sure that the year is valid otherwise.


Line 280:       if (orig_date.is_special()) return TimestampVal::null();
Same here - should we check special first?


Line 289:     // used by DateTrunc only
This comment seems unnecessary (it's documented in the enum).


Line 296:     // used by DateTrunc only
Same here.


Line 334:     // used by DateTrunc only
Same here.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: sandeep akinapelli <sa...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: sandeep akinapelli <sa...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5317: add DATE TRUNC() function

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5317: add DATE_TRUNC() function
......................................................................


Patch Set 3: Code-Review+2

I was able to look at this with fresh eyes. Looks good, I will start the merge.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: sandeep akinapelli <sa...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: sandeep akinapelli <sa...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5317: add DATE TRUNC() function

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5317: add DATE_TRUNC() function
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

LGTM but would like someone else to check I didn't miss any edge cases

http://gerrit.cloudera.org:8080/#/c/7313/2/be/src/exprs/udf-builtins.cc
File be/src/exprs/udf-builtins.cc:

Line 260: }
> Done. Its already inside the the namespace {} block.
Ahh I see. Didn't notice that.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: sandeep akinapelli <sa...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: sandeep akinapelli <sa...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5317: add DATE TRUNC() function

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5317: add DATE_TRUNC() function
......................................................................


Patch Set 1:

Ping.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: sandeep akinapelli <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5317: add DATE TRUNC() function

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5317: add DATE_TRUNC() function
......................................................................


Patch Set 1:

(8 comments)

Looks good overall. I had a few minor style comments then a bigger question about code-sharing with Trunc()

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

Line 7307: 
Extra blank line


Line 7311:       TimestampValue::Parse("9000-01-01 00:00:00"));
Nit should indent continuations by 4 extra spaces.


Line 7355: 
Extra blank line


Line 7358:     TestIsNull("date_trunc('MILLENNIUM', '1416-05-08 10:30:00')", TYPE_TIMESTAMP);
I think it would be clearer to separate the test cases where the input is valid but the output is out-of-range (i.e. the first two).


http://gerrit.cloudera.org:8080/#/c/7313/1/be/src/exprs/udf-builtins.cc
File be/src/exprs/udf-builtins.cc:

Line 56: struct DateTruncUnit {
Let's convert this and TruncUnit to use the C++11 "strongly typed enum" feature. 

I think this was written pre-C++11 in an attempt to emulate that.


Line 99:   }
else if goes on the same line. https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=65868536

If you want to run clang-tidy to format your last commit you can run:
 
  git diff -U HEAD^ | $IMPALA_TOOLCHAIN/llvm-$IMPALA_LLVM_VERSION/share/clang/clang-format-diff.py -i -p1


Line 420:   switch (date_trunc_unit) {
There's a lot of code duplication with Trunc(). 

What do you think about combining the implementations? Was there a specific reason not to?

I was thinking we could combine the sets of units (documenting those which are supported by Trunc()/ DateTrunc()/both) then merge the implementations except StrToDateTruncUnit() and StrToTruncUnit().


http://gerrit.cloudera.org:8080/#/c/7313/1/be/src/exprs/udf-builtins.h
File be/src/exprs/udf-builtins.h:

Line 99: 
Extra blank line.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: sandeep akinapelli <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5317: add DATE TRUNC() function

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5317: add DATE_TRUNC() function
......................................................................


IMPALA-5317: add DATE_TRUNC() function

Added a UDF builtin function date_trunc.
Reuse many of the Trunc functions implemented already for trunc() including
truncate unit and except strToTruncUnit
Added checks to ensure that truncation results that fall outside of
posix timestamp range are returned as NULL.
Added ctest for the date_trunc function.

Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Reviewed-on: http://gerrit.cloudera.org:8080/7313
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/exprs/expr-test.cc
M be/src/exprs/udf-builtins-ir.cc
M be/src/exprs/udf-builtins.cc
M be/src/exprs/udf-builtins.h
M common/function-registry/impala_functions.py
5 files changed, 383 insertions(+), 47 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Tim Armstrong: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: sandeep akinapelli <sa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: sandeep akinapelli <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-5317: add DATE TRUNC() function

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5317: add DATE_TRUNC() function
......................................................................


Patch Set 4: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: sandeep akinapelli <sa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: sandeep akinapelli <sa...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5317: add DATE TRUNC() function

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5317: add DATE_TRUNC() function
......................................................................


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1204/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: sandeep akinapelli <sa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: sandeep akinapelli <sa...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5317: add DATE TRUNC() function

Posted by "sandeep akinapelli (Code Review)" <ge...@cloudera.org>.
sandeep akinapelli has uploaded a new patch set (#3).

Change subject: IMPALA-5317: add DATE_TRUNC() function
......................................................................

IMPALA-5317: add DATE_TRUNC() function

Added a UDF builtin function date_trunc.
Reuse many of the Trunc functions implemented already for trunc() including
truncate unit and except strToTruncUnit
Added checks to ensure that truncation results that fall outside of
posix timestamp range are returned as NULL.
Added ctest for the date_trunc function.

Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
---
M be/src/exprs/expr-test.cc
M be/src/exprs/udf-builtins-ir.cc
M be/src/exprs/udf-builtins.cc
M be/src/exprs/udf-builtins.h
M common/function-registry/impala_functions.py
5 files changed, 383 insertions(+), 47 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: sandeep akinapelli <sa...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: sandeep akinapelli <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-5317: add DATE TRUNC() function

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5317: add DATE_TRUNC() function
......................................................................


Patch Set 1:

Any updates here? Can I help move this along somehow?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: sandeep akinapelli <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5317: add DATE TRUNC() function

Posted by "sandeep akinapelli (Code Review)" <ge...@cloudera.org>.
sandeep akinapelli has posted comments on this change.

Change subject: IMPALA-5317: add DATE_TRUNC() function
......................................................................


Patch Set 2:

(7 comments)

addressed review comments.

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

Line 7307:       TimestampValue::Parse("2000-01-01 00:00:00  "));
> Extra blank line
Done


Line 7311:       TimestampValue::Parse("2110-01-01 00:00:00"));
> Nit should indent continuations by 4 extra spaces.
Done


Line 7355:   TestTimestampValue("date_trunc('WEEK', '9999-12-31 23:59:59.999999999')",
> Extra blank line
Done


Line 7358:       TimestampValue::Parse("9999-12-31 00:00:00"));
> I think it would be clearer to separate the test cases where the input is v
Done


http://gerrit.cloudera.org:8080/#/c/7313/1/be/src/exprs/udf-builtins.cc
File be/src/exprs/udf-builtins.cc:

Line 56:   WEEK,
> Let's convert this and TruncUnit to use the C++11 "strongly typed enum" fea
Removed the DateTruncUnit enum altogether and made the existing TruncUnit strongly typed enum.


Line 99:   } else if (unit == "hour") {
> else if goes on the same line. https://cwiki.apache.org/confluence/pages/vi
Done


Line 420: 
> There's a lot of code duplication with Trunc(). 
Yes agree with you. MY thoughts initially were that if there are conflicts with how we interpret the units in the implementation, then it is murky. However thinking again, I dont think the definition of these functions will change over time. hence merged the code excpet the StrtoTruncunit.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: sandeep akinapelli <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: sandeep akinapelli <sa...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5317: add DATE TRUNC() function

Posted by "sandeep akinapelli (Code Review)" <ge...@cloudera.org>.
sandeep akinapelli has posted comments on this change.

Change subject: IMPALA-5317: add DATE_TRUNC() function
......................................................................


Patch Set 2:

(12 comments)

Addressed review comments.

http://gerrit.cloudera.org:8080/#/c/7313/2/be/src/exprs/udf-builtins.cc
File be/src/exprs/udf-builtins.cc:

PS2, Line 52: untils
> units?
Done


Line 245:   // fractional seconds are nanoseconds as per the build configuration
> This comment is a bit cryptic - not clear which build configuration it mean
Done


Line 254:   // fractional seconds are nanoseconds as per the build configuration.
> Same here.
Done


Line 260: // used by both Trunc and DateTrunc functions to perform the truncation
> Put doTrunc() in an anonymous namespace -  "namespace {" - to avoid avoid e
Done. Its already inside the the namespace {} block.


PS2, Line 261: doTrunc
> nit: casing should be DoTrunc().
Done


Line 267:   static const date week_limit_date(1400, 1, 6);
> local static variables have weird semantics - it would be best to avoid the
Done


PS2, Line 272:       // for millenium < 2000 year value goes to 1000
             :       // (outside impala supported range)
> nit: comment fits on one line.
Done


Line 275:       if (orig_date.is_special()) return TimestampVal::null();
> Shouldn't we check whether the date is special before checking the year? I'
Yes. you are correct. switched the ifs.


Line 280:       if (orig_date.is_special()) return TimestampVal::null();
> Same here - should we check special first?
Done


Line 289:     // used by DateTrunc only
> This comment seems unnecessary (it's documented in the enum).
Done


Line 296:     // used by DateTrunc only
> Same here.
Done


Line 334:     // used by DateTrunc only
> Same here.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: sandeep akinapelli <sa...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: sandeep akinapelli <sa...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5317: add DATE TRUNC() function

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5317: add DATE_TRUNC() function
......................................................................


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: sandeep akinapelli <sa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: sandeep akinapelli <sa...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5317: add DATE TRUNC() function

Posted by "sandeep akinapelli (Code Review)" <ge...@cloudera.org>.
sandeep akinapelli has uploaded a new patch set (#2).

Change subject: IMPALA-5317: add DATE_TRUNC() function
......................................................................

IMPALA-5317: add DATE_TRUNC() function

Added a UDF builtin function date_trunc.
Reuse many of the Trunc functions implemented already for trunc() including
truncate unit and except strToTruncUnit
Added checks to ensure that truncation results that fall outside of
posix timestamp range are returned as NULL.
Added ctest for the date_trunc function.

Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
---
M be/src/exprs/expr-test.cc
M be/src/exprs/udf-builtins-ir.cc
M be/src/exprs/udf-builtins.cc
M be/src/exprs/udf-builtins.h
M common/function-registry/impala_functions.py
5 files changed, 378 insertions(+), 47 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: sandeep akinapelli <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>