[Impala-ASF-CR] IMPALA-6230, IMPALA-6468: Fix the output type of round() and related fns

2018-03-23 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9346 )

Change subject: IMPALA-6230, IMPALA-6468: Fix the output type of round() and 
related fns
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77541678012edab70b182378b11ca8753be53f97
Gerrit-Change-Number: 9346
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Sat, 24 Mar 2018 04:43:00 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6230, IMPALA-6468: Fix the output type of round() and related fns

2018-03-23 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9346 )

Change subject: IMPALA-6230, IMPALA-6468: Fix the output type of round() and 
related fns
..

IMPALA-6230, IMPALA-6468: Fix the output type of round() and related fns

Before this patch, the output type of round() ceil() floor() trunc() was
not always the same as the input type. It was also inconsistent in
general. For example, round(double) returned an integer, but
round(double, int) returned a double.

After looking at other database systems, we decided that the guideline
should be that the output type should be the same as the input type. In
this patch, we change the behavior of the previously mentioned functions
so that if a double is given then a double is returned.

We also modify the rounding behavior to always round away from zero.
Before, we were rounding towards positive infinity in some cases.

Testinging:
- Updated tests
- Ran an exhaustive build which passed.

Cherry-picks: not for 2.x

Change-Id: I77541678012edab70b182378b11ca8753be53f97
Reviewed-on: http://gerrit.cloudera.org:8080/9346
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M be/src/exprs/expr-test.cc
M be/src/exprs/math-functions-ir.cc
M be/src/exprs/math-functions.h
M be/src/exprs/scalar-fn-call.cc
M common/function-registry/impala_functions.py
M testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
7 files changed, 110 insertions(+), 68 deletions(-)

Approvals:
  Alex Behm: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I77541678012edab70b182378b11ca8753be53f97
Gerrit-Change-Number: 9346
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-6670: refresh lib-cache entries from plan

2018-03-23 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9697 )

Change subject: IMPALA-6670: refresh lib-cache entries from plan
..

IMPALA-6670: refresh lib-cache entries from plan

When an impalad is in executor-only mode, it receives no
catalog updates. As a result, lib-cache entries are never
refreshed. A consequence is that udf queries can return
incorrect results or may not run due to resolution issues.
Both cases are caused by the executor using a stale copy
of the lib file. For incorrect results, an old version of
the method may be used. Resolution issues can come up if
a method is added to a lib file.

The solution in this change is to capture the coordinator's
view of the lib file's last modified time when planning.
This last modified time is then shipped with the plan to
executors. Executors must then use both the lib file path
and the last modified time as a key for the lib-cache.
If the coordinator's last modified time is more recent than
the executor's lib-cache entry, then the entry is refreshed.

Brief discussion of alternatives:

- lib-cache always checks last modified time
  + easy/local change to lib-cache
  - adds an fs lookup always. rejected for this reason

- keep the last modified time in the catalog
  - bound on staleness is too loose. consider the case where
fn's f1, f2, f3 are created with last modified times of
t1, t2, t3. treat the fn's last modified time as a low-watermark;
if the cache entry has a more recent time, use it. Such a scheme
would allow the version at t2 to persist. An old fn may keep the
state from converging to the latest. This could end up with strange
cases where different versions of the lib are used across executors
for a single query.

In contrast, the change in this path relies on the statestore to
push versions forward at all coordinators, so will push all
versions at all caches forward as well.

Testing:
- added an e2e custom cluster test

Change-Id: Icf740ea8c6a47e671427d30b4d139cb8507b7ff6
Reviewed-on: http://gerrit.cloudera.org:8080/9697
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M be/src/codegen/codegen-callgraph.cc
M be/src/codegen/llvm-codegen-test.cc
M be/src/codegen/llvm-codegen.cc
M be/src/codegen/llvm-codegen.h
M be/src/exec/external-data-source-executor.cc
M be/src/exprs/agg-fn.cc
M be/src/exprs/hive-udf-call.cc
M be/src/exprs/scalar-fn-call.cc
M be/src/runtime/lib-cache.cc
M be/src/runtime/lib-cache.h
M be/src/service/fe-support.cc
M common/thrift/Frontend.thrift
M common/thrift/Types.thrift
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/catalog/AggregateFunction.java
M fe/src/main/java/org/apache/impala/catalog/Function.java
M fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M testdata/bin/copy-udfs-udas.sh
M tests/custom_cluster/test_coordinators.py
M tests/test-hive-udfs/src/main/java/org/apache/impala/TestUpdateUdf.java
22 files changed, 440 insertions(+), 83 deletions(-)

Approvals:
  Alex Behm: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icf740ea8c6a47e671427d30b4d139cb8507b7ff6
Gerrit-Change-Number: 9697
Gerrit-PatchSet: 23
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zach Amsden

[Impala-ASF-CR] IMPALA-6670: refresh lib-cache entries from plan

2018-03-23 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9697 )

Change subject: IMPALA-6670: refresh lib-cache entries from plan
..


Patch Set 22: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf740ea8c6a47e671427d30b4d139cb8507b7ff6
Gerrit-Change-Number: 9697
Gerrit-PatchSet: 22
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Sat, 24 Mar 2018 04:38:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6647: Add CREATE fine-grained privilege

2018-03-23 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has uploaded a new patch set (#18). ( 
http://gerrit.cloudera.org:8080/9738 )

Change subject: IMPALA-6647: Add CREATE fine-grained privilege
..

IMPALA-6647: Add CREATE fine-grained privilege

This patch allows executing CREATE statements by granting CREATE
privilege.

These are the new GRANT/REVOKE statements introduced at server and
database scopes.

GRANT CREATE on SERVER svr TO ROLE testrole;
GRANT CREATE on DATABASE db TO ROLE testrole;

REVOKE CREATE on SERVER svr FROM ROLE testrole;
REVOKE CREATE on DATABASE db FROM ROLE testrole;

Testing:
- Ran front-end tests

Cherry-picks: not for 2.x

Change-Id: Id540e78fc9201fc1b4e6cac9b81ea54b8ae9eecd
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizeableFn.java
M fe/src/main/java/org/apache/impala/authorization/Privilege.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/resources/authz-policy.ini.template
13 files changed, 176 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/9738/18
--
To view, visit http://gerrit.cloudera.org:8080/9738
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id540e78fc9201fc1b4e6cac9b81ea54b8ae9eecd
Gerrit-Change-Number: 9738
Gerrit-PatchSet: 18
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya

[Impala-ASF-CR] IMPALA-6230, IMPALA-6468: Fix the output type of round() and related fns

2018-03-23 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9346 )

