[Impala-ASF-CR] IMPALA-5315: Cast to timestamp fails for YYYY-M-D format

2017-11-01 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7009 )

Change subject: IMPALA-5315: Cast to timestamp fails for -M-D format
..


Patch Set 5:

I will certainly get back to this change. Needed to handle some personal issues 
and got off track.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
Gerrit-Change-Number: 7009
Gerrit-PatchSet: 5
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran 
Gerrit-Comment-Date: Wed, 01 Nov 2017 17:18:59 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5336: Fix partition pruning when column is cast

2017-07-28 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5336: Fix partition pruning when column is cast
..


Patch Set 1: Code-Review+1

Great analysis.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94f597a6589f5e34d2b74abcd29be77c4161cd99
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5336: Fix partition pruning when column is cast

2017-07-27 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5336: Fix partition pruning when column is cast
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7521/1/fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java
File fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java:

PS1, Line 185: if (bp.getChild(0).isImplicitCast()) return false;
Will this also change the behaviour of implicitly casted integer types (e.g. 
BIGINT  INT)?
One question that I didn't quite have an answer/explanation for while working 
on this was that this bug didn't seem to affect implicitly casted integer-type 
partition keys.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94f597a6589f5e34d2b74abcd29be77c4161cd99
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5315: Cast to timestamp fails for YYYY-M-D format

2017-07-20 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5315: Cast to timestamp fails for -M-D format
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7009/5/be/src/runtime/timestamp-parse-util.cc
File be/src/runtime/timestamp-parse-util.cc:

Line 291:   // If we failed to parse using the default templates we will try 
lazy format.
> how much slower is it doing it this way, compared to using a stamped out fo
Virtually same over 11000 datetime strings. 
http://github.mtv.cloudera.com/gist/anonymous/a605d18c955198ecccda2c42d912d690

But maybe I need a larger dataset?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5524: Fixes NPE during planning with DISABLE UNSAFE SPILLS=1

2017-07-11 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5524: Fixes NPE during planning with 
DISABLE_UNSAFE_SPILLS=1
..


Patch Set 5:

Done.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iccae7fdeaf0ade0b8728a7d249981c8270e8c327
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5524: Fixes NPE during planning with DISABLE UNSAFE SPILLS=1

2017-07-11 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#4).

Change subject: IMPALA-5524: Fixes NPE during planning with 
DISABLE_UNSAFE_SPILLS=1
..

IMPALA-5524: Fixes NPE during planning with DISABLE_UNSAFE_SPILLS=1

This change will avoid a NPE during query planning under the following
conditions:
- DISABLE_UNSAFE_SPILLS is TRUE or 1
- All tables involved in the query have stats

Change-Id: Iccae7fdeaf0ade0b8728a7d249981c8270e8c327
---
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
2 files changed, 20 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iccae7fdeaf0ade0b8728a7d249981c8270e8c327
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran 
Gerrit-Reviewer: anujphadke 


[Impala-ASF-CR] IMPALA-5524: Fixes NPE during planning with DISABLE UNFASE SPILLS=1

2017-06-30 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5524: Fixes NPE during planning with 
DISABLE_UNFASE_SPILLS=1
..


Patch Set 2:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/7219/2/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

Line 1027:   && queryCtx.isSetTables_missing_stats()
> I think this should be
Done


http://gerrit.cloudera.org:8080/#/c/7219/2/fe/src/test/java/org/apache/impala/planner/PlannerTest.java
File fe/src/test/java/org/apache/impala/planner/PlannerTest.java:

Line 432: queryCtx.client_request.setStmt("compute stats 
functional.alltypes");
> It might help to explain the choice of queries. Is it significant that one 
Done


PS2, Line 435: request_with_disable_spill_off
> Variables in the fe are capitalised like: requestWithDisableSpillOff
Done


Line 437: try {
> You can just remove the try()/catch(). If an exception is thrown and not ca
Done
frontend_.createExecRequest() throws, so we either have to catch it or the test 
method have to throw as well. Is there any implication within the unit test 
framework? (i.e. is it possible that it may halt all of the tests?)


Line 438:   request_with_disable_spill_off = 
frontend_.createExecRequest(queryCtx, explainBuilder);
> Long line - can you wrap to 90 characters?
Done


PS2, Line 442: Preconditions.checkNotNull
> Use JUnit's assertNotNull() in tests
Done


Line 445: try {
> Same here - the try/catch isn't really necessary - we can make the test a b
Done


Line 446:   request_with_disable_spill_on = 
frontend_.createExecRequest(queryCtx, explainBuilder);
> Long line - can you wrap to 90 characters?
Done


Line 448:   Assert.fail("Failed to create exec request with 
DISABLE_UNSAFE_SPILLS=true:" + e.getMessage());
> Long line - can you wrap to 90 characters?
Done


Line 450: Preconditions.checkNotNull(request_with_disable_spill_on);
> Use JUnit's assertNotNull() in tests
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iccae7fdeaf0ade0b8728a7d249981c8270e8c327
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5524: Fixes NPE during planning with DISABLE UNFASE SPILLS=1

2017-06-30 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#3).

Change subject: IMPALA-5524: Fixes NPE during planning with 
DISABLE_UNFASE_SPILLS=1
..

IMPALA-5524: Fixes NPE during planning with DISABLE_UNFASE_SPILLS=1

This change will avoid a NPE during query planning under the following
conditions:
- DISABLE_UNSAFE_SPILLS is TRUE or 1
- All tables involved in the query have stats

Change-Id: Iccae7fdeaf0ade0b8728a7d249981c8270e8c327
---
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
2 files changed, 20 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iccae7fdeaf0ade0b8728a7d249981c8270e8c327
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran 
Gerrit-Reviewer: anujphadke 


[Impala-ASF-CR] IMPALA-5316: Adds last day() function

2017-06-26 Thread Vincent Tran (Code Review)
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 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-2782: Allow impala-shell to connect directly to impalad when configured with load balancer and kerberos.

2017-06-22 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-2782: Allow impala-shell to connect directly to impalad 
when configured with load balancer and kerberos.
..


Patch Set 2:

Added default option as None. Still missing a test though, I will make sure to 
work on the test after I get feedback.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4726226a7a3817421b133f74dd4f4cf8c52135f9
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-2782: Allow impala-shell to connect directly to impalad when configured with load balancer and kerberos.

2017-06-22 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#2).

Change subject: IMPALA-2782: Allow impala-shell to connect directly to impalad 
when configured with load balancer and kerberos.
..

IMPALA-2782: Allow impala-shell to connect directly to impalad when
configured with load balancer and kerberos.

This change adds an impala-shell option -b . This allows
user to optionally specify the load-balancer's host:port so that
impala-shell will accept a direct connection to impala daemons in
a kerberized cluster.

Change-Id: I4726226a7a3817421b133f74dd4f4cf8c52135f9
---
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
4 files changed, 12 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4726226a7a3817421b133f74dd4f4cf8c52135f9
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-2782: Allow impala-shell to connect directly to impalad when configured with load balancer and kerberos.

2017-06-20 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-2782: Allow impala-shell to connect directly to impalad 
when configured with load balancer and kerberos.
..


Patch Set 1:

I'm not sure how to write a test for this one as the mini cluster lacks 
kerberos and lb. Looking for suggestions.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4726226a7a3817421b133f74dd4f4cf8c52135f9
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-2782: Allow impala-shell to connect directly to impalad when configured with load balancer and kerberos.

2017-06-20 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new change for review.

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

Change subject: IMPALA-2782: Allow impala-shell to connect directly to impalad 
when configured with load balancer and kerberos.
..

IMPALA-2782: Allow impala-shell to connect directly to impalad when
configured with load balancer and kerberos.

This change adds an impala-shell option -b . This allows
user to optionally specify the load-balancer's host:port so that
impala-shell will accept a direct connection to impala daemons in
a kerberized cluster.

Change-Id: I4726226a7a3817421b133f74dd4f4cf8c52135f9
---
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
3 files changed, 11 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4726226a7a3817421b133f74dd4f4cf8c52135f9
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 


[Impala-ASF-CR] IMPALA-5524: Fixes NPE during planning with DISABLE UNSAFE SPILLS=1

2017-06-18 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5524: Fixes NPE during planning with 
DISABLE_UNSAFE_SPILLS=1
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7219/1//COMMIT_MSG
Commit Message:

PS1, Line 7: DISABLE_UNSAFE_SPILLS
> typo
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iccae7fdeaf0ade0b8728a7d249981c8270e8c327
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Vincent Tran 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5524: Fixes NPE during planning with DISABLE UNSAFE SPILLS=1

2017-06-18 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#2).

Change subject: IMPALA-5524: Fixes NPE during planning with 
DISABLE_UNSAFE_SPILLS=1
..

IMPALA-5524: Fixes NPE during planning with DISABLE_UNSAFE_SPILLS=1

This change will avoid a NPE during query planning under the following
conditions:
- DISABLE_UNSAFE_SPILLS is TRUE or 1
- All tables involved in the query have stats

Change-Id: Iccae7fdeaf0ade0b8728a7d249981c8270e8c327
---
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
2 files changed, 26 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iccae7fdeaf0ade0b8728a7d249981c8270e8c327
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: anujphadke 


[Impala-ASF-CR] IMPALA-5524: Fixes NPE during planning with DISABLE UNFASE SPILLS=1

2017-06-17 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new change for review.

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

Change subject: IMPALA-5524: Fixes NPE during planning with 
DISABLE_UNFASE_SPILLS=1
..

IMPALA-5524: Fixes NPE during planning with DISABLE_UNFASE_SPILLS=1

This change will avoid a NPE during query planning under the following
conditions:
- DISABLE_UNSAFE_SPILLS is TRUE or 1
- All tables involved in the query have stats

Change-Id: Iccae7fdeaf0ade0b8728a7d249981c8270e8c327
---
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
2 files changed, 26 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iccae7fdeaf0ade0b8728a7d249981c8270e8c327
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 


[Impala-ASF-CR] IMPALA-5315: Cast to timestamp fails for YYYY-M-D format

2017-06-17 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5315: Cast to timestamp fails for -M-D format
..


Patch Set 5:

Fixed some nits.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5315: Cast to timestamp fails for YYYY-M-D format

2017-06-17 Thread Vincent Tran (Code Review)
Hello Lars Volker,

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

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

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

Change subject: IMPALA-5315: Cast to timestamp fails for -M-D format
..

IMPALA-5315: Cast to timestamp fails for -M-D format

This change allows casting of a string in 'lazy' date
format to timestamp. The supported lazy date formats are:
  -[M]M-[d]d
  -[M]M=[d]d [H]H:[m]m:[s]s[.S]
  -[M]M=[d]dT[H]H:[m]m:[s]s[.S]

Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
---
M be/src/exprs/expr-test.cc
M be/src/runtime/timestamp-parse-util.cc
M be/src/runtime/timestamp-parse-util.h
3 files changed, 95 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/7009/5
-- 
To view, visit http://gerrit.cloudera.org:8080/7009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-5315 Cast to timestamp fails for YYYY-M-D format

2017-06-17 Thread Vincent Tran (Code Review)
Hello Lars Volker,

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

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

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

Change subject: IMPALA-5315 Cast to timestamp fails for -M-D format
..

IMPALA-5315 Cast to timestamp fails for -M-D format

This change allows casting of a string in 'lazy' date
format to timestamp. The supported lazy date formats are:
  -[M]M-[d]d
  -[M]M=[d]d [H]H:[m]m:[s]s[.S]
  -[M]M=[d]dT[H]H:[m]m:[s]s[.S]

Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
---
M be/src/exprs/expr-test.cc
M be/src/runtime/timestamp-parse-util.cc
M be/src/runtime/timestamp-parse-util.h
3 files changed, 95 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-5315 Cast to timestamp fails for YYYY-M-D format

2017-06-17 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5315 Cast to timestamp fails for -M-D format
..


Patch Set 3:

(1 comment)

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

Line 2216:   TimestampValue::Parse("2001-01-21 00:00:00"));
> Sorry for the delayed response, I missed this question.
Generating it on-the-fly is an excellent idea. Done.
I've also added a few more cases so that it's inline with the current default 
templates.
-[M]M-[d]d[T][H]H:[m]m:[s]s[.S]


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5494: Fixes the selectivity of NOT IN predicates

2017-06-16 Thread Vincent Tran (Code Review)
Hello Impala Public Jenkins, Alex Behm,

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

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

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

Change subject: IMPALA-5494: Fixes the selectivity of NOT IN predicates
..

IMPALA-5494: Fixes the selectivity of NOT IN predicates

This change modifies the logic of NOT IN predicate so that
the planner can calculate the correct node cardinality. Prior
to this change, both IN and NOT IN predicates shared the
same selectivity, which resulted in the same cardinality
during planning.

The selectivity is calculated by the following heuristic:

selectivity = 1 - (num of predicate children /
num of distinct values)

Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
---
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
A fe/src/test/java/org/apache/impala/analysis/ExprSelectivityTest.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
3 files changed, 73 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/7168/8
-- 
To view, visit http://gerrit.cloudera.org:8080/7168
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-5494: Fixes the selectivity of NOT IN predicates

2017-06-16 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5494: Fixes the selectivity of NOT IN predicates
..


Patch Set 7:

One query plan in kudu-selectivity.test had a NOT IN predicate. The actual scan 
node cardinality was changed from 4 to 5. The order of predicates was also 
changed (NOT IN has the highest cardinality so was moved to the most right side 
of the predicate list). Private build looks good now.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4418: Fixes extra blank lines in query result

2017-06-15 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#4).

Change subject: IMPALA-4418: Fixes extra blank lines in query result
..

IMPALA-4418: Fixes extra blank lines in query result

This change avoids printing blank lines when the Impala
shell fetches 0 rows from a statement.

Change-Id: I6e18ce36be07ee90a16b007b1e30d5255ef8a839
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 18 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6e18ce36be07ee90a16b007b1e30d5255ef8a839
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-4418: Extra blank lines in query result

2017-06-15 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-4418: Extra blank lines in query result
..


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7055/3//COMMIT_MSG
Commit Message:

Line 11: that returns 0 row.
> This change avoids printing blank lines when the Impala shell fetches 0 row
Done


http://gerrit.cloudera.org:8080/#/c/7055/3/shell/impala_shell.py
File shell/impala_shell.py:

Line 922:   # IMPALA-4418: Breaking out of the loop to prevent an empty 
newline printing
> Break out of the loop to prevent printing an unnecessary empty line.
Done


http://gerrit.cloudera.org:8080/#/c/7055/3/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

Line 293: # IMPALA-4418
> We use this format
Done


Line 294: # DROP and USE are exception cases that client does not fetch.
> DROP and USE are generally exceptional statements where the client does not
Done


Line 296: # CREATE [DATABASE|TABLE] should trigger a 0 row fetch
> Remove this part about CREATE in favor of the more general formulation abov
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6e18ce36be07ee90a16b007b1e30d5255ef8a839
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4418: Extra blank lines in query result

2017-06-14 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-4418: Extra blank lines in query result
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7055/2/shell/impala_shell.py
File shell/impala_shell.py:

Line 919:   if len(rows) == 0:
> Comment on why we are doing this
Done


http://gerrit.cloudera.org:8080/#/c/7055/2/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

Line 294:   # DROP and USE are exception cases that client does not fetch
> Prefix with IMPALA-4418
Done


Line 297:   run_impala_shell_interactive("drop database if exists d1;")
> Why not use a test similar to what was reported in the JIRA, i.e. issue a u
Good point. Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6e18ce36be07ee90a16b007b1e30d5255ef8a839
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5494: Fixes the selectivity of NOT IN predicates

2017-06-14 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5494: Fixes the selectivity of NOT IN predicates
..


Patch Set 6:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/7168/6/fe/src/test/java/org/apache/impala/analysis/ExprSelectivityTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprSelectivityTest.java:

Line 31: public void verifySelectivityStmt(String stmtStr, double 
expectedSel)
> we indent 2
Done


Line 33:   SelectStmt stmt = (SelectStmt) ParsesOk(stmtStr);
> simplify with AnalyzesOk()
Done


Line 38:   assertEquals(calculatedSel,expectedSel);
> space after comma
Done


Line 43:   String stmtStr = "select * from functional.alltypes where id " +
> simplify this to "select  from functional.alltypes"
Done


Line 52: private double getPredSel(int numPredChild) {
> Very specific to IN predicate. How about we hardcode the expected values fo
Done


Line 69:   verifySel("in (1,3,5,7)", getPredSel(4));
> Seems cleaner to provide the full expr as input and not assume "id". That w
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5494: Fixes the selectivity of NOT IN predicates

2017-06-13 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#6).

Change subject: IMPALA-5494: Fixes the selectivity of NOT IN predicates
..

IMPALA-5494: Fixes the selectivity of NOT IN predicates

This change modifies the logic of NOT IN predicate so that
the planner can calculate the correct node cardinality. Prior
to this change, both IN and NOT IN predicates shared the
same selectivity, which resulted in the same cardinality
during planning.

The selectivity is calculated by the following heuristic:

selectivity = 1 - (num of predicate children /
num of distinct values)

Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
---
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
A fe/src/test/java/org/apache/impala/analysis/ExprSelectivityTest.java
2 files changed, 79 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-5494: Fixes the selectivity of NOT IN predicates

2017-06-13 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#5).

Change subject: IMPALA-5494: Fixes the selectivity of NOT IN predicates
..

IMPALA-5494: Fixes the selectivity of NOT IN predicates

This change modifies the logic of NOT IN predicate so that
the planner can calculate the correct node cardinality. Prior
to this change, both IN and NOT IN predicates shared the
same selectivity, which resulted in the same cardinality
during planning.

The selectivity is calculated by the following heuristic:

selectivity = 1 - (num of predicate children /
num of distinct values)

Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
---
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
A fe/src/test/java/org/apache/impala/analysis/ExprSelectivityTest.java
2 files changed, 79 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/7168/5
-- 
To view, visit http://gerrit.cloudera.org:8080/7168
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-5494: Fixes the selectivity of NOT IN predicates

2017-06-13 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#4).

Change subject: IMPALA-5494: Fixes the selectivity of NOT IN predicates
..

IMPALA-5494: Fixes the selectivity of NOT IN predicates

This change modifies the logic of NOT IN predicate so that
the planner can calculate the correct node cardinality. Prior
to this change, both IN and NOT IN predicates shared the
same selectivity, which resulted in the same cardinality
during planning.

The selectivity is calculated by the following heuristic:

selectivity = 1 - (num of predicate children /
num of distinct values)

Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
---
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
1 file changed, 7 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-5494: Fixes the selectivity of NOT IN predicates

2017-06-13 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5494: Fixes the selectivity of NOT IN predicates
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7168/3/tests/metadata/test_explain.py
File tests/metadata/test_explain.py:

Line 105: result = self.execute_query("explain select * from %s.%s where id 
"
> Instead of this end-to-end test, please add a new FE unit test ExprSelectiv
That makes sense. Done.


Line 105: result = self.execute_query("explain select * from %s.%s where id 
"
> Agree, e-e test seems like an overkill.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5494: Fixes the selectivity of NOT IN predicates

2017-06-13 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5494: Fixes the selectivity of NOT IN predicates
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7168/2//COMMIT_MSG
Commit Message:

PS2, Line 7: NOT IN predicate shares the same selectivity as
   : IN predicate.
> Change to what this commit fixes may be? Something like Fix the selectivity
Done


PS2, Line 18: predicae
> typo
Done


http://gerrit.cloudera.org:8080/#/c/7168/2/tests/metadata/test_explain.py
File tests/metadata/test_explain.py:

Line 104: # IN predicate, cardinality should be 7300*(num of children/num 
of distinct values)
> Not totally sure if there is a better place to add these tests, Alex any id
Maybe I should just specify the expected value as a constant?


Line 109: # NOT IN predicate, cardinality should be 7300*(1-(num of 
children/num of distinct values))
> nit: long line.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5494: Fixes the selectivity of NOT IN predicates

2017-06-13 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#3).

Change subject: IMPALA-5494: Fixes the selectivity of NOT IN predicates
..

IMPALA-5494: Fixes the selectivity of NOT IN predicates

This change modifies the logic of NOT IN predicate so that
the planner can calculate the correct node cardinality. Prior
to this change, both IN and NOT IN predicates shared the
same selectivity, which resulted in the same cardinality
during planning.

The selectivity is calculated by the following heuristic:

selectivity = 1 - (num of predicate children /
num of distinct values)

Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
---
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
M tests/metadata/test_explain.py
2 files changed, 18 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-5494: NOT IN predicate shares the same selectivity as IN predicate.

2017-06-13 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5494: NOT IN predicate shares the same selectivity as IN 
predicate.
..


Patch Set 2:

removed extraneous space characters.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5494: NOT IN predicate shares the same selectivity as IN predicate.

2017-06-13 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#2).

Change subject: IMPALA-5494: NOT IN predicate shares the same selectivity as IN 
predicate.
..

IMPALA-5494: NOT IN predicate shares the same selectivity as
IN predicate.

This change modifies the logic of NOT IN predicate so that
the planner can calculate the correct node cardinality. Prior
to this change, both IN and NOT IN predicates shared the
same selectivity, which resulted in the same cardinality
during planning.

The selectivity is calculated by the following heuristic:

selectivity = 1 - (num of predicae children /
num of distinct values)

Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
---
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
M tests/metadata/test_explain.py
2 files changed, 17 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 


[Impala-ASF-CR] IMPALA-5494: NOT IN predicate shares the same selectivity as IN predicate.

2017-06-13 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new change for review.

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

Change subject: IMPALA-5494: NOT IN predicate shares the same selectivity as IN 
predicate.
..

IMPALA-5494: NOT IN predicate shares the same selectivity as
IN predicate.

This change modifies the logic of NOT IN predicate so that
the planner can calculate the correct node cardinality. Prior
to this change, both IN and NOT IN predicates shared the
same selectivity, which resulted in the same cardinality
during planning.

The selectivity is calculated by the following heuristic:

selectivity = 1 - (num of predicae children /
num of distinct values)

Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
---
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
M tests/metadata/test_explain.py
2 files changed, 17 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I69e6217257b5618cb63e13b32ba3347fa0483b63
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 


[Impala-ASF-CR] IMPALA-5315 Cast to timestamp fails for YYYY-M-D format

2017-06-08 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5315 Cast to timestamp fails for -M-D format
..


Patch Set 3:

(1 comment)

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

Line 2216:   TimestampValue::Parse("2001-01-21 00:00:00"));
> Please see the new example that Greg added to the JIRA.
So looks like both of these formats should be supported:

-M[M]-d[d] 
-M[M]-d[d] H[H]:m[m]:s[s]

The simplest solution is to stamp them all out with format templates.

Is this what you have in mind?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5315 Cast to timestamp fails for YYYY-M-D format

2017-06-06 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#3).

Change subject: IMPALA-5315 Cast to timestamp fails for -M-D format
..

IMPALA-5315 Cast to timestamp fails for -M-D format

This change allows casting of a string in 'lazy' date
format to timestamp. The supported lazy date formats are:
  -M-D
  -MM-D
  -M-DD

Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
---
M be/src/exprs/expr-test.cc
M be/src/runtime/timestamp-parse-util.cc
M be/src/runtime/timestamp-parse-util.h
3 files changed, 43 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-5315 Cast to timestamp fails for YYYY-M-D format

2017-06-06 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5315 Cast to timestamp fails for -M-D format
..


Patch Set 3:

(2 comments)

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

Line 2210:   // Test casting of lazy day and/or month format string to timestamp
> nit: this should probably now read day, not date.
Done


http://gerrit.cloudera.org:8080/#/c/7009/2/be/src/runtime/timestamp-parse-util.cc
File be/src/runtime/timestamp-parse-util.cc:

PS2, Line 260: DEFAULT_DATE_FMT_LEN - 2:
> Maybe it would be clearer to replace these with DEFAULT_DATE_FMT_LEN-1 and 
That makes sense. Done.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5316: Adds last day() function

2017-06-06 Thread Vincent Tran (Code Review)
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 -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 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-5316: Adds last day() function

2017-06-06 Thread Vincent Tran (Code Review)
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 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5315 Cast to timestamp fails for YYYY-M-D format

2017-06-06 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#2).

Change subject: IMPALA-5315 Cast to timestamp fails for -M-D format
..

IMPALA-5315 Cast to timestamp fails for -M-D format

This change allows casting of a string in 'lazy' date
format to timestamp. The supported lazy date formats are:
  -M-D
  -MM-D
  -M-DD

Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
---
M be/src/exprs/expr-test.cc
M be/src/runtime/timestamp-parse-util.cc
M be/src/runtime/timestamp-parse-util.h
3 files changed, 45 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-5315 Cast to timestamp fails for YYYY-M-D format

2017-06-06 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5315 Cast to timestamp fails for -M-D format
..


Patch Set 1:

(4 comments)

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

Line 2216:   TimestampValue::Parse("2001-01-21 00:00:00"));
> Should this also work for lazy years? If not, please add a test to make sur
Done


Line 2217: 
> What about timestamps with a time, like "2012-1-1 09:10:11"? Should we pars
You're right as that will likely require a rewrite of the parser to handle lazy 
date and time more intelligently. Though, I wasn't planning to do that here.

Added negative tests to make sure all of these cases don't parse.


http://gerrit.cloudera.org:8080/#/c/7009/1/be/src/runtime/timestamp-parse-util.h
File be/src/runtime/timestamp-parse-util.h:

Line 217:   static const int DEFAULT_LAZY_SINGLE_DATE_FMT_LEN = 8;
> single and double are often used for floating point precision. Can you thin
Done


Line 233:   static DateTimeFormatContext DEFAULT_LAZY_SINGLE_DATE_CTX;
> same here
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4418: Extra blank lines in query result

2017-06-01 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#2).

Change subject: IMPALA-4418: Extra blank lines in query result
..

IMPALA-4418: Extra blank lines in query result

This change will remove the blank lines resulting
from the impala-shell client fetching on statements
that returns 0 row.

Change-Id: I6e18ce36be07ee90a16b007b1e30d5255ef8a839
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 21 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6e18ce36be07ee90a16b007b1e30d5255ef8a839
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 


[Impala-ASF-CR] IMPALA-4418: Extra blank lines in query result

2017-06-01 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new change for review.

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

Change subject: IMPALA-4418: Extra blank lines in query result
..

IMPALA-4418: Extra blank lines in query result

This change will remove the blank lines resulting
from the impala-shell client fetching on statements
that returns 0 row.

Change-Id: I6e18ce36be07ee90a16b007b1e30d5255ef8a839
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 22 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6e18ce36be07ee90a16b007b1e30d5255ef8a839
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 


[Impala-ASF-CR] IMPALA-5030: [TESTS] Adds support for NVL2() function

2017-06-01 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#5).

Change subject: IMPALA-5030: [TESTS] Adds support for NVL2() function
..

IMPALA-5030: [TESTS] Adds support for NVL2() function

This change adds value tests to expr-test.cc to ensure
that NVL2() function is rewritten correctly to IF().

Change-Id: Ide0352d611322de637a61a0004c39f5663192c52
---
M be/src/exprs/expr-test.cc
1 file changed, 18 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/7000/5
-- 
To view, visit http://gerrit.cloudera.org:8080/7000
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide0352d611322de637a61a0004c39f5663192c52
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-5030: Adds support for NVL2() function

2017-06-01 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5030: Adds support for NVL2() function
..


Patch Set 3:

(2 comments)

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

PS3, Line 5643: TimestampValue
> Doesn't compile - duplicate declaration of then_val/else_val about.
oops. 
Done


PS3, Line 5644: TRUE
> Maybe make this FALSE? Just for some variety.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide0352d611322de637a61a0004c39f5663192c52
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5030: [TESTS] Adds support for NVL2() function

2017-06-01 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#4).

Change subject: IMPALA-5030: [TESTS] Adds support for NVL2() function
..

IMPALA-5030: [TESTS] Adds support for NVL2() function

This change adds value tests to expr-test.cc to ensure
that NVL2() function is rewritten correctly to IF().

Change-Id: Ide0352d611322de637a61a0004c39f5663192c52
---
M be/src/exprs/expr-test.cc
M shell/impala_shell.py
2 files changed, 20 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide0352d611322de637a61a0004c39f5663192c52
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-5315 Cast to timestamp fails for YYYY-M-D format

2017-05-27 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new change for review.

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

Change subject: IMPALA-5315 Cast to timestamp fails for -M-D format
..

IMPALA-5315 Cast to timestamp fails for -M-D format

This change allows casting of a string in 'lazy' date
format to timestamp. The supported lazy date formats are:
  -M-D
  -MM-D
  -M-DD

Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
---
M be/src/exprs/expr-test.cc
M be/src/runtime/timestamp-parse-util.cc
M be/src/runtime/timestamp-parse-util.h
3 files changed, 40 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 


[Impala-ASF-CR] IMPALA-5316: Adds last day() function

2017-05-26 Thread Vincent Tran (Code Review)
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 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5316: Adds last day() function

2017-05-26 Thread Vincent Tran (Code Review)
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 -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 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-5030: Adds support for NVL2() function

2017-05-26 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#3).

Change subject: IMPALA-5030: Adds support for NVL2() function
..

IMPALA-5030: Adds support for NVL2() function

This change adds value tests to expr-test.cc to ensure
that NVL2() function is rewritten correctly to IF().

Change-Id: Ide0352d611322de637a61a0004c39f5663192c52
---
M be/src/exprs/expr-test.cc
1 file changed, 18 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide0352d611322de637a61a0004c39f5663192c52
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 


[Impala-ASF-CR] IMPALA-5030: Adds support for NVL2() function

2017-05-26 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#2).

Change subject: IMPALA-5030: Adds support for NVL2() function
..

IMPALA-5030: Adds support for NVL2() function

This change adds value and null tests to expr-test.cc for
IMPALA-5030.

Change-Id: Ide0352d611322de637a61a0004c39f5663192c52
---
M be/src/exprs/expr-test.cc
1 file changed, 18 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide0352d611322de637a61a0004c39f5663192c52
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 


[Impala-ASF-CR] IMPALA-5030: Adds support for NVL2() function

2017-05-26 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new change for review.

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

Change subject: IMPALA-5030: Adds support for NVL2() function
..

IMPALA-5030: Adds support for NVL2() function

This change adds support to the function NVL2(expr1, expr2, expr3).
This function returns expr2 if expr1 is not null, else it returns
expr3. NVL2() is converted to IF() prior to analysis.

Added value and null tests to expr-test.cc

Change-Id: Ide0352d611322de637a61a0004c39f5663192c52
---
M be/src/exprs/expr-test.cc
1 file changed, 18 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ide0352d611322de637a61a0004c39f5663192c52
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 


[Impala-ASF-CR] IMPALA-5316: Adds last day() function

2017-05-26 Thread Vincent Tran (Code Review)
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 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-5316: Adds last day() function

2017-05-25 Thread Vincent Tran (Code Review)
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 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5030: Adds support for NVL2() function

2017-05-25 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5030: Adds support for NVL2() function
..


Patch Set 5:

I can make that happen. Can I just push the patch again as normal?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32b03e9864f46c9c5e482280d1aa676ff7f02644
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5316: Adds last day() function

2017-05-25 Thread Vincent Tran (Code Review)
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 


[Impala-ASF-CR] IMPALA-5316: Adds last day() function

2017-05-25 Thread Vincent Tran (Code Review)
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 


[Impala-ASF-CR] IMPALA-5030: Adds support for NVL2() function

2017-05-24 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#4).

Change subject: IMPALA-5030: Adds support for NVL2() function
..

IMPALA-5030: Adds support for NVL2() function

This change adds support to the function NVL2(expr1, expr2, expr3).
This function returns expr2 if expr1 is not null, else it returns
expr3. NVL2() is converted to IF() prior to analysis.

Change-Id: I32b03e9864f46c9c5e482280d1aa676ff7f02644
---
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
2 files changed, 29 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32b03e9864f46c9c5e482280d1aa676ff7f02644
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-5030: Adds support for NVL2() function

2017-05-24 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5030: Adds support for NVL2() function
..


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6962/3/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java:

PS3, Line 100: params.exprs().get(0)
> Will this work if the user simply calls nvl2() ? Should this be part of the
Done


Line 100:   plist.add(new IsNullPredicate(params.exprs().get(0), true));
> What if NVL2() is called without params? How about doing:
Done


PS3, Line 106:}
> nit: Wrong indentation.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32b03e9864f46c9c5e482280d1aa676ff7f02644
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5030: Adds support for NVL2() function

2017-05-24 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new patch set (#3).

Change subject: IMPALA-5030: Adds support for NVL2() function
..

IMPALA-5030: Adds support for NVL2() function

This change adds support to the function NVL2(expr1, expr2, expr3).
This function returns expr2 if expr1 is not null, else it returns
expr3. NVL2() is converted to IF() prior to analysis.

Change-Id: I32b03e9864f46c9c5e482280d1aa676ff7f02644
---
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
2 files changed, 30 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32b03e9864f46c9c5e482280d1aa676ff7f02644
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Vincent Tran 


[Impala-ASF-CR] IMPALA-5030: Adds support for NVL2() function

2017-05-24 Thread Vincent Tran (Code Review)
Vincent Tran has posted comments on this change.

Change subject: IMPALA-5030: Adds support for NVL2() function
..


Patch Set 2:

(16 comments)

http://gerrit.cloudera.org:8080/#/c/6962/1//COMMIT_MSG
Commit Message:

Line 17
> Clean up commit msg
Done


http://gerrit.cloudera.org:8080/#/c/6962/2//COMMIT_MSG
Commit Message:

Line 11: expr3.
> Briefly mention that NVL2() is converted to IF before analysis.
Done


http://gerrit.cloudera.org:8080/#/c/6962/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

Line 329:   // Binary predicates must be rewritten to a canonical form for 
both Kudu predicate
> Please look the places that reference BetweenToCompoundRule.INSTANCE. There
Abandoned this approach in favor of IF FunctionCallExpr


http://gerrit.cloudera.org:8080/#/c/6962/1/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java:

Line 85:   public static Expr createExpr(FunctionName fnName, FunctionParams 
params) {
> The current approach looks too complicated. I think we should directly tran
Done


http://gerrit.cloudera.org:8080/#/c/6962/2/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java:

Line 94: if (fnName.getFnNamePath().get(0).equalsIgnoreCase("nvl2")) {
> Please follow the same checks as above for "decode". You could have:
Done


Line 95:   Preconditions.checkArgument(params.size()==3, "NVL2() "
> Preconditions checks are like asserts. They are used to enforce code invari
Done


Line 97:   Preconditions.checkArgument(
> Same here, inappropriate use of Preconditions check. In addition, the match
Done


http://gerrit.cloudera.org:8080/#/c/6962/1/fe/src/main/java/org/apache/impala/rewrite/Nvl2ToCaseRule.java
File fe/src/main/java/org/apache/impala/rewrite/Nvl2ToCaseRule.java:

Line 33
> Provide class comment with examples like in the other ExprRewriteRules
Done


Line 39
> negate this condition and inline the Nvl2RewriteFunction for brevity
Done


Line 53
> Remove tabs.
Done


Line 55
> Why not IF? That seems shorter and more appropriate. Take a look at TupleIs
Done


http://gerrit.cloudera.org:8080/#/c/6962/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java:

Line 1605: 
> remove whitespace
Done


Line 1606: // NVL2() 
> mention that NVL2() is converted to IF before analysis
Done


Line 1607: AnalyzesOk("select nvl2(1, 'not null', 'null')");
> Add negative cases to show what error message will be shown, for example:
Done


http://gerrit.cloudera.org:8080/#/c/6962/1/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java:

Line 81: 
> Remove tabs
Done


Line 85: RewritesOk("int_col not between float_col and double_col", rule,
> Add additional tests:
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32b03e9864f46c9c5e482280d1aa676ff7f02644
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5030: Adds support for NVL2() function

2017-05-23 Thread Vincent Tran (Code Review)
Vincent Tran has abandoned this change.

Change subject: IMPALA-5030: Adds support for NVL2() function
..


Abandoned

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I5d5618293fa3bc798d2611a24709ad65e6636ad7
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 


[Impala-ASF-CR] IMPALA-5030: Adds support for NVL2() function

2017-05-23 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new change for review.

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

Change subject: IMPALA-5030: Adds support for NVL2() function
..

IMPALA-5030: Adds support for NVL2() function

This change adds support to the function NVL2(expr1, expr2, expr3).
This function returns expr2 if expr1 is not null, else it returns
expr3.

Change-Id: I5d5618293fa3bc798d2611a24709ad65e6636ad7
---
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
2 files changed, 24 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5d5618293fa3bc798d2611a24709ad65e6636ad7
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 


[Impala-ASF-CR] IMPALA-5030: Add support to NVL2() function

2017-05-23 Thread Vincent Tran (Code Review)
Vincent Tran has uploaded a new change for review.

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

Change subject: IMPALA-5030: Add support to NVL2() function
..

IMPALA-5030: Add support to NVL2() function

This change adds support to the NVL2(x,t,t) function.
It returns the second argument if the first argument is not
null, otherwise it returns the third argument. NVL2 is short
hand for the case expression “case when x is not null then y
else z end".

Change-Id: I472d973c28a00376a7c7f2bd153ddca9bb89fc70

IMPALA-5030 - Add NVL2() Function

This patch adds support for the the NVL2() function
It returns the second argument if the first argument is not
null, otherwise it returns the third argument. NVL2 is
shorthand for the case expression “case when x is not
null then y else z end.”

Change-Id: I32b03e9864f46c9c5e482280d1aa676ff7f02644
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
A fe/src/main/java/org/apache/impala/rewrite/Nvl2ToCaseRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
4 files changed, 103 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I32b03e9864f46c9c5e482280d1aa676ff7f02644
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran