[Impala-ASF-CR] IMPALA-6728: Always use Kudu based kinit if FLAGS use krpc=true

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9797 )

Change subject: IMPALA-6728: Always use Kudu based kinit if FLAGS_use_krpc=true
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7086e5c9b460233e9e957f886141b3e6bba414b
Gerrit-Change-Number: 9797
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 27 Mar 2018 05:52:18 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6728: Always use Kudu based kinit if FLAGS use krpc=true

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9797 )

Change subject: IMPALA-6728: Always use Kudu based kinit if FLAGS_use_krpc=true
..


Patch Set 4: Code-Review+2

Carry +2. Fixed clang-tidy errors.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7086e5c9b460233e9e957f886141b3e6bba414b
Gerrit-Change-Number: 9797
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 27 Mar 2018 05:51:33 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6728: Always use Kudu based kinit if FLAGS use krpc=true

[Michael Ho (Code Review)]

Hello Sailesh Mukil, Dan Hecht, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-6728: Always use Kudu based kinit if FLAGS_use_krpc=true
..

IMPALA-6728: Always use Kudu based kinit if FLAGS_use_krpc=true

We rely on the KPRC logic to do the Kerberos authentication
when KRPC is enabled. Therefore, when FLAGS_ues_krpc=true,
we must always call kudu::security::InitKerberosForServer()
to initialize the Kerberos related logic. This change makes
Impala ignore FLAGS_use_kudu_kinit=false when FLAGS_use_krpc=true.

Change-Id: Ia7086e5c9b460233e9e957f886141b3e6bba414b
---
M be/src/rpc/auth-provider.h
M be/src/rpc/authentication.cc
M be/src/rpc/rpc-mgr-kerberized-test.cc
M be/src/rpc/rpc-mgr-test-base.h
M be/src/rpc/thrift-server-test.cc
M be/src/testutil/mini-kdc-wrapper.cc
M be/src/testutil/mini-kdc-wrapper.h
7 files changed, 149 insertions(+), 113 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7086e5c9b460233e9e957f886141b3e6bba414b
Gerrit-Change-Number: 9797
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-6747: Automate diagnostics collection.

[Bharath Vissapragada (Code Review)]

Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9815 )

Change subject: IMPALA-6747: Automate diagnostics collection.
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9815/1/bin/diagnostics/__init__.py
File bin/diagnostics/__init__.py:

http://gerrit.cloudera.org:8080/#/c/9815/1/bin/diagnostics/__init__.py@1
PS1, Line 1:
> The Rat check might trip over this file without a license header, even if i
Good point, added it to rat-excludes.


http://gerrit.cloudera.org:8080/#/c/9815/1/bin/diagnostics/collect_diagnostics.py
File bin/diagnostics/collect_diagnostics.py:

http://gerrit.cloudera.org:8080/#/c/9815/1/bin/diagnostics/collect_diagnostics.py@54
PS1, Line 54: thout breakpad suppor
> This should now refer to upstream versions
Done.


http://gerrit.cloudera.org:8080/#/c/9815/1/bin/diagnostics/collect_shared_libs.sh
File bin/diagnostics/collect_shared_libs.sh:

http://gerrit.cloudera.org:8080/#/c/9815/1/bin/diagnostics/collect_shared_libs.sh@1
PS1, Line 1: #!/usr/bin/env bash
> What's the befit of this over /bin/bash?
It just searches the PATH for bash. So, it also works in non-default 
installations of bash (/bin/bash doesn't exist on some systems maybe?) Not an 
expert on this but I read this makes it more portable. I guess it doesn't make 
much difference in our environments though.

More here 
https://stackoverflow.com/questions/16365130/the-difference-between-usr-bin-env-bash-and-usr-bin-bash



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib29caec7c3be5b6a31e60461294979c318300f64
Gerrit-Change-Number: 9815
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Tue, 27 Mar 2018 05:04:31 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6747: Automate diagnostics collection.

[Bharath Vissapragada (Code Review)]

Hello Lars Volker, Philip Zeyliger,

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

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

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

Change subject: IMPALA-6747: Automate diagnostics collection.
..

IMPALA-6747: Automate diagnostics collection.

This commit adds the necessary tooling to automate diagnostics
collection for Impala daemons. Following diagnostics are supported.

1. Native core dump (+ shared libs)
2. GDB/Java thread dump (pstack + jstack)
3. Java heap dump (jmap)
4. Minidumps (using breakpad) *
5. Profiles

Given the required inputs, the script outputs a zip compressed
impala diagnostic bundle with all the diagnostics collected.

The script can be run manually with the following command.

python collect_diagnostics.py --help

* minidumps collected here correspond to the state of the Impala
process at the time this script is triggered. This is different
from collect_minidumps.py which archives the entire minidump
directory.

Change-Id: Ib29caec7c3be5b6a31e60461294979c318300f64
---
A bin/diagnostics/__init__.py
A bin/diagnostics/collect_diagnostics.py
A bin/diagnostics/collect_shared_libs.sh
M bin/rat_exclude_files.txt
A tests/unittests/test_command.py
5 files changed, 619 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib29caec7c3be5b6a31e60461294979c318300f64
Gerrit-Change-Number: 9815
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 

[Impala-ASF-CR] IMPALA-4123 (prep): Parquet column reader cleanup

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9799 )

Change subject: IMPALA-4123 (prep): Parquet column reader cleanup
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc00352df3a0b2d605f872ae7e43db2dc90faab1
Gerrit-Change-Number: 9799
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 27 Mar 2018 04:41:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6747: Automate diagnostics collection.

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9815 )

Change subject: IMPALA-6747: Automate diagnostics collection.
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9815/1/bin/diagnostics/__init__.py
File bin/diagnostics/__init__.py:

http://gerrit.cloudera.org:8080/#/c/9815/1/bin/diagnostics/__init__.py@1
PS1, Line 1:
The Rat check might trip over this file without a license header, even if it's 
otherwise empty (I might be wrong, please check).


http://gerrit.cloudera.org:8080/#/c/9815/1/bin/diagnostics/collect_diagnostics.py
File bin/diagnostics/collect_diagnostics.py:

http://gerrit.cloudera.org:8080/#/c/9815/1/bin/diagnostics/collect_diagnostics.py@54
PS1, Line 54: CDH versions <= 5.8.x
This should now refer to upstream versions


http://gerrit.cloudera.org:8080/#/c/9815/1/bin/diagnostics/collect_shared_libs.sh
File bin/diagnostics/collect_shared_libs.sh:

http://gerrit.cloudera.org:8080/#/c/9815/1/bin/diagnostics/collect_shared_libs.sh@1
PS1, Line 1: #!/usr/bin/env bash
What's the befit of this over /bin/bash?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib29caec7c3be5b6a31e60461294979c318300f64
Gerrit-Change-Number: 9815
Gerrit-PatchSet: 1
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Tue, 27 Mar 2018 04:30:45 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6728: Always use Kudu based kinit if FLAGS use krpc=true

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9797 )

Change subject: IMPALA-6728: Always use Kudu based kinit if FLAGS_use_krpc=true
..


Patch Set 3: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7086e5c9b460233e9e957f886141b3e6bba414b
Gerrit-Change-Number: 9797
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 27 Mar 2018 04:16:48 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6747: Automate diagnostics collection.

[Bharath Vissapragada (Code Review)]

Bharath Vissapragada has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9815


Change subject: IMPALA-6747: Automate diagnostics collection.
..

IMPALA-6747: Automate diagnostics collection.

This commit adds the necessary tooling to automate diagnostics
collection for Impala daemons. Following diagnostics are supported.

1. Native core dump (+ shared libs)
2. GDB/Java thread dump (pstack + jstack)
3. Java heap dump (jmap)
4. Minidumps (using breakpad) *
5. Profiles

Given the required inputs, the script outputs a zip compressed
impala diagnostic bundle with all the diagnostics collected.

The script can be run manually with the following command.

python collect_diagnostics.py --help

* minidumps collected here correspond to the state of the Impala
process at the time this script is triggered. This is different
from collect_minidumps.py which archives the entire minidump
directory.

Change-Id: Ib29caec7c3be5b6a31e60461294979c318300f64
---
A bin/diagnostics/__init__.py
A bin/diagnostics/collect_diagnostics.py
A bin/diagnostics/collect_shared_libs.sh
A tests/unittests/test_command.py
4 files changed, 619 insertions(+), 0 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib29caec7c3be5b6a31e60461294979c318300f64
Gerrit-Change-Number: 9815
Gerrit-PatchSet: 1
Gerrit-Owner: Bharath Vissapragada 

[Impala-ASF-CR] Use "mvn -B" in builds to avoid dowloading progress bars in logs.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9792 )

Change subject: Use "mvn -B" in builds to avoid dowloading progress bars in 
logs.
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa117272c2a86b63b0f9062099a4145324eb6fc
Gerrit-Change-Number: 9792
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Tue, 27 Mar 2018 04:04:27 +
Gerrit-HasComments: No

[Impala-ASF-CR] Use "mvn -B" in builds to avoid dowloading progress bars in logs.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9792 )

Change subject: Use "mvn -B" in builds to avoid dowloading progress bars in 
logs.
..

Use "mvn -B" in builds to avoid dowloading progress bars in logs.

Maven's batch (or non-interactive) mode prevents progress bar output
when Maven is downloading artifacts, which isn't generally useful.
Now that we keep Maven logs in logs/mvn/mvn.log, this makes
them slightly more tidy.

Change-Id: I5aa117272c2a86b63b0f9062099a4145324eb6fc
Reviewed-on: http://gerrit.cloudera.org:8080/9792
Reviewed-by: Michael Brown 
Tested-by: Impala Public Jenkins
---
M common/yarn-extras/CMakeLists.txt
M ext-data-source/CMakeLists.txt
M fe/CMakeLists.txt
M impala-parent/CMakeLists.txt
4 files changed, 4 insertions(+), 4 deletions(-)

Approvals:
  Michael Brown: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5aa117272c2a86b63b0f9062099a4145324eb6fc
Gerrit-Change-Number: 9792
Gerrit-PatchSet: 2
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 

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

[Fredy Wijaya (Code Review)]

Fredy Wijaya has uploaded a new patch set (#3). ( 
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 tests.

Change-Id: Id880aa559cec0afe8fbb7d33ccce83f7b5e474cb
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.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(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/9788/3
--
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: newpatchset
Gerrit-Change-Id: Id880aa559cec0afe8fbb7d33ccce83f7b5e474cb
Gerrit-Change-Number: 9788
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR](2.x) IMPALA-6743: bump from 2.11 to 2.12

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9809 )

Change subject: IMPALA-6743: bump from 2.11 to 2.12
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaeaf230bf6f0cbf299edd4cf5ede4cb808523f1c
Gerrit-Change-Number: 9809
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Tue, 27 Mar 2018 02:50:07 +
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-6743: bump from 2.11 to 2.12

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9809 )

Change subject: IMPALA-6743: bump from 2.11 to 2.12
..

IMPALA-6743: bump from 2.11 to 2.12

Next release is 2.12 so update the 2.x branch to
refer to 2.12 (2.11 has already been released).

Change-Id: Iaeaf230bf6f0cbf299edd4cf5ede4cb808523f1c
Reviewed-on: http://gerrit.cloudera.org:8080/9809
Reviewed-by: Lars Volker 
Tested-by: Impala Public Jenkins
---
M bin/save-version.sh
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Lars Volker: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: merged
Gerrit-Change-Id: Iaeaf230bf6f0cbf299edd4cf5ede4cb808523f1c
Gerrit-Change-Number: 9809
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 

[Impala-ASF-CR] KUDU-2374: Add RpcContext::GetTimeReceived()

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9807 )

Change subject: KUDU-2374: Add RpcContext::GetTimeReceived()
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b39c7f2ea856eccfdab8c1bb1433829e979ae13
Gerrit-Change-Number: 9807
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Tue, 27 Mar 2018 02:22:23 +
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2374: Add RpcContext::GetTimeReceived()

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9807 )

Change subject: KUDU-2374: Add RpcContext::GetTimeReceived()
..

KUDU-2374: Add RpcContext::GetTimeReceived()

This change adds RpcContext::GetTimeReceived() which returns
the time at which the inbound call associated with the RpcContext
was received. It's helpful to make this accessible to the RPC
handlers for its own book-keeping purpose (e.g. reporting the
average dispatch latency as part of query profile in Impala).

Change-Id: I6b39c7f2ea856eccfdab8c1bb1433829e979ae13
Reviewed-on: http://gerrit.cloudera.org:8080/9796
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
Reviewed-on: http://gerrit.cloudera.org:8080/9807
Reviewed-by: Joe McDonnell 
Tested-by: Impala Public Jenkins
---
M be/src/kudu/rpc/rpc-test-base.h
M be/src/kudu/rpc/rpc_context.cc
M be/src/kudu/rpc/rpc_context.h
3 files changed, 10 insertions(+), 0 deletions(-)

Approvals:
  Joe McDonnell: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6b39c7f2ea856eccfdab8c1bb1433829e979ae13
Gerrit-Change-Number: 9807
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] IMPALA-6510: [DOCS] Remove refresh after connect

[Alex Rodoni (Code Review)]

Alex Rodoni has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9813


Change subject: IMPALA-6510: [DOCS] Remove refresh_after_connect
..

IMPALA-6510: [DOCS] Remove refresh_after_connect

Removed refresh_after_connect option from impala shell options.
Removed the refresh_after_connect from INVALIDATE METADATA doc.

Change-Id: I7bd49cb32a952362dcefc230d8feb1a7d6c13ea0
---
M docs/topics/impala_invalidate_metadata.xml
M docs/topics/impala_shell_options.xml
2 files changed, 0 insertions(+), 29 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7bd49cb32a952362dcefc230d8feb1a7d6c13ea0
Gerrit-Change-Number: 9813
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 

[Impala-ASF-CR] IMPALA-4123 (prep): Parquet column reader cleanup

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9799 )

Change subject: IMPALA-4123 (prep): Parquet column reader cleanup
..


Patch Set 2:

Carry +1. Running tests just to get clang-tidy coverage, etc.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc00352df3a0b2d605f872ae7e43db2dc90faab1
Gerrit-Change-Number: 9799
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 27 Mar 2018 00:56:49 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4123 (prep): Parquet column reader cleanup

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9799 )

Change subject: IMPALA-4123 (prep): Parquet column reader cleanup
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc00352df3a0b2d605f872ae7e43db2dc90faab1
Gerrit-Change-Number: 9799
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 27 Mar 2018 00:56:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4123 (prep): Parquet column reader cleanup

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9799 )

Change subject: IMPALA-4123 (prep): Parquet column reader cleanup
..


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/9799/1/be/src/exec/parquet-column-readers.cc
File be/src/exec/parquet-column-readers.cc:

http://gerrit.cloudera.org:8080/#/c/9799/1/be/src/exec/parquet-column-readers.cc@518
PS1, Line 518:   if (def_level < 
def_level_of_immediate_repeated_ancestor()) {
 : // A containing repeated field is empty or NULL. Skip 
the value but
 : // move to the next repetition level if necessary.
 : if (pos_slot_desc_ != nullptr) 
rep_levels_.CacheSkipLevels(1);
 : continue;
 :   }
 :   if (pos_slot_desc_ != nullptr) {
 : ReadPositionBatched(rep_levels_.CacheGetNext(),
 : 
static_cast(tuple->GetSlot(pos_slot_desc_->tuple_offset(;
 :   }
> I would move this whole block to ReadPositionBatched, and rename it to some
I think the current structure is the lesser of evils for a couple of reads:
* It is tied into the control flow of the loop, since we use "continue" to jump 
to the top of the loop based on the def level. I don't see a clean way to 
factor out the def_level < .. branch.
* I want to use ReadPositionBatched() as a helper function in a follow-on patch.


http://gerrit.cloudera.org:8080/#/c/9799/1/be/src/exec/parquet-column-readers.cc@531
PS1, Line 531: def_level >= max_def_level()
> This was not changed, but I am curious: can def_level be greater than max_d
It shouldn't be possible in a valid file. It would break our decoding in a lot 
of places, since we infer the maximum from the schema, then use that to 
determine the bit width when decoding levels.

This predates me working on the code, but I suspect the looser check was used 
to avoid crashing without paying any extra runtime overhead to validate each 
value.


http://gerrit.cloudera.org:8080/#/c/9799/1/be/src/exec/parquet-column-readers.cc@586
PS1, Line 586: if (!continue_execution) return false;
> Replacing this with !parent_->parse_status_.ok() and removing continue_exec
Done


http://gerrit.cloudera.org:8080/#/c/9799/1/be/src/exec/parquet-column-readers.cc@597
PS1, Line 597: template 
> This could be split to two specialized template functions.
Ahh, I missed this. Unfortunately it looks like there's some reason that isn't 
not possible to specialize a member function template when the class template 
is unspecialised - I'm getting this from clang:

  template 
  template <>
  bool ScalarColumnReader::DecodeValue(

 error| cannot specialize (with 'template<>') a member of an unspecialized 
template


If there's an alternative you know of, I'm open to it.


http://gerrit.cloudera.org:8080/#/c/9799/1/be/src/exec/parquet-column-readers.cc@600
PS1, Line 600: MemPool* RESTRICT pool
> "pool" could be removed, as it is not used in the function (ReadSlot() used
Oh good point. I removed the pool argument.

I'll hold off on making the other change. I can see the argument for it, but 
one advantage of the current code is that it's more obvious in 
HdfsParquetScanner::AssembleRows() that ReadValueBatch() allocates memory from 
aux_mem_pool, so the memory lifetime is a bit clearer.


http://gerrit.cloudera.org:8080/#/c/9799/1/be/src/exec/parquet-column-readers.cc@1320
PS1, Line 1320: return 
ReadSlot(static_cast(tuple->GetSlot(tuple_offset_)), pool);
> Tuple has a GetCollectionSlot() functions that does the cast. Some new type
Ah I forgot about those functions, good catch. Added some new functions. I used 
GetBigIntVal() since I think the name should reflect the logical type of the 
slot.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc00352df3a0b2d605f872ae7e43db2dc90faab1
Gerrit-Change-Number: 9799
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 27 Mar 2018 00:55:42 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4123 (prep): Parquet column reader cleanup

[Tim Armstrong (Code Review)]

Hello Csaba Ringhofer,

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

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

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

Change subject: IMPALA-4123 (prep): Parquet column reader cleanup
..

IMPALA-4123 (prep): Parquet column reader cleanup

Some miscellaneous cleanup to make it easier to understand and
make future changes to the Parquet scanner.

A lot of the refactoring is about more cleanly separating functions
so that they have clearer purpose, e.g.:
* Functions that strictly do decoding, i.e. materialize values, convert
  and validate them. These are changed to operate on single values, not tuples.
* Functions that are used for the non-batched decoding path (i.e. driven
  by CollectionColumnReader or BoolColumnReader).
* Functions that dispatch to a templated implementation based on one or
  more runtime values.

Other misc changes:
* Move large functions out of class bodies.
* Use parquet::Encoding instead of bool to indicate encoding.
* Add some additional DCHECKs.

Testing:
* Ran exhaustive tests
* Ran fuzz test in a loop

Change-Id: Ibc00352df3a0b2d605f872ae7e43db2dc90faab1
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/parquet-column-readers.cc
M be/src/exec/parquet-column-readers.h
M be/src/runtime/tuple.h
M be/src/util/bit-stream-utils.h
M be/src/util/rle-encoding.h
6 files changed, 228 insertions(+), 168 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibc00352df3a0b2d605f872ae7e43db2dc90faab1
Gerrit-Change-Number: 9799
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 

[Impala-ASF-CR] IMPALA-6685: Improve profiles in KrpcDataStreamRecvr and KrpcDataStreamSender

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9690 )

Change subject: IMPALA-6685: Improve profiles in KrpcDataStreamRecvr and 
KrpcDataStreamSender
..


Patch Set 5:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/9690/3/be/src/runtime/krpc-data-stream-recvr.cc
File be/src/runtime/krpc-data-stream-recvr.cc:

http://gerrit.cloudera.org:8080/#/c/9690/3/be/src/runtime/krpc-data-stream-recvr.cc@457
PS3, Line 457: kudu::Slice tuple_offsets;
 : kudu::Slice tuple_data;
 : int64_t batch_size;
 : status = UnpackRequest(ctx->request, ctx->rpc_context, 
_offsets,
 : _data, _size);
 : // Reply with error status if the entry cannot be unpacked.
 : if (UNLIKELY(!status.ok())) {
 :   DataStreamService::RespondAndReleaseRpc(status, 
ctx->response, ctx->rpc_context,
 :   recvr_->deferred_rpc_tracker());
 :   DequeueDeferredRpc();
 :   return;
 : }
 :
 :
> Right, but it also gives the serialized size components. What I meant was:
Done


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc
File be/src/runtime/krpc-data-stream-recvr.cc:

http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@117
PS4, Line 117: doe
> does
Done


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@121
PS4, Line 121: doe
> does
Done


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@207
PS4, Line 207: l_has_deferred_rpcs_timer_'.
> how about calling it: has_deferred_rpcs_start_time_ns_?
Done


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@318
PS4, Line 318:   deferred_rpcs_.pop();
> DCHECK_NE(has_deferred_rpcs_start_time_ns, 0)?
Done


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@320
PS4, Line 320: ed_rpcs_start_time_ns_, 0
> how about calling this: total_has_deferred_rpcs_time_ to make it clearer th
Done


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@642
PS4, Line 642: chWork()
> sometimes we have "counter_" and sometimes not. Let's try to be consistent
Done


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@646
PS4, Line 646: rious
> okay to ignore, but I think it'd be good to call these timer_ (but calling
Done


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@673
PS4, Line 673: total_deferred_rpcs_count
> as mentioned earlier, how about total_has_deferred_rpcs_time_ to distinguis
Done


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@674
PS4, Line 674:  "TotalRPCsDeferred",
> TotalHasDeferredRPCsTime
Done


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-sender.h
File be/src/runtime/krpc-data-stream-sender.h:

http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-sender.h@183
PS4, Line 183: m
> maybe add: but not meaningful by itself, and therefore not placed in a prof
Done


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-sender.cc
File be/src/runtime/krpc-data-stream-sender.cc:

http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-sender.cc@405
PS4, Line 405: Substitute("TransmitData() to $0 failed", 
TNetworkAddressToString(address_));
> maybe set rpc_start_time_ns_ to 0 here and then DCHECK at that it's not 0 a
Set to 0 in MarkDone().


http://gerrit.cloudera.org:8080/#/c/9690/4/common/protobuf/data_stream_service.proto
File common/protobuf/data_stream_service.proto:

http://gerrit.cloudera.org:8080/#/c/9690/4/common/protobuf/data_stream_service.proto@53
PS4, Line 53: receivin
> let's clarify that, since it could be intepretted as the data stream recvr
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8ba405921b3df920c1e85b940ce9c8d02fc647cd
Gerrit-Change-Number: 9690
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Comment-Date: Tue, 27 Mar 2018 00:46:22 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6685: Improve profiles in KrpcDataStreamRecvr and KrpcDataStreamSender

[Michael Ho (Code Review)]

Michael Ho has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/9690 )

Change subject: IMPALA-6685: Improve profiles in KrpcDataStreamRecvr and 
KrpcDataStreamSender
..

IMPALA-6685: Improve profiles in KrpcDataStreamRecvr and KrpcDataStreamSender

This change implements a couple of improvements to the profiles of
KrpcDataStreamRecvr and KrpcDataStreamSender:

- track pending number of deferred row batches over time in KrpcDataStreamRecvr
- track the number of bytes dequeued over time in KrpcDataStreamRecvr
- track the total time deferred RPCs queues are not empty
- track the number of bytes sent from KrpcDataStreamSender over time
- track the total amount of time spent in KrpcDataStreamSender, including time
  spent waiting for RPC completion.

Sample profile of an Exchange node instance:

  EXCHANGE_NODE (id=21):(Total: 2s284ms, non-child: 64.926ms, % 
non-child: 2.84%)
 - ConvertRowBatchTime: 44.380ms
 - PeakMemoryUsage: 124.04 KB (127021)
 - RowsReturned: 287.51K (287514)
 - RowsReturnedRate: 125.88 K/sec
Buffer pool:
   - AllocTime: 1.109ms
   - CumulativeAllocationBytes: 10.96 MB (11493376)
   - CumulativeAllocations: 562 (562)
   - PeakReservation: 112.00 KB (114688)
   - PeakUnpinnedBytes: 0
   - PeakUsedReservation: 112.00 KB (114688)
   - ReadIoBytes: 0
   - ReadIoOps: 0 (0)
   - ReadIoWaitTime: 0.000ns
   - WriteIoBytes: 0
   - WriteIoOps: 0 (0)
   - WriteIoWaitTime: 0.000ns
Dequeue:
  BytesDequeued(500.000ms): 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 700.00 KB, 2.00 MB, 3.49 
MB, 4.39 MB, 5.86 MB, 6.85 MB
   - FirstBatchWaitTime: 0.000ns
   - TotalBytesDequeued: 6.85 MB (7187850)
   - TotalGetBatchTime: 2s237ms
 - DataWaitTime: 2s219ms
Enqueue:
  BytesReceived(500.000ms): 0, 23.36 KB, 23.36 KB, 23.36 KB, 23.36 
KB, 23.36 KB, 23.36 KB, 23.36 KB, 23.36 KB, 23.36 KB, 23.36 KB, 23.36 KB, 23.36 
KB, 23.36 KB, 23.36 KB, 23.36 KB, 23.36 KB, 23.36 KB, 23.36 KB, 23.36 KB, 23.36 
KB, 23.36 KB, 23.36 KB, 23.36 KB, 23.36 KB, 23.36 KB, 23.36 KB, 23.36 KB, 23.36 
KB, 23.36 KB, 23.36 KB, 328.73 KB, 963.79 KB, 1.64 MB, 2.09 MB, 2.76 MB, 3.23 MB
  DeferredQueueSize(500.000ms): 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0
   - DispatchTime: (Avg: 108.593us ; Min: 30.525us ; Max: 1.524ms ; 
Number of samples: 281)
   - DeserializeRowBatchTime: 8.395ms
   - TotalBatchesEnqueued: 281 (281)
   - TotalBatchesReceived: 281 (281)
   - TotalBytesReceived: 3.23 MB (3387144)
   - TotalEarlySenders: 0 (0)
   - TotalEosReceived: 1 (1)
   - TotalHasDeferredRPCsTime: 15s446ms
   - TotalRPCsDeferred: 38 (38)

Sample sender's profile:

KrpcDataStreamSender (dst_id=21):(Total: 17s923ms, non-child: 
604.494ms, % non-child: 3.37%)
  BytesSent(500.000ms): 0, 0, 0, 34.78 KB, 34.78 KB, 34.78 KB, 34.78 
KB, 34.78 KB, 34.78 KB, 34.78 KB, 34.78 KB, 34.78 KB, 34.78 KB, 34.78 KB, 34.78 
KB, 34.78 KB, 34.78 KB, 34.78 KB, 34.78 KB, 34.78 KB, 34.78 KB, 34.78 KB, 34.78 
KB, 46.54 KB, 46.54 KB, 46.54 KB, 58.31 KB, 58.31 KB, 58.31 KB, 58.31 KB, 58.31 
KB, 58.31 KB, 58.31 KB, 974.44 KB, 2.82 MB, 4.93 MB, 6.27 MB, 8.28 MB, 9.69 MB
   - EosSent: 3 (3)
   - NetworkThroughput: 4.61 MB/sec
   - PeakMemoryUsage: 22.57 KB (23112)
   - RowsSent: 287.51K (287514)
   - RpcFailure: 0 (0)
   - RpcRetry: 0 (0)
   - SerializeBatchTime: 329.162ms
   - TotalBytesSent: 9.69 MB (10161432)
   - UncompressedRowBatchSize: 20.56 MB (21563550)

Change-Id: I8ba405921b3df920c1e85b940ce9c8d02fc647cd
---
M be/src/kudu/rpc/rpc_context.cc
M be/src/kudu/rpc/rpc_context.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-recvr.cc
M be/src/runtime/krpc-data-stream-recvr.h
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/krpc-data-stream-sender.h
M be/src/runtime/runtime-state.h
M be/src/service/data-stream-service.cc
M be/src/service/data-stream-service.h
M common/protobuf/data_stream_service.proto
12 files changed, 288 insertions(+), 138 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8ba405921b3df920c1e85b940ce9c8d02fc647cd

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

[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 5:

(1 comment)

Thanks, I fixed a few other comments as well.

http://gerrit.cloudera.org:8080/#/c/9693/5/be/src/exec/parquet-column-stats.h
File be/src/exec/parquet-column-stats.h:

http://gerrit.cloudera.org:8080/#/c/9693/5/be/src/exec/parquet-column-stats.h@162
PS5, Line 162: i
> nit: Capital I
Done



--
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: comment
Gerrit-Change-Id: Icbacf7fe3b7672e3ce719261ecef445b16f8dec9
Gerrit-Change-Number: 9693
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 27 Mar 2018 00:41:02 +
Gerrit-HasComments: Yes

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

[Zoltan Borok-Nagy (Code Review)]

Hello Lars Volker, Tim Armstrong, Csaba Ringhofer,

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

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

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

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, 547 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/9693/6
--
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: 6
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 

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

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9788 )

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


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9788/1/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@60
PS1, Line 60: db.toLowerCase()
> I like introducing new DbName since it's pretty consistent with TableName a
In general, toSql() represents SQL of an analyzed statement where all entities 
and databases have been resolved. It's not required that toSql() preserves the 
user input exactly. In fact, it would often be wrong to do that, think

create view v as select * from T;

Before creating the view we need to resolve "t" and fully qualify it. The 
toSql() of the SelectStmt will output something like:

SELECT * from default.t

Notice how casing has changed in various places, and now "t" is fully qualified.



--
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: comment
Gerrit-Change-Id: Id880aa559cec0afe8fbb7d33ccce83f7b5e474cb
Gerrit-Change-Number: 9788
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 27 Mar 2018 00:38:07 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6728: Always use Kudu based kinit if FLAGS use krpc=true

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9797 )

Change subject: IMPALA-6728: Always use Kudu based kinit if FLAGS_use_krpc=true
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7086e5c9b460233e9e957f886141b3e6bba414b
Gerrit-Change-Number: 9797
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 27 Mar 2018 00:33:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6728: Always use Kudu based kinit if FLAGS use krpc=true

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9797 )

Change subject: IMPALA-6728: Always use Kudu based kinit if FLAGS_use_krpc=true
..


Patch Set 3: Code-Review+2

Carry +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7086e5c9b460233e9e957f886141b3e6bba414b
Gerrit-Change-Number: 9797
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 27 Mar 2018 00:32:54 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6728: Always use Kudu based kinit if FLAGS use krpc=true

[Michael Ho (Code Review)]

Hello Sailesh Mukil, Dan Hecht,

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

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

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

Change subject: IMPALA-6728: Always use Kudu based kinit if FLAGS_use_krpc=true
..

IMPALA-6728: Always use Kudu based kinit if FLAGS_use_krpc=true

We rely on the KPRC logic to do the Kerberos authentication
when KRPC is enabled. Therefore, when FLAGS_ues_krpc=true,
we must always call kudu::security::InitKerberosForServer()
to initialize the Kerberos related logic. This change makes
Impala ignore FLAGS_use_kudu_kinit=false when FLAGS_use_krpc=true.

Change-Id: Ia7086e5c9b460233e9e957f886141b3e6bba414b
---
M be/src/rpc/auth-provider.h
M be/src/rpc/authentication.cc
M be/src/rpc/rpc-mgr-kerberized-test.cc
M be/src/rpc/rpc-mgr-test-base.h
M be/src/rpc/thrift-server-test.cc
M be/src/testutil/mini-kdc-wrapper.cc
M be/src/testutil/mini-kdc-wrapper.h
7 files changed, 146 insertions(+), 113 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7086e5c9b460233e9e957f886141b3e6bba414b
Gerrit-Change-Number: 9797
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-6728: Always use Kudu based kinit if FLAGS use krpc=true

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9797 )

Change subject: IMPALA-6728: Always use Kudu based kinit if FLAGS_use_krpc=true
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9797/2/be/src/rpc/rpc-mgr-kerberized-test.cc
File be/src/rpc/rpc-mgr-kerberized-test.cc:

http://gerrit.cloudera.org:8080/#/c/9797/2/be/src/rpc/rpc-mgr-kerberized-test.cc@57
PS2, Line 57: // TODO: IMPALA-6477: This test breaks on CentOS 6.4. Re-enable 
after a fix.
This can be removed.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7086e5c9b460233e9e957f886141b3e6bba414b
Gerrit-Change-Number: 9797
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 27 Mar 2018 00:32:24 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Use "mvn -B" in builds to avoid dowloading progress bars in logs.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9792 )