Change subject: IMPALA-6230, IMPALA-6468: Fix the output type of round() and 
related fns
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77541678012edab70b182378b11ca8753be53f97
Gerrit-Change-Number: 9346
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Sat, 24 Mar 2018 00:56:54 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6230, IMPALA-6468: Fix the output type of round() and related fns

2018-03-23 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9346 )

Change subject: IMPALA-6230, IMPALA-6468: Fix the output type of round() and 
related fns
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77541678012edab70b182378b11ca8753be53f97
Gerrit-Change-Number: 9346
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Sat, 24 Mar 2018 00:55:03 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6670: refresh lib-cache entries from plan

2018-03-23 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9697 )

Change subject: IMPALA-6670: refresh lib-cache entries from plan
..


Patch Set 22: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf740ea8c6a47e671427d30b4d139cb8507b7ff6
Gerrit-Change-Number: 9697
Gerrit-PatchSet: 22
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Sat, 24 Mar 2018 00:50:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6670: refresh lib-cache entries from plan

2018-03-23 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9697 )

Change subject: IMPALA-6670: refresh lib-cache entries from plan
..


Patch Set 22:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf740ea8c6a47e671427d30b4d139cb8507b7ff6
Gerrit-Change-Number: 9697
Gerrit-PatchSet: 22
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Sat, 24 Mar 2018 00:49:54 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6670: refresh lib-cache entries from plan

2018-03-23 Thread Alex Behm (Code Review) 
Alex Behm has removed a vote on this change.

Change subject: IMPALA-6670: refresh lib-cache entries from plan
..


Removed Code-Review-2 by Alex Behm 
--
To view, visit http://gerrit.cloudera.org:8080/9697
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Icf740ea8c6a47e671427d30b4d139cb8507b7ff6
Gerrit-Change-Number: 9697
Gerrit-PatchSet: 21
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zach Amsden

[Impala-ASF-CR] IMPALA-6230, IMPALA-6468: Fix the output type of round() and related fns

2018-03-23 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9346 )

Change subject: IMPALA-6230, IMPALA-6468: Fix the output type of round() and 
related fns
..


Patch Set 4: Code-Review+1

I'm happy with this change. I think a BE person should +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77541678012edab70b182378b11ca8753be53f97
Gerrit-Change-Number: 9346
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Sat, 24 Mar 2018 00:47:18 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6694: fix "buffer pool" child profile order

2018-03-23 Thread Tim Armstrong (Code Review) 
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9749 )

Change subject: IMPALA-6694: fix "buffer pool" child profile order
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9749/1/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:

http://gerrit.cloudera.org:8080/#/c/9749/1/be/src/util/runtime-profile.cc@359
PS1, Line 359: while (insert_pos != children_.end() && !found_child) {
 :   found_child = insert_pos->first == child;
 :   ++insert_pos;
 : }
> Is it fair to expect that children_ here should end up having the same numb
It should have node.num_children entries unless the source profile had some 
entries removed (which isn't an operation this is allowed).

I don't think that invariant is generally true. If the relative order of the 
child profiles is changed (e.g. by calling SortChildren()) then it can be 
violated

If first we have children A, B, C in that order in the initial update and then 
the order is changed, it's violated.

  Update([A, B, C]) -> children_ = [A, B, C]

  Update(B, A, D, C) -> children_ = [A, B, C, D]

That happens because A and B are out of order, and the searching forward skips 
over A when looking for B then skips over C when looking for A, which means 
that D is inserted at i = 3 instead of i=2.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I230f0673edf20a846fdb13191b7a292d329c1bb8
Gerrit-Change-Number: 9749
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 24 Mar 2018 00:41:35 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6694: fix "buffer pool" child profile order

2018-03-23 Thread Michael Ho (Code Review) 
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9749 )

Change subject: IMPALA-6694: fix "buffer pool" child profile order
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9749/1/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:

http://gerrit.cloudera.org:8080/#/c/9749/1/be/src/util/runtime-profile.cc@359
PS1, Line 359: while (insert_pos != children_.end() && !found_child) {
 :   found_child = insert_pos->first == child;
 :   ++insert_pos;
 : }
Is it fair to expect that children_ here should end up having the same number 
of entries as nodes[] after this loop is over ?

If so, for entries in nodes[] which don't already exist in child_map_, should 
we expect the insertion position be "i" in this case ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I230f0673edf20a846fdb13191b7a292d329c1bb8
Gerrit-Change-Number: 9749
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 23 Mar 2018 23:47:18 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6641: Support more separators between date and time in default timestamp format

2018-03-23 Thread Vincent Tran (Code Review) 
Hello Tim Armstrong,

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

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

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

Change subject: IMPALA-6641: Support more separators between date and time in 
default timestamp format
..

IMPALA-6641: Support more separators between date and time in default
timestamp format

This change adds support to the multi-space separator and the
'T' separator between the date and time component of a datetime
string during a cast (x as timestamp).

Testing:
Added valid and invalid tests to expr-test.cc to validate
the functionality during a cast.

Change-Id: Id2ce3ba09256b3996170e42d42d49d12776cab97
---
M be/src/exprs/expr-test.cc
M be/src/runtime/timestamp-parse-util.cc
2 files changed, 53 insertions(+), 16 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-6641: Support more separators between date and time in default timestamp format

2018-03-23 Thread Vincent Tran (Code Review) 
Vincent Tran has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9725 )

Change subject: IMPALA-6641: Support more separators between date and time in 
default timestamp format
..


Patch Set 2:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9725/2/be/src/runtime/timestamp-parse-util.cc@422
PS2, Line 422: break;
> Break needs to go outside if, otherwise we'll fall through to the next case
Done


http://gerrit.cloudera.org:8080/#/c/9725/2/be/src/runtime/timestamp-parse-util.cc@435
PS2, Line 435: break;
> Same here.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2ce3ba09256b3996170e42d42d49d12776cab97
Gerrit-Change-Number: 9725
Gerrit-PatchSet: 2
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran 
Gerrit-Comment-Date: Fri, 23 Mar 2018 23:41:11 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6230, IMPALA-6468: Fix the output type of round() and related fns

