You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Vincent Tran (Code Review)" <ge...@cloudera.org> on 2017/05/25 12:09:28 UTC
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Vincent Tran has uploaded a new change for review.
http://gerrit.cloudera.org:8080/6991
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
IMPALA-5316: Adds last_day() function
This change adds last_day(TIMESTAMP) function.
The function takes exactly one TIMESTAMP argument
and returns a TIMESTAMP that is the last date of the
input TIMESTAMP's calendar month.
Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
---
M be/src/exprs/expr-test.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M common/function-registry/impala_functions.py
4 files changed, 61 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/6991/1
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Vincent Tran has uploaded a new patch set (#4).
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
IMPALA-5316: Adds last_day() function
This change adds last_day() function.
The function takes exactly one TIMESTAMP argument
and returns a TIMESTAMP that is the last date of the
input date's calendar month.
The function will return NULL when:
1) The input argument cannot be implicitly casted to
a TIMESTAMP.
2) The TIMESTAMP argument is missing a date component.
3) The TIMESTAMP argument is outside of the supported range:
between 1400-01-31 00:00:00 and 9999-12-31 23:59:59
Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
---
M be/src/exprs/expr-test.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M common/function-registry/impala_functions.py
4 files changed, 76 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/6991/4
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Vincent Tran has posted comments on this change.
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6991/4/be/src/exprs/timestamp-functions.h
File be/src/exprs/timestamp-functions.h:
PS4, Line 202: /// 1) The argument cannot be implicitly casted to TIMESTAMP.
> Remove this line. I don't think this is relevant to this function implement
That makes sense. Done.
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Vincent Tran has posted comments on this change.
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/6991/2/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
Line 279: void TestLastDayFunction() {
> I didn't mean in automated tests but rather checking by manual inspection,
Done
http://gerrit.cloudera.org:8080/#/c/6991/3/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
Line 285: TestTimestampValue("last_day('2003-03-02 03:21:12.0058')",
> add some fractional seconds too
Done
http://gerrit.cloudera.org:8080/#/c/6991/3/be/src/exprs/timestamp-functions.h
File be/src/exprs/timestamp-functions.h:
PS3, Line 199: date c
> Impala doesn't have a DATE type so this is misleading. I think you can say
Done
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Vincent Tran has uploaded a new patch set (#2).
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
IMPALA-5316: Adds last_day() function
This change adds last_day(TIMESTAMP) function.
The function takes exactly one TIMESTAMP argument
and returns a TIMESTAMP that is the last date of the
input TIMESTAMP's calendar month.
Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
---
M be/src/exprs/expr-test.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M common/function-registry/impala_functions.py
4 files changed, 60 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/6991/2
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6991/4/be/src/exprs/timestamp-functions.h
File be/src/exprs/timestamp-functions.h:
PS4, Line 202: /// 1) The argument cannot be implicitly casted to TIMESTAMP.
Remove this line. I don't think this is relevant to this function implementation (i.e. what we're documenting in this header file) because you know the argument is already a TIMESTAMP. It makes sense in the context of the commit msg and ultimately the Impala documentation.
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
Patch Set 2:
(7 comments)
Thanks for the contribution, Vincent.
http://gerrit.cloudera.org:8080/#/c/6991/2/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
Line 279: void TestLastDayFunction() {
can you check the results of all of these tests are the same as a reference database?
Line 281: TestTimestampValue("last_day('2003-01-02')",
can you add times to some of these?
Line 282: TimestampValue::Parse("2003-01-31 00:00:00", 19));
for all of the below, use the Parse() overload that takes std::string, i.e. remove the str len.
Line 316: TimestampValue::Parse("2100-02-28 00:00:00", 19));
I pointed out a few special cases to consider in the .cc file
In addition:
'1400-01-01 00:00:00' (first supported date)
'9999-12-31 23:59:59' (last supported date)
http://gerrit.cloudera.org:8080/#/c/6991/2/be/src/exprs/timestamp-functions-ir.cc
File be/src/exprs/timestamp-functions-ir.cc:
PS2, Line 562: end_of_month()
1) I wonder if you can get a TimestampValue without a Date or with a "special" date, eg. maybe if you create a TIMESTAMP with just a time. Please test last_day(cast("00:00:00" as timestamp));
2) I think we have to be sure to check the case when timestamp.date() is at the end of the supported date range (year 12-31-9999 23:59:59), I suspect it should return a valid date but we should check.
I can't think of any other corner cases.
PS2, Line 562: ptime pdate(timestamp.date().end_of_month());
: TimestampValue tsv(pdate);
I think you can avoid converting to ptime and just use the TimestampValue constructor that takes a gregorian date and a time_duration, e.g.
tsv(timestamp.date().end_of_month(), time_duration(0,0,0,0))
http://gerrit.cloudera.org:8080/#/c/6991/2/be/src/exprs/timestamp-functions.h
File be/src/exprs/timestamp-functions.h:
Line 198: static TimestampVal LastDay(FunctionContext* context, const TimestampVal& ts);
please add a comment. look to other fns in the file for examples.
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6991/6/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
Line 279: void TestLastDayFunction() {
Vincent, MJ, I don't really see how this code is executed, are you sure these tests are running? I don't see this code called from a GTest test.
You should put these test cases into a GTest like this:
TEST_F(ExprTest, LastDay) {
// Code goes here
}
The ExprTest class should only contain helper functions and not test cases.
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
Patch Set 5: Code-Review+1
Nice work, thanks for doing this! Let's see if Marcel, Alex, or Dan want to check anything else.
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
Patch Set 5:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/733/
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Vincent Tran has posted comments on this change.
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
Patch Set 2:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/6991/2/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
Line 279: void TestLastDayFunction() {
> can you check the results of all of these tests are the same as a reference
MariaDB returns only the DATE portion of the timestamp. That would be neat to do here as well. But I don't see a way to do it unless we convert the output to StringVal.
Line 281: TestTimestampValue("last_day('2003-01-02')",
> can you add times to some of these?
Done
Line 282: TimestampValue::Parse("2003-01-31 00:00:00", 19));
> for all of the below, use the Parse() overload that takes std::string, i.e.
Done
Line 316: TimestampValue::Parse("2100-02-28 00:00:00", 19));
> I pointed out a few special cases to consider in the .cc file
Done
http://gerrit.cloudera.org:8080/#/c/6991/2/be/src/exprs/timestamp-functions-ir.cc
File be/src/exprs/timestamp-functions-ir.cc:
PS2, Line 562: end_of_month()
> 1) I wonder if you can get a TimestampValue without a Date or with a "speci
Done
PS2, Line 562: ptime pdate(timestamp.date().end_of_month());
: TimestampValue tsv(pdate);
> I think you can avoid converting to ptime and just use the TimestampValue c
Done
http://gerrit.cloudera.org:8080/#/c/6991/2/be/src/exprs/timestamp-functions.h
File be/src/exprs/timestamp-functions.h:
Line 198: static TimestampVal LastDay(FunctionContext* context, const TimestampVal& ts);
> please add a comment. look to other fns in the file for examples.
Done
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Vincent Tran has uploaded a new patch set (#5).
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
IMPALA-5316: Adds last_day() function
This change adds last_day() function.
The function takes exactly one TIMESTAMP argument
and returns a TIMESTAMP that is the last date of the
input date's calendar month.
The function will return NULL when:
1) The input argument cannot be implicitly casted to
a TIMESTAMP.
2) The TIMESTAMP argument is missing a date component.
3) The TIMESTAMP argument is outside of the supported range:
between 1400-01-31 00:00:00 and 9999-12-31 23:59:59
Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
---
M be/src/exprs/expr-test.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M common/function-registry/impala_functions.py
4 files changed, 75 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/6991/5
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6991/6/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
Line 279: void TestLastDayFunction() {
> I'm fixing this.
Filed IMPALA-5585
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Vincent Tran has posted comments on this change.
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6991/6/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
Line 279: void TestLastDayFunction() {
> Good catch, I suspect the intention was to call this in the same place Test
I'm fixing this.
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/6991/2/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
Line 279: void TestLastDayFunction() {
> MariaDB returns only the DATE portion of the timestamp. That would be neat
I didn't mean in automated tests but rather checking by manual inspection, sorry if it's a pain but we should just check.
http://gerrit.cloudera.org:8080/#/c/6991/3/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
Line 285: TestTimestampValue("last_day('2003-03-02 03:21:12')",
add some fractional seconds too
http://gerrit.cloudera.org:8080/#/c/6991/3/be/src/exprs/timestamp-functions.h
File be/src/exprs/timestamp-functions.h:
PS3, Line 199: DATE or
Impala doesn't have a DATE type so this is misleading. I think you can say "The input timestamp requires a date component, it may or may not have a time component."
Comment when this returns a TimestampVal set to null.
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6991/6/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
Line 279: void TestLastDayFunction() {
> Vincent, MJ, I don't really see how this code is executed, are you sure the
Good catch, I suspect the intention was to call this in the same place TestNextDayFunction() is called.
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5316: Adds last day() 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-5316: Adds last_day() function
......................................................................
IMPALA-5316: Adds last_day() function
This change adds last_day() function.
The function takes exactly one TIMESTAMP argument
and returns a TIMESTAMP that is the last date of the
input date's calendar month.
The function will return NULL when:
1) The input argument cannot be implicitly casted to
a TIMESTAMP.
2) The TIMESTAMP argument is missing a date component.
3) The TIMESTAMP argument is outside of the supported range:
between 1400-01-31 00:00:00 and 9999-12-31 23:59:59
Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Reviewed-on: http://gerrit.cloudera.org:8080/6991
Reviewed-by: Matthew Jacobs <mj...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/exprs/expr-test.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M common/function-registry/impala_functions.py
4 files changed, 75 insertions(+), 0 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Matthew Jacobs: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
[Impala-ASF-CR] IMPALA-5316: Adds last day() function
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Vincent Tran has uploaded a new patch set (#3).
Change subject: IMPALA-5316: Adds last_day() function
......................................................................
IMPALA-5316: Adds last_day() function
This change adds last_day() function.
The function takes exactly one DATE/TIMESTAMP argument
and returns a TIMESTAMP that is the last date of the
input date's calendar month.
Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
---
M be/src/exprs/expr-test.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M common/function-registry/impala_functions.py
4 files changed, 70 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/6991/3
--
To view, visit http://gerrit.cloudera.org:8080/6991
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I429c8734bddca3c37a2eedc211a16a4ffcb04370
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>