Change subject: Use "mvn -B" in builds to avoid dowloading progress bars in 
logs.
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa117272c2a86b63b0f9062099a4145324eb6fc
Gerrit-Change-Number: 9792
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Tue, 27 Mar 2018 00:19:46 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6314: Add run time scalar subquery check for uncorrelated subqueries

[Zoltan Borok-Nagy (Code Review)]

Hello Attila Jeges, Dimitris Tsirogiannis, Tim Armstrong, Csaba Ringhofer, Alex 
Behm, Vuk Ercegovac,

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

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

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

Change subject: IMPALA-6314: Add run time scalar subquery check for 
uncorrelated subqueries
..

IMPALA-6314: Add run time scalar subquery check for uncorrelated subqueries

If a scalar subquery is used with a binary predicate,
or, used in an arithmetic expression, it must return
only one row/column to be valid. If this cannot be
guaranteed at parse time through a single row aggregate
or limit clause, Impala fails the query like such.

E.g., currently the following query is not allowed:
SELECT bigint_col
FROM alltypesagg
WHERE id = (SELECT id FROM alltypesagg WHERE id = 1)

However, it would be allowed if the query contained
a LIMIT 1 clause, or instead of id it was max(id).

This commit makes the example valid by introducing a
runtime check to test if the subquery returns a single
row. If the subquery returns more than one row, it
aborts the query with an error.

I added a new node type, called CardinalityCheckNode. It
is created during planning on top of the subquery when
needed, then during execution it checks if its child
only returns a single row.

I extended the frontend tests and e2e tests as well.

Change-Id: I0f52b93a60eeacedd242a2f17fa6b99c4fc38e06
---
M be/src/exec/CMakeLists.txt
A be/src/exec/cardinality-check-node.cc
A be/src/exec/cardinality-check-node.h
M be/src/exec/exec-node.cc
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/HdfsCachingOp.java
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
M fe/src/main/java/org/apache/impala/analysis/Subquery.java
A fe/src/main/java/org/apache/impala/planner/CardinalityCheckNode.java
M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
M testdata/workloads/functional-query/queries/QueryTest/subquery.test
27 files changed, 912 insertions(+), 64 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/9005/17
--
To view, visit http://gerrit.cloudera.org:8080/9005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0f52b93a60eeacedd242a2f17fa6b99c4fc38e06
Gerrit-Change-Number: 9005
Gerrit-PatchSet: 17
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zoltan Borok-Nagy 

[Impala-ASF-CR] IMPALA-6314: Add run time scalar subquery check for uncorrelated subqueries

[Zoltan Borok-Nagy (Code Review)]

Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9005 )

Change subject: IMPALA-6314: Add run time scalar subquery check for 
uncorrelated subqueries
..


Patch Set 11:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9005/11/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
File fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java:

http://gerrit.cloudera.org:8080/#/c/9005/11/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java@175
PS11, Line 175:   StmtRewriter.rewriteNonScalarSubqueries(operand, 
analyzer);
> Thanks for making these changes. I did not look at all failing tests, but I
Thanks!

At some point I was suspecting that some code parts might rely on the exact 
type of a subquery. So I made the type of the subquery ArrayType in InPredicate 
and ExistsPredicate. But, it didn't seem to solve the problem, so I removed the 
ArrayTypes.

Now that you mentioned it, I figured out that I forgot to "transfer" the exact 
type of the subquery to the new subquery in StmtRewriter.rewriteExpr().

I also tried your version of Expr.isScalarSubquery(), but I got a bunch of 
IllegalStateExceptions because the limit element wasn't analyzed (you already 
wrote a comment about it, but still I couldn't always make it analyzed, and my 
other approach already worked).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f52b93a60eeacedd242a2f17fa6b99c4fc38e06
Gerrit-Change-Number: 9005
Gerrit-PatchSet: 11
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 27 Mar 2018 00:02:59 +
Gerrit-HasComments: Yes

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

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9788 )

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


Patch Set 2:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9788/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3076
PS2, Line 3076: req.getDb_name().toLowerCase()
put the lower-case into the refreshFunctions method. that will make it 
consistent with the toLowerCase's in the other CatalogServiceCatalog and 
Catalog methods (they do the toLowerCase in the method, so its not expected for 
the caller to do so).


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

http://gerrit.cloudera.org:8080/#/c/9788/2/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java@424
PS2, Line 424: AnalyzesOk(String.format("invalidate metadata %s.%s", 
dbName, tableName));
just checking that this is testing what's intended, but without the fix, these 
fail, correct?



--
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: comment
Gerrit-Change-Id: Id880aa559cec0afe8fbb7d33ccce83f7b5e474cb
Gerrit-Change-Number: 9788
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 26 Mar 2018 23:47:37 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [experimental] Clang Tidy Diff trial balloon

[Joe McDonnell (Code Review)]

Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9751 )

Change subject: [experimental] Clang Tidy Diff trial balloon
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9751/1//COMMIT_MSG@22
PS1, Line 22: The main change is to find a way to get the
> I like your last suggestion as an alternative to doing #3 first. Maybe it w
Changed the names of the clang-tidy diff scripts and make targets to make the 
distinction clearer. Added some comments in clang_tidy_diff.py and 
run_clang_tidy.sh to say clearly what they are doing.

I'm looking into compilation flags we can enable. I will do a run with several 
flags disabled and see what pops.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2fb6a13400367fd3d12a4738bbb2dfc944466a7
Gerrit-Change-Number: 9751
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 26 Mar 2018 23:40:49 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [experimental] Clang Tidy Diff trial balloon

[Joe McDonnell (Code Review)]

Hello Jim Apple, Philip Zeyliger, Tim Armstrong,

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

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

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

Change subject: [experimental] Clang Tidy Diff trial balloon
..

[experimental] Clang Tidy Diff trial balloon

Kudu has a nice script that does clang tidy on
a diff rather than the whole source code. Kudu
uses this for a Tidy Bot that comments on Gerrit
reviews if the code change introduces an issue.

This is an experimental change to bring those
scripts over and adapt them to Impala. The
Kudu scripts are mostly unchanged from the
Kudu versions. The changes are:
 - The scripts are in ${IMPALA_HOME}/bin,
   requiring corresponding path fixups.
 - clang_tidy_gerrit.py has been renamed to
   clang_tidy_diff.py to make a distinction
   between this clang-tidy functionality
   and the existing run_clang_tidy.sh. For the same
   reason, tidy.sh is now tidy-diff.sh and the
   Kudu compile target "make tidy" is now
   "make tidy-diff".
 - bin/clang_tidy_diff.py use Impala's locations
   for clang-tidy and clang-tidy-diff.py
 - All of the clang-tidy diff includes comments
   indicating it is experimental.

The main change is to find a way to get the
appropriate compilation flags for clang. This is
hard-coded in Kudu, but our build is more
complicated. This pulls the appropriate compile
flags from CMake's internal variables and uses
them to populate a templated python file
(bin/compile_flags.py.template) that can be
imported by clang_tidy_diff.py.

The script can be invoked directly, but this
change also adds a CMake target (stolen from
Kudu). A developer can run "make tidy-diff" and
the script will be invoked for changes since
the last upstream commit.

Change-Id: Ie2fb6a13400367fd3d12a4738bbb2dfc944466a7
---
M CMakeLists.txt
M be/CMakeLists.txt
A bin/clang_tidy_diff.py
A bin/compile_flags.py.template
A bin/get-upstream-commit.sh
M bin/run_clang_tidy.sh
A bin/tidy-diff.sh
M cmake_modules/FindKRPC.cmake
8 files changed, 366 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie2fb6a13400367fd3d12a4738bbb2dfc944466a7
Gerrit-Change-Number: 9751
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6338: Fix flaky test profile fragment instances

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9718 )

Change subject: IMPALA-6338: Fix flaky test_profile_fragment_instances
..


Patch Set 1:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/9718/1//COMMIT_MSG@7
PS1, Line 7: Fix flaky test_profile_fragment_instances
This subject makes it sound like you're fixing the flaky test. But you're 
actually making an impala change, so could you reword this to summarize the 
change?

Otherwise, when looking at git commits, it's hard to know that this change is 
making a subtle coordinator behavior change.


http://gerrit.cloudera.org:8080/#/c/9718/1//COMMIT_MSG@43
PS1, Line 43: Returning results for queries that are cancelled by the user is
: unaffected as the manual cancel path causes 
WaitForBackendCompletion().
I don't follow that. What does this have to do with query results?  Also, what 
do you mean by the "path causes WaitForBackendCompletion()". Which callpath are 
you referring to?


http://gerrit.cloudera.org:8080/#/c/9718/1/be/src/runtime/coordinator-backend-state.h
File be/src/runtime/coordinator-backend-state.h:

http://gerrit.cloudera.org:8080/#/c/9718/1/be/src/runtime/coordinator-backend-state.h@119
PS1, Line 119:   boost::mutex* lock() { return _; }
> I don't think it will help with the iteration part, but it'll prevent mista
I think we should try hard to not expose the lock out of a class. That's a big 
problem with ClientRequestState.  The fact that it's exposed usually means some 
abstraction is missing or wrong.

(If we really do end up needing to take the series of locks, then it'd be good 
to follow the pattern here: 
https://github.com/apache/impala/blob/dc2f69e5a0b36ca721eeadeec661a251c957410b/be/src/runtime/bufferpool/buffer-allocator.cc#L341)


http://gerrit.cloudera.org:8080/#/c/9718/1/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/9718/1/be/src/runtime/coordinator.cc@1028
PS1, Line 1028:   // ensure the profile isn't being updated when we call 
Add(Split|Exec)Stats below.
What are the backend locks protecting in Add*Stats()?  Which profile is the 
comment referring to?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f8295e4ec580b476f7668c77aec1348fb2e868c
Gerrit-Change-Number: 9718
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Mon, 26 Mar 2018 23:36:03 +
Gerrit-HasComments: Yes

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

[Fredy Wijaya (Code Review)]

Fredy Wijaya has uploaded a new patch set (#2). ( 
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:
- Ran front-end tests.

Change-Id: Id880aa559cec0afe8fbb7d33ccce83f7b5e474cb
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
2 files changed, 15 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/9788/2
--
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: newpatchset
Gerrit-Change-Id: Id880aa559cec0afe8fbb7d33ccce83f7b5e474cb
Gerrit-Change-Number: 9788
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR](2.x) IMPALA-6743: bump from 2.11 to 2.12

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9809 )

Change subject: IMPALA-6743: bump from 2.11 to 2.12
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaeaf230bf6f0cbf299edd4cf5ede4cb808523f1c
Gerrit-Change-Number: 9809
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Mon, 26 Mar 2018 23:02:27 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6728: Always use Kudu based kinit if FLAGS use krpc=true

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9797 )

Change subject: IMPALA-6728: Always use Kudu based kinit if FLAGS_use_krpc=true
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7086e5c9b460233e9e957f886141b3e6bba414b
Gerrit-Change-Number: 9797
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 26 Mar 2018 22:56:33 +
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-6743: bump from 2.11 to 2.12

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9809 )

Change subject: IMPALA-6743: bump from 2.11 to 2.12
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaeaf230bf6f0cbf299edd4cf5ede4cb808523f1c
Gerrit-Change-Number: 9809
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Mon, 26 Mar 2018 22:47:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6731: Use private index in bootstrap virtualenv

[Lars Volker (Code Review)]

Lars Volker has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9798 )

Change subject: IMPALA-6731: Use private index in bootstrap_virtualenv
..

IMPALA-6731: Use private index in bootstrap_virtualenv

This change switches to using a private pypi index url when using a
private pypi mirror. This allows to run the tests without relying on the
public Python pypi mirrors.

Some packages can not detect their dependencies correctly when they get
installed together with the dependencies in the same call to pip. This
change adds a second stage of package installation to separate these
packages from their dependencies.

It also adds a few missing packages and updates some packages to newer
versions.

Testing: Ran this on a box where I blocked DNS resolution to Python's
upstream pypi.

Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
Reviewed-on: http://gerrit.cloudera.org:8080/9798
Reviewed-by: Philip Zeyliger 
Tested-by: Lars Volker 
---
M infra/python/bootstrap_virtualenv.py
M infra/python/deps/compiled-requirements.txt
M infra/python/deps/pip_download.py
M infra/python/deps/requirements.txt
A infra/python/deps/stage2-requirements.txt
5 files changed, 71 insertions(+), 15 deletions(-)

Approvals:
  Philip Zeyliger: Looks good to me, approved
  Lars Volker: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
Gerrit-Change-Number: 9798
Gerrit-PatchSet: 5
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 

[Impala-ASF-CR] IMPALA-6731: Use private index in bootstrap virtualenv

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9798 )

Change subject: IMPALA-6731: Use private index in bootstrap_virtualenv
..


Patch Set 4: Verified+1

PS1 had passed a private build. Since then only comments have changed.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
Gerrit-Change-Number: 9798
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 26 Mar 2018 22:44:57 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6731: Use private index in bootstrap virtualenv

[Philip Zeyliger (Code Review)]

Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9798 )

Change subject: IMPALA-6731: Use private index in bootstrap_virtualenv
..


Patch Set 4: Code-Review+2

To the extent this is facilitating fixing builds, I think this makes sense.

David: do we have a JIRA for thinking about this from more first principles? I 
think you filed one recently? If we can get rid of bootstrap_virtualenv, I'm 
sure that's preferable.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
Gerrit-Change-Number: 9798
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 26 Mar 2018 22:43:08 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6614: ClientRequestState should use HS2 TOperationState

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9501 )

Change subject: IMPALA-6614: ClientRequestState should use HS2 TOperationState
..

IMPALA-6614: ClientRequestState should use HS2 TOperationState

Currently it uses beeswax's QueryState enum, but the TOperationState
is a superset. In order to remove dependencies on beeswax, and also
set things up for a future change to use the TOperationState explicit
CANCELED_STATE (see IMPALA-1262), migrate CLR to use TOperationState.

The intent of this change is to make no client visible change.

Change-Id: I36287eaf8f1dac23c306b470f95f379dfdc6bb5b
Reviewed-on: http://gerrit.cloudera.org:8080/9501
Reviewed-by: Dan Hecht 
Tested-by: Impala Public Jenkins
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
6 files changed, 84 insertions(+), 67 deletions(-)

Approvals:
  Dan Hecht: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I36287eaf8f1dac23c306b470f95f379dfdc6bb5b
Gerrit-Change-Number: 9501
Gerrit-PatchSet: 7
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-6614: ClientRequestState should use HS2 TOperationState

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9501 )

Change subject: IMPALA-6614: ClientRequestState should use HS2 TOperationState
..


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I36287eaf8f1dac23c306b470f95f379dfdc6bb5b
Gerrit-Change-Number: 9501
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 26 Mar 2018 22:40:43 +
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-6743: bump from 2.11 to 2.12

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9809


Change subject: IMPALA-6743: bump from 2.11 to 2.12
..

IMPALA-6743: bump from 2.11 to 2.12

Next release is 2.12 so update the 2.x branch to
refer to 2.12 (2.11 has already been released).

Change-Id: Iaeaf230bf6f0cbf299edd4cf5ede4cb808523f1c
---
M bin/save-version.sh
1 file changed, 1 insertion(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaeaf230bf6f0cbf299edd4cf5ede4cb808523f1c
Gerrit-Change-Number: 9809
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 

[Impala-ASF-CR] KUDU-2374: Add RpcContext::GetTimeReceived()

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9807 )

Change subject: KUDU-2374: Add RpcContext::GetTimeReceived()
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b39c7f2ea856eccfdab8c1bb1433829e979ae13
Gerrit-Change-Number: 9807
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Mon, 26 Mar 2018 22:37:04 +
Gerrit-HasComments: No

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

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9758 )

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


Patch Set 5: Verified+1


--
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: 5
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 26 Mar 2018 22:26:11 +
Gerrit-HasComments: No

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

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9758 )

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

IMPALA-6715,IMPALA-6736: fix stress TPC workload selection

IMPALA-6715:
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.

- While testing on hardware clusters down stream, I noticed...

IMPALA-6736:
  TPC-DS Q67A is 10x more expensive to run without spilling than any
  other query. I fixed the --filter-query-mem-ratio option to work. This
  will still run Q67A during the binary search phase, but if a cluster
  is too small, the query will be skipped.

Change-Id: I3e26b64d38aa8d63a176daf95c4ac5dee89508da
Reviewed-on: http://gerrit.cloudera.org:8080/9758
Reviewed-by: David Knupp 
Tested-by: Impala Public Jenkins
---
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, 88 insertions(+), 27 deletions(-)

Approvals:
  David Knupp: Looks good to me, approved
  Impala Public Jenkins: Verified

--
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: merged
Gerrit-Change-Id: I3e26b64d38aa8d63a176daf95c4ac5dee89508da
Gerrit-Change-Number: 9758
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6724: Incorrect exception handling in create/drop function statements

[Fredy Wijaya (Code Review)]

Fredy Wijaya has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/9800 )

Change subject: IMPALA-6724: Incorrect exception handling in create/drop 
function statements
..

IMPALA-6724: Incorrect exception handling in create/drop function statements

This patch removes restriction on creating a function with the same name
as the built-in function. The reason for lifting the reestriction is to
avoid to name clash when introducing new built-in functions. The patch
also fixes some inconsistent behavior when creating or dropping function
whether the name specified is fully-qualified or not.

Refer to the below tables for more information.

Create function:
+-+-+-+---+---+
| FQ Name | Built-in DB | Function Name   | Existing Behavior   
  | New Behavior  |
+-+-+-+---+---+
| Yes | Yes | Same as built-in| Same name exception 
  | Cannot modify system database |
| Yes | Yes | Different than built-in | Cannot modify system 
database | Cannot modify system database |
| Yes | No  | Same as built-in| Function created
  | Function created  |
| Yes | No  | Different than built-in | Function created
  | Function created  |
| No  | Yes | Same as built-in| Same name exception 
  | Cannot modify system database |
| No  | Yes | Different than built-in | Cannot modify system 
database | Cannot modify system database |
| No  | No  | Same as built-in| Same name exception 
  | Function created  |
| No  | No  | Different than built-in | Function created
  | Function created  |
+-+-+-+---+---+

Drop function:
+-+-+-+---+---+
| FQ Name | Built-in DB | Function Name   | Existing Behavior   
  | New Behavior  |
+-+-+-+---+---+
| Yes | Yes | Same as built-in| Cannot modify system 
database | Cannot modify system database |
| Yes | Yes | Different than built-in | Cannot modify system 
database | Cannot modify system database |
| Yes | No  | Same as built-in| Function dropped
  | Function dropped  |
| Yes | No  | Different than built-in | Function dropped
  | Function dropped  |
| No  | Yes | Same as built-in| Cannot modify system 
database | Cannot modify system database |
| No  | Yes | Different than built-in | Cannot modify system 
database | Cannot modify system database |
| No  | No  | Same as built-in| Cannot modify system 
database | Function dropped  |
| No  | No  | Different than built-in | Function dropped
  | Function dropped  |
+-+-+-+---+---+

Select function (no new behavior):
+-+-+-++
| FQ Name | Built-in DB | Function Name   | Behavior
   |
+-+-+-++
| Yes | Yes | Same as built-in| Function in the specified 
database (built-in) executed |
| Yes | Yes | Different than built-in | Unknown function exception  
   |
| Yes | No  | Same as built-in| Function in the specified 
database executed|
| Yes | No  | Different than built-in | Function in the specified 
database executed|
| No  | Yes | Same as built-in| Built-in function executed  
   |
| No  | Yes | Different than built-in | Unknown function exception  
   |
| No  | No  | Same as built-in| Built-in function executed  
   |
| No  | No  | Different than built-in | Function in the current 
database executed  |
+-+-+-++

Testing:
- Ran front-end tests

Cherry-picks: not for 2.x

Change-Id: 

[Impala-ASF-CR] IMPALA-6724: Incorrect exception handling in create/drop function statements

[Fredy Wijaya (Code Review)]

Fredy Wijaya has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/9800 )

Change subject: IMPALA-6724: Incorrect exception handling in create/drop 
function statements
..

IMPALA-6724: Incorrect exception handling in create/drop function statements

This patch removes restriction on creating a function with the same name
as the built-in function. The reason for lifting the reestriction is to
avoid to name clash when introducing new built-in functions. The patch
also fixes some inconsistent behavior when creating or dropping function
whether the name specified is fully-qualified or not.

Refer to the below tables for more information.

Create function:
+---+-+---+---+
| FQ Name | Built-in DB | Function Name   | Existing Behavior   
  | New Behavior  |
+---+-+---+---+
| Yes | Yes | Same as built-in| Same name exception 
  | Cannot modify system database |
| Yes | Yes | Different than built-in | Cannot modify system 
database | Cannot modify system database |
| Yes | No  | Same as built-in| Function created
  | Function created  |
| Yes | No  | Different than built-in | Function created
  | Function created  |
| No  | Yes | Same as built-in| Same name exception 
  | Cannot modify system database |
| No  | Yes | Different than built-in | Cannot modify system 
database | Cannot modify system database |
| No  | No  | Same as built-in| Same name exception 
  | Function created  |
| No  | No  | Different than built-in | Function created
  | Function created  |
+-+-+-+---+---+

Drop function:
+---+-+---+---+
| FQ Name | Built-in DB | Function Name   | Existing Behavior   
  | New Behavior  |
+---+-+---+---+
| Yes | Yes | Same as built-in| Cannot modify system 
database | Cannot modify system database |
| Yes | Yes | Different than built-in | Cannot modify system 
database | Cannot modify system database |
| Yes | No  | Same as built-in| Function dropped
  | Function dropped  |
| Yes | No  | Different than built-in | Function dropped
  | Function dropped  |
| No  | Yes | Same as built-in| Cannot modify system 
database | Cannot modify system database |
| No  | Yes | Different than built-in | Cannot modify system 
database | Cannot modify system database |
| No  | No  | Same as built-in| Cannot modify system 
database | Function dropped  |
| No  | No  | Different than built-in | Function dropped
  | Function dropped  |
+-+-+-+---+---+

Select function (no new behavior):
+---+-++
| FQ Name | Built-in DB | Function Name   | Behavior
   |
+---+-++
| Yes | Yes | Same as built-in| Function in the specified 
database (built-in) executed |
| Yes | Yes | Different than built-in | Unknown function exception  
   |
| Yes | No  | Same as built-in| Function in the specified 
database executed|
| Yes | No  | Different than built-in | Function in the specified 
database executed|
| No  | Yes | Same as built-in| Built-in function executed  
   |
| No  | Yes | Different than built-in | Unknown function exception  
   |
| No  | No  | Same as built-in| Built-in function executed  
   |
| No  | No  | Different than built-in | Function in the current 
database executed  |
+-+-+-++

Testing:
- Ran front-end tests

Cherry-picks: not for 2.x

Change-Id: 

[Impala-ASF-CR] KUDU-2374: Add RpcContext::GetTimeReceived()

[Joe McDonnell (Code Review)]

Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9807 )

Change subject: KUDU-2374: Add RpcContext::GetTimeReceived()
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b39c7f2ea856eccfdab8c1bb1433829e979ae13
Gerrit-Change-Number: 9807
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Mon, 26 Mar 2018 21:41:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2374: Add RpcContext::GetTimeReceived()

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9807 )

Change subject: KUDU-2374: Add RpcContext::GetTimeReceived()
..


Patch Set 1:

Clean cherry-pick.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b39c7f2ea856eccfdab8c1bb1433829e979ae13
Gerrit-Change-Number: 9807
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Mon, 26 Mar 2018 21:38:40 +
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2374: Add RpcContext::GetTimeReceived()

[Michael Ho (Code Review)]

Michael Ho has removed Todd Lipcon from this change.  ( 
http://gerrit.cloudera.org:8080/9807 )

Change subject: KUDU-2374: Add RpcContext::GetTimeReceived()
..


Removed reviewer Todd Lipcon.
--
To view, visit http://gerrit.cloudera.org:8080/9807
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I6b39c7f2ea856eccfdab8c1bb1433829e979ae13
Gerrit-Change-Number: 9807
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 

[Impala-ASF-CR] KUDU-2374: Add RpcContext::GetTimeReceived()

[Michael Ho (Code Review)]

Michael Ho has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/9807 )

Change subject: KUDU-2374: Add RpcContext::GetTimeReceived()
..


Removed reviewer Kudu Jenkins.
--
To view, visit http://gerrit.cloudera.org:8080/9807
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I6b39c7f2ea856eccfdab8c1bb1433829e979ae13
Gerrit-Change-Number: 9807
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Todd Lipcon 

[Impala-ASF-CR] KUDU-2374: Add RpcContext::GetTimeReceived()

[Michael Ho (Code Review)]

Hello Kudu Jenkins, Todd Lipcon,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: KUDU-2374: Add RpcContext::GetTimeReceived()
..

KUDU-2374: Add RpcContext::GetTimeReceived()

This change adds RpcContext::GetTimeReceived() which returns
the time at which the inbound call associated with the RpcContext
was received. It's helpful to make this accessible to the RPC
handlers for its own book-keeping purpose (e.g. reporting the
average dispatch latency as part of query profile in Impala).

Change-Id: I6b39c7f2ea856eccfdab8c1bb1433829e979ae13
Reviewed-on: http://gerrit.cloudera.org:8080/9796
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M be/src/kudu/rpc/rpc-test-base.h
M be/src/kudu/rpc/rpc_context.cc
M be/src/kudu/rpc/rpc_context.h
3 files changed, 10 insertions(+), 0 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6b39c7f2ea856eccfdab8c1bb1433829e979ae13
Gerrit-Change-Number: 9807
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[Impala-ASF-CR] IMPALA-6728: Always use Kudu based kinit if FLAGS use krpc=true

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9797 )

Change subject: IMPALA-6728: Always use Kudu based kinit if FLAGS_use_krpc=true
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9797/1/be/src/rpc/thrift-server-test.cc
File be/src/rpc/thrift-server-test.cc:

http://gerrit.cloudera.org:8080/#/c/9797/1/be/src/rpc/thrift-server-test.cc@106
PS1, Line 106: FLAGS_prinicpal
> FLAGS_principal
Done


http://gerrit.cloudera.org:8080/#/c/9797/1/be/src/rpc/thrift-server-test.cc@112
PS1, Line 112:   FLAGS_be_principal = "";
> how does this work to turn off kerberos?
Impala internally relies on !FLAGS_principal.empty() to infer whether Kerberos 
is enabled. Please see 
https://github.com/apache/impala/blob/master/be/src/util/auth-util.cc#L103-L105


http://gerrit.cloudera.org:8080/#/c/9797/1/be/src/testutil/mini-kdc-wrapper.cc
File be/src/testutil/mini-kdc-wrapper.cc:

http://gerrit.cloudera.org:8080/#/c/9797/1/be/src/testutil/mini-kdc-wrapper.cc@a68
PS1, Line 68:
> related to other question: why don't we need to skip this stuff still when
The goal is to keep this function simple by just being responsible for setting 
up the KDC. It's okay to start KDC but not enable Kerberos in Impala. The 
previous logic to mix in the KerberosSwitch seems a bit unnecessary as this 
kind of FLAGS configuration should be left in the BE tests themselves.


http://gerrit.cloudera.org:8080/#/c/9797/1/be/src/testutil/mini-kdc-wrapper.cc@a89
PS1, Line 89:
:
The initialization of these flags are moved to the BE tests instead.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7086e5c9b460233e9e957f886141b3e6bba414b
Gerrit-Change-Number: 9797
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 26 Mar 2018 20:56:25 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6728: Always use Kudu based kinit if FLAGS use krpc=true

[Michael Ho (Code Review)]

Hello Sailesh Mukil, Dan Hecht,

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

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

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

Change subject: IMPALA-6728: Always use Kudu based kinit if FLAGS_use_krpc=true
..

IMPALA-6728: Always use Kudu based kinit if FLAGS_use_krpc=true

We rely on the KPRC logic to do the Kerberos authentication
when KRPC is enabled. Therefore, when FLAGS_ues_krpc=true,
we must always call kudu::security::InitKerberosForServer()
to initialize the Kerberos related logic. This change makes
Impala ignore FLAGS_use_kudu_kinit=false when FLAGS_use_krpc=true.

Change-Id: Ia7086e5c9b460233e9e957f886141b3e6bba414b
---
M be/src/rpc/auth-provider.h
M be/src/rpc/authentication.cc
M be/src/rpc/rpc-mgr-kerberized-test.cc
M be/src/rpc/rpc-mgr-test-base.h
M be/src/rpc/thrift-server-test.cc
M be/src/testutil/mini-kdc-wrapper.cc
M be/src/testutil/mini-kdc-wrapper.h
7 files changed, 146 insertions(+), 112 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7086e5c9b460233e9e957f886141b3e6bba414b
Gerrit-Change-Number: 9797
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-6731: Use private index in bootstrap virtualenv

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9798 )

Change subject: IMPALA-6731: Use private index in bootstrap_virtualenv
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9798/2/infra/python/bootstrap_virtualenv.py
File infra/python/bootstrap_virtualenv.py:

http://gerrit.cloudera.org:8080/#/c/9798/2/infra/python/bootstrap_virtualenv.py@154
PS2, Line 154: cmd.append("--no-index")
> I was under the impression that infra/python/deps/download_requirements ->
My understanding is that we're not relying on an external index to obtain the 
packages themselves but do rely on it for dependency resolution. I.e., we're 
making all dependency decisions based on the index, and then expect the 
resulting file to exist locally.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
Gerrit-Change-Number: 9798
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 26 Mar 2018 20:36:13 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6731: Use private index in bootstrap virtualenv

[David Knupp (Code Review)]

David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9798 )

Change subject: IMPALA-6731: Use private index in bootstrap_virtualenv
..


Patch Set 4:

I've always been a little perplexed by the complexity of the way we manage our 
python environments. This review seems like a good context to bring this up.

>From past experience, a familiar pattern to me would be set up the environment 
>once, use "pip freeze" to generate a requirements.txt, and then use that 
>requirements.txt to set up future environments as needed.

As an experiment, I took a centos69 machine with python 2.6.6 installed. I had 
to make sure that python-devel and cyrus-sasl-devel were installed, and that 
the Kudu repo was added to /etc/yum.repos.d/. Then I went to machine which has 
an Impala dev environment set up, and pulled this list:

  $ pip freeze
  You are using pip version 7.1.0, however version 9.0.3 is available.
  You should consider upgrading via the 'pip install --upgrade pip' command.
  AllPairs==2.0.1
  apipkg==1.4
  argparse==1.4.0
  bitarray==0.8.1
  boto3==1.2.3
  botocore==1.3.30
  cm-api==10.0.0
  Cython==0.23.4
  docopt==0.6.2
  docutils==0.12
  ecdsa==0.13
  execnet==1.4.0
  Fabric==1.10.2
  Flask==0.10.1
  futures==3.0.5
  hdfs==2.0.2
  impyla==0.14.0
  ipython==1.2.1
  itsdangerous==0.24
  Jinja2==2.8
  jmespath==0.9.0
  kazoo==2.2.1
  kudu-python==1.2.0
  MarkupSafe==0.23
  numpy==1.10.4
  ordereddict==1.1
  paramiko==1.15.2
  pbr==1.8.1
  pexpect==3.3
  pg8000==1.10.2
  prettytable==0.7.2
  psutil==0.7.1
  py==1.4.32
  pycrypto==2.6.1
  pyparsing==2.0.3
  pytest==2.9.2
  pytest-random==0.2
  pytest-xdist==1.15.0
  python-dateutil==2.5.2
  python-magic==0.4.11
  pytz==2018.3
  pywebhdfs==0.3.2
  requests==2.7.0
  sasl==0.1.3
  setuptools-scm==1.15.4
  sh==1.11
  simplejson==3.3.0
  six==1.9.0
  sqlparse==0.1.15
  texttable==0.8.3
  thrift==0.9.0
  thrift-sasl==0.1.0
  virtualenv==13.1.0
  Werkzeug==0.11.3

I saved this list (minus the warning about the outdated pip version) to a 
req.txt file on my test centos6.9 box. Then I created a virtualenv, and 
installed all of the packages into it with one command:

  $ pip install -i https://pypi.infra.cloudera.com/api/pypi/pypi-public/simple 
-r req.txt

This seems successful, which I checked by running "pip freeze" in the new 
environment:

  (pip_test5) [systest@impala-centos6-client ~]$ pip freeze
  DEPRECATION: Python 2.6 is no longer supported by the Python core team, 
please upgrade your Python. A future version of pip will drop support for 
Python 2.6
  AllPairs==2.0.1
  apipkg==1.4
  argparse==1.4.0
  bitarray==0.8.1
  boto3==1.2.3
  botocore==1.3.30
  cm-api==10.0.0
  Cython==0.23.4
  docopt==0.6.2
  docutils==0.12
  ecdsa==0.13
  execnet==1.4.0
  Fabric==1.10.2
  Flask==0.10.1
  futures==3.0.5
  hdfs==2.0.2
  impyla==0.14.0
  ipython==1.2.1
  itsdangerous==0.24
  Jinja2==2.8
  jmespath==0.9.0
  kazoo==2.2.1
  kudu-python==1.2.0
  MarkupSafe==0.23
  numpy==1.10.4
  ordereddict==1.1
  paramiko==1.15.2
  pbr==1.8.1
  pexpect==3.3
  pg8000==1.10.2
  prettytable==0.7.2
  psutil==0.7.1
  py==1.4.32
  pycrypto==2.6.1
  pyparsing==2.0.3
  pytest==2.9.2
  pytest-random==0.2
  pytest-xdist==1.15.0
  python-dateutil==2.5.2
  python-magic==0.4.11
  pytz==2018.3
  pywebhdfs==0.3.2
  requests==2.7.0
  sasl==0.1.3
  setuptools-scm==1.15.4
  sh==1.11
  simplejson==3.3.0
  six==1.9.0
  sqlparse==0.1.15
  texttable==0.8.3
  thrift==0.9.0
  thrift-sasl==0.1.0
  virtualenv==13.1.0
  Werkzeug==0.11.3

Would something like this work for us?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
Gerrit-Change-Number: 9798
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 26 Mar 2018 20:19:25 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6649: Add fine-graned ALTER privilege

[Fredy Wijaya (Code Review)]

Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9805 )

Change subject: IMPALA-6649: Add fine-graned ALTER privilege
..


Patch Set 1:

(16 comments)

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

http://gerrit.cloudera.org:8080/#/c/9805/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-6649: Add fine-graned ALTER privilege
typo: grained


http://gerrit.cloudera.org:8080/#/c/9805/1//COMMIT_MSG@9
PS1, Line 9: Updated support and analysis files to provide ALTER privilege.
We need ALTER on server scope as well.


http://gerrit.cloudera.org:8080/#/c/9805/1//COMMIT_MSG@11
PS1, Line 11: Example statements:
Please mention the privilege scope.


http://gerrit.cloudera.org:8080/#/c/9805/1//COMMIT_MSG@19
PS1, Line 19: Added ALTER tests to cover scope of existing ALTER tests but in
Make sure to run all front-end tests.


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

http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java@250
PS1, Line 250:   AnalyzesOk(String.format("%s ALTER ON DATABASE functional 
%s myrole", formatArgs));
Add test for server scope.


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

http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@338
PS1, Line 338: sentryService.grantRolePrivilege(USER, roleName, privilege);
Add TestServerLevelAlter similar to TestServerLevelCreate


http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1387
PS1, Line 1387: AuthzOk("ALTER TABLE functional_seq_snap.alltypes RENAME TO 
functional_seq_snap.t1");
I think for RENAME TO, we need a CREATE privilege at the database-level and 
ALTER privilege at the table-level. Please add the comment.


http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1434
PS1, Line 1434: "'hdfs://localhost:20500/test-warehouse/no_access'",
nit: formatting


http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1438
PS1, Line 1438: "'/test-warehouse/no_access'",
nit: formatting


http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1442
PS1, Line 1442: "SET LOCATION '/test-warehouse/no_access'",
nit: formatting


http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1472
PS1, Line 1472: "PARTITION(year=2011, month=3) " +
nit: formatting


http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1473
PS1, Line 1473: "PARTITION(year=2011, month=4) LOCATION 
'/test-warehouse/no_access'",
nit: formatting


http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1475
PS1, Line 1475: "hdfs://localhost:20500/test-warehouse/no_access");
nit: formatting


http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1478
PS1, Line 1478: "'hdfs://localhost:20510/test-warehouse/new_table'",
nit: formatting


http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1480
PS1, Line 1480: "hdfs://localhost:20510/test-warehouse/new_table");
nit: formatting


http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1572
PS1, Line 1572: // ALTER privilege on view only.
Update the comment about required privileges to do RENAME TO



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b25d10a8634829fbe90e308dfc7efc8182fef2d
Gerrit-Change-Number: 9805
Gerrit-PatchSet: 1
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Mon, 26 Mar 2018 19:55:06 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6685: Improve profiles in KrpcDataStreamRecvr and KrpcDataStreamSender

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9690 )

Change subject: IMPALA-6685: Improve profiles in KrpcDataStreamRecvr and 
KrpcDataStreamSender
..


Patch Set 4:

(12 comments)

http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc
File be/src/runtime/krpc-data-stream-recvr.cc:

http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@117
PS4, Line 117: did
does


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@121
PS4, Line 121: did
does


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@207
PS4, Line 207: deferred_rpcs_start_time_ns_
how about calling it: has_deferred_rpcs_start_time_ns_?


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@318
PS4, Line 318:   if (deferred_rpcs_.empty()) {
DCHECK_NE(has_deferred_rpcs_start_time_ns, 0)?


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@320
PS4, Line 320: total_deferred_rpcs_time_
how about calling this: total_has_deferred_rpcs_time_ to make it clearer that 
it's a total of the !isempty() property (as opposed to total over all deferred 
rpcs).


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@642
PS4, Line 642: counter_
sometimes we have "counter_" and sometimes not. Let's try to be consistent to 
make it easier to read.


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@646
PS4, Line 646: time_
okay to ignore, but I think it'd be good to call these timer_ (but calling the 
thing it ultimately aggregates to a "time" e.g. DataWaitTime makes sense to 
me). Otherwise, there's no way to know they should be used with the timer 
macros since they are all Counter* types.


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@673
PS4, Line 673: total_deferred_rpcs_time_
as mentioned earlier, how about total_has_deferred_rpcs_time_ to distinguish 
that this isn't the total time across deferred rpcs, and the fact that it 
doesn't really relate to TotalRPCsDeferred.


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-recvr.cc@674
PS4, Line 674: TotalRPCsDeferralTime
TotalHasDeferredRPCsTime


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-sender.h
File be/src/runtime/krpc-data-stream-sender.h:

http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-sender.h@183
PS4, Line 183: .
maybe add: but not meaningful by itself, and therefore not placed in a profile.


http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-sender.cc
File be/src/runtime/krpc-data-stream-sender.cc:

http://gerrit.cloudera.org:8080/#/c/9690/4/be/src/runtime/krpc-data-stream-sender.cc@405
PS4, Line 405:   }
maybe set rpc_start_time_ns_ to 0 here and then DCHECK at that it's not 0 at 
the beginning of this function, to verify it was always set to a valid start 
time for this rpc.


http://gerrit.cloudera.org:8080/#/c/9690/4/common/protobuf/data_stream_service.proto
File common/protobuf/data_stream_service.proto:

http://gerrit.cloudera.org:8080/#/c/9690/4/common/protobuf/data_stream_service.proto@53
PS4, Line 53: receiver
let's clarify that, since it could be intepretted as the data stream recvr 
only.  Maybe say "receiving daemon" or similar.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8ba405921b3df920c1e85b940ce9c8d02fc647cd
Gerrit-Change-Number: 9690
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Comment-Date: Mon, 26 Mar 2018 19:54:05 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6724: Incorrect exception handling in create function statement

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9800 )

Change subject: IMPALA-6724: Incorrect exception handling in create function 
statement
..


Patch Set 3:

Tim, you are correct. We could still break existing scripts that invoke 
unqualified UDFs. The best practice is to always fully-qualify to avoid this.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic30df56ac276970116715c14454a5a2477b185fa
Gerrit-Change-Number: 9800
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 26 Mar 2018 19:53:24 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6731: Use private index in bootstrap virtualenv

[David Knupp (Code Review)]

David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9798 )

Change subject: IMPALA-6731: Use private index in bootstrap_virtualenv
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9798/2/infra/python/bootstrap_virtualenv.py
File infra/python/bootstrap_virtualenv.py:

http://gerrit.cloudera.org:8080/#/c/9798/2/infra/python/bootstrap_virtualenv.py@154
PS2, Line 154: cmd.extend(["--index-url", "%s/simple" % 
os.environ["PYPI_MIRROR"]])
> After more consideration, this should prevent us from fetching additional p
I was under the impression that infra/python/deps/download_requirements -> 
infra/python/deps/pip_download.py was supposed to insure that we aren't relying 
on an external index (either public or private) by the time we get to 
bootstrap_virtualenv.py. Shouldn't we only be installing from pre-downloaded 
packages at this point?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
Gerrit-Change-Number: 9798
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 26 Mar 2018 19:50:53 +
Gerrit-HasComments: Yes

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

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9725 )

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
Reviewed-on: http://gerrit.cloudera.org:8080/9725
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
M be/src/exprs/expr-test.cc
M be/src/runtime/timestamp-parse-util.cc
2 files changed, 53 insertions(+), 16 deletions(-)

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

--
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: merged
Gerrit-Change-Id: Id2ce3ba09256b3996170e42d42d49d12776cab97
Gerrit-Change-Number: 9725
Gerrit-PatchSet: 5
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran 

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

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins 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 4: Verified+1


--
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: 4
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran 
Gerrit-Comment-Date: Mon, 26 Mar 2018 19:49:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6731: Use private index in bootstrap virtualenv

[Lars Volker (Code Review)]

Hello Philip Zeyliger, David Knupp,

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

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

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

Change subject: IMPALA-6731: Use private index in bootstrap_virtualenv
..

IMPALA-6731: Use private index in bootstrap_virtualenv

This change switches to using a private pypi index url when using a
private pypi mirror. This allows to run the tests without relying on the
public Python pypi mirrors.

Some packages can not detect their dependencies correctly when they get
installed together with the dependencies in the same call to pip. This
change adds a second stage of package installation to separate these
packages from their dependencies.

It also adds a few missing packages and updates some packages to newer
versions.

Testing: Ran this on a box where I blocked DNS resolution to Python's
upstream pypi.

Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
---
M infra/python/bootstrap_virtualenv.py
M infra/python/deps/compiled-requirements.txt
M infra/python/deps/pip_download.py
M infra/python/deps/requirements.txt
A infra/python/deps/stage2-requirements.txt
5 files changed, 71 insertions(+), 15 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
Gerrit-Change-Number: 9798
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 

[Impala-ASF-CR] IMPALA-6731: Use private index in bootstrap virtualenv

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9798 )

Change subject: IMPALA-6731: Use private index in bootstrap_virtualenv
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9798/2/infra/python/bootstrap_virtualenv.py
File infra/python/bootstrap_virtualenv.py:

http://gerrit.cloudera.org:8080/#/c/9798/2/infra/python/bootstrap_virtualenv.py@154
PS2, Line 154: cmd.append("--no-index")
> I don't know. Can you tell if it seems to do anything?
After more consideration, this should prevent us from fetching additional 
packages from the index. If we forget to add some to the various 
requirements.txt files, this should trigger an error. However, we still seem to 
access the index for version/dependency resolution, hence we need to change it 
when using a private mirror. I updated the comment.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
Gerrit-Change-Number: 9798
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 26 Mar 2018 19:39:49 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6731: Use private index in bootstrap virtualenv

[Philip Zeyliger (Code Review)]

Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9798 )

Change subject: IMPALA-6731: Use private index in bootstrap_virtualenv
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9798/2/infra/python/bootstrap_virtualenv.py
File infra/python/bootstrap_virtualenv.py:

http://gerrit.cloudera.org:8080/#/c/9798/2/infra/python/bootstrap_virtualenv.py@154
PS2, Line 154: cmd.append("--no-index")
> I don't know, should we remove it?
I don't know. Can you tell if it seems to do anything?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
Gerrit-Change-Number: 9798
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 26 Mar 2018 19:29:44 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6649: Add fine-graned ALTER privilege

[Adam Holley (Code Review)]

Adam Holley has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9805


Change subject: IMPALA-6649: Add fine-graned ALTER privilege
..

IMPALA-6649: Add fine-graned ALTER privilege

Updated support and analysis files to provide ALTER privilege.

Example statements:
GRANT ALTER on DATABASE svr TO ROLE testrole;
GRANT ALTER on TABLE db TO ROLE testrole;

REVOKE ALTER on DATABASE svr FROM ROLE testrole;
REVOKE ALTER on TABLE db FROM ROLE testrole;

Tests:
Added ALTER tests to cover scope of existing ALTER tests but in
the context of only having ALTER privilege.

Change-Id: I0b25d10a8634829fbe90e308dfc7efc8182fef2d
Cherry-picks: not for 2.x
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
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/AuthorizationTest.java
M fe/src/test/resources/authz-policy.ini.template
6 files changed, 113 insertions(+), 6 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0b25d10a8634829fbe90e308dfc7efc8182fef2d
Gerrit-Change-Number: 9805
Gerrit-PatchSet: 1
Gerrit-Owner: Adam Holley 

[Impala-ASF-CR] IMPALA-6724: Incorrect exception handling in create function statement

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9800 )

Change subject: IMPALA-6724: Incorrect exception handling in create function 
statement
..


Patch Set 3:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9800/3//COMMIT_MSG@10
PS3, Line 10: The reason for doing this
Does this statement refer to the current state or the proposed state? The 
current state blocks the clash via an exception. The proposed change will avoid 
the clash for the default database and allow "clashes" with builtin's for 
non-default databases.


http://gerrit.cloudera.org:8080/#/c/9800/3//COMMIT_MSG@10
PS3, Line 10: .
would it be more accurate to further specify:
... when the fn name is qualified (either explicitly or implicitly via "use") 
for a database other than the default database.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic30df56ac276970116715c14454a5a2477b185fa
Gerrit-Change-Number: 9800
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 26 Mar 2018 19:25:52 +
Gerrit-HasComments: Yes

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

[Csaba Ringhofer (Code Review)]

Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9693 )

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


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9693/5/be/src/exec/parquet-column-stats.h
File be/src/exec/parquet-column-stats.h:

http://gerrit.cloudera.org:8080/#/c/9693/5/be/src/exec/parquet-column-stats.h@162
PS5, Line 162:
nit: Capital I



--
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: comment
Gerrit-Change-Id: Icbacf7fe3b7672e3ce719261ecef445b16f8dec9
Gerrit-Change-Number: 9693
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 26 Mar 2018 19:17:16 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4123 (prep): Parquet column reader cleanup

[Csaba Ringhofer (Code Review)]

Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9799 )

Change subject: IMPALA-4123 (prep): Parquet column reader cleanup
..


Patch Set 1: Code-Review+1

(6 comments)

My suggestions are mainly from the "why not refactor a bit, if we are already 
touching this area" type, so feel free to skip them.

http://gerrit.cloudera.org:8080/#/c/9799/1/be/src/exec/parquet-column-readers.cc
File be/src/exec/parquet-column-readers.cc:

http://gerrit.cloudera.org:8080/#/c/9799/1/be/src/exec/parquet-column-readers.cc@518
PS1, Line 518:   if (def_level < 
def_level_of_immediate_repeated_ancestor()) {
 : // A containing repeated field is empty or NULL. Skip 
the value but
 : // move to the next repetition level if necessary.
 : if (pos_slot_desc_ != nullptr) 
rep_levels_.CacheSkipLevels(1);
 : continue;
 :   }
 :   if (pos_slot_desc_ != nullptr) {
 : ReadPositionBatched(rep_levels_.CacheGetNext(),
 : 
static_cast(tuple->GetSlot(pos_slot_desc_->tuple_offset(;
 :   }
I would move this whole block to ReadPositionBatched, and rename it to 
something like HandleNextRepLevel(Tuple*, def_level) to make 
MaterializeValueBatch()'s non collection logic easier read.

Line 509 could be moved to, as rep_levels_.CacheHasNext() should be true at the 
start of every iteration of the loop.


http://gerrit.cloudera.org:8080/#/c/9799/1/be/src/exec/parquet-column-readers.cc@531
PS1, Line 531: def_level >= max_def_level()
This was not changed, but I am curious: can def_level be greater than 
max_def_level?


http://gerrit.cloudera.org:8080/#/c/9799/1/be/src/exec/parquet-column-readers.cc@586
PS1, Line 586: if (!continue_execution) return false;
Replacing this with !parent_->parse_status_.ok() and removing 
continue_execution would be a bit shorter.

Marking this branch as UNLIKELY could probably make the other branch a bit 
faster.


http://gerrit.cloudera.org:8080/#/c/9799/1/be/src/exec/parquet-column-readers.cc@597
PS1, Line 597: template 
This could be split to two specialized template functions.


http://gerrit.cloudera.org:8080/#/c/9799/1/be/src/exec/parquet-column-readers.cc@600
PS1, Line 600: MemPool* RESTRICT pool
"pool" could be removed, as it is not used in the function (ReadSlot() used it 
only as an argument to ConvertSlot()). 

It could be also turned into a member (it is always the scanners 
scratch_batch_->aux_mem_pool) instead of passing it as an argument.


http://gerrit.cloudera.org:8080/#/c/9799/1/be/src/exec/parquet-column-readers.cc@1320
PS1, Line 1320: return 
ReadSlot(static_cast(tuple->GetSlot(tuple_offset_)), pool);
Tuple has a GetCollectionSlot() functions that does the cast. Some new typed 
function like GetInt64Slot() could be added too replace the other GetSlot() 
casts.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc00352df3a0b2d605f872ae7e43db2dc90faab1
Gerrit-Change-Number: 9799
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Comment-Date: Mon, 26 Mar 2018 19:12:22 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6724: Incorrect exception handling in create function statement

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9800 )

Change subject: IMPALA-6724: Incorrect exception handling in create function 
statement
..


Patch Set 3:

Oh ok, that makes a lot of sense if that's the rule. So we would still break 
scripts that use the UDFs without specifying the database name, but it's easier 
to fix the breakage by going through and qualifying function names.

I guess that's probably more convenient.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic30df56ac276970116715c14454a5a2477b185fa
Gerrit-Change-Number: 9800
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 26 Mar 2018 19:03:59 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6731: Use private index in bootstrap virtualenv

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9798 )

Change subject: IMPALA-6731: Use private index in bootstrap_virtualenv
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9798/2/infra/python/bootstrap_virtualenv.py
File infra/python/bootstrap_virtualenv.py:

http://gerrit.cloudera.org:8080/#/c/9798/2/infra/python/bootstrap_virtualenv.py@23
PS2, Line 23: # 1. install basic non-C/C++ packages into the virtualenv
> Is there a new stage here?
Done


http://gerrit.cloudera.org:8080/#/c/9798/2/infra/python/bootstrap_virtualenv.py@154
PS2, Line 154: cmd.extend(["--index-url", "%s/simple" % 
os.environ["PYPI_MIRROR"]])
> Do you know why we use --no-index in the mirror-less case?
I don't know, should we remove it?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
Gerrit-Change-Number: 9798
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 26 Mar 2018 19:02:23 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6731: Use private index in bootstrap virtualenv

[Lars Volker (Code Review)]

Hello Philip Zeyliger, David Knupp,

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

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

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

Change subject: IMPALA-6731: Use private index in bootstrap_virtualenv
..

IMPALA-6731: Use private index in bootstrap_virtualenv

This change switches to using a private pypi index url when using a
private pypi mirror. This allows to run the tests without relying on the
public Python pypi mirrors.

Some packages can not detect their dependencies correctly when they get
installed together with the dependencies in the same call to pip. This
change adds a second stage of package installation to separate these
packages from their dependencies.

It also adds a few missing packages and updates some packages to newer
versions.

Testing: Ran this on a box where I blocked DNS resolution to Python's
upstream pypi.

Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
---
M infra/python/bootstrap_virtualenv.py
M infra/python/deps/compiled-requirements.txt
M infra/python/deps/pip_download.py
M infra/python/deps/requirements.txt
A infra/python/deps/stage2-requirements.txt
5 files changed, 67 insertions(+), 15 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
Gerrit-Change-Number: 9798
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 

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

[Tim Armstrong (Code Review)]

Tim Armstrong 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:

(5 comments)

Minor comments on the clang-tidy fixes

http://gerrit.cloudera.org:8080/#/c/9584/4/be/src/rpc/impala-service-pool.cc
File be/src/rpc/impala-service-pool.cc:

http://gerrit.cloudera.org:8080/#/c/9584/4/be/src/rpc/impala-service-pool.cc@80
PS4, Line 80:   ImpalaServicePool::Shutdown();
This was required because Shutdown() is virtual. However, I don't see why 
Shutdown() needs to be virtual since we don't subclass ImpalaServicePool as far 
as I can see. We should generally prefer non-virtual methods. I think Init() 
also is unnecessarily virtual.


http://gerrit.cloudera.org:8080/#/c/9584/4/be/src/transport/TSasl.cpp
File be/src/transport/TSasl.cpp:

http://gerrit.cloudera.org:8080/#/c/9584/4/be/src/transport/TSasl.cpp@57
PS4, Line 57: (const char*)
Switch this cast too?


http://gerrit.cloudera.org:8080/#/c/9584/4/be/src/transport/TSasl.cpp@72
PS4, Line 72: (const char*
Switch this cast too?


http://gerrit.cloudera.org:8080/#/c/9584/4/be/src/transport/TSasl.cpp@242
PS4, Line 242: (const char*)
Switch this cast and the one just below too?


http://gerrit.cloudera.org:8080/#/c/9584/4/be/src/udf/udf-test.cc
File be/src/udf/udf-test.cc:

http://gerrit.cloudera.org:8080/#/c/9584/4/be/src/udf/udf-test.cc@140
PS4, Line 140:   // ptime t(*(date*)); is this conversion correct?
Leftover comment. This looks right to me.



--
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: Mon, 26 Mar 2018 18:59:51 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6614: ClientRequestState should use HS2 TOperationState

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9501 )

Change subject: IMPALA-6614: ClientRequestState should use HS2 TOperationState
..


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I36287eaf8f1dac23c306b470f95f379dfdc6bb5b
Gerrit-Change-Number: 9501
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 26 Mar 2018 18:55:39 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6731: Use private index in bootstrap virtualenv

[Philip Zeyliger (Code Review)]

Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9798 )

Change subject: IMPALA-6731: Use private index in bootstrap_virtualenv
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9798/2/infra/python/bootstrap_virtualenv.py
File infra/python/bootstrap_virtualenv.py:

http://gerrit.cloudera.org:8080/#/c/9798/2/infra/python/bootstrap_virtualenv.py@23
PS2, Line 23: # 1. install basic non-C/C++ packages into the virtualenv
Is there a new stage here?


http://gerrit.cloudera.org:8080/#/c/9798/2/infra/python/bootstrap_virtualenv.py@154
PS2, Line 154: cmd.append("--no-index")
Do you know why we use --no-index in the mirror-less case?

https://pip.pypa.io/en/stable/reference/pip_wheel/#no-index was not clear to me 
in the context of what's going on here.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
Gerrit-Change-Number: 9798
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 26 Mar 2018 18:51:47 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6685: Improve profiles in KrpcDataStreamRecvr and KrpcDataStreamSender

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9690 )

Change subject: IMPALA-6685: Improve profiles in KrpcDataStreamRecvr and 
KrpcDataStreamSender
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9690/3/be/src/runtime/krpc-data-stream-recvr.cc
File be/src/runtime/krpc-data-stream-recvr.cc:

http://gerrit.cloudera.org:8080/#/c/9690/3/be/src/runtime/krpc-data-stream-recvr.cc@457
PS3, Line 457: static int64_t GetSerializedBatchSize(
 : const TransmitDataRequestPB* request, RpcContext* 
rpc_context) {
 :   kudu::Slice tuple_offsets;
 :   if (UNLIKELY(!rpc_context->GetInboundSidecar(
 :   request->tuple_offsets_sidecar_idx(), 
_offsets).ok())) {
 : return 0;
 :   }
 :   kudu::Slice tuple_data;
 :   if (UNLIKELY(!rpc_context->GetInboundSidecar(
 :   request->tuple_data_sidecar_idx(), _data).ok())) 
{
 : return 0;
 :   }
 :   return tuple_data.size() + tuple_offsets.size();
 : }
> Convert to a static function. UnpackRequest() was returning deserialized si
Right, but it also gives the serialized size components. What I meant was:

Status status = UnpackRequest(request, rpc_context, _offset, _data, 
_used);
return status.ok() ? tuple_data.size() + tuple_offsets.size() : 0;

okay to ignore, but was just thinking it'd be good if things can't get out of 
sync by sharing code (for example, if another sidecar were to be added).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8ba405921b3df920c1e85b940ce9c8d02fc647cd
Gerrit-Change-Number: 9690
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Comment-Date: Mon, 26 Mar 2018 18:50:22 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6724: Incorrect exception handling in create function statement

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9800 )

Change subject: IMPALA-6724: Incorrect exception handling in create function 
statement
..


Patch Set 3:

I think our hands could be similarly tied if we don't allow built-in names. 
That limits our ability to add new built-in functions because a user may 
already have a UDF with that same name.

I agree we need to resolve your concern, Tim. My understanding is that UDFs 
that clash with a built-in must be fully-qualified. An unqualified function 
invokation should default to "_impala_builtins", and only resolve to a UDF if 
there is no match in "_impala_builtins".


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic30df56ac276970116715c14454a5a2477b185fa
Gerrit-Change-Number: 9800
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 26 Mar 2018 18:48:25 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5721,IMPALA-6717,IMPALA-6738: improve stress test binary search

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9770 )

Change subject: IMPALA-5721,IMPALA-6717,IMPALA-6738: improve stress test binary 
search
..


Patch Set 6:

I didn't review the code in detail but the high-level goals of the patch make 
sense to me.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I33d036ec93df3016cd4703205078dbdba0168acb
Gerrit-Change-Number: 9770
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 26 Mar 2018 18:41:02 +
Gerrit-HasComments: No

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

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9758 )

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


Patch Set 5:

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


--
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: 5
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 26 Mar 2018 18:38:08 +
Gerrit-HasComments: No

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

[David Knupp (Code Review)]

David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9758 )

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


Patch Set 5: Code-Review+2

Sorry, meant to complete my last sentence and add a +2 with my last comment, 
but I clicked "Send" instead by accident.


--
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: 5
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 26 Mar 2018 18:37:03 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6724: Incorrect exception handling in create function statement

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9800 )

Change subject: IMPALA-6724: Incorrect exception handling in create function 
statement
..


Patch Set 3:

Doesn't this allow a bunch of shenanigans with hijacking builtin functions? 
E.g. if I have permissions to create functions in a database, I could override 
a builtin function like "substring" and send every input string somewhere if 
someone uses the substring() function if that db is their current database?

I know that we don't have sandboxing now so there are worse things someone who 
has create function privileges can do, but this behavioural change seems to tie 
our hands in future.

Even if the person creating the function isn't malicious, you could 
accidentally cause a lot of chaos by accidentally overriding a builtin.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic30df56ac276970116715c14454a5a2477b185fa
Gerrit-Change-Number: 9800
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 26 Mar 2018 18:36:14 +
Gerrit-HasComments: No

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

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9758 )

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


Patch Set 4:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9758/4//COMMIT_MSG@37
PS4, Line 37: - While testing on hardware clusters down stream, I noticed 
IMPALA-6736:
> Nit: since you've set up IMPALA-6715 as a "sub-header" of sorts, for symmet
Done


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

http://gerrit.cloudera.org:8080/#/c/9758/4/tests/stress/concurrent_select.py@2071
PS4, Line 2071: Filtering non-spilling query due to mem ratio option
> Nt:
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: 4
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 26 Mar 2018 18:34:53 +
Gerrit-HasComments: Yes

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

[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 (#5).

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

IMPALA-6715,IMPALA-6736: fix stress TPC workload selection

IMPALA-6715:
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.

- While testing on hardware clusters down stream, I noticed...

IMPALA-6736:
  TPC-DS Q67A is 10x more expensive to run without spilling than any
  other query. I fixed the --filter-query-mem-ratio option to work. This
  will still run Q67A during the binary search phase, but if a cluster
  is too small, the query will be skipped.

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, 88 insertions(+), 27 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/9758/5
--
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: 5
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Reviewer: Tim Armstrong 

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

[David Knupp (Code Review)]

David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9758 )

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


Patch Set 4:

(2 comments)

Feel free to commit

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

http://gerrit.cloudera.org:8080/#/c/9758/4//COMMIT_MSG@37
PS4, Line 37: - While testing on hardware clusters down stream, I noticed 
IMPALA-6736:
Nit: since you've set up IMPALA-6715 as a "sub-header" of sorts, for symmetry, 
maybe do the same with IMPALA-6736.


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

http://gerrit.cloudera.org:8080/#/c/9758/4/tests/stress/concurrent_select.py@2071
PS4, Line 2071: Filtering non-spilling query due to mem ratio option
Nt:

  "Filtering non-spilling query that exceeds mem ratio option"



--
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: 4
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 26 Mar 2018 18:25:08 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6731: Use private index in bootstrap virtualenv

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9798 )

Change subject: IMPALA-6731: Use private index in bootstrap_virtualenv
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9798/1//COMMIT_MSG@7
PS1, Line 7: 7631
> IMPALA-6731
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
Gerrit-Change-Number: 9798
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Mon, 26 Mar 2018 18:26:00 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6731: Use private index in bootstrap virtualenv

[Lars Volker (Code Review)]

Hello David Knupp,

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

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

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

Change subject: IMPALA-6731: Use private index in bootstrap_virtualenv
..

IMPALA-6731: Use private index in bootstrap_virtualenv

This change switches to using a private pypi index url when using a
private pypi mirror. This allows to run the tests without relying on the
public Python pypi mirrors.

Some packages can not detect their dependencies correctly when they get
installed together with the dependencies in the same call to pip. This
change adds a second stage of package installation to separate these
packages from their dependencies.

It also adds a few missing packages and updates some packages to newer
versions.

Testing: Ran this on a box where I blocked DNS resolution to Python's
upstream pypi.

Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
---
M infra/python/bootstrap_virtualenv.py
M infra/python/deps/compiled-requirements.txt
M infra/python/deps/pip_download.py
M infra/python/deps/requirements.txt
A infra/python/deps/stage2-requirements.txt
5 files changed, 65 insertions(+), 15 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
Gerrit-Change-Number: 9798
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 

[Impala-ASF-CR] IMPALA-5721,IMPALA-6717,IMPALA-6738: improve stress test binary search

[Michael Brown (Code Review)]

Michael Brown has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9770


Change subject: IMPALA-5721,IMPALA-6717,IMPALA-6738: improve stress test binary 
search
..

IMPALA-5721,IMPALA-6717,IMPALA-6738: improve stress test binary search

IMPALA-5721:
- Save profiles of queries at the end of both the spilling and
  non-spilling binary search. These were not being saved before. Note
  these profiles won't have ExecSummary until IMPALA-6640 is addressed.

- Save the profile of any query that produces incorrect results during
  binary search. These were not being saved before, either.

- Use descriptive names, like
  tpch_100_parquet_q12_profile_without_spilling.txt, for profiles
  mentioned above. We do this by introducing the concept of a
  "logical_query_id" whose values look like "tpch_100_parquet_q12".

- Use the logical_query_id in critical error paths and include the
  logical_query_id in result hash files.

IMPALA-6717:
- Plumb --common-query-options through to the binary search.

IMPALA-6738:
- Begin a refactoring to reduce the number of parameters used when doing
  the binary search.

- Introduce a notion of "curated args" via class that dispatches
  curating args from parse_args() into internal form.

- Adjust populate_all_queries() to use curated_args

Change-Id: I33d036ec93df3016cd4703205078dbdba0168acb
---
M tests/stress/concurrent_select.py
1 file changed, 214 insertions(+), 76 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I33d036ec93df3016cd4703205078dbdba0168acb
Gerrit-Change-Number: 9770
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6724: Incorrect exception handling in create function statement

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9800 )

Change subject: IMPALA-6724: Incorrect exception handling in create function 
statement
..


Patch Set 3:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9800/3//COMMIT_MSG@13
PS3, Line 13: Existing behavior:
To get ahead of all the comments regarding behavior and compatibility, it might 
be good to make an exhaustive list of all cases, and then show the old/new 
behaviors.

The 2 dimensions are:
* targets _impala_builtins database or not
* function name has a built-in with the same name or not
So a total of 4 cases, each with old vs. new behavior.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic30df56ac276970116715c14454a5a2477b185fa
Gerrit-Change-Number: 9800
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 26 Mar 2018 18:20:18 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6724: Incorrect exception handling in create function statement

[Bharath Vissapragada (Code Review)]

Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9800 )

Change subject: IMPALA-6724: Incorrect exception handling in create function 
statement
..


Patch Set 2:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9800/2//COMMIT_MSG@13
PS2, Line 13: Testing:
Looks to me like a compat-breaking change, add that to the commit message and 
not to cherry-pick to 2.x?

Just curious if everyone is ok with this approach. Vuk/Alex?

One concerning thing is that if some user overrides an inbuilt function with a 
custom behavior and other users don't know that, that could result in 
unexpected results. Thoughts?


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

http://gerrit.cloudera.org:8080/#/c/9800/2/fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java@137
PS2, Line 137: fnName_.analyze(analyzer, true);
Should you update other places that resolve functions (like DropStmt for 
example) to reflect the same behavior? Else, I guess it might be confusing if 
create/drop/selects resolve functions their own ways. I haven't tried the patch 
locally, but looks like it could happen?

-- succeeds
use mydb;
create function sin(...

---
select sin()  <-- what should this resolve to, given we have both mydb.sin() 
and inbuilt.sin()

-- fails
drop function sin(...
can't modify system db...



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic30df56ac276970116715c14454a5a2477b185fa
Gerrit-Change-Number: 9800
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 26 Mar 2018 18:13:30 +
Gerrit-HasComments: Yes

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

[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 5:

(16 comments)

Thanks Csaba!
(and I also updated my vimrc... :) )

http://gerrit.cloudera.org:8080/#/c/9693/4/be/src/exec/parquet-column-stats.h
File be/src/exec/parquet-column-stats.h:

http://gerrit.cloudera.org:8080/#/c/9693/4/be/src/exec/parquet-column-stats.h@159
PS4, Line 159:   // If true, min/max values are ascending.
> nit: missing .
Done


http://gerrit.cloudera.org:8080/#/c/9693/4/be/src/exec/parquet-column-stats.h@162
PS4, Line 162:   // if true min/max values are descending.
> nit: missing .
Done


http://gerrit.cloudera.org:8080/#/c/9693/4/be/src/exec/parquet-column-stats.inline.h
File be/src/exec/parquet-column-stats.inline.h:

http://gerrit.cloudera.org:8080/#/c/9693/4/be/src/exec/parquet-column-stats.inline.h@207
PS4, Line 207:   if (prev_page_min_buffer_.IsEmpty()) {
 : RETURN_IF_ERROR(CopyToBuffer(_page_min_buffer_, 
_page_min_value_));
> nit: long lines
Done


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

http://gerrit.cloudera.org:8080/#/c/9693/4/tests/query_test/test_parquet_page_index.py@71
PS4, Line 71: column_index_length)
> nit: not enough indentation
Done


http://gerrit.cloudera.org:8080/#/c/9693/4/tests/query_test/test_parquet_page_index.py@82
PS4, Line 82: offset_index_length)
> nit: not enough indentation
Done


http://gerrit.cloudera.org:8080/#/c/9693/4/tests/query_test/test_parquet_page_index.py@85
PS4, Line 85:   page_header = get_page_header(parquet_file, 
page_loc.offset,
:   page_loc.compressed_page_size)
:   page_headers.append(page_header)
> nit: too much indentation
Done


http://gerrit.cloudera.org:8080/#/c/9693/4/tests/query_test/test_parquet_page_index.py@113
PS4, Line 113: for null_page, value in zip(null_pages, values):
> Are these prints intentional?
No, jus left them here.


http://gerrit.cloudera.org:8080/#/c/9693/4/tests/query_test/test_parquet_page_index.py@118
PS4, Line 118: elif ordering == BoundaryOrder.DESCENDING and 
previouse_value is not None:
> Is this print intentional?
Done


http://gerrit.cloudera.org:8080/#/c/9693/4/tests/query_test/test_parquet_page_index.py@120
PS4, Line 120: previous_value = current_value
> nit: too much indentation
Done


http://gerrit.cloudera.org:8080/#/c/9693/4/tests/query_test/test_parquet_page_index.py@122
PS4, Line 122:
> nit: too much indentation
Done


http://gerrit.cloudera.org:8080/#/c/9693/4/tests/query_test/test_parquet_page_index.py@164
PS4, Line 164:   assert null_page
 : # Everything is None, no further checks needed.
 : return
 :
 :   min_value = decode_stats_value(column_info.schema, 
min_value_str)
 :   for null_p
> nit: too much indentation
Done


http://gerrit.cloudera.org:8080/#/c/9693/4/tests/query_test/test_parquet_page_index.py@200
PS4, Line 200:   ).format(qualified_table_name, sorting_column, 
source_table)
> nit: not enough indentation
Done


http://gerrit.cloudera.org:8080/#/c/9693/4/tests/query_test/test_parquet_page_index.py@203
PS4, Line 203: self._validate_parquet_index(hdfs_path, sorting_column, 
tmpdir)
> nit: not enough indentation
Done


http://gerrit.cloudera.org:8080/#/c/9693/4/tests/query_test/test_parquet_page_index.py@244
PS4, Line 244:
> nit: missing .
Done


http://gerrit.cloudera.org:8080/#/c/9693/4/tests/query_test/test_parquet_page_index.py@249
PS4, Line 249: self._ctas_table_and_verify_index(vector, unique_database,
> nit: missing .
Done


http://gerrit.cloudera.org:8080/#/c/9693/4/tests/util/get_parquet_metadata.py
File tests/util/get_parquet_metadata.py:

http://gerrit.cloudera.org:8080/#/c/9693/4/tests/util/get_parquet_metadata.py@169
PS4, Line 169: def get_column_index(filename, file_pos, length):
 :   """Reads a ColumnIndex object from a file at the given 
position."""
 :   serialized_column_index = read_serialized_object(filename, 
file_pos, length)
 :   protocol = create_protocol(serialized_column_index)
 :   column_index = ColumnIndex()
 :   column_index.read(protocol)
 :   return column_index
 :
 :
 : def get_page_header(filename, file_pos, length):
 :   """Reads a PageHeader object from a file at the given 
position."""
 :   serialized_page_header = read_serialized_object(filename, 
file_pos, length)
 :   protocol = create_protocol(serialized_page_header)
 :   page_header = PageHeader()
 :   page_header.read(protocol)
 :   

[Impala-ASF-CR] IMPALA-6724: Incorrect exception handling in create function statement

[Fredy Wijaya (Code Review)]

Fredy Wijaya has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/9800 )

Change subject: IMPALA-6724: Incorrect exception handling in create function 
statement
..

IMPALA-6724: Incorrect exception handling in create function statement

This patch removes restriction on creating a function with the same name
as the built-in function. The reason for doing this is to avoid name
clash when introducing new built-in functions.

Existing behavior:
> use mydb;
> create function sin(double) ... -- this is not allowed

New behavior:
> use mydb;
> create function sin(double) ... -- this is allowed

Existing behavior:
> create function _impala_builtins.sin(double) ... -- same name exception

New behavior:
> create function _impala_builtins.sin(double) ... -- cannot modify
system database exception

Testing:
- Ran front-end tests

Change-Id: Ic30df56ac276970116715c14454a5a2477b185fa
---
M fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/FunctionName.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
4 files changed, 48 insertions(+), 19 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic30df56ac276970116715c14454a5a2477b185fa
Gerrit-Change-Number: 9800
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-6724: Incorrect exception handling in create function statement

[Fredy Wijaya (Code Review)]

Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9800 )

Change subject: IMPALA-6724: Incorrect exception handling in create function 
statement
..


Patch Set 3:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9800/2//COMMIT_MSG@10
PS2, Line 10: The reason for doing this is to avoid name
: clash when introducing new built-in functions.
> avoiding a name clash sounds reasonable (current behavior). pls clarify her
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic30df56ac276970116715c14454a5a2477b185fa
Gerrit-Change-Number: 9800
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 26 Mar 2018 17:53:05 +
Gerrit-HasComments: Yes

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

[Zoltan Borok-Nagy (Code Review)]

Hello Lars Volker, Tim Armstrong, Csaba Ringhofer,

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

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

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

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, 547 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/9693/5
--
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: 5
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 

[Impala-ASF-CR] IMPALA-7631: Use private index in bootstrap virtualenv

[David Knupp (Code Review)]

David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9798 )

Change subject: IMPALA-7631: Use private index in bootstrap_virtualenv
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9798/1//COMMIT_MSG@7
PS1, Line 7: 7631
IMPALA-6731



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I85f75f1f1a305f3043e0910ab88a880eeb30f00b
Gerrit-Change-Number: 9798
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Mon, 26 Mar 2018 17:40:27 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6724: Incorrect exception handling in create function statement

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9800 )

Change subject: IMPALA-6724: Incorrect exception handling in create function 
statement
..


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9800/2//COMMIT_MSG@10
PS2, Line 10: The reason for doing this is to avoid name
: clashing when introducing new built-in functions
avoiding a name clash sounds reasonable (current behavior). pls clarify here 
what is incorrect and what is the proposed new behavior.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic30df56ac276970116715c14454a5a2477b185fa
Gerrit-Change-Number: 9800
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 26 Mar 2018 17:33:52 +
Gerrit-HasComments: Yes