2018-03-23 Thread Taras Bobrovytsky (Code Review) 
Taras Bobrovytsky has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/9346 )

Change subject: IMPALA-6230, IMPALA-6468: Fix the output type of round() and 
related fns
..

IMPALA-6230, IMPALA-6468: Fix the output type of round() and related fns

Before this patch, the output type of round() ceil() floor() trunc() was
not always the same as the input type. It was also inconsistent in
general. For example, round(double) returned an integer, but
round(double, int) returned a double.

After looking at other database systems, we decided that the guideline
should be that the output type should be the same as the input type. In
this patch, we change the behavior of the previously mentioned functions
so that if a double is given then a double is returned.

We also modify the rounding behavior to always round away from zero.
Before, we were rounding towards positive infinity in some cases.

Testinging:
- Updated tests
- Ran an exhaustive build which passed.

Cherry-picks: not for 2.x

Change-Id: I77541678012edab70b182378b11ca8753be53f97
---
M be/src/exprs/expr-test.cc
M be/src/exprs/math-functions-ir.cc
M be/src/exprs/math-functions.h
M be/src/exprs/scalar-fn-call.cc
M common/function-registry/impala_functions.py
M testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
7 files changed, 110 insertions(+), 68 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I77541678012edab70b182378b11ca8753be53f97
Gerrit-Change-Number: 9346
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-6715: fix stress TPC workload selection

2018-03-23 Thread Michael Brown (Code Review) 
Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9758 )

Change subject: IMPALA-6715: fix stress TPC workload selection
..


Patch Set 1:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/9758/1//COMMIT_MSG@9
PS1, Line 9: A recent commit caused the stress test to double-count queries. 
This
> By this comment, I'm taking that you mean the addition of the tpcds-decimal
Done


http://gerrit.cloudera.org:8080/#/c/9758/1/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

http://gerrit.cloudera.org:8080/#/c/9758/1/tests/stress/concurrent_select.py@91
PS1, Line 91: 71
> After our conversation yesterday, I understood this to be the case. Should
Imo it just falls under the umbrella of IMPALA-3947.


http://gerrit.cloudera.org:8080/#/c/9758/1/tests/stress/concurrent_select.py@1076
PS1, Line 1076:   file_name_pattern = 
re.compile(r"^{0}-(q.*).test$".format(workload))
> Can you add a comment explaining why this is necessary -- e.g., that there
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3e26b64d38aa8d63a176daf95c4ac5dee89508da
Gerrit-Change-Number: 9758
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 23 Mar 2018 22:37:18 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6230, IMPALA-6468: Fix the output type of round() and related fns

2018-03-23 Thread Taras Bobrovytsky (Code Review) 
Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9346 )

Change subject: IMPALA-6230, IMPALA-6468: Fix the output type of round() and 
related fns
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9346/2/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/9346/2/be/src/exprs/math-functions-ir.cc@152
PS2, Line 152:   // TODO: should we support non-constant seed?
> I think this will DCHECK if a negative scale is passed
oops, fixed.


http://gerrit.cloudera.org:8080/#/c/9346/1/common/function-registry/impala_functions.py
File common/function-registry/impala_functions.py:

http://gerrit.cloudera.org:8080/#/c/9346/1/common/function-registry/impala_functions.py@266
PS1, Line 266:   [['sign'], 'DOUBLE', ['DOUBLE'], 
'impala::MathFunctions::Sign'],
> I think this is a question for Greg. It's mostly about whether this functio
Agreed.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77541678012edab70b182378b11ca8753be53f97
Gerrit-Change-Number: 9346
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 23 Mar 2018 22:37:20 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6715: fix stress TPC workload selection

2018-03-23 Thread Michael Brown (Code Review) 
Hello Nithya Janarthanan, David Knupp, Tim Armstrong,

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

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

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

Change subject: IMPALA-6715: fix stress TPC workload selection
..

IMPALA-6715: fix stress TPC workload selection

This commit
  IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL
added additional decimal_v2 queries to the stress test that amount to
running the same query twice. This makes the binary search run
incredibly slow.

- Fix the query selection. Add additional queries that weren't matching
  before, like the tpcds-q[0-9]+a.test series.

- Add a test that will at least ensure if
  testdata/workloads/tpc*/queries is modified, the stress test will
  still find the same number of queries for the given workload. There's
  no obvious place to put this test: it's not testing the product at
  all, so:

- Add a new directory tests/infra for such tests and add it to
  tests/run-tests.py.

- Move the test from IMPALA-6441 into tests/infra.

Testing:
- Core private build passed. I manually looked to make sure the moved
  and new tests ran.

- Short stress test run. I checked the runtime info and saw the new
  TPCDS queries in the JSON.

Change-Id: I3e26b64d38aa8d63a176daf95c4ac5dee89508da
---
A tests/infra/test_stress_infra.py
M tests/metadata/test_explain.py
M tests/run-tests.py
M tests/stress/concurrent_select.py
4 files changed, 78 insertions(+), 24 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3e26b64d38aa8d63a176daf95c4ac5dee89508da
Gerrit-Change-Number: 9758
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5842: Write page index in Parquet files

2018-03-23 Thread Zoltan Borok-Nagy (Code Review) 
Hello Lars Volker, Tim Armstrong,

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

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

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

Change subject: IMPALA-5842: Write page index in Parquet files
..

IMPALA-5842: Write page index in Parquet files

This commit builds on the previous work of
Pooja Nilangekar: https://gerrit.cloudera.org/#/c/7464/

The commit implements the write path of PARQUET-922:
"Add column indexes to parquet.thrift". As specified in the
parquet-format, Impala writes the page indexes just before
the footer. This allows much more efficient page filtering
than using the same information from the 'statistics' field
of DataPageHeader.

I updated Pooja's python tests as well.

Change-Id: Icbacf7fe3b7672e3ce719261ecef445b16f8dec9
---
M be/src/exec/hdfs-parquet-table-writer.cc
M be/src/exec/hdfs-parquet-table-writer.h
M be/src/exec/parquet-column-stats.h
M be/src/exec/parquet-column-stats.inline.h
M common/thrift/parquet.thrift
A tests/query_test/test_parquet_page_index.py
M tests/util/get_parquet_metadata.py
7 files changed, 546 insertions(+), 12 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icbacf7fe3b7672e3ce719261ecef445b16f8dec9
Gerrit-Change-Number: 9693
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy

[Impala-ASF-CR] IMPALA-5842: Write page index in Parquet files

2018-03-23 Thread Zoltan Borok-Nagy (Code Review) 
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9693 )

Change subject: IMPALA-5842: Write page index in Parquet files
..


Patch Set 2:

(20 comments)

http://gerrit.cloudera.org:8080/#/c/9693/2/be/src/exec/hdfs-parquet-table-writer.cc
File be/src/exec/hdfs-parquet-table-writer.cc:

http://gerrit.cloudera.org:8080/#/c/9693/2/be/src/exec/hdfs-parquet-table-writer.cc@300
PS2, Line 300:   std::vector null_pages_;
> Just saw Tim's comment. I think we discussed this a while ago in the Parque
OK, I'll implement this part after that.


http://gerrit.cloudera.org:8080/#/c/9693/2/be/src/exec/hdfs-parquet-table-writer.cc@739
PS2, Line 739: null_pages_
> Sry for the confusion. The spec says that this must be set if and only if a
Oh, sure.


http://gerrit.cloudera.org:8080/#/c/9693/2/be/src/exec/hdfs-parquet-table-writer.cc@1195
PS2, Line 1195:   // If the file contains more than one row_group, don't write 
the index.
> I missed this earlier - this seems weird. Do we ever write more than one ro
AFAIK we always write only one row group. I just copy-pasted this part from 
Pooja's work. Should I substitute it to a DCHECK maybe?


http://gerrit.cloudera.org:8080/#/c/9693/2/be/src/exec/hdfs-parquet-table-writer.cc@1224
PS2, Line 1224: <
> That sounds good to me.
Great, I implemented it that way.


http://gerrit.cloudera.org:8080/#/c/9693/3/be/src/exec/hdfs-parquet-table-writer.cc
File be/src/exec/hdfs-parquet-table-writer.cc:

http://gerrit.cloudera.org:8080/#/c/9693/3/be/src/exec/hdfs-parquet-table-writer.cc@1211
PS3, Line 1211: // Let's see if there's an ordering on min/max values.
> After moving column.min_values_ above it is invalid here. It also leaves th
We are using columns_ in the next section to write the offset index, so can't 
reset them here, but at the end of the function.

I only call BaseColumnWriter::Reset() on them, because unique_ptr::reset 
generates segfaults later.


http://gerrit.cloudera.org:8080/#/c/9693/3/tests/query_test/test_parquet_page_index.py
File tests/query_test/test_parquet_page_index.py:

http://gerrit.cloudera.org:8080/#/c/9693/3/tests/query_test/test_parquet_page_index.py@55
PS3, Line 55:
> We should assert that the schema is flat, e.g. by comparing lengths of row_
Added an assert.


http://gerrit.cloudera.org:8080/#/c/9693/3/tests/query_test/test_parquet_page_index.py@63
PS3, Line 63:
> You should not need "is not None" here and elsewhere. The parentheses also
I removed it from couple of places.
Kept them in asserts, because I think it is more readable that way.
Also kept for "if max_value is not None" and such, because max_value can be set 
to 0.


http://gerrit.cloudera.org:8080/#/c/9693/3/tests/query_test/test_parquet_page_index.py@84
PS3, Line 84:
> This looks complicated enough to warrant a namedtuple, maybe even a class.
I chose namedtuple


http://gerrit.cloudera.org:8080/#/c/9693/3/tests/query_test/test_parquet_page_index.py@87
PS3, Line 87:
> ...row_groups... (plural)
Done


http://gerrit.cloudera.org:8080/#/c/9693/3/tests/query_test/test_parquet_page_index.py@90
PS3, Line 90:
> this comment seems wrong
Removed the last sentence.


http://gerrit.cloudera.org:8080/#/c/9693/3/tests/query_test/test_parquet_page_index.py@103
PS3, Line 103:
> nit: All comments should follow PEP8 class comment format.
Done


http://gerrit.cloudera.org:8080/#/c/9693/3/tests/query_test/test_parquet_page_index.py@118
PS3, Line 118:
> long line
Done


http://gerrit.cloudera.org:8080/#/c/9693/3/tests/query_test/test_parquet_page_index.py@122
PS3, Line 122:
> It seems that this method is only called once, and without skip_col_idxs. R
Done


http://gerrit.cloudera.org:8080/#/c/9693/3/tests/query_test/test_parquet_page_index.py@147
PS3, Line 147:
> Impala currently does not write these, add an assertion
Done


http://gerrit.cloudera.org:8080/#/c/9693/3/tests/query_test/test_parquet_page_index.py@164
PS3, Line 164:
> What happens if it is None?
Added some extra asserts about it. If either stats.min_value or stats.max_value 
is None, the other must be None as well. And all pages need to be null.


http://gerrit.cloudera.org:8080/#/c/9693/3/tests/query_test/test_parquet_page_index.py@165
PS3, Line 165:
> Don't overwrite the variable
Done


http://gerrit.cloudera.org:8080/#/c/9693/3/tests/query_test/test_parquet_page_index.py@205
PS3, Line 205:
> copy-paste error?
yup, done.


http://gerrit.cloudera.org:8080/#/c/9693/3/tests/query_test/test_parquet_page_index.py@230
PS3, Line 230:
> We also should have a test with a file that has 4K of 0xFF in it, just to m
Do you know a quick way of generate a parquet file like that?

I added the other tests.


http://gerrit.cloudera.org:8080/#/c/9693/3/tests/util/get_parquet_index.py
File tests/util/get_parquet_index.py:

PS3:
> Maybe we should merge this file with get_parquet_metadata.py (and clean it
Moved the contents to get_parquet_metadata.

[Impala-ASF-CR] IMPALA-6670: refresh lib-cache entries from plan

2018-03-23 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9697 )

Change subject: IMPALA-6670: refresh lib-cache entries from plan
..


Patch Set 21: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf740ea8c6a47e671427d30b4d139cb8507b7ff6
Gerrit-Change-Number: 9697
Gerrit-PatchSet: 21
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 23 Mar 2018 21:26:41 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4277: Support multiple versions of Hadoop ecosystem

2018-03-23 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9716 )

Change subject: IMPALA-4277: Support multiple versions of Hadoop ecosystem
..


