[Impala-ASF-CR] IMPALA-3942: Fix wrongly escaped string literal in front-end

[Alex Behm (Code Review)]

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

Change subject: IMPALA-3942: Fix wrongly escaped string literal in front-end
..


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8818/6/tests/query_test/test_queries.py
File tests/query_test/test_queries.py:

http://gerrit.cloudera.org:8080/#/c/8818/6/tests/query_test/test_queries.py@310
PS6, Line 310:
> I'm ok with leaving some E2E tests.
I just realized this comment has not been addressed, sorry. Can you move these 
tests?

You might put them in views-ddl.test or test_ddl.py



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc4b5f5d8ffaa8feb96a466959427a04b3b06fec
Gerrit-Change-Number: 8818
Gerrit-PatchSet: 9
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jan 2018 06:06:00 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4835: prerequisite buffer pool changes

[Tim Armstrong (Code Review)]

Hello Bikramjeet Vig,

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

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

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

Change subject: IMPALA-4835: prerequisite buffer pool changes
..

IMPALA-4835: prerequisite buffer pool changes

The scanner/buffer pool changes will have different scanner
threads sharing the same buffer pool client. This requires that the
AllocateBuffer() API is safe to call concurrently from different
threads, which was true previously but not documented or tested.

This updates the comments and adds a couple of tests.

Change-Id: I8f2196722df59f2d367787c0550058022e296e24
---
M be/src/runtime/bufferpool/buffer-pool-test.cc
M be/src/runtime/bufferpool/buffer-pool.h
2 files changed, 142 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8f2196722df59f2d367787c0550058022e296e24
Gerrit-Change-Number: 9097
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 

[Impala-ASF-CR] IMPALA-6399: Fix timeout logic in test query profile thrift timestamps

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6399: Fix timeout logic in 
test_query_profile_thrift_timestamps
..


Patch Set 8: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
Gerrit-Change-Number: 9079
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Tue, 23 Jan 2018 05:13:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6410: Tool to cherrypick changes across branches.

[Jim Apple (Code Review)]

Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9045 )

Change subject: IMPALA-6410: Tool to cherrypick changes across branches.
..


Patch Set 5: Code-Review+2

Nice work!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6120ec2d6e914a1e5fda568178b32aafda8722a9
Gerrit-Change-Number: 9045
Gerrit-PatchSet: 5
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Tue, 23 Jan 2018 04:34:53 +
Gerrit-HasComments: No

[native-toolchain-CR] IMPALA-6401 : Re-apply the PPC Breakpad patches

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9077 )

Change subject: IMPALA-6401 : Re-apply the PPC Breakpad patches
..


Patch Set 1:

> Change has been successfully merged by Lars Volker

Thanks, Lars!


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia40e6f9a0054b55e72350ddbb96c9ecac53b0419
Gerrit-Change-Number: 9077
Gerrit-PatchSet: 1
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-Comment-Date: Tue, 23 Jan 2018 04:24:02 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3942: Fix wrongly escaped string literal in front-end

[Kim Jin Chul (Code Review)]

Kim Jin Chul has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8818 )

Change subject: IMPALA-3942: Fix wrongly escaped string literal in front-end
..


Patch Set 8:

(1 comment)

Applied the update.

http://gerrit.cloudera.org:8080/#/c/8818/8/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
File fe/src/main/java/org/apache/impala/analysis/StringLiteral.java:

http://gerrit.cloudera.org:8080/#/c/8818/8/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@92
PS8, Line 92:*  String literal can come directly from the SQL of a query or 
from rewrites like
> Some grammar issues, suggest this correction:
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc4b5f5d8ffaa8feb96a466959427a04b3b06fec
Gerrit-Change-Number: 8818
Gerrit-PatchSet: 8
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jan 2018 04:19:16 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3942: Fix wrongly escaped string literal in front-end

[Kim Jin Chul (Code Review)]

Hello Thomas Tauber-Marshall, Gabor Kaszab, Jim Apple, Tim Armstrong, Alex Behm,

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

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

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

Change subject: IMPALA-3942: Fix wrongly escaped string literal in front-end
..

IMPALA-3942: Fix wrongly escaped string literal in front-end

String literal can be wrapped by either single or double quotes.
There are some holes in escaping the string literal:
- toSql() always returns strings that are single quoted, resulting in
improper escaping in the output if the original string was actually
double quoted.
- It is not always possible to determine if a string "should" be single
or double quoted. e.g. concat('a', "b")

The solution is to normalize any string which comes from user's given
string or a generated string (e.g. constant fold by the rewritter rule)
to single quotes.

Testing:
Added unit tests into expr-test, ToSqlTest
Added E2E tests into test_queries

Change-Id: Ibc4b5f5d8ffaa8feb96a466959427a04b3b06fec
---
M be/src/exprs/expr-test.cc
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M tests/query_test/test_queries.py
4 files changed, 113 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibc4b5f5d8ffaa8feb96a466959427a04b3b06fec
Gerrit-Change-Number: 8818
Gerrit-PatchSet: 9
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6346: Potential deadlock in KrpcDataStreamMgr

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8950 )

Change subject: IMPALA-6346: Potential deadlock in KrpcDataStreamMgr
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8950/5/be/src/runtime/data-stream-test.cc
File be/src/runtime/data-stream-test.cc:

http://gerrit.cloudera.org:8080/#/c/8950/5/be/src/runtime/data-stream-test.cc@658
PS5, Line 658:  DataStreamTestForImpala2931
> Given there is nothing special about this class, may be this should be call
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7d1a8f12a4821092ca61ccc8a6f20c0404d56c7
Gerrit-Change-Number: 8950
Gerrit-PatchSet: 5
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 23 Jan 2018 03:26:40 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6346: Potential deadlock in KrpcDataStreamMgr

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Lars Volker,

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

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

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

Change subject: IMPALA-6346: Potential deadlock in KrpcDataStreamMgr
..

IMPALA-6346: Potential deadlock in KrpcDataStreamMgr

In KrpcDataStreamMgr::CreateRecvr() we take the lock_ and
then call recvr->TakeOverEarlySender() for all contexts.
recvr->TakeOverEarlySender() then calls
recvr_->mgr_->EnqueueDeserializeTask((), which can block if the
deserialize pool queue is full. The next thread to become available
in that queue will also have to acquire lock_, thus leading to a
deadlock.

We fix this by moving the EarlySendersList out of the
EarlySendersMap and dropping the lock before taking any actions on
the RPC contexts in the EarlySendersList. All functions called after
dropping 'lock_' do not require the lock to protect them as they are
thread safe.

Additionally modified the BE test data-stream-test to work with KRPC
as well.

Testing: Added a new test to data-stream-test to verify that the
deadlock does not happen. Also, I verified that this test hangs
without the fix.

Change-Id: Ib7d1a8f12a4821092ca61ccc8a6f20c0404d56c7
---
M be/src/runtime/data-stream-test.cc
M be/src/runtime/krpc-data-stream-mgr.cc
2 files changed, 256 insertions(+), 57 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib7d1a8f12a4821092ca61ccc8a6f20c0404d56c7
Gerrit-Change-Number: 8950
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-3942: Fix wrongly escaped string literal in front-end

[Alex Behm (Code Review)]

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

Change subject: IMPALA-3942: Fix wrongly escaped string literal in front-end
..


Patch Set 8: Code-Review+2

(1 comment)

Thanks! I'll merge after you've addressed the final change.

http://gerrit.cloudera.org:8080/#/c/8818/8/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
File fe/src/main/java/org/apache/impala/analysis/StringLiteral.java:

http://gerrit.cloudera.org:8080/#/c/8818/8/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@92
PS8, Line 92:*  String literal can come directly from the SQL of a query or 
from rewrites like
Some grammar issues, suggest this correction:

String literals can come directly from the SQL of a query or from rewrites like 
constant folding. So this value normalization to a single-quoted string is 
necessary because we do not know whether single or double quotes are 
appropriate.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc4b5f5d8ffaa8feb96a466959427a04b3b06fec
Gerrit-Change-Number: 8818
Gerrit-PatchSet: 8
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jan 2018 02:15:37 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6418: Find a reliable way to detect supported TLS versions

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6418: Find a reliable way to detect supported TLS 
versions
..

IMPALA-6418: Find a reliable way to detect supported TLS versions

Due to a RHEL bug, finding the version of the dynamically linked
OpenSSL is unreliable on certain RHEL platforms. This means that
if Impala is built against OpenSSL 1.0.0 and run against OpenSSL
1.0.1, due to the bug, Impala will not be able to access the
capabilities of OpenSSL 1.0.1. The most significant drawback of
this is that in this scenario, Impala cannot use TLSv1.2, even though
it is available.

The RHEL bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1497859

This patch imitates Kudu's way of determining what TLS versions are
supported via this patch:
https://github.com/apache/kudu/commit/b88117415a02699c12a6eacbf065c4140ee0963c

This is tested and works on all supported platforms of Impala.

Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
Reviewed-on: http://gerrit.cloudera.org:8080/9060
Reviewed-by: Sailesh Mukil 
Tested-by: Impala Public Jenkins
---
M be/src/rpc/thrift-client.cc
M be/src/rpc/thrift-server.cc
M be/src/util/openssl-util.cc
M be/src/util/openssl-util.h
4 files changed, 45 insertions(+), 12 deletions(-)

Approvals:
  Sailesh Mukil: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
Gerrit-Change-Number: 9060
Gerrit-PatchSet: 7
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-6418: Find a reliable way to detect supported TLS versions

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6418: Find a reliable way to detect supported TLS 
versions
..


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
Gerrit-Change-Number: 9060
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 23 Jan 2018 01:59:40 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3282: Adds regexp escape built-in function

[Kim Jin Chul (Code Review)]

Hello Tianyi Wang, Jim Apple, Philip Zeyliger, Tim Armstrong,

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

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

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

Change subject: IMPALA-3282: Adds regexp_escape built-in function
..

IMPALA-3282: Adds regexp_escape built-in function

Escapes the following special characters in RE2 library:
.\+*?[^]$(){}=!<>|:-

Testing:
Add some unit tests into ExprTest.StringRegexpFunctions
Add some E2E tests into exprs.test

Change-Id: I84c3e0ded26f6eb20794c38b75be9b25cd111e4b
---
M be/src/exprs/expr-test.cc
M be/src/exprs/string-functions-ir.cc
M be/src/exprs/string-functions.h
M common/function-registry/impala_functions.py
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
5 files changed, 91 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84c3e0ded26f6eb20794c38b75be9b25cd111e4b
Gerrit-Change-Number: 8900
Gerrit-PatchSet: 7
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-3998: Remove refresh after connect option from shell

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-3998: Remove refresh_after_connect option from shell
..

IMPALA-3998: Remove refresh_after_connect option from shell

This removes the deprecated option in time for 3.0.

Testing:
Ran core tests. Manually ran the shell with the argument to confirm that
it reported "no such option".

Cherry-picks: not for 2.x.

Change-Id: I8f430bad0578e150d5e80066b9e7572041af4a15
Reviewed-on: http://gerrit.cloudera.org:8080/9072
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/bad_impalarc
M tests/shell/good_impalarc
M tests/shell/test_shell_commandline.py
6 files changed, 1 insertion(+), 30 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8f430bad0578e150d5e80066b9e7572041af4a15
Gerrit-Change-Number: 9072
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-3998: Remove refresh after connect option from shell

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-3998: Remove refresh_after_connect option from shell
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f430bad0578e150d5e80066b9e7572041af4a15
Gerrit-Change-Number: 9072
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jan 2018 01:48:49 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6190/6246: Add instances tab and event sequence

[Lars Volker (Code Review)]

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

Change subject: IMPALA-6190/6246: Add instances tab and event sequence
..


Patch Set 14: Code-Review+2

Rebased, carrying Tim's +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I626456b6afa9101ffd5cda10c4096d63d7f9
Gerrit-Change-Number: 8758
Gerrit-PatchSet: 14
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jan 2018 01:36:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6190/6246: Add instances tab and event sequence

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6190/6246: Add instances tab and event sequence
..


Patch Set 14:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I626456b6afa9101ffd5cda10c4096d63d7f9
Gerrit-Change-Number: 8758
Gerrit-PatchSet: 14
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jan 2018 01:36:33 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6399: Fix timeout logic in test query profile thrift timestamps

[Lars Volker (Code Review)]

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

Change subject: IMPALA-6399: Fix timeout logic in 
test_query_profile_thrift_timestamps
..


Patch Set 8: Code-Review-2

Still testing this.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
Gerrit-Change-Number: 9079
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Tue, 23 Jan 2018 01:35:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6399: Fix timeout logic in test query profile thrift timestamps

[Lars Volker (Code Review)]

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

Change subject: IMPALA-6399: Fix timeout logic in 
test_query_profile_thrift_timestamps
..


Patch Set 8:

Rebased to get the workaround for IMPALA-6092.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
Gerrit-Change-Number: 9079
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Tue, 23 Jan 2018 01:35:03 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6190/6246: Add instances tab and event sequence

[Lars Volker (Code Review)]

Hello Michael Ho, Joe McDonnell, Tim Armstrong, Dan Hecht,

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

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

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

Change subject: IMPALA-6190/6246: Add instances tab and event sequence
..

IMPALA-6190/6246: Add instances tab and event sequence

This change adds tracking of the current state during the execution of a
fragment instance. The current state is then reported back to the
coordinator and exposed to users via a new tab in the query detail debug
webpage.

This change also adds an event timeline to fragment instances in the
query profile. The timeline measures the time since backend-local query
start at which particular events complete. Events are derived from the
current state of the execution of a fragment instance. For example:

- Prepare Finished: 13.436ms (13.436ms)
- First Batch Produced: 1s022ms (1s008ms)
- First Batch Sent: 1s022ms (455.558us)
- ExecInternal Finished: 2s783ms (1s760ms)

I added automated tests for both extensions and additionally verified
the change by manual inspection.

Here are the TPCH performance comparison results between this change and
the previous commit on a 16 node cluster.

++---+-++++
| Workload   | File Format   | Avg (s) | Delta(Avg) | GeoMean(s) | 
Delta(GeoMean) |
++---+-++++
| TPCH(_300) | parquet / none / none | 18.47   | -0.94% | 9.72   | 
-1.08% |
++---+-++++

++--+---++-++++-+---+
| Workload   | Query| File Format   | Avg(s) | Base Avg(s) | 
Delta(Avg) | StdDev(%)  | Base StdDev(%) | Num Clients | Iters |
++--+---++-++++-+---+
| TPCH(_300) | TPCH-Q5  | parquet / none / none | 48.88  | 46.93   |   
+4.15%   |   0.14%|   3.61%| 1   | 3 |
| TPCH(_300) | TPCH-Q13 | parquet / none / none | 21.64  | 21.15   |   
+2.29%   |   2.06%|   1.84%| 1   | 3 |
| TPCH(_300) | TPCH-Q11 | parquet / none / none | 1.71   | 1.70|   
+1.12%   |   0.54%|   2.51%| 1   | 3 |
| TPCH(_300) | TPCH-Q18 | parquet / none / none | 33.15  | 32.79   |   
+1.09%   |   0.13%|   2.03%| 1   | 3 |
| TPCH(_300) | TPCH-Q14 | parquet / none / none | 5.95   | 5.90|   
+0.82%   |   2.19%|   0.49%| 1   | 3 |
| TPCH(_300) | TPCH-Q1  | parquet / none / none | 13.99  | 13.90   |   
+0.63%   |   0.25%|   1.39%| 1   | 3 |
| TPCH(_300) | TPCH-Q2  | parquet / none / none | 3.44   | 3.44|   
+0.00%   | * 20.29% * | * 20.76% * | 1   | 3 |
| TPCH(_300) | TPCH-Q6  | parquet / none / none | 1.21   | 1.22|   
-0.01%   |   0.06%|   0.06%| 1   | 3 |
| TPCH(_300) | TPCH-Q20 | parquet / none / none | 3.51   | 3.51|   
-0.11%   |   7.15%|   7.30%| 1   | 3 |
| TPCH(_300) | TPCH-Q16 | parquet / none / none | 6.89   | 6.91|   
-0.21%   |   0.65%|   0.55%| 1   | 3 |
| TPCH(_300) | TPCH-Q4  | parquet / none / none | 4.78   | 4.80|   
-0.38%   |   0.06%|   0.59%| 1   | 3 |
| TPCH(_300) | TPCH-Q19 | parquet / none / none | 30.78  | 31.04   |   
-0.83%   |   0.45%|   1.03%| 1   | 3 |
| TPCH(_300) | TPCH-Q22 | parquet / none / none | 6.06   | 6.12|   
-1.02%   |   1.51%|   2.12%| 1   | 3 |
| TPCH(_300) | TPCH-Q10 | parquet / none / none | 9.43   | 9.58|   
-1.54%   |   0.69%|   3.30%| 1   | 3 |
| TPCH(_300) | TPCH-Q21 | parquet / none / none | 93.41  | 95.18   |   
-1.86%   |   0.08%|   0.81%| 1   | 3 |
| TPCH(_300) | TPCH-Q15 | parquet / none / none | 3.40   | 3.47|   
-1.99%   |   0.72%|   1.27%| 1   | 3 |
| TPCH(_300) | TPCH-Q7  | parquet / none / none | 44.98  | 46.24   |   
-2.71%   |   1.83%|   1.27%| 1   | 3 |
| TPCH(_300) | TPCH-Q3  | parquet / none / none | 28.06  | 29.11   |   
-3.61%   |   1.62%|   1.23%| 1   | 3 |
| TPCH(_300) | TPCH-Q12 | parquet / none / none | 3.15   | 3.28|   
-3.80%   |   0.96%|   1.32%| 1   | 3 |
| TPCH(_300) | TPCH-Q9  | parquet / none / none | 29.47  | 30.80   |   
-4.30%   |   0.29%|   0.34%| 1   | 3 |
| TPCH(_300) | TPCH-Q17 | parquet / none / none 

[Impala-ASF-CR] IMPALA-6318: Revert "Adjustment for hanging query cancellation test"

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6318: Revert "Adjustment for hanging query cancellation 
test"
..

IMPALA-6318: Revert "Adjustment for hanging query cancellation test"

Jenkins jobs occasionally hang on test_query_cancellation_during_fetch.
There was a workaround proposal submitted under this Jira ID, however,
apparently jobs still hang on this test randomly. Reverting the
workaround and skipping the test until further fix proposal provided.

This reverts commit 7810d1f9a2c7d59b4b916d4d1793672cd8c33143.

Change-Id: I51acee49b5a17c4852410b7568fd1d092b114a6d
Reviewed-on: http://gerrit.cloudera.org:8080/8972
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
M tests/shell/test_shell_commandline.py
M tests/shell/util.py
2 files changed, 11 insertions(+), 13 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I51acee49b5a17c4852410b7568fd1d092b114a6d
Gerrit-Change-Number: 8972
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6318: Revert "Adjustment for hanging query cancellation test"

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6318: Revert "Adjustment for hanging query cancellation 
test"
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51acee49b5a17c4852410b7568fd1d092b114a6d
Gerrit-Change-Number: 8972
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jan 2018 01:24:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3282: Adds regexp escape built-in function

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-3282: Adds regexp_escape built-in function
..


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8900/6/be/src/exprs/string-functions-ir.cc
File be/src/exprs/string-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/8900/6/be/src/exprs/string-functions-ir.cc@624
PS6, Line 624: const uint8_t *
nit: no space before the *, i.e.

  const uint8_t* const start_ptr;


http://gerrit.cloudera.org:8080/#/c/8900/6/be/src/exprs/string-functions-ir.cc@635
PS6, Line 635:   *dest_ptr = '\0';
StringVals don't need to be null-terminated.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84c3e0ded26f6eb20794c38b75be9b25cd111e4b
Gerrit-Change-Number: 8900
Gerrit-PatchSet: 6
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jan 2018 01:21:31 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4835: Part 3: switch I/O buffers to buffer pool

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-4835: Part 3: switch I/O buffers to buffer pool
..


Patch Set 10:

This is now ready for review. There might still be tweaks required to memory 
limits of some tests, but I don't expect any major changes.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic09c6196b31e55b301df45cc56d0b72cfece6786
Gerrit-Change-Number: 8966
Gerrit-PatchSet: 10
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jan 2018 01:01:33 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3282: Adds regexp escape built-in function

[Kim Jin Chul (Code Review)]

Kim Jin Chul has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8900 )

Change subject: IMPALA-3282: Adds regexp_escape built-in function
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8900/5/be/src/exprs/string-functions-ir.cc
File be/src/exprs/string-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/8900/5/be/src/exprs/string-functions-ir.cc@623
PS5, Line 623:   const strings::CharSet to_escape(".\\+*?[^]$(){}=!<>|:-");
> This is a constant (perhaps "REGEX_ESCAPE_CHARACTERS") and could be declare
Thanks for the suggestion. The name is changed and static keyword is also 
added. By the way, I'd like to leave it in the function because it is just used 
in this function.


http://gerrit.cloudera.org:8080/#/c/8900/5/be/src/exprs/string-functions-ir.cc@637
PS5, Line 637:   DCHECK_GE(result.len, 0);
> DCHECK_GE(result.len, str.len) should be true too, no?
Correct. the length of a new generated string should be greater or equal than 
the length of the original string. Your idea is more close to what I intended.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84c3e0ded26f6eb20794c38b75be9b25cd111e4b
Gerrit-Change-Number: 8900
Gerrit-PatchSet: 5
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jan 2018 00:57:24 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3282: Adds regexp escape built-in function

[Kim Jin Chul (Code Review)]

Hello Tianyi Wang, Jim Apple, Philip Zeyliger, Tim Armstrong,

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

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

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

Change subject: IMPALA-3282: Adds regexp_escape built-in function
..

IMPALA-3282: Adds regexp_escape built-in function

Escapes the following special characters in RE2 library:
.\+*?[^]$(){}=!<>|:-

Testing:
Add some unit tests into ExprTest.StringRegexpFunctions
Add some E2E tests into exprs.test

Change-Id: I84c3e0ded26f6eb20794c38b75be9b25cd111e4b
---
M be/src/exprs/expr-test.cc
M be/src/exprs/string-functions-ir.cc
M be/src/exprs/string-functions.h
M common/function-registry/impala_functions.py
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
5 files changed, 92 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84c3e0ded26f6eb20794c38b75be9b25cd111e4b
Gerrit-Change-Number: 8900
Gerrit-PatchSet: 6
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-4835: prerequisite buffer pool changes

[Bikramjeet Vig (Code Review)]

Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9097 )

Change subject: IMPALA-4835: prerequisite buffer pool changes
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9097/1/be/src/runtime/bufferpool/buffer-pool.h
File be/src/runtime/bufferpool/buffer-pool.h:

http://gerrit.cloudera.org:8080/#/c/9097/1/be/src/runtime/bufferpool/buffer-pool.h@254
PS1, Line 254: TransferBuffer
should we add a test for this too?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f2196722df59f2d367787c0550058022e296e24
Gerrit-Change-Number: 9097
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Comment-Date: Tue, 23 Jan 2018 00:56:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6075: Add Impala daemon metric for catalog version.

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6075: Add Impala daemon metric for catalog version.
..


Patch Set 6:

Is this ready to merge?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2307eb434561ed74ff058106541c0ebda017d97
Gerrit-Change-Number: 8949
Gerrit-PatchSet: 6
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jan 2018 00:56:09 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Doc for MURMUR HASH() function

[Tim Armstrong (Code Review)]

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

Change subject: [DOCS] Doc for MURMUR_HASH() function
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I784a5a080d7d13192aac2fca67f841d2d19fc99b
Gerrit-Change-Number: 9031
Gerrit-PatchSet: 1
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jan 2018 00:53:23 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6395: Add a flag for data stream sender's buffer size

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6395: Add a flag for data stream sender's buffer size
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I385f4b7a0671bb2d7872bee60d476c375680b5c2
Gerrit-Change-Number: 9026
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jan 2018 00:53:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4132: Use -fno-omit-frame-pointer

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-4132: Use -fno-omit-frame-pointer
..


Patch Set 2:

I'll go ahead and merge this unless there are any remaining concerns.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7d0d88ba015a847356ed0274fe91017b98cb941
Gerrit-Change-Number: 8612
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jan 2018 00:41:59 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6430: Log relevant debug pages if wait for metric value times out

[Bikramjeet Vig (Code Review)]

Bikramjeet Vig has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9098


Change subject: IMPALA-6430: Log relevant debug pages if wait_for_metric_value 
times out
..

IMPALA-6430: Log relevant debug pages if wait_for_metric_value times out

Log the memz, metrics and query page if the method wait_for_metric_value
times out. This would help us understand the state of the defaulting
impalad when the time out happens.

Change-Id: I069dad48ede709c4114f4d7175861f98321be6cf
---
M tests/common/impala_service.py
1 file changed, 6 insertions(+), 2 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I069dad48ede709c4114f4d7175861f98321be6cf
Gerrit-Change-Number: 9098
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 

[Impala-ASF-CR] PREVIEW: IMPALA-4835: Part 3: switch I/O buffers to buffer pool

[Tim Armstrong (Code Review)]

Hello Bikramjeet Vig,

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

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

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

Change subject: PREVIEW: IMPALA-4835: Part 3: switch I/O buffers to buffer pool
..

PREVIEW: IMPALA-4835: Part 3: switch I/O buffers to buffer pool

This is the final patch to switch the Disk I/O manager to allocate all
buffer from the buffer pool and to reserve the buffers required for
a query upfront.

* The planner reserves enough memory to run a single scanner per
  scan node.
* The multi-threaded scan node must increase reservation before
  spinning up more threads.
* The scanner implementations must be careful to stay within their
  assigned reservation.

The row-oriented scanners were most straightforward, since they only
have a single scan range active at a time. A single I/O buffer is
sufficient to scan the whole file but more I/O buffers can improve I/O
throughput.

Parquet is more complex because it issues a scan range per column and
the sizes of the columns on disk are not known during planning. To
deal with this, the reservation in the frontend is based on a
heuristic involving the file size and # columns. The Parquet scanner
can then divvy up reservation to columns based on the size of column
data on disk.

I adjusted how the 'mem_limit' is divided between buffer pool and non
buffer pool memory for low mem_limits to account for the increase in
buffer pool memory.

Testing:
* Added more planner tests to cover reservation calcs for scan node.
* Test scanners for all file formats with the reservation denial debug
  action, to test behaviour when the scanners hit reservation limits.
* Updated memory and buffer pool limits for tests.

Change-Id: Ic09c6196b31e55b301df45cc56d0b72cfece6786
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-parquet-scanner.h
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scan-node-mt.cc
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M be/src/exec/parquet-column-readers.cc
M be/src/exec/parquet-column-readers.h
M be/src/exec/scanner-context.cc
M be/src/exec/scanner-context.h
M be/src/runtime/bufferpool/reservation-tracker-test.cc
M be/src/runtime/bufferpool/reservation-util.cc
M be/src/runtime/io/disk-io-mgr-stress-test.cc
M be/src/runtime/io/disk-io-mgr-stress.cc
M be/src/runtime/io/disk-io-mgr-stress.h
M be/src/runtime/io/disk-io-mgr-test.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
M be/src/runtime/io/request-context.cc
M be/src/runtime/io/request-context.h
M be/src/runtime/io/request-ranges.h
M be/src/runtime/io/scan-range.cc
M be/src/runtime/tmp-file-mgr.cc
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/util/BitUtil.java
M fe/src/test/java/org/apache/impala/util/BitUtilTest.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/disable-codegen.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/partition-pruning.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/sort-expr-materialization.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
M testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test
M testdata/workloads/functional-query/queries/QueryTest/codegen-mem-limit.test
M 
testdata/workloads/functional-query/queries/QueryTest/disk-spill-encryption.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level0.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level1.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
M testdata/workloads/functional-query/queries/QueryTest/nested-types-tpch.test
M testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
M testdata/workloads/functional-query/queries/QueryTest/scanners.test
M testdata/workloads/functional-query/queries/QueryTest/set.test
M testdata/workloads/functional-query/queries/QueryTest/spilling-aggs.test
M 

[Impala-ASF-CR] IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation
..


Patch Set 20: Code-Review+2

carry


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If5cb42437d11c13bc4a55c3ab426b66777332bd1
Gerrit-Change-Number: 8414
Gerrit-PatchSet: 20
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jan 2018 00:09:24 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation

[Tim Armstrong (Code Review)]

Hello Tianyi Wang, Dan Hecht,

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

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

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

Change subject: IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation
..

IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation

In preparation for switching the I/O mgr to the buffer pool, this
removes and cleans up a lot of code so that the switchover patch starts
from a cleaner slate.

* Remove the free buffer cache (which will be replaced by buffer pool's
  own caching).
* Make memory limit exceeded error checking synchronous (in anticipation
  of having to propagate buffer pool errors synchronously).
* Simplify error propagation - remove the (ineffectual) code that
  enqueued BufferDescriptors containing error statuses.
* Document locking scheme better in a few places, make it part of the
  function signature when it seemed reasonable.
* Move ReturnBuffer() to ScanRange, because it is intrinsically
  connected with the lifecycle of a scan range.
* Separate external ReturnBuffer() and internal CleanUpBuffer()
  interfaces - previously callers of ReturnBuffer() were fudging
  the num_buffers_in_reader accounting to make the external interface work.
* Eliminate redundant state in ScanRange: 'eosr_returned_' and
  'is_cancelled_'.
* Clarify the logic around calling Close() for the last
  BufferDescriptor.
  -> There appeared to be an implicit assumption that buffers would be
 freed in the order they were returned from the scan range, so that
 the "eos" buffer was returned last. Instead just count the number
 of outstanding buffers to detect the last one.
  -> Touching the is_cancelled_ field without holding a lock was hard to
 reason about - violated locking rules and it was unclear that it
 was race-free.
* Remove DiskIoMgr::Read() to simplify interface. It is trivial to
  inline at the callsites.

This will probably regress performance somewhat because of the cache
removal, so my plan is to merge it around the same time as switching
the I/O mgr to allocate from the buffer pool. I'm keeping the patches
separate to make reviewing easier.

Testing:
* Ran exhaustive tests
* Ran the disk-io-mgr-stress-test overnight

Change-Id: If5cb42437d11c13bc4a55c3ab426b66777332bd1
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/scanner-context.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/io/disk-io-mgr-stress.cc
M be/src/runtime/io/disk-io-mgr-test.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
M be/src/runtime/io/request-context.cc
M be/src/runtime/io/request-context.h
M be/src/runtime/io/request-ranges.h
M be/src/runtime/io/scan-range.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/test-env.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
19 files changed, 577 insertions(+), 909 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5cb42437d11c13bc4a55c3ab426b66777332bd1
Gerrit-Change-Number: 8414
Gerrit-PatchSet: 20
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6383: free memory after skipping parquet row groups

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6383: free memory after skipping parquet row groups
..


Patch Set 4: Code-Review+1

Carry +1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95713675455f7635fa3f72616b166f35e2a46c1a
Gerrit-Change-Number: 9059
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jan 2018 00:09:10 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5931: Generates scan ranges in planner for s3/adls

[Vuk Ercegovac (Code Review)]

Hello Dimitris Tsirogiannis,

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

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

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

Change subject: IMPALA-5931: Generates scan ranges in planner for s3/adls
..

IMPALA-5931: Generates scan ranges in planner for s3/adls

Currently, for filesystems that do not include physical
block information (e.g., block replica locations, caching),
synthetic blocks are generated and stored in the catalog
when metadata is loaded. Example file systems for which this is done
includes S3, ADLS, and local fs.

This change avoids generating these blocks when metadata is loaded.
Instead, scan ranges are directly generated from such files by the
backend coordinator. Previously, all scan ranges were produced by
the planner in HDFSScanNode in the frontend. Now, those files without
block information are sent to the coordinator represented by a split
specification that determines how the coordinator will create scan ranges
to send to executors.

This change reduces the space needed in the catalog and reduces the
scan range data structures that are passed from the frontend to the
backend when planning and coordinating a query.
In addition a bug is avoided where non-splittable files were being
split anyways to support the query parameter that places a limit on
scan ranges.

Testing:
- local fs tests cover the code paths in this change
- all core tests pass when configured with s3
- manually tried larger local filesystem tables (tpch) with multiple
  partitions and observed the same scan ranges.
- TODO: adls testing

Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
---
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler-test-util.cc
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M be/src/util/CMakeLists.txt
A be/src/util/flat_buffer.cc
A be/src/util/flat_buffer.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
11 files changed, 399 insertions(+), 179 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
Gerrit-Change-Number: 8523
Gerrit-PatchSet: 8
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-5931: Generates scan ranges in planner for s3/adls

[Vuk Ercegovac (Code Review)]

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

Change subject: IMPALA-5931: Generates scan ranges in planner for s3/adls
..


Patch Set 7:

(6 comments)

in addition to the comments, decided to move that util function to its own file.

http://gerrit.cloudera.org:8080/#/c/8523/7/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/8523/7/be/src/scheduling/scheduler.cc@509
PS7, Line 509:   vector* 
expanded_locations,
 :  int* num_generated) {
> Indentation is off
Done


http://gerrit.cloudera.org:8080/#/c/8523/7/be/src/util/compress.h
File be/src/util/compress.h:

http://gerrit.cloudera.org:8080/#/c/8523/7/be/src/util/compress.h@136
PS7, Line 136: flat buffer
> nit: FlatBuffer so that it is clear what this thing represents
Done


http://gerrit.cloudera.org:8080/#/c/8523/7/be/src/util/compress.h@139
PS7, Line 139:  
> nit: indentation
Done


http://gerrit.cloudera.org:8080/#/c/8523/7/be/src/util/compress.cc
File be/src/util/compress.cc:

http://gerrit.cloudera.org:8080/#/c/8523/7/be/src/util/compress.cc@325
PS7, Line 325:  THdfsCompression::type* 
thrift_compression) {
> Your indentation is off is several places
Done


http://gerrit.cloudera.org:8080/#/c/8523/7/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:

http://gerrit.cloudera.org:8080/#/c/8523/7/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@154
PS7, Line 154:
> nit: extra space
Done


http://gerrit.cloudera.org:8080/#/c/8523/7/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/8523/7/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@738
PS7, Line 738: supportsBlocks
> in the other file you're using hasBlockInfo. I think I prefer this one bett
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
Gerrit-Change-Number: 8523
Gerrit-PatchSet: 7
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 22 Jan 2018 23:55:32 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6190/6246: Add instances tab and event sequence

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6190/6246: Add instances tab and event sequence
..


Patch Set 13: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I626456b6afa9101ffd5cda10c4096d63d7f9
Gerrit-Change-Number: 8758
Gerrit-PatchSet: 13
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jan 2018 23:06:09 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6190/6246: Add instances tab and event sequence

[Lars Volker (Code Review)]

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

Change subject: IMPALA-6190/6246: Add instances tab and event sequence
..


Patch Set 13:

(5 comments)

Thanks for the review. Please see my inline comments and PS13.

http://gerrit.cloudera.org:8080/#/c/8758/12/be/src/common/atomic.h
File be/src/common/atomic.h:

http://gerrit.cloudera.org:8080/#/c/8758/12/be/src/common/atomic.h@148
PS12, Line 148:
> Can we add a static assert that the enum type is <= 32 bits. It seems like
Done


http://gerrit.cloudera.org:8080/#/c/8758/12/be/src/runtime/fragment-instance-state.cc
File be/src/runtime/fragment-instance-state.cc:

http://gerrit.cloudera.org:8080/#/c/8758/12/be/src/runtime/fragment-instance-state.cc@133
PS12, Line 133:   profile()->AddEventSequence("Fragment Instance Lifecycle 
Event Timeline");
> We should be careful to document what the fragment instance timings are rel
Done


http://gerrit.cloudera.org:8080/#/c/8758/12/be/src/runtime/fragment-instance-state.cc@515
PS12, Line 515: PRODUCING_DAT
> Should we call this LAST_BATCH_SENT instead for consistency?
Done


http://gerrit.cloudera.org:8080/#/c/8758/12/be/src/util/runtime-profile-counters.h
File be/src/util/runtime-profile-counters.h:

http://gerrit.cloudera.org:8080/#/c/8758/12/be/src/util/runtime-profile-counters.h@357
PS12, Line 357: return event1.second < event2.second;
> So this comment is incorrect about it being ordered?
Yes, I updated it.


http://gerrit.cloudera.org:8080/#/c/8758/12/www/query_finstances.tmpl
File www/query_finstances.tmpl:

http://gerrit.cloudera.org:8080/#/c/8758/12/www/query_finstances.tmpl@113
PS12, Line 113: clearInterval(intervalId);
> The query could also not have started executing yet.
I changed the error message, as well as the behavior to display the error 
message when the query has finished. I also noticed that query_backends freezes 
the last state of the table (by throwing an unhandled error in the JS code :o 
), which I think leaves the user wondering why it stopped updating. Since the 
query will immediately be removed upon completion we won't be able to pull a 
fully completed table from the server, so I think it's cleaner to remove it 
upon completion. Let me know if you'd prefer to preserve the last state.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I626456b6afa9101ffd5cda10c4096d63d7f9
Gerrit-Change-Number: 8758
Gerrit-PatchSet: 13
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jan 2018 22:55:41 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6190/6246: Add instances tab and event sequence

[Lars Volker (Code Review)]

Hello Michael Ho, Joe McDonnell, Tim Armstrong, Dan Hecht,

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

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

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

Change subject: IMPALA-6190/6246: Add instances tab and event sequence
..

IMPALA-6190/6246: Add instances tab and event sequence

This change adds tracking of the current state during the execution of a
fragment instance. The current state is then reported back to the
coordinator and exposed to users via a new tab in the query detail debug
webpage.

This change also adds an event timeline to fragment instances in the
query profile. The timeline measures the time since backend-local query
start at which particular events complete. Events are derived from the
current state of the execution of a fragment instance. For example:

- Prepare Finished: 13.436ms (13.436ms)
- First Batch Produced: 1s022ms (1s008ms)
- First Batch Sent: 1s022ms (455.558us)
- ExecInternal Finished: 2s783ms (1s760ms)

I added automated tests for both extensions and additionally verified
the change by manual inspection.

Here are the TPCH performance comparison results between this change and
the previous commit on a 16 node cluster.

++---+-++++
| Workload   | File Format   | Avg (s) | Delta(Avg) | GeoMean(s) | 
Delta(GeoMean) |
++---+-++++
| TPCH(_300) | parquet / none / none | 18.47   | -0.94% | 9.72   | 
-1.08% |
++---+-++++

++--+---++-++++-+---+
| Workload   | Query| File Format   | Avg(s) | Base Avg(s) | 
Delta(Avg) | StdDev(%)  | Base StdDev(%) | Num Clients | Iters |
++--+---++-++++-+---+
| TPCH(_300) | TPCH-Q5  | parquet / none / none | 48.88  | 46.93   |   
+4.15%   |   0.14%|   3.61%| 1   | 3 |
| TPCH(_300) | TPCH-Q13 | parquet / none / none | 21.64  | 21.15   |   
+2.29%   |   2.06%|   1.84%| 1   | 3 |
| TPCH(_300) | TPCH-Q11 | parquet / none / none | 1.71   | 1.70|   
+1.12%   |   0.54%|   2.51%| 1   | 3 |
| TPCH(_300) | TPCH-Q18 | parquet / none / none | 33.15  | 32.79   |   
+1.09%   |   0.13%|   2.03%| 1   | 3 |
| TPCH(_300) | TPCH-Q14 | parquet / none / none | 5.95   | 5.90|   
+0.82%   |   2.19%|   0.49%| 1   | 3 |
| TPCH(_300) | TPCH-Q1  | parquet / none / none | 13.99  | 13.90   |   
+0.63%   |   0.25%|   1.39%| 1   | 3 |
| TPCH(_300) | TPCH-Q2  | parquet / none / none | 3.44   | 3.44|   
+0.00%   | * 20.29% * | * 20.76% * | 1   | 3 |
| TPCH(_300) | TPCH-Q6  | parquet / none / none | 1.21   | 1.22|   
-0.01%   |   0.06%|   0.06%| 1   | 3 |
| TPCH(_300) | TPCH-Q20 | parquet / none / none | 3.51   | 3.51|   
-0.11%   |   7.15%|   7.30%| 1   | 3 |
| TPCH(_300) | TPCH-Q16 | parquet / none / none | 6.89   | 6.91|   
-0.21%   |   0.65%|   0.55%| 1   | 3 |
| TPCH(_300) | TPCH-Q4  | parquet / none / none | 4.78   | 4.80|   
-0.38%   |   0.06%|   0.59%| 1   | 3 |
| TPCH(_300) | TPCH-Q19 | parquet / none / none | 30.78  | 31.04   |   
-0.83%   |   0.45%|   1.03%| 1   | 3 |
| TPCH(_300) | TPCH-Q22 | parquet / none / none | 6.06   | 6.12|   
-1.02%   |   1.51%|   2.12%| 1   | 3 |
| TPCH(_300) | TPCH-Q10 | parquet / none / none | 9.43   | 9.58|   
-1.54%   |   0.69%|   3.30%| 1   | 3 |
| TPCH(_300) | TPCH-Q21 | parquet / none / none | 93.41  | 95.18   |   
-1.86%   |   0.08%|   0.81%| 1   | 3 |
| TPCH(_300) | TPCH-Q15 | parquet / none / none | 3.40   | 3.47|   
-1.99%   |   0.72%|   1.27%| 1   | 3 |
| TPCH(_300) | TPCH-Q7  | parquet / none / none | 44.98  | 46.24   |   
-2.71%   |   1.83%|   1.27%| 1   | 3 |
| TPCH(_300) | TPCH-Q3  | parquet / none / none | 28.06  | 29.11   |   
-3.61%   |   1.62%|   1.23%| 1   | 3 |
| TPCH(_300) | TPCH-Q12 | parquet / none / none | 3.15   | 3.28|   
-3.80%   |   0.96%|   1.32%| 1   | 3 |
| TPCH(_300) | TPCH-Q9  | parquet / none / none | 29.47  | 30.80   |   
-4.30%   |   0.29%|   0.34%| 1   | 3 |
| TPCH(_300) | TPCH-Q17 | parquet / none / none 

[Impala-ASF-CR] IMPALA-4835: prerequisite buffer pool changes

[Tim Armstrong (Code Review)]

Tim Armstrong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9097


Change subject: IMPALA-4835: prerequisite buffer pool changes
..

IMPALA-4835: prerequisite buffer pool changes

The scanner/buffer pool changes will have different scanner
threads sharing the same buffer pool client. This requires that the
AllocateBuffer() API is safe to call concurrently from different
threads, which was true previously but not documented or tested.

This updates the comments and adds a test.

Change-Id: I8f2196722df59f2d367787c0550058022e296e24
---
M be/src/runtime/bufferpool/buffer-pool-test.cc
M be/src/runtime/bufferpool/buffer-pool.h
2 files changed, 70 insertions(+), 4 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8f2196722df59f2d367787c0550058022e296e24
Gerrit-Change-Number: 9097
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5654: Disallow setting Kudu table name in CREATE TABLE

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5654: Disallow setting Kudu table name in CREATE TABLE
..


Patch Set 18: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieca037498abf8f5fde67b77e824b720482cdbe6f
Gerrit-Change-Number: 8820
Gerrit-PatchSet: 18
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 22 Jan 2018 22:34:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5654: Disallow setting Kudu table name in CREATE TABLE

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5654: Disallow setting Kudu table name in CREATE TABLE
..

IMPALA-5654: Disallow setting Kudu table name in CREATE TABLE

This change disallows explicitly setting the Kudu table name property
for managed Kudu tables in a CREATE TABLE statement. The Kudu table
name property gets a generated value as the following:
'impala::db_name.table_name' where table_name is the one given in
the CREATE TABLE statement.
Providing the Kudu table name property when creating a managed Kudu
table results in an error without creating the table. E.g.:
CREATE TABLE t (i INT) STORED AS KUDU
  TBLPROPERTIES('kudu.table_name'='some_name');

Alongside the CREATE TABLE statement also the ALTER TABLE statement
is changed not to allow the modification of Kudu.table_name of
managed Kudu tables.

Change-Id: Ieca037498abf8f5fde67b77e824b720482cdbe6f
Reviewed-on: http://gerrit.cloudera.org:8080/8820
Reviewed-by: Dimitris Tsirogiannis 
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
M tests/query_test/test_kudu.py
7 files changed, 160 insertions(+), 109 deletions(-)

Approvals:
  Dimitris Tsirogiannis: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ieca037498abf8f5fde67b77e824b720482cdbe6f
Gerrit-Change-Number: 8820
Gerrit-PatchSet: 19
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 

[Impala-ASF-CR] IMPALA-6418: Find a reliable way to detect supported TLS versions

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6418: Find a reliable way to detect supported TLS 
versions
..


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
Gerrit-Change-Number: 9060
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 22 Jan 2018 22:25:49 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3916: Reserve SQL:2016 reserved words

[Tianyi Wang (Code Review)]

Tianyi Wang has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9096


Change subject: IMPALA-3916: Reserve SQL:2016 reserved words
..

IMPALA-3916: Reserve SQL:2016 reserved words

This patch reserves SQL:2016 reserved words, excluding:
1. Impala builtin function names.
2. Time unit words(year, month, etc.).
3. An exception list based on a discussion.

Some test cases are modified to avoid these words. A flag
reserve_impala3_words is added. It is true by default.

Change-Id: If1b295e6a77e840cf1b794c2eb73e1b9d2b8ddd6
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M testdata/workloads/functional-query/queries/QueryTest/empty-build-joins.test
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
M testdata/workloads/functional-query/queries/QueryTest/partition-col-types.test
M testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test
M testdata/workloads/functional-query/queries/QueryTest/spilling-large-rows.test
M testdata/workloads/functional-query/queries/QueryTest/values.test
M tests/custom_cluster/test_stats_extrapolation.py
M tests/query_test/test_sort.py
23 files changed, 487 insertions(+), 433 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If1b295e6a77e840cf1b794c2eb73e1b9d2b8ddd6
Gerrit-Change-Number: 9096
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 

[Impala-ASF-CR] IMPALA-6418: Find a reliable way to detect supported TLS versions

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9060 )

Change subject: IMPALA-6418: Find a reliable way to detect supported TLS 
versions
..


Patch Set 6: Code-Review+2

Carry +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
Gerrit-Change-Number: 9060
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 22 Jan 2018 22:25:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6418: Find a reliable way to detect supported TLS versions

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Dan Burkert, Philip Zeyliger,

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

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

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

Change subject: IMPALA-6418: Find a reliable way to detect supported TLS 
versions
..

IMPALA-6418: Find a reliable way to detect supported TLS versions

Due to a RHEL bug, finding the version of the dynamically linked
OpenSSL is unreliable on certain RHEL platforms. This means that
if Impala is built against OpenSSL 1.0.0 and run against OpenSSL
1.0.1, due to the bug, Impala will not be able to access the
capabilities of OpenSSL 1.0.1. The most significant drawback of
this is that in this scenario, Impala cannot use TLSv1.2, even though
it is available.

The RHEL bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1497859

This patch imitates Kudu's way of determining what TLS versions are
supported via this patch:
https://github.com/apache/kudu/commit/b88117415a02699c12a6eacbf065c4140ee0963c

This is tested and works on all supported platforms of Impala.

Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
---
M be/src/rpc/thrift-client.cc
M be/src/rpc/thrift-server.cc
M be/src/util/openssl-util.cc
M be/src/util/openssl-util.h
4 files changed, 45 insertions(+), 12 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
Gerrit-Change-Number: 9060
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-6418: Find a reliable way to detect supported TLS versions

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Dan Burkert, Philip Zeyliger,

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

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

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

Change subject: IMPALA-6418: Find a reliable way to detect supported TLS 
versions
..

IMPALA-6418: Find a reliable way to detect supported TLS versions

Due to a RHEL bug, finding the version of the dynamically linked
OpenSSL is unreliable on certain RHEL platforms. This means that
if Impala is built against OpenSSL 1.0.0 and run against OpenSSL
1.0.1, due to the bug, Impala will not be able to access the
capabilities of OpenSSL 1.0.1. The most significant drawback of
this is that in this scenario, Impala cannot use TLSv1.2, even though
it is available.

The RHEL bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1497859

This patch imitates Kudu's way of determining what TLS versions are
supported via this patch:
https://github.com/apache/kudu/commit/b88117415a02699c12a6eacbf065c4140ee0963c

This is tested and works on all supported platforms of Impala.

Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
---
M be/src/rpc/thrift-client.cc
M be/src/rpc/thrift-server.cc
M be/src/util/openssl-util.cc
M be/src/util/openssl-util.h
4 files changed, 45 insertions(+), 12 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
Gerrit-Change-Number: 9060
Gerrit-PatchSet: 5
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5152: Introduce metadata loading phase

[Alex Behm (Code Review)]

Hello Bharath Vissapragada, Dimitris Tsirogiannis, Vuk Ercegovac,

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

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

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

Change subject: IMPALA-5152: Introduce metadata loading phase
..

IMPALA-5152: Introduce metadata loading phase

Reworks the collection and loading of missing metadata
when compiling a statement. Introduces a new
metadata-loading phase between parsing and analysis.
Summary of the new compilation flow:
1. Parse statement.
2. Collect all table references from the parsed
   statement and generate a list of tables that need
   to be loaded for analysis to succeed.
3. Request missing metadata and wait for it to arrive.
   As views become loaded we expand the set of required
   tables based on the view definitions.
   This step populates a statement-local table cache
   that contains all loaded tables relevant to the
   statement.
4. Create a new Analyzer with the table cache and
   analyze the statement. During analysis only the
   table cache is consulted for table metadata, the
   ImpaladCatalog is not used for that purpose anymore.
5. Authorize the statement.
6. Plan generation as usual.

The intent of the existing code was to collect all tables
missing metadata during analysis, load the metadata, and then
re-analyze the statement (and repeat those steps until all
metadata is loaded).
Unfortunately, the relevant code was hard-to-follow, subtle
and not well tested, and therefore it was broken in several
ways over the course of time. For example, the introduction
of path analysis for nested types subtly broke the intended
behavior, and there are other similar examples.

The serial table loading observed in the JIRA was caused by the
following code in the resolution of table references:
for (all path interpretations) {
  try {
// Try to resolve the path; might call getTable() which
// throws for nonexistent tables.
  } catch (AnalysisException e) {
if (analyzer.hasMissingTbls()) throw e;
  }
}

The following example illustrates the problem:
SELECT * FROM a.b, x.y
When resolving the path "a.b" we consider that "a" could be a
database or a table. Similarly, "b" could be a table or a
nested collection.
If the path resolution for "a.b" adds a missing table entry,
then the path resolution for "x.y" could exit prematurely,
without trying the other path interpretations that would
lead to adding the expected missing table. So effectively,
the tables end up being loaded one-by-one.

Testing:
- A core/hdfs run succeeded
- No new tests were added because the existing functional tests
  provide good coverage of various metadata loading scenarios.
- The issue reported in IMPALA-5152 is basically impossible now.
  Adding FE unit tests for that bug specifically would require
  ugly changes to the new code to enable such testing.

Change-Id: I68d32d5acd4a6f6bc6cedb05e6cc5cf604d24a55
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/AuthorizationStmt.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LimitElement.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SetStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java
M 

[Impala-ASF-CR] IMPALA-5152: Introduce metadata loading phase

[Alex Behm (Code Review)]

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

Change subject: IMPALA-5152: Introduce metadata loading phase
..


Patch Set 1:

(39 comments)

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

http://gerrit.cloudera.org:8080/#/c/8958/1/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java@73
PS1, Line 73: if (!fromClauseOnly) tblRefs.add(new 
TableRef(tableName_.toPath(), null));
> Maybe replace the if check with a Preconditions.checkState(!fromClauseOnly)
Good point. This Preconditions check would be sensible in many places, so I 
opted to clean up
the collectTabeRefs() interface instead because fromClauseonly=true is only 
needed for QueryStmts.


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

http://gerrit.cloudera.org:8080/#/c/8958/1/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@66
PS1, Line 66: // Set in analyzeAndAuthorize().
:   private ImpaladCatalog catalog_;
> If I understand it correctly, that shouldn't be needed, given that analysis
Need this for AnalysisContext#checkSystemDbAccess which is annoying but also 
difficult to clean up (I tried).


http://gerrit.cloudera.org:8080/#/c/8958/1/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@367
PS1, Line 367:
> IMO, this method should be documented, given this is the starting point of
Done


http://gerrit.cloudera.org:8080/#/c/8958/1/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@518
PS1, Line 518: analysisResult_.isAlterTableStmt() ||
 : analysisResult_.isAlterViewStmt());
> Curious, where did this come from?
We can add column priv requests for something like this:

use functional;
alter table allcomplextypes.int_array_col set fileformat sequencefile;

The table path resolves to a column and we register an auth request.

Actually, I think any alter table could hit this, so generalized this check to 
any AlterTableStmt.


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

http://gerrit.cloudera.org:8080/#/c/8958/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@309
PS1, Line 309: private final Map loadedTables;
> nit: My feeling is that we can call this referencedTables_ (and backup with
Renamed to stmtTableCache.

I renamed this because "referencedTables" could be misleading in the sense that 
this map could contain tables that are needed for resolving a path, but are not 
actually the target of that path. So in some sense not all tables are really 
"referenced" by the statement.


http://gerrit.cloudera.org:8080/#/c/8958/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2361
PS1, Line 2361: or the db
> I don't think the existence of db is checked in this function anymore.
Done


http://gerrit.cloudera.org:8080/#/c/8958/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2372
PS1, Line 2372: if (table.isLoaded() && table instanceof IncompleteTable) {
> Preconditions.checkState(table.isLoaded())?
Done


http://gerrit.cloudera.org:8080/#/c/8958/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2383
PS1, Line 2383: wit
> nit: with
Done


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

http://gerrit.cloudera.org:8080/#/c/8958/1/fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java@82
PS1, Line 82: tableName_
> other statements assert that tableName is not null on construction. this st
Done


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

http://gerrit.cloudera.org:8080/#/c/8958/1/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java@85
PS1, Line 85: tableName_
> can tableName_ be null?
Added Preonditions check in c'tor.


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

http://gerrit.cloudera.org:8080/#/c/8958/1/fe/src/main/java/org/apache/impala/analysis/FromClause.java@68
PS1, Line 68: public
> @Override
Does not override since it's not a StatementBase.


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

[Impala-ASF-CR] IMPALA-5801: [draft] Clean up codegen GetType() interface

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-5801: [draft] Clean up codegen GetType() interface
..


Patch Set 1:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/9063/1/be/src/codegen/llvm-codegen.h
File be/src/codegen/llvm-codegen.h:

http://gerrit.cloudera.org:8080/#/c/9063/1/be/src/codegen/llvm-codegen.h@253
PS1, Line 253: GetInternalRepresentation
Maybe GetSlotType() since it's how the slot within a Tuple is represented? We 
use that terminology in some other places.


http://gerrit.cloudera.org:8080/#/c/9063/1/be/src/codegen/llvm-codegen.h@263
PS1, Line 263: GetType
I wonder if we should rename this to GetNamedType() or GetTypeFromModule() or 
something along those lines?


http://gerrit.cloudera.org:8080/#/c/9063/1/be/src/codegen/llvm-codegen.h@496
PS1, Line 496:   llvm::Type* boolean_type() { return 
GetInternalRepresentation(TYPE_BOOLEAN); }
> Maybe bool_type would be more logical in a c++ context.
Makes sense to me.


http://gerrit.cloudera.org:8080/#/c/9063/1/be/src/codegen/llvm-codegen.h@497
PS1, Line 497:   llvm::Type* i8_type() { return 
GetInternalRepresentation(TYPE_TINYINT); }
Any reason not to call the LLVM functions directly, like i128_type() i.e. 
llvm::Type::getInt8Ty(context());


http://gerrit.cloudera.org:8080/#/c/9063/1/be/src/codegen/llvm-codegen.h@497
PS1, Line 497:   llvm::Type* i8_type() { return 
GetInternalRepresentation(TYPE_TINYINT); }
 :   llvm::Type* i16_type() { return 
GetInternalRepresentation(TYPE_SMALLINT); }
 :   llvm::Type* i32_type() { return 
GetInternalRepresentation(TYPE_INT); }
 :   llvm::Type* i64_type() { return 
GetInternalRepresentation(TYPE_BIGINT); }
> I have chosen i8_type() instead of int8_type(), because i128_type() already
The short type names seem good.


http://gerrit.cloudera.org:8080/#/c/9063/1/be/src/codegen/llvm-codegen.h@524
PS1, Line 524: I8Constant
I think GetI8Constant() would be more consistent with other name (although more 
verbose unfortunately).


http://gerrit.cloudera.org:8080/#/c/9063/1/be/src/codegen/llvm-codegen.h@525
PS1, Line 525: return GetIntConstant(TYPE_TINYINT, val);
We could also just call the LLVM function directly here.


http://gerrit.cloudera.org:8080/#/c/9063/1/be/src/exprs/slot-ref.cc
File be/src/exprs/slot-ref.cc:

http://gerrit.cloudera.org:8080/#/c/9063/1/be/src/exprs/slot-ref.cc@191
PS1, Line 191:   llvm::Value* tuple_offset = 
llvm::ConstantInt::get(codegen->i32_type(), tuple_idx_);
I feel like this is more readable, thank you.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib146ca21af82023b0b460f813eae3435b4b2eb77
Gerrit-Change-Number: 9063
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jan 2018 22:05:00 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6394: Enable HDFS debug logging

[Philip Zeyliger (Code Review)]

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

Change subject: IMPALA-6394: Enable HDFS debug logging
..


Patch Set 1:

I don't really have a sense of how frequent this is. I would recommend 
triggering 
https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/build?delay=0sec ~10 
times in a row with this patch to see what falls out.

(You can probably disable the test execution too, to speed it up considerably.)

If it's more infrequent than that, I'm fine with checking this in and observing 
builds.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide09d5abb141dbbb6bc1ee66b69144ac41f841d9
Gerrit-Change-Number: 9082
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 22 Jan 2018 22:04:57 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3998: Remove refresh after connect option from shell

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-3998: Remove refresh_after_connect option from shell
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f430bad0578e150d5e80066b9e7572041af4a15
Gerrit-Change-Number: 9072
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jan 2018 22:03:36 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3998: Remove refresh after connect option from shell

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-3998: Remove refresh_after_connect option from shell
..


Patch Set 4: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9072/2/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/9072/2/tests/shell/test_shell_commandline.py@a418
PS2, Line 418:
> This config file still has the 'refresh_after_connect=true' option. You may
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f430bad0578e150d5e80066b9e7572041af4a15
Gerrit-Change-Number: 9072
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jan 2018 22:03:24 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3998: Remove refresh after connect option from shell

[Tim Armstrong (Code Review)]

Hello Dimitris Tsirogiannis,

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

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

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

Change subject: IMPALA-3998: Remove refresh_after_connect option from shell
..

IMPALA-3998: Remove refresh_after_connect option from shell

This removes the deprecated option in time for 3.0.

Testing:
Ran core tests. Manually ran the shell with the argument to confirm that
it reported "no such option".

Cherry-picks: not for 2.x.

Change-Id: I8f430bad0578e150d5e80066b9e7572041af4a15
---
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/bad_impalarc
M tests/shell/good_impalarc
M tests/shell/test_shell_commandline.py
6 files changed, 1 insertion(+), 30 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8f430bad0578e150d5e80066b9e7572041af4a15
Gerrit-Change-Number: 9072
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dimitris Tsirogiannis 

[Impala-ASF-CR] IMPALA-6219: Use AES-GCM for spill-to-disk encryption

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6219: Use AES-GCM for spill-to-disk encryption
..


Patch Set 7:

(1 comment)

Thank you for the patch! It looks good, we just need to update the interface 
and comments to EncryptionKey because the usage pattern of the interface has 
changed subtle.

http://gerrit.cloudera.org:8080/#/c/9032/7/be/src/util/openssl-util.h
File be/src/util/openssl-util.h:

http://gerrit.cloudera.org:8080/#/c/9032/7/be/src/util/openssl-util.h@116
PS7, Line 116: const
This isn't really const if it modifies gcm_tag_, so we should remove the const 
specifier.

This also changes the relationship between Encrypt() and Decrypt(), because 
they are now stateful and Decrypt() can only decrypt the result of the last 
Encrypt() call. This actually seems fine, but we should remove the "const" and 
update the comments for Encrypt() and Decrypt().



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ea87b82a8897ee8bfa187715ac1c52883790d24
Gerrit-Change-Number: 9032
Gerrit-PatchSet: 7
Gerrit-Owner: Xianda Ke 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Xianda Ke 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 22 Jan 2018 22:00:04 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6418: Find a reliable way to detect supported TLS versions

[Michael Ho (Code Review)]

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

Change subject: IMPALA-6418: Find a reliable way to detect supported TLS 
versions
..


Patch Set 4: Code-Review+2

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9060/4/be/src/rpc/thrift-server.cc
File be/src/rpc/thrift-server.cc:

http://gerrit.cloudera.org:8080/#/c/9060/4/be/src/rpc/thrift-server.cc@93
PS4, Line 93:
Is it fair to DCHECK_LE(protocol, TLSv1_2_plus) given the way we treat default 
below in the switch statement ?


http://gerrit.cloudera.org:8080/#/c/9060/4/be/src/rpc/thrift-server.cc@99
PS4, Line 99:default:
DCHECK_GE(max_supported_tls_version, TLS_1_2_VERSION);


http://gerrit.cloudera.org:8080/#/c/9060/4/be/src/rpc/thrift-server.cc@102
PS4, Line 102:   // All other versions supported by OpenSSL 1.0.1 and later.
 :   return true;
not needed ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
Gerrit-Change-Number: 9060
Gerrit-PatchSet: 4
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 22 Jan 2018 21:55:49 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6059: Enhance ltrim()/rtrim() functions to trim any set of characters.

[Zoram Thanga (Code Review)]

Zoram Thanga has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8349 )

Change subject: IMPALA-6059: Enhance ltrim()/rtrim() functions to trim any set 
of characters.
..


Patch Set 11:

Let's wait for https://gerrit.cloudera.org/#/c/9079/ to be merged before 
rebasing, so that we're not again held up by a flaky test failing.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a5ae3f59762e70c3268a01e14ed57a9e36b8d79
Gerrit-Change-Number: 8349
Gerrit-PatchSet: 11
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Mon, 22 Jan 2018 21:36:07 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6059: Enhance ltrim()/rtrim() functions to trim any set of characters.

[Michael Ho (Code Review)]

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

Change subject: IMPALA-6059: Enhance ltrim()/rtrim() functions to trim any set 
of characters.
..


Patch Set 11: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a5ae3f59762e70c3268a01e14ed57a9e36b8d79
Gerrit-Change-Number: 8349
Gerrit-PatchSet: 11
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Mon, 22 Jan 2018 21:33:08 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5903: Inconsistent specification of result set and result set metadata

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9090 )

Change subject: IMPALA-5903: Inconsistent specification of result set and 
result set metadata
..


Patch Set 1:

I see that this patch fixes some inconsistencies in the code but if you take a 
look at all the ddl statements, it's kind of random which return a result set 
('summary') and which don't. My thinking is that if we want to go down that 
path we should establish a consistent behavior across all statements. The other 
concern with this patch is about testing. How do you know if this change will 
break some clients? How do you plan to test this?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic542fb8e49e850052416ac663ee329ee3974e3b9
Gerrit-Change-Number: 9090
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jan 2018 20:30:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6418: Find a reliable way to detect supported TLS versions

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Dan Burkert, Philip Zeyliger,

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

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

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

Change subject: IMPALA-6418: Find a reliable way to detect supported TLS 
versions
..

IMPALA-6418: Find a reliable way to detect supported TLS versions

Due to a RHEL bug, finding the version of the dynamically linked
OpenSSL is unreliable on certain RHEL platforms. This means that
if Impala is built against OpenSSL 1.0.0 and run against OpenSSL
1.0.1, due to the bug, Impala will not be able to access the
capabilities of OpenSSL 1.0.1. The most significant drawback of
this is that in this scenario, Impala cannot use TLSv1.2, even though
it is available.

The RHEL bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1497859

This patch imitates Kudu's way of determining what TLS versions are
supported via this patch:
https://github.com/apache/kudu/commit/b88117415a02699c12a6eacbf065c4140ee0963c

This is tested and works on all supported platforms of Impala.

Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
---
M be/src/rpc/thrift-client.cc
M be/src/rpc/thrift-server.cc
M be/src/util/openssl-util.cc
M be/src/util/openssl-util.h
4 files changed, 43 insertions(+), 10 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
Gerrit-Change-Number: 9060
Gerrit-PatchSet: 4
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5931: Generates scan ranges in planner for s3/adls

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8523 )

Change subject: IMPALA-5931: Generates scan ranges in planner for s3/adls
..


Patch Set 7: Code-Review+1

(7 comments)

Mostly minor comments but I am happy with the FE changes. You should ask 
someone to take another look at BE as well.

http://gerrit.cloudera.org:8080/#/c/8523/7/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/8523/7/be/src/scheduling/scheduler.cc@509
PS7, Line 509: expanded_locations
Maybe rename to 'generated_scan_ranges'?


http://gerrit.cloudera.org:8080/#/c/8523/7/be/src/scheduling/scheduler.cc@509
PS7, Line 509:   vector* 
expanded_locations,
 :  int* num_generated) {
Indentation is off


http://gerrit.cloudera.org:8080/#/c/8523/7/be/src/util/compress.h
File be/src/util/compress.h:

http://gerrit.cloudera.org:8080/#/c/8523/7/be/src/util/compress.h@136
PS7, Line 136: flat buffer
nit: FlatBuffer so that it is clear what this thing represents


http://gerrit.cloudera.org:8080/#/c/8523/7/be/src/util/compress.h@139
PS7, Line 139:  
nit: indentation


http://gerrit.cloudera.org:8080/#/c/8523/7/be/src/util/compress.cc
File be/src/util/compress.cc:

http://gerrit.cloudera.org:8080/#/c/8523/7/be/src/util/compress.cc@325
PS7, Line 325:  THdfsCompression::type* 
thrift_compression) {
Your indentation is off is several places


http://gerrit.cloudera.org:8080/#/c/8523/7/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:

http://gerrit.cloudera.org:8080/#/c/8523/7/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@154
PS7, Line 154:
nit: extra space


http://gerrit.cloudera.org:8080/#/c/8523/7/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/8523/7/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@738
PS7, Line 738: supportsBlocks
in the other file you're using hasBlockInfo. I think I prefer this one better :)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
Gerrit-Change-Number: 8523
Gerrit-PatchSet: 7
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 22 Jan 2018 19:50:13 +
Gerrit-HasComments: Yes

[native-toolchain-CR] Bump Kudu version to c6beta-impala-toolchain-tag1

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9086 )

Change subject: Bump Kudu version to c6beta-impala-toolchain-tag1
..


Patch Set 1:

The toolchain job failed because it pulls in Kudu from the public github, but 
the tag we want to build against here only exists in the internal github.

Working on getting the job fixed, and I'll post here again when I get a 
successful run.


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I409cf30dc1554630bdebca1e831c22fc36e024d0
Gerrit-Change-Number: 9086
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Mon, 22 Jan 2018 19:38:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6418: Find a reliable way to detect supported TLS versions

[Michael Ho (Code Review)]

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

Change subject: IMPALA-6418: Find a reliable way to detect supported TLS 
versions
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9060/3/be/src/rpc/thrift-server.cc
File be/src/rpc/thrift-server.cc:

http://gerrit.cloudera.org:8080/#/c/9060/3/be/src/rpc/thrift-server.cc@94
PS3, Line 94:   bool is_openssl_1_0_0_or_lower = (max_supported_tls_version < 
TLS1_1_VERSION);
:   if (is_openssl_1_0_0_or_lower) return (protocol == TLSv1_0_plus 
|| protocol == TLSv1_0)
if (max_supported_tls_version < TLS_1_VERSION) {
   return protocol == TLSv1_0_plus || protocol == TLSv1_0;
}


http://gerrit.cloudera.org:8080/#/c/9060/3/be/src/rpc/thrift-server.cc@97
PS3, Line 97: // All other versions supported by OpenSSL 1.0.1 and later.
The old code made the assumption that Open SSL 1.0.1 or later will always 
support TLS 1.1 and TLS 1.2.

The new code is inferring the version by checking the max TLS version supported 
but this may break if there exists a version of the library which supports TLS 
1.1 but not TLS 1.2. I am not entirely sure if such a version exists but we 
don't have proof that it doesn't either.

I wonder if the code will be more robust if we do the following instead:

switch (max_supported_tls_version) {
   case TLS1_VERSION:
   return protocol == TLSv1_0_plus || protocol == TLSv1_0;
   case TLS1_1_VERSION:
   return protocol != TLSv1_2_plus && protocol != TLS_v1_2_0;
   default:
   return true;
}



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
Gerrit-Change-Number: 9060
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 22 Jan 2018 19:35:52 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6399: Fix timeout logic in test query profile thrift timestamps

[Lars Volker (Code Review)]

Hello Sailesh Mukil, Zoram Thanga, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-6399: Fix timeout logic in 
test_query_profile_thrift_timestamps
..

IMPALA-6399: Fix timeout logic in test_query_profile_thrift_timestamps

In the previous fix for IMPALA-6399 we increased the number of retries.
However, there was a bug in the retry logic: If the profile was missing,
we would call 'continue' without sleeping, thus eating through the
maximum number of retries in a short period of time.

This change now switches to a time-based wait-loop instead of a set
number of retries. It also moves the sleep() to the beginning of the
loop to make it less likely to forget  it when changing the code in the
future. Calling sleep() before trying to fetch the profile for the first
time also reduces the likelihood of hitting a warning when the profile
is not yet available.

This change also increases the timeout to 300 seconds and marks the test
for serial execution to decrease the likelihood of races.

Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
---
M tests/query_test/test_observability.py
1 file changed, 17 insertions(+), 14 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
Gerrit-Change-Number: 9079
Gerrit-PatchSet: 6
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zoram Thanga 

[Impala-ASF-CR] IMPALA-6418: Find a reliable way to detect supported TLS versions

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Dan Burkert, Philip Zeyliger,

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

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

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

Change subject: IMPALA-6418: Find a reliable way to detect supported TLS 
versions
..

IMPALA-6418: Find a reliable way to detect supported TLS versions

Due to a RHEL bug, finding the version of the dynamically linked
OpenSSL is unreliable on certain RHEL platforms. This means that
if Impala is built against OpenSSL 1.0.0 and run against OpenSSL
1.0.1, due to the bug, Impala will not be able to access the
capabilities of OpenSSL 1.0.1. The most significant drawback of
this is that in this scenario, Impala cannot use TLSv1.2, even though
it is available.

The RHEL bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1497859

This patch imitates Kudu's way of determining what TLS versions are
supported via this patch:
https://github.com/apache/kudu/commit/b88117415a02699c12a6eacbf065c4140ee0963c

This is tested and works on all supported platforms of Impala.

Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
---
M be/src/rpc/thrift-client.cc
M be/src/rpc/thrift-server.cc
M be/src/util/openssl-util.cc
M be/src/util/openssl-util.h
4 files changed, 38 insertions(+), 10 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
Gerrit-Change-Number: 9060
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 

[native-toolchain-CR] IMPALA-6401 : Re-apply the PPC Breakpad patches

[Lars Volker (Code Review)]

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

Change subject: IMPALA-6401 : Re-apply the PPC Breakpad patches
..

IMPALA-6401 : Re-apply the PPC Breakpad patches

This change adds the updated PPC patch for Breakpad

Change-Id: Ia40e6f9a0054b55e72350ddbb96c9ecac53b0419
---
M buildall.sh
A 
source/breakpad/breakpad-97a98836768f8f0154f8f86e5e14c2bb7e74132e-patches/0002-Build-breakpad-97a98836-on-ppc64le.patch
2 files changed, 475 insertions(+), 1 deletion(-)

Approvals:
  Lars Volker: Looks good to me, approved; Verified

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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia40e6f9a0054b55e72350ddbb96c9ecac53b0419
Gerrit-Change-Number: 9077
Gerrit-PatchSet: 1
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Valencia Edna Serrao 

[Impala-ASF-CR] IMPALA-5654: Disallow setting Kudu table name in CREATE TABLE

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8820 )

Change subject: IMPALA-5654: Disallow setting Kudu table name in CREATE TABLE
..


Patch Set 18: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieca037498abf8f5fde67b77e824b720482cdbe6f
Gerrit-Change-Number: 8820
Gerrit-PatchSet: 18
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 22 Jan 2018 18:37:25 +
Gerrit-HasComments: No

[native-toolchain-CR] IMPALA-6401 : Re-apply the PPC Breakpad patches

[Lars Volker (Code Review)]

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

Change subject: IMPALA-6401 : Re-apply the PPC Breakpad patches
..


Patch Set 1:

I ran md5sum on x86 on all binaries with and without the patch and found that 
the core2md binary has a different checksum. All other binaries match between 
the builds.

Do you know why that checksum changed? The source doesn't get patched as far as 
I can see.


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia40e6f9a0054b55e72350ddbb96c9ecac53b0419
Gerrit-Change-Number: 9077
Gerrit-PatchSet: 1
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-Comment-Date: Mon, 22 Jan 2018 17:21:22 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4132: Use -fno-omit-frame-pointer

[Lars Volker (Code Review)]

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

Change subject: IMPALA-4132: Use -fno-omit-frame-pointer
..


Patch Set 2: Code-Review+1

> Patch Set 2:
>
> Checked the binary sizes for release builds:
>
> Binary size of impalad (output of ls in bytes):
> -fno-omit-frame-pointers: 494185576
> -fomit-frame-pointers:491523224
>
> size of the whole be/build/debug dir (output of du in kbytes):
> -fno-omit-frame-pointers: 51365652
> -fomit-frame-pointers:51087284

Thanks for the update. I think all open questions are answered now.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7d0d88ba015a847356ed0274fe91017b98cb941
Gerrit-Change-Number: 8612
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jan 2018 16:58:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4132: Use -fno-omit-frame-pointer

[Gabor Kaszab (Code Review)]

Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8612 )

Change subject: IMPALA-4132: Use -fno-omit-frame-pointer
..


Patch Set 2:

Checked the binary sizes for release builds:

Binary size of impalad (output of ls in bytes):
-fno-omit-frame-pointers: 494185576
-fomit-frame-pointers:491523224

size of the whole be/build/debug dir (output of du in kbytes):
-fno-omit-frame-pointers: 51365652
-fomit-frame-pointers:51087284


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7d0d88ba015a847356ed0274fe91017b98cb941
Gerrit-Change-Number: 8612
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jan 2018 15:59:36 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3942: Fix wrongly escaped string literal in front-end

[Kim Jin Chul (Code Review)]

Kim Jin Chul has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8818 )

Change subject: IMPALA-3942: Fix wrongly escaped string literal in front-end
..


Patch Set 7:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/8818/7/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
File fe/src/main/java/org/apache/impala/analysis/StringLiteral.java:

http://gerrit.cloudera.org:8080/#/c/8818/7/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@87
PS7, Line 87: return BaseSemanticAnalyzer.unescapeSQLString("'" + 
getNormalizedValue()
> whitespace
Done


http://gerrit.cloudera.org:8080/#/c/8818/7/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@91
PS7, Line 91:   // Assumes a string literal from single quotes and escapes it 
because:
> What does this function do and return? Please consider these snippets which
Done


http://gerrit.cloudera.org:8080/#/c/8818/7/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@100
PS7, Line 100:   private String getNormalizedValue() {
> nit: we typically use /* */ style comment for class and function comments
Done


http://gerrit.cloudera.org:8080/#/c/8818/7/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@101
PS7, Line 101: final int lengthOfValue = value_.length();
> len?
Done


http://gerrit.cloudera.org:8080/#/c/8818/7/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
File fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java:

http://gerrit.cloudera.org:8080/#/c/8818/7/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java@468
PS7, Line 468:   public void escapeStringLiteralTest() {
> normalizeStringLiteralsTest
Done


http://gerrit.cloudera.org:8080/#/c/8818/7/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java@471
PS7, Line 471: testToSql("select \"'\"", "SELECT '\\''");
> add a few tests with single quotes that show they are not transformed
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc4b5f5d8ffaa8feb96a466959427a04b3b06fec
Gerrit-Change-Number: 8818
Gerrit-PatchSet: 7
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jan 2018 15:11:58 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5903: Inconsistent specification of result set and result set metadata

[Zoltan Borok-Nagy (Code Review)]

Zoltan Borok-Nagy has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9090


Change subject: IMPALA-5903: Inconsistent specification of result set and 
result set metadata
..

IMPALA-5903: Inconsistent specification of result set and result set metadata

Some statements that return a result set declare it in
Frontend.java, but others do not. E.g. COMPUTE STATS returns
a result set, but in Frontend.createCatalogOpRequest() we
declare an empty schema for the result set.

I found inconsistencies amongst the ALTER TABLE statements.
They all handled the same way in Frontend.java, but in
CatalogOpExecutor.alterTable() around half of the various
ALTER TABLE statements return a result set, the other half
don't. Now in Frontend.java we declare the appropriate result
set schema for the ALTER TABLE statements.

Change-Id: Ic542fb8e49e850052416ac663ee329ee3974e3b9
---
M 
fe/src/main/java/org/apache/impala/analysis/AlterTableAddDropRangePartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddReplaceColsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewRenameStmt.java
M 
fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetCachedStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetColumnStats.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
18 files changed, 59 insertions(+), 2 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic542fb8e49e850052416ac663ee329ee3974e3b9
Gerrit-Change-Number: 9090
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 

[Impala-ASF-CR] IMPALA-3942: Fix wrongly escaped string literal in front-end

[Kim Jin Chul (Code Review)]

Hello Thomas Tauber-Marshall, Gabor Kaszab, Jim Apple, Tim Armstrong, Alex Behm,

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

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

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

Change subject: IMPALA-3942: Fix wrongly escaped string literal in front-end
..

IMPALA-3942: Fix wrongly escaped string literal in front-end

String literal can be wrapped by either single or double quotes.
There are some holes in escaping the string literal:
- toSql() always returns strings that are single quoted, resulting in
improper escaping in the output if the original string was actually
double quoted.
- It is not always possible to determine if a string "should" be single
or double quoted. e.g. concat('a', "b")

The solution is to normalize any string which comes from user's given
string or a generated string (e.g. constant fold by the rewritter rule)
to single quotes.

Testing:
Added unit tests into expr-test, ToSqlTest
Added E2E tests into test_queries

Change-Id: Ibc4b5f5d8ffaa8feb96a466959427a04b3b06fec
---
M be/src/exprs/expr-test.cc
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M tests/query_test/test_queries.py
4 files changed, 113 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibc4b5f5d8ffaa8feb96a466959427a04b3b06fec
Gerrit-Change-Number: 8818
Gerrit-PatchSet: 8
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6318: Revert "Adjustment for hanging query cancellation test"

[Gabor Kaszab (Code Review)]

Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8972 )

Change subject: IMPALA-6318: Revert "Adjustment for hanging query cancellation 
test"
..


Patch Set 2:

Instead of dropping the test, just skipping it. (Still reverting the fix 
proposal that didn't work as expected)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51acee49b5a17c4852410b7568fd1d092b114a6d
Gerrit-Change-Number: 8972
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Comment-Date: Mon, 22 Jan 2018 13:47:17 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6318: Revert "Adjustment for hanging query cancellation test"

[Gabor Kaszab (Code Review)]

Gabor Kaszab has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/8972 )

Change subject: IMPALA-6318: Revert "Adjustment for hanging query cancellation 
test"
..

IMPALA-6318: Revert "Adjustment for hanging query cancellation test"

Jenkins jobs occasionally hang on test_query_cancellation_during_fetch.
There was a workaround proposal submitted under this Jira ID, however,
apparently jobs still hang on this test randomly. Reverting the
workaround and skipping the test until further fix proposal provided.

This reverts commit 7810d1f9a2c7d59b4b916d4d1793672cd8c33143.

Change-Id: I51acee49b5a17c4852410b7568fd1d092b114a6d
---
M tests/shell/test_shell_commandline.py
M tests/shell/util.py
2 files changed, 11 insertions(+), 13 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I51acee49b5a17c4852410b7568fd1d092b114a6d
Gerrit-Change-Number: 8972
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab 

[Impala-ASF-CR] IMPALA-6219: Use AES-GCM for spill-to-disk encryption

[Zoltan Borok-Nagy (Code Review)]

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

Change subject: IMPALA-6219: Use AES-GCM for spill-to-disk encryption
..


Patch Set 6: Code-Review+1

(3 comments)

Thanks for applying these changes!

http://gerrit.cloudera.org:8080/#/c/9032/6/be/src/util/openssl-util.h
File be/src/util/openssl-util.h:

http://gerrit.cloudera.org:8080/#/c/9032/6/be/src/util/openssl-util.h@140
PS6, Line 140: Return the a
Returns the


http://gerrit.cloudera.org:8080/#/c/9032/4/be/src/util/openssl-util.cc
File be/src/util/openssl-util.cc:

http://gerrit.cloudera.org:8080/#/c/9032/4/be/src/util/openssl-util.cc@173
PS4, Line 173: LOG(WARNING) << "This mode is not supported, fall back to 
the default mode.";
 : mode_ = GetSupportedDefaultMode();
 :   }
 : }
 :
 : bool EncryptionKey::IsModeSupported(AES_CIPHER_MODE m) const {
 :   switch (m) {
 : case AES_256_GCM:
 :   return (CpuInfo::IsSupported(CpuInfo::PCLMULQDQ)
 :   && SSLeay() >= OPENSSL_VERSION_1_0_1D && 
EVP_aes_256_gcm);
 :
 : case AES_256_CTR:
 :
> good suggestion. Thanks a lot! duplicated logic is refactored. "falling cha
Yeah, I think this is reasonable.
Thanks for applying the changes.


http://gerrit.cloudera.org:8080/#/c/9032/6/be/src/util/openssl-util.cc
File be/src/util/openssl-util.cc:

http://gerrit.cloudera.org:8080/#/c/9032/6/be/src/util/openssl-util.cc@173
PS6, Line 173: LOG(WARNING) << "This mode is not supported, fall back to 
the default mode.";
Maybe you could create a 'string ToString(enum AES_CIPHER_MODE)' helper 
function and output the not supported mode and the default mode as well.

For this you need string formatting, by convention we use the Substitute() 
function for that in Impala. See example in L59.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ea87b82a8897ee8bfa187715ac1c52883790d24
Gerrit-Change-Number: 9032
Gerrit-PatchSet: 6
Gerrit-Owner: Xianda Ke 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Xianda Ke 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 22 Jan 2018 12:46:54 +
Gerrit-HasComments: Yes

[native-toolchain-CR] IMPALA-6401 : Re-apply the PPC Breakpad patches

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9077 )

Change subject: IMPALA-6401 : Re-apply the PPC Breakpad patches
..


Patch Set 1:

> Thanks for providing an updated patch. Can you briefly describe how
 > you tested this?

Thanks for reviewing.


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia40e6f9a0054b55e72350ddbb96c9ecac53b0419
Gerrit-Change-Number: 9077
Gerrit-PatchSet: 1
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-Comment-Date: Mon, 22 Jan 2018 11:21:55 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5654: Disallow setting Kudu table name in CREATE TABLE

[Gabor Kaszab (Code Review)]

Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8820 )

Change subject: IMPALA-5654: Disallow setting Kudu table name in CREATE TABLE
..


Patch Set 17:

Again failed on a flaky test (but on a different on than last time): 
TestUdfExecution.test_java_udfs

It seems an occurrence of this: 
https://issues.apache.org/jira/browse/IMPALA-6092

Would it be possible to re-execute the build?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieca037498abf8f5fde67b77e824b720482cdbe6f
Gerrit-Change-Number: 8820
Gerrit-PatchSet: 17
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 22 Jan 2018 10:46:43 +
Gerrit-HasComments: No