Patch Set 10: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a2ab50331986c7394c2bbfd6c865232bca975f7
Gerrit-Change-Number: 9716
Gerrit-PatchSet: 10
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 23 Mar 2018 20:55:58 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4277: Support multiple versions of Hadoop ecosystem

2018-03-23 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9716 )

Change subject: IMPALA-4277: Support multiple versions of Hadoop ecosystem
..

IMPALA-4277: Support multiple versions of Hadoop ecosystem

Adds support for building against two sets of Hadoop ecosystem
components. The control variable is IMPALA_MINICLUSTER_PROFILE_OVERRIDE,
which can either be set to 2 (for Hadoop 2, Hive 1, and so on) or 3 (for
Hadoop 3, Hive 2, and so on).

We intend (in a trivial follow-on change soon) to make 3 the new default
and to explicitly deprecate 2, but this change only does not switch the
default yet. We support both to facilitate a smoother transition, but
support will be removed soon in the Impala 3.x line.

The switch is done at build time, following the pattern from IMPALA-5184
(build fe against both Hive 1 & 2 APIs). Switching back and forth
requires running 'cmake' again. Doing this at build-time avoids
complicating the Java code with classloader configuration.

There are relatively few incompatible APIs. This implementation
encapsulates that by extracting some Java code into
fe/src/compat-minicluminicluster-profile-{2,3}. (This follows the
pattern established by IMPALA-5184, but, to avoid a proliferation
of directories, I've moved the Hive files into the same tree.)
pattern from IMPALA-5184 (build fe against both Hive 1 & 2 APIs). I
consolidated the Hive changes into the same directory structure.

For Maven, I introduced Maven "profiles" to handle the two cases where
the dependencies (and exclusions) differ. These are driven by the
$IMPALA_MINICLUSTER_PROFILE environment variable.

For Sentry, exception class names changed. We work around this by adding
"isSentry...(Exception)" methods with two different implementations.
Sentry is also doing some odd shading, whereby some exceptions are
"sentry.org.apache.sentry..."; we handle both. Similarly, the mechanism
to create a SentryAuthProvider is slightly different. The easiest way to
see the differences is to run:

  diff -u 
fe/src/compat-minicluster-profile-{2,3}/java/org/apache/impala/util/SentryUtil.java
  diff -u 
fe/src/compat-minicluster-profile-{2,3}/java/org/apache/impala/authorization/SentryAuthProvider.java

The Sentry work is based on a change by Zach Amsden.

In addition, we recently added an explicit "refresh" permission.  In
Sentry 2, this required creating an ImpalaPrivilegeModel to capture
that. It's a slight customization of Hive's equivalent class.

For Parquet, the difference is even more mechanical. The package names
gone from "parquet" to "org.apache.parquet". The affected code
was extracted into ParquetHelper, but only one copy exists. The second
copy is generated at build-time using sed.

In the rare cases where we need to behave differently at runtime,
MiniclusterProfile.MINICLUSTER_PROFILE is a class which encapsulates
what version we were built aginst. One of the cases is the results
expected by various frontend tests. I avoided the issue by translating
one error string into another, which handled the diversion in one place,
rather than complicating the several locations which look for "No
FileSystem for scheme..." errors.

The HBase APIs we use for splitting regions at test time changed.
This patch includes a re-write of that code for the new APIs. This
piece was contributed by Zach Amsden.

To work with newer versions of dependencies, I updated the version of
httpcomponents.core we use to 4.4.9.

We (Thomas Tauber-Marshall and I) uploaded new Hadoop/Hive/Sentry/HBase
binaries to s3://native-toolchain, and amended the shell scripts to
launch the right things. There are minor mechanical differences.  Some
of this was based on earlier work by Joe McDonnell and Zach Amsden.
Hive's logging is changed in Hive 2, necessitating creating a
log4j2.properties template and using it appropriately. Furthermore,
Hadoop3's new shell script re-writes do a certain amount of classpath
de-duplication, causing some issues with locating the relevant logging
configurations. Accomodations exist in the code to deal with that.

parquet-filtering.test was updated to turn off stats filtering. Older
Hive didn't write Parquet statistics, but newer Hive does. By turning
off stats filtering, we test what the test had intended to test.

For views-compatibility.test, it seems that Hive 2 has fixed certain
bugs that we were testing for in Hive. I've added a
HIVE=SUCCESS_PROFILE_3_ONLY mechanism to capture that.

For AuthorizationTest, different hive versions show slightly different
things for extended output.

To facilitate easier reviewing, the following files are 100% renames as 
identified by git; nothing
to see here.

 rename fe/src/{compat-hive-1 => 
compat-minicluster-profile-2}/java/org/apache/hive/service/rpc/thrift/TGetCatalogsReq.java
 (100%)
 rename fe/src/{compat-hive-1 =>

[Impala-ASF-CR] IMPALA-6722: include fs prefix for udf test

2018-03-23 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9782 )

Change subject: IMPALA-6722: include fs prefix for udf test
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I314d8c32e4bc3857aefd244b524fb6718d234f30
Gerrit-Change-Number: 9782
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 23 Mar 2018 19:50:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6722: include fs prefix for udf test

2018-03-23 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9782 )

Change subject: IMPALA-6722: include fs prefix for udf test
..

IMPALA-6722: include fs prefix for udf test

test_native_functions_race failed tests since it did not
include a path prefix. The fix uses get_fs_path to include
the fs prefix.

Change-Id: I314d8c32e4bc3857aefd244b524fb6718d234f30
Reviewed-on: http://gerrit.cloudera.org:8080/9782
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M tests/query_test/test_udfs.py
1 file changed, 3 insertions(+), 2 deletions(-)

Approvals:
  Alex Behm: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I314d8c32e4bc3857aefd244b524fb6718d234f30
Gerrit-Change-Number: 9782
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-6719: Reset metadata database name case sensitivity

2018-03-23 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9788


Change subject: IMPALA-6719: Reset metadata database name case sensitivity
..

IMPALA-6719: Reset metadata database name case sensitivity

Fix issue with database case name case sensitivity in reset metadata
statement.

Testing:
- Created end-to-end reset metadata test.

Change-Id: Id880aa559cec0afe8fbb7d33ccce83f7b5e474cb
---
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
A 
testdata/workloads/functional-query/queries/QueryTest/reset-metadata-case-sensitivity.test
A tests/metadata/test_reset_metadata.py
3 files changed, 85 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id880aa559cec0afe8fbb7d33ccce83f7b5e474cb
Gerrit-Change-Number: 9788
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya

[Impala-ASF-CR] IMPALA-6715: fix stress TPC workload selection

2018-03-23 Thread David Knupp (Code Review) 
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9758 )

Change subject: IMPALA-6715: fix stress TPC workload selection
..


Patch Set 2:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/9758/1//COMMIT_MSG@9
PS1, Line 9: A recent commit caused the stress test to double-count queries. 
This
By this comment, I'm taking that you mean the addition of the tpcds-decimal-* 
queries. Can you actually reference that commit here, and say that queries were 
added to the workload that don't apply to the stress test? As is, I read this 
to mean that a bug had been introduced into the stress test that was literally 
causing it to process the same query files twice as many times as necessary.


http://gerrit.cloudera.org:8080/#/c/9758/1/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

http://gerrit.cloudera.org:8080/#/c/9758/1/tests/stress/concurrent_select.py@91
PS1, Line 91: 71
> Your matching isn't right ;). There are duplicate decimal_v2 queries, hence
After our conversation yesterday, I understood this to be the case. Should we 
file a JIRA to move them to a separate workload?


http://gerrit.cloudera.org:8080/#/c/9758/1/tests/stress/concurrent_select.py@1076
PS1, Line 1076:   file_name_pattern = 
re.compile(r"^{0}-(q.*).test$".format(workload))
Can you add a comment explaining why this is necessary -- e.g., that there are 
queries (e.g., decimal_v2) in the TPCDS workload/queries directory that the 
stress test should overlook. Reference the JIRA re: creating a new workload if 
you filed one.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3e26b64d38aa8d63a176daf95c4ac5dee89508da
Gerrit-Change-Number: 9758
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 23 Mar 2018 18:23:08 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle exit code 5

2018-03-23 Thread Nithya Janarthanan (Code Review) 
Nithya Janarthanan has uploaded a new patch set (#14). ( 
http://gerrit.cloudera.org:8080/9494 )

Change subject: IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle 
exit_code 5
..

IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle exit_code 5

exit_code for EE tests when no tests are collected.After this
change return_code will be either 0 if no tests are expected
to be collected (dry-run) and 1 if tests are expected to be
collected but are not collected due to some error

Testing:
 - Ran end-to-end shell, hs2 tests for IMPALA-5886 with debug
   statements to verify the exit_codes
 - Ran end-to-end shell tests with collect-only for IMPALA-4812

Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150
---
M tests/run-tests.py
1 file changed, 69 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/9494/14
--
To view, visit http://gerrit.cloudera.org:8080/9494
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150
Gerrit-Change-Number: 9494
Gerrit-PatchSet: 14
Gerrit-Owner: Nithya Janarthanan 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan

[Impala-ASF-CR] IMPALA-6670: refresh lib-cache entries from plan

2018-03-23 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9697 )

Change subject: IMPALA-6670: refresh lib-cache entries from plan
..


Patch Set 21:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf740ea8c6a47e671427d30b4d139cb8507b7ff6
Gerrit-Change-Number: 9697
Gerrit-PatchSet: 21
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 23 Mar 2018 17:47:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5980: Upgrade to LLVM 5.0.1

2018-03-23 Thread Bikramjeet Vig (Code Review) 
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9584 )

Change subject: IMPALA-5980: Upgrade to LLVM 5.0.1
..


Patch Set 4:

Fixed a bunch of new clang-tidy warnings


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0a15cb53feab89e7b35a56b67b3b30eb3e62c6b
Gerrit-Change-Number: 9584
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 23 Mar 2018 17:41:04 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5980: Upgrade to LLVM 5.0.1

2018-03-23 Thread Bikramjeet Vig (Code Review) 
Hello Tim Armstrong, Dan Hecht, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-5980: Upgrade to LLVM 5.0.1
..

IMPALA-5980: Upgrade to LLVM 5.0.1

Highlighting a few changes in LLVM:
- Minor changes to some function signatures
- Minor changes to error handling
- Split Bitcode/ReaderWriter.h - https://reviews.llvm.org/D26502
- Introduced an optional new GVN optimization pass.

Needed to fix a bunch of new clang-tidy warnings.

Testing:
Ran core and ASAN tests successfully.

Performance:
Ran single node TPC-H and targeted perf with scale factor 60. Both
improved on average. Identified regression in
"primitive_filter_in_predicate" which will be addressed by IMPALA-6621.

+---+---+-++++
| Workload  | File Format   | Avg (s) | Delta(Avg) | GeoMean(s) 
| Delta(GeoMean) |
+---+---+-++++
| TARGETED-PERF(60) | parquet / none / none | 22.29   | -0.12% | 3.90   
| +3.16% |
| TPCH(60)  | parquet / none / none | 15.97   | -3.64% | 10.14  
| -4.92% |
+---+---+-++++

+---++---++-++++-+---+
| Workload  | Query  | 
File Format   | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%)  | Base 
StdDev(%) | Num Clients | Iters |
+---++---++-++++-+---+
| TARGETED-PERF(60) | PERF_LIMIT-Q1  | 
parquet / none / none | 0.01   | 0.00| R +156.43% | * 25.80% * | * 
17.14% * | 1   | 5 |
| TARGETED-PERF(60) | primitive_filter_in_predicate  | 
parquet / none / none | 3.39   | 1.92| R +76.33%  |   3.23%|   
4.37%| 1   | 5 |
| TARGETED-PERF(60) | primitive_filter_string_non_selective  | 
parquet / none / none | 1.25   | 1.11|   +12.46%  |   3.41%|   
5.36%| 1   | 5 |
| TARGETED-PERF(60) | primitive_filter_decimal_selective | 
parquet / none / none | 1.40   | 1.25|   +12.25%  |   3.57%|   
3.44%| 1   | 5 |
| TARGETED-PERF(60) | primitive_filter_string_like   | 
parquet / none / none | 16.87  | 15.65   |   +7.78%   |   5.05%|   
0.37%| 1   | 5 |
| TARGETED-PERF(60) | primitive_min_max_runtime_filter   | 
parquet / none / none | 1.79   | 1.71|   +4.77%   |   0.71%|   
1.73%| 1   | 5 |
| TARGETED-PERF(60) | primitive_broadcast_join_2 | 
parquet / none / none | 0.60   | 0.58|   +3.64%   |   3.19%|   
3.81%| 1   | 5 |
| TARGETED-PERF(60) | primitive_filter_string_selective  | 
parquet / none / none | 0.95   | 0.93|   +2.91%   |   5.23%|   
5.85%| 1   | 5 |
| TARGETED-PERF(60) | primitive_broadcast_join_3 | 
parquet / none / none | 4.33   | 4.21|   +2.83%   |   5.46%|   
3.25%| 1   | 5 |
| TARGETED-PERF(60) | primitive_groupby_bigint_lowndv| 
parquet / none / none | 4.59   | 4.47|   +2.82%   |   3.73%|   
1.14%| 1   | 5 |
| TARGETED-PERF(60) | primitive_conjunct_ordering_3  | 
parquet / none / none | 0.20   | 0.19|   +2.65%   |   4.76%|   
2.24%| 1   | 5 |
| TARGETED-PERF(60) | PERF_AGG-Q1| 
parquet / none / none | 2.49   | 2.43|   +2.31%   |   1.06%|   
1.93%| 1   | 5 |
| TARGETED-PERF(60) | PERF_AGG-Q6| 
parquet / none / none | 2.04   | 2.00|   +2.09%   |   3.51%|   
2.80%| 1   | 5 |
| TPCH(60)  | TPCH-Q3| 
parquet / none / none | 12.37  | 12.17   |   +1.62%   |   0.80%|   
2.45%| 1   | 5 |
| TARGETED-PERF(60) | PERF_STRING-Q5 | 
parquet / none / none | 4.52   | 4.45|   +1.54%   |   1.23%|   
1.08%| 1   | 5 |
| TPCH(60)  | TPCH-Q6

[Impala-ASF-CR] IMPALA-4277: Support multiple versions of Hadoop ecosystem

2018-03-23 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9716 )

Change subject: IMPALA-4277: Support multiple versions of Hadoop ecosystem
..


Patch Set 10:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a2ab50331986c7394c2bbfd6c865232bca975f7
Gerrit-Change-Number: 9716
Gerrit-PatchSet: 10
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 23 Mar 2018 17:13:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6715: fix stress TPC workload selection

2018-03-23 Thread Michael Brown (Code Review) 
Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9758 )

Change subject: IMPALA-6715: fix stress TPC workload selection
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9758/1/tests/infra/test_infra.py
File tests/infra/test_infra.py:

http://gerrit.cloudera.org:8080/#/c/9758/1/tests/infra/test_infra.py@1
PS1, Line 1: # Licensed to the Apache Software Foundation (ASF) under one
> This is awesome. We've needed a place to have tests for the infra code for
Done


http://gerrit.cloudera.org:8080/#/c/9758/1/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

http://gerrit.cloudera.org:8080/#/c/9758/1/tests/stress/concurrent_select.py@91
PS1, Line 91: 71
> Why is this number so low? When I list the contents of tpch and tpch_nested
Your matching isn't right ;). There are duplicate decimal_v2 queries, hence the 
bug and the double-counting. Really, when those were added, I should have asked 
to make them a separate workload.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3e26b64d38aa8d63a176daf95c4ac5dee89508da
Gerrit-Change-Number: 9758
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 23 Mar 2018 16:31:30 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6715: fix stress TPC workload selection

2018-03-23 Thread Michael Brown (Code Review) 
Hello Nithya Janarthanan, David Knupp, Tim Armstrong,

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

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

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

Change subject: IMPALA-6715: fix stress TPC workload selection
..

IMPALA-6715: fix stress TPC workload selection

A recent commit caused the stress test to double-count queries. This
makes the binary search run incredibly slow.

- Fix the query selection. Add additional queries that weren't matching
  before, like the tpcds-q[0-9]+a.test series.

- Add a test that will at least ensure if
  testdata/workloads/tpc*/queries is modified, the stress test will
  still find the same number of queries for the given workload. There's
  no obvious place to put this test: it's not testing the product at
  all, so:

- Add a new directory tests/infra for such tests and add it to
  tests/run-tests.py.

- Move the test from IMPALA-6441 into tests/infra.

Testing:
- Core private build passed. I manually looked to make sure the moved
  and new tests ran.

- Short stress test run. I checked the runtime info and saw the new
  TPCDS queries in the JSON.

Change-Id: I3e26b64d38aa8d63a176daf95c4ac5dee89508da
---
A tests/infra/test_stress_infra.py
M tests/metadata/test_explain.py
M tests/run-tests.py
M tests/stress/concurrent_select.py
4 files changed, 75 insertions(+), 24 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3e26b64d38aa8d63a176daf95c4ac5dee89508da
Gerrit-Change-Number: 9758
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-6722: include fs prefix for udf test

2018-03-23 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9782 )

Change subject: IMPALA-6722: include fs prefix for udf test
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I314d8c32e4bc3857aefd244b524fb6718d234f30
Gerrit-Change-Number: 9782
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 23 Mar 2018 15:56:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6722: include fs prefix for udf test

2018-03-23 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9782 )

Change subject: IMPALA-6722: include fs prefix for udf test
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I314d8c32e4bc3857aefd244b524fb6718d234f30
Gerrit-Change-Number: 9782
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Comment-Date: Fri, 23 Mar 2018 15:55:59 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6641: Support more separators between date and time in default timestamp format

2018-03-23 Thread Tim Armstrong (Code Review) 
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9725 )

Change subject: IMPALA-6641: Support more separators between date and time in 
default timestamp format
..


Patch Set 2:

(2 comments)

LGTM once these two things are fixed.

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

http://gerrit.cloudera.org:8080/#/c/9725/2/be/src/runtime/timestamp-parse-util.cc@422
PS2, Line 422: break;
Break needs to go outside if, otherwise we'll fall through to the next case 
when str[13] is != ":".


http://gerrit.cloudera.org:8080/#/c/9725/2/be/src/runtime/timestamp-parse-util.cc@435
PS2, Line 435: break;
Same here.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2ce3ba09256b3996170e42d42d49d12776cab97
Gerrit-Change-Number: 9725
Gerrit-PatchSet: 2
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran 
Gerrit-Comment-Date: Fri, 23 Mar 2018 15:20:43 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6324: Support reading RLE-encoded boolean values in Parquet scanner

2018-03-23 Thread Tim Armstrong (Code Review) 
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9403 )

Change subject: IMPALA-6324: Support reading RLE-encoded boolean values in 
Parquet scanner
..


Patch Set 13:

I've gotten into a similar situation before and git commit --amend 
--reset-author has gotten me out of it. Anyway, not a big deal, I just don't 
want to get credit (or blame!) for your work :)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4644bf8cf5d2b7238b05076407fbf78ab5d2c14f
Gerrit-Change-Number: 9403
Gerrit-PatchSet: 13
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 23 Mar 2018 15:10:37 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6324: Support reading RLE-encoded boolean values in Parquet scanner

2018-03-23 Thread Csaba Ringhofer (Code Review) 
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9403 )

Change subject: IMPALA-6324: Support reading RLE-encoded boolean values in 
Parquet scanner
..


Patch Set 13:

I think that it is related to an incident during my commit:
- by mistake, I committed first with "commit --amend", which modified the last 
commit in my repo (which was yours)
- I searched for a solution to redo this change without loosing my 
modification, and did the one I found (I don't remember the exact commands, I 
can look it up if necessary)
- everything looked ok to me, my changes were in separate commit, so I pushed 
the change to gerrit

My guess is that gerrit was tricked somehow by this maneuver, and the Author 
field was inherited from the last commit.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4644bf8cf5d2b7238b05076407fbf78ab5d2c14f
Gerrit-Change-Number: 9403
Gerrit-PatchSet: 13
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 23 Mar 2018 12:17:52 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6699: Fix DST end time for Australian time zones

2018-03-23 Thread Gabor Kaszab (Code Review) 
Hello Attila Jeges, Tim Armstrong,

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

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

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

Change subject: IMPALA-6699: Fix DST end time for Australian time zones
..

IMPALA-6699: Fix DST end time for Australian time zones

In Australian time zones where Daylight Saving Time is used (except
LHDT) DST should end at 3am on the first Sunday of April when the
clock is set back to 2am. However, the current time zone DB contains
wrong DST end time for them. This fix sets the DST end time to 3am
for the mentioned time zones.

Change-Id: I461cd4a9057dfebfe8dd85b568cba4f1e87ad215
---
M be/src/exprs/expr-test.cc
M be/src/exprs/timezone_db.cc
2 files changed, 61 insertions(+), 14 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I461cd4a9057dfebfe8dd85b568cba4f1e87ad215
Gerrit-Change-Number: 9724
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-6699: Fix DST end time for Australian time zones

2018-03-23 Thread Gabor Kaszab (Code Review) 
Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9724 )

Change subject: IMPALA-6699: Fix DST end time for Australian time zones
..


Patch Set 3:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/9724/2//COMMIT_MSG@10
PS2, Line 10: DST
> not necessary
Done


http://gerrit.cloudera.org:8080/#/c/9724/2//COMMIT_MSG@11
PS2, Line 11: However, the current time zone DB contains
: wrong DST end time for them. This fix sets the DS
> Maybe something like this instead:
Thanks for the suggestion. Rephrased a little bit. Done


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

http://gerrit.cloudera.org:8080/#/c/9724/2/be/src/exprs/expr-test.cc@268
PS2, Line 268: Tests that DST of the given timezone ends at 3am
> Tests that DST of the given timezone ends at 3am
Thanks for the suggestion. Done


http://gerrit.cloudera.org:8080/#/c/9724/2/be/src/exprs/expr-test.cc@5684
PS2, Line 5684: // timestamp otherwise.
  :   TestStringValue("cast(to_u
> Please comment on why we expect NULL here and below
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I461cd4a9057dfebfe8dd85b568cba4f1e87ad215
Gerrit-Change-Number: 9724
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 23 Mar 2018 11:53:49 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6641: Support more separators between date and time in default timestamp format

2018-03-23 Thread Vincent Tran (Code Review) 
Hello Tim Armstrong,

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

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

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

Change subject: IMPALA-6641: Support more separators between date and time in 
default timestamp format
..

IMPALA-6641: Support more separators between date and time in default
timestamp format

This change adds support to the multi-space separator and the
'T' separator between the date and time component of a datetime
string during a cast (x as timestamp).

Testing:
Added valid and invalid tests to expr-test.cc to validate
the functionality during a cast.

Change-Id: Id2ce3ba09256b3996170e42d42d49d12776cab97
---
M be/src/exprs/expr-test.cc
M be/src/runtime/timestamp-parse-util.cc
2 files changed, 55 insertions(+), 18 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id2ce3ba09256b3996170e42d42d49d12776cab97
Gerrit-Change-Number: 9725
Gerrit-PatchSet: 2
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran

[Impala-ASF-CR] IMPALA-6641: Support more separators between date and time in default timestamp format

2018-03-23 Thread Vincent Tran (Code Review) 
Vincent Tran has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9725 )

Change subject: IMPALA-6641: Support more separators between date and time in 
default timestamp format
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9725/1/be/src/runtime/timestamp-parse-util.cc@407
PS1, Line 407: if (str[4] == '-' && str[7] == '-' && str[13] == ':') {
> Checking str[13] here doesn't seem right - that will read past the end of t
Good point.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2ce3ba09256b3996170e42d42d49d12776cab97
Gerrit-Change-Number: 9725
Gerrit-PatchSet: 1
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran 
Gerrit-Comment-Date: Fri, 23 Mar 2018 09:47:49 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6670: refresh lib-cache entries from plan

2018-03-23 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9697 )

Change subject: IMPALA-6670: refresh lib-cache entries from plan
..


Patch Set 21: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2164/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf740ea8c6a47e671427d30b4d139cb8507b7ff6
Gerrit-Change-Number: 9697
Gerrit-PatchSet: 21
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 23 Mar 2018 07:10:27 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6722: include fs prefix for udf test

2018-03-23 Thread Vuk Ercegovac (Code Review) 
Vuk Ercegovac has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9782


Change subject: IMPALA-6722: include fs prefix for udf test
..

IMPALA-6722: include fs prefix for udf test

test_native_functions_race failed tests since it did not
include a path prefix. The fix uses get_fs_path to include
the fs prefix.

Change-Id: I314d8c32e4bc3857aefd244b524fb6718d234f30
---
M tests/query_test/test_udfs.py
1 file changed, 3 insertions(+), 2 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I314d8c32e4bc3857aefd244b524fb6718d234f30
Gerrit-Change-Number: 9782
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac