date:20201022

Hello Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-10266: Identify FileSystem type based on the protocol 
scheme.
..

IMPALA-10266: Identify FileSystem type based on the protocol scheme.

Frontend identifies the type of FileSystem in two ways. The first is
done using the instanceof operator with subclasses of
org.apache.hadoop.fs.FileSystem. The second is by checking the
FileSystem protocol scheme. This patch standardizes the FileSystem
identification based on the scheme only.

Testing:
- Add several tests in FileSystemUtilTest to check validity of some
  FileSystemUtil functions.
- Run and pass core tests.

Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
---
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java
5 files changed, 261 insertions(+), 67 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
Gerrit-Change-Number: 16628
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-10245: Disable test kudu scanner when run with erasure coding

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

Change subject: IMPALA-10245: Disable test_kudu_scanner when run with erasure 
coding
..


Patch Set 3:

previous GVO failed due to some unrelated flaky issue


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia383c1a788c6c0c66e2ef7c6494fe5fe643956df
Gerrit-Change-Number: 16623
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 23 Oct 2020 00:47:36 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10245: Disable test kudu scanner when run with erasure coding

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10245: Disable test_kudu_scanner when run with erasure 
coding
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6601/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia383c1a788c6c0c66e2ef7c6494fe5fe643956df
Gerrit-Change-Number: 16623
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 23 Oct 2020 00:47:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10245: Disable test kudu scanner when run with erasure coding

Bikramjeet Vig has removed a vote on this change.

Change subject: IMPALA-10245: Disable test_kudu_scanner when run with erasure 
coding
..


Removed Verified-1 by Impala Public Jenkins 
--
To view, visit http://gerrit.cloudera.org:8080/16623
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ia383c1a788c6c0c66e2ef7c6494fe5fe643956df
Gerrit-Change-Number: 16623
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-10245: Disable test kudu scanner when run with erasure coding

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10245: Disable test_kudu_scanner when run with erasure 
coding
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia383c1a788c6c0c66e2ef7c6494fe5fe643956df
Gerrit-Change-Number: 16623
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 23 Oct 2020 00:47:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10244: Make non-scalable failures to dequeue observable.

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

Change subject: IMPALA-10244: Make non-scalable failures to dequeue observable.
..


Patch Set 4: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16613/2/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:

http://gerrit.cloudera.org:8080/#/c/16613/2/be/src/scheduling/admission-controller.cc@1785
PS2, Line 1785: ited_->In
> That's a clever idea, so I could make sure that for every queued query, my
makes sense.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3456396ac139c562ad9cd3ac1a624d8f35487518
Gerrit-Change-Number: 16613
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Comment-Date: Fri, 23 Oct 2020 00:44:09 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10245: Disable test kudu scanner when run with erasure coding

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10245: Disable test_kudu_scanner when run with erasure 
coding
..


Patch Set 2: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia383c1a788c6c0c66e2ef7c6494fe5fe643956df
Gerrit-Change-Number: 16623
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 23 Oct 2020 00:33:02 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10244: Make non-scalable failures to dequeue observable.

Andrew Sherman has posted comments on this change. (
http://gerrit.cloudera.org:8080/16613 )

Change subject: IMPALA-10244: Make non-scalable failures to dequeue observable.
..

Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16613/2/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:

http://gerrit.cloudera.org:8080/#/c/16613/2/be/src/scheduling/admission-controller.cc@1785
PS2, Line 1785: Increment
> we can add a bool to the QueueNode to keep track of this
That's a clever idea, so I could make sure that for every queued query, my new
metric would be bumped only once. But from my point of view this makes it less
observable. If a query gets stuck at the front of the queue for some time I
actually want to be able to observe that. So I could add a timestamp to
QueueNode and only bump the counter every X seconds. But I'm not sure this
complexity is worth it. In some ways the raw number does reflect something that
is actually happening. What do you think?

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3456396ac139c562ad9cd3ac1a624d8f35487518
Gerrit-Change-Number: 16613
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Qifan Chen
Gerrit-Comment-Date: Fri, 23 Oct 2020 00:33:07 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10210: Skip Authentication for connection from a trusted domain

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10210: Skip Authentication for connection from a trusted 
domain
..


Patch Set 4:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/7538/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09234078e2314dbc3177d0e869ae028e216ca699
Gerrit-Change-Number: 16542
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Fri, 23 Oct 2020 00:20:50 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10210: Skip Authentication for connection from a trusted domain

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

Change subject: IMPALA-10210: Skip Authentication for connection from a trusted 
domain
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16542/3/be/src/rpc/authentication-util.cc
File be/src/rpc/authentication-util.cc:

http://gerrit.cloudera.org:8080/#/c/16542/3/be/src/rpc/authentication-util.cc@178
PS3, Line 178: VLOG(2) << "Unable to parse origin address. Error: " << 
s.ToString();
> Not sure if this is really an error? I think this is just handling the case
Its not an error per say, but i put this in and set the log level to 2 in case 
we want to debug and we expect an ip address instead of a hostname. Dont have a 
strong preference here so, lemme know if you prefer removing this or increasing 
its log level further


http://gerrit.cloudera.org:8080/#/c/16542/3/be/src/rpc/authentication-util.cc@202
PS3, Line 202:   std::size_t colon = decoded.find(':');
> I guess we're requiring that in the trusted domain case the Auth header is
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09234078e2314dbc3177d0e869ae028e216ca699
Gerrit-Change-Number: 16542
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Fri, 23 Oct 2020 00:07:44 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10210: Skip Authentication for connection from a trusted domain

Hello Thomas Tauber-Marshall, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-10210: Skip Authentication for connection from a trusted 
domain
..

IMPALA-10210: Skip Authentication for connection from a trusted domain

Adds the ability to skip authentication for connection requests
originating from a trusted domain over the hs2 http endpoint and
the http webserver endpoint. The trusted domain can be specified
using the newly added "--trusted_domain" startup flag. Additionally,
if the startup flag "--trusted_domain_use_xff_header" is set to true,
impala will switch to using the 'X-Forwarded-For' HTML header to
extract the origin address while attempting to check if the connection
originated from a trusted domain.

Other highlights:
- This still requires the client to specify a username via a basic
  auth header.
- To avoid looking up hostname for every http request, a cookie is
  returned on the first auth attempt which will then be subsequently
  used for further communication on the same connection.

Testing:
Added tests for both the hs2 http endpoint and the webserver http endpoint

Change-Id: I09234078e2314dbc3177d0e869ae028e216ca699
---
M be/src/rpc/CMakeLists.txt
R be/src/rpc/authentication-util.cc
R be/src/rpc/authentication-util.h
M be/src/rpc/authentication.cc
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M be/src/util/webserver.cc
M be/src/util/webserver.h
M common/thrift/metrics.json
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
11 files changed, 422 insertions(+), 46 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I09234078e2314dbc3177d0e869ae028e216ca699
Gerrit-Change-Number: 16542
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall

[Impala-ASF-CR] IMPALA-10216: add logging to help debug flaky test

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10216: add logging to help debug flaky test
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09984dd3cea686808115ca4cb8c88d24271d8cc1
Gerrit-Change-Number: 16620
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 22 Oct 2020 23:44:32 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10216: add logging to help debug flaky test

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10216: add logging to help debug flaky test
..

IMPALA-10216: add logging to help debug flaky test

This commit adds additional info to the assertions to
help debug it if it reoccurs.

Change-Id: I09984dd3cea686808115ca4cb8c88d24271d8cc1
Reviewed-on: http://gerrit.cloudera.org:8080/16620
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/runtime/bufferpool/buffer-pool-test.cc
1 file changed, 43 insertions(+), 10 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I09984dd3cea686808115ca4cb8c88d24271d8cc1
Gerrit-Change-Number: 16620
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-10244: Make non-scalable failures to dequeue observable.

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10244: Make non-scalable failures to dequeue observable.
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7537/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3456396ac139c562ad9cd3ac1a624d8f35487518
Gerrit-Change-Number: 16613
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Comment-Date: Thu, 22 Oct 2020 23:20:56 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10244: Make non-scalable failures to dequeue observable.

Andrew Sherman has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/16613 )

Change subject: IMPALA-10244: Make non-scalable failures to dequeue observable.
..

IMPALA-10244: Make non-scalable failures to dequeue observable.

One of the important ways to observe Impala throughput is by looking at
when queries are queued. This can be an indication that more resources
should be added to the cluster by adding more executor groups. This is
only a good strategy if adding more resources will help with the current
workload. In some situations the head of the query queue cannot be
executed because of resource constraints on the coordinator. In these
cases the coordinator is the bottleneck so adding more executor groups
will not help. This change is to make these cases observable by adding a
new counter which is incremented when a dequeue fails because of
resource constraints on the coordinator.

The two cases that cause the counter to be incremented are:
- when there are not enough admission control slots on the coordinator
- when there is not enough memory on the coordinator
but it is possible that other conditions may be added in future.

TESTING:
Added new unit tests.
Ran all end-to-end tests.

Change-Id: I3456396ac139c562ad9cd3ac1a624d8f35487518
---
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M common/thrift/metrics.json
M tests/custom_cluster/test_admission_controller.py
5 files changed, 161 insertions(+), 55 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3456396ac139c562ad9cd3ac1a624d8f35487518
Gerrit-Change-Number: 16613
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen

[Impala-ASF-CR] IMPALA-9990: Support SET OWNER for Kudu tables

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-9990: Support SET OWNER for Kudu tables
..


Patch Set 8:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7536/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I29d641efc8db314964bc5ee9828a86d4a44ae95c
Gerrit-Change-Number: 16273
Gerrit-PatchSet: 8
Gerrit-Owner: Fang-Yu Rao 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Thu, 22 Oct 2020 22:50:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10244: Make non-scalable failures to dequeue observable.

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

Change subject: IMPALA-10244: Make non-scalable failures to dequeue observable.
..


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/16613/2/be/src/scheduling/admission-controller-test.cc
File be/src/scheduling/admission-controller-test.cc:

http://gerrit.cloudera.org:8080/#/c/16613/2/be/src/scheduling/admission-controller-test.cc@382
PS2, Line 382: CanAdmitRequestMemory
> There is a test as part of 'Test 2' of DedicatedCoordAdmissionChecks on lin
oh sorry missed that


http://gerrit.cloudera.org:8080/#/c/16613/2/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:

http://gerrit.cloudera.org:8080/#/c/16613/2/be/src/scheduling/admission-controller.cc@1785
PS2, Line 1785: Increment
> I understand what you mean these numbers can get quite big. But I am not su
we can add a bool to the QueueNode to keep track of this


http://gerrit.cloudera.org:8080/#/c/16613/2/common/thrift/metrics.json
File common/thrift/metrics.json:

http://gerrit.cloudera.org:8080/#/c/16613/2/common/thrift/metrics.json@2731
PS2, Line 2731: Query dequeue failed because of a non-scalable limit
> I will change to use the name, suggested by Quigan
wfm



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3456396ac139c562ad9cd3ac1a624d8f35487518
Gerrit-Change-Number: 16613
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Comment-Date: Thu, 22 Oct 2020 22:35:04 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9990: Support SET OWNER for Kudu tables

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-9990: Support SET OWNER for Kudu tables
..


Patch Set 8:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6600/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I29d641efc8db314964bc5ee9828a86d4a44ae95c
Gerrit-Change-Number: 16273
Gerrit-PatchSet: 8
Gerrit-Owner: Fang-Yu Rao 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Thu, 22 Oct 2020 22:30:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9990: Support SET OWNER for Kudu tables

2020-10-22 Thread Fang-Yu Rao (Code Review)

Fang-Yu Rao has uploaded a new patch set (#8). ( 
http://gerrit.cloudera.org:8080/16273 )

Change subject: IMPALA-9990: Support SET OWNER for Kudu tables
..

IMPALA-9990: Support SET OWNER for Kudu tables

KUDU-3090 adds the support for table ownership and exposes the API's of
setting owner on creating and altering tables, which allows Impala to
also pass to Kudu the new owner of the Kudu table for the ALTER TABLE
SET OWNER statement.

Specifically, based on the API of AlterTableOptions#setOwner(), this
patch stores the ownership information of the Kudu table in the
corresponding instance of AlterTableOptions, which will then be passed
to Kudu via a KuduClient.

Testing:
- Added a FE test in AnalyzeKuduDDLTest.java to verify the statement
  could be correctly analyzed.
- Added an E2E test in kudu_alter.test to verify the statement could be
  correctly executed when the integration between Kudu and HMS is not
  enabled.
- Added an E2E test in kudu_hms_alter.test and verified that the
  statement could be correctly executed when the integration between
  Kudu and HMS is enabled after manually re-enabling
  TestKuduHMSIntegration::test_kudu_alter_table(). Note that this was
  not possible before IMPALA-10092 was resolved due to a bug in the
  class of CustomClusterTestSuite. In addition, we may need to delete
  the Kudu table 'simple' via a Kudu-Python client if the E2E test
  complains that the Kudu table already exists, which may be related to
  IMPALA-8751.
- Manually verified that the views of Kudu server and HMS are consistent
  for a synchronized Kudu table after the ALTER TABLE SET OWNER
  statement even though the Kudu table was once an external and
  non-synchronized table, meaning that the owner from Kudu's perspective
  could be different than that from HMS' perspective. Such a discrepancy
  could be created if we execute the ALTER TABLE SET OWNER statement for
  an external Kudu table with the property of 'external.table.purge'
  being false. The test is performed manually because currently the
  Kudu-Python client adopted in Impala's E2E tests is not up to date so
  that the field of 'owner' cannot be accessed in the E2E tests. On the
  other hand, to verify the owner of a Kudu table from Kudu's
  perspective, we used the latest Kudu-Python client as provided at
  github.com/apache/kudu/tree/master/examples/python/basic-python-example.
- Verified that the patch could pass the exhaustive tests in the DEBUG
  mode.

Change-Id: I29d641efc8db314964bc5ee9828a86d4a44ae95c
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test
5 files changed, 65 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I29d641efc8db314964bc5ee9828a86d4a44ae95c
Gerrit-Change-Number: 16273
Gerrit-PatchSet: 8
Gerrit-Owner: Fang-Yu Rao 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vihang Karajgaonkar

[Impala-ASF-CR] IMPALA-10210: Skip Authentication for connection from a trusted domain

2020-10-22 Thread Thomas Tauber-Marshall (Code Review)

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

Change subject: IMPALA-10210: Skip Authentication for connection from a trusted 
domain
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16542/3/be/src/rpc/authentication-util.cc
File be/src/rpc/authentication-util.cc:

http://gerrit.cloudera.org:8080/#/c/16542/3/be/src/rpc/authentication-util.cc@178
PS3, Line 178: VLOG(2) << "Unable to parse origin address. Error: " << 
s.ToString();
Not sure if this is really an error? I think this is just handling the case 
where 'origin' is a hostname instead of an IP, which is expected in the case of 
trusted_domain_use_xff_header=false I think


http://gerrit.cloudera.org:8080/#/c/16542/3/be/src/rpc/authentication-util.cc@202
PS3, Line 202:   std::size_t colon = decoded.find(':');
I guess we're requiring that in the trusted domain case the Auth header is 
encoded with a ':' and any password, rather than just being a username?
Might want to document that, eg. with the --trusted_domain flag info



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09234078e2314dbc3177d0e869ae028e216ca699
Gerrit-Change-Number: 16542
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Thu, 22 Oct 2020 22:06:04 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode
..


Patch Set 2:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/7534/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie075389b3ab65c612d64ba58e16a10b19bdf4d6f
Gerrit-Change-Number: 16630
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Thu, 22 Oct 2020 21:52:08 +
Gerrit-HasComments: No

[Impala-ASF-CR] WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7535/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie075389b3ab65c612d64ba58e16a10b19bdf4d6f
Gerrit-Change-Number: 16630
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Thu, 22 Oct 2020 21:52:33 +
Gerrit-HasComments: No

[Impala-ASF-CR] WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode

2020-10-22 Thread Wenzhe Zhou (Code Review)

Wenzhe Zhou has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/16630 )

Change subject: WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode
..

WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode

To compliant with FIPS requirement, we should use OpenSSL libraries
for cryptographic hash functions, instead of own hash functions.
This patch replace MD5 and SHA1 functions in Squeasel Web server
with OpenSSL APIs. It also turn off HTTP Digest Authorization in
FIPS mode since Digest Authorization use MD5 hash.

Testing:
 - Passed core tests.
 - TODO: Verify HTTP Digest Authorization could not be enabled
   on FIPS enabled cluster.

Change-Id: Ie075389b3ab65c612d64ba58e16a10b19bdf4d6f
---
M be/src/thirdparty/squeasel/squeasel.c
M be/src/util/webserver.cc
2 files changed, 38 insertions(+), 11 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie075389b3ab65c612d64ba58e16a10b19bdf4d6f
Gerrit-Change-Number: 16630
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Wenzhe Zhou

[Impala-ASF-CR] WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode

2020-10-22 Thread Wenzhe Zhou (Code Review)

Wenzhe Zhou has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/16630 )

Change subject: WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode
..

WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode

To compliant with FIPS requirement, we should use OpenSSL libraries
for cryptographic hash functions, instead of own hash functions.
This patch replace MD5 and SHA1 functions in Squeasel Web server
with OpenSSL APIs. It also turn off HTTP Digest Authorization in
FIPS mode since Digest Authorization use MD5 hash.

Testing:
 - Passed core tests.
 - TODO: Verify HTTP Digest Authorization could not be enabled
   on FIPS enabled cluster.

Change-Id: Ie075389b3ab65c612d64ba58e16a10b19bdf4d6f
---
A Impala.cbp
A be/ASSEMBLER.cbp
M be/src/thirdparty/squeasel/squeasel.c
M be/src/util/webserver.cc
4 files changed, 41,306 insertions(+), 11 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie075389b3ab65c612d64ba58e16a10b19bdf4d6f
Gerrit-Change-Number: 16630
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Wenzhe Zhou

[Impala-ASF-CR] IMPALA-10244: Make non-scalable failures to dequeue observable.

Andrew Sherman has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16613 )

Change subject: IMPALA-10244: Make non-scalable failures to dequeue observable.
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16613/2/common/thrift/metrics.json
File common/thrift/metrics.json:

http://gerrit.cloudera.org:8080/#/c/16613/2/common/thrift/metrics.json@2734
PS2, Line 2734: admission-controller.total-dequeue-failed-cannot-scale"
> Thanks! I will use this name
Actually admission-controller.total-dequeue-failed-coordinator-limited
seems clearer



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3456396ac139c562ad9cd3ac1a624d8f35487518
Gerrit-Change-Number: 16613
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Comment-Date: Thu, 22 Oct 2020 21:28:44 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10266: Identify FileSystem type based on the protocol scheme.

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10266: Identify FileSystem type based on the protocol 
scheme.
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7533/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
Gerrit-Change-Number: 16628
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 22 Oct 2020 21:24:48 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10215: Implement INSERT INTO for non-partitioned Iceberg tables (Parquet)

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10215: Implement INSERT INTO for non-partitioned Iceberg 
tables (Parquet)
..


Patch Set 6: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5690fb6c2cc51f0033fa26caf8597c80a11bcd8e
Gerrit-Change-Number: 16545
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng 
Gerrit-Comment-Date: Thu, 22 Oct 2020 21:20:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10244: Make non-scalable failures to dequeue observable.

Andrew Sherman has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16613 )

Change subject: IMPALA-10244: Make non-scalable failures to dequeue observable.
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16613/2/common/thrift/metrics.json
File common/thrift/metrics.json:

http://gerrit.cloudera.org:8080/#/c/16613/2/common/thrift/metrics.json@2731
PS2, Line 2731: Query dequeue failed because of a non-scalable limit
> I originally planned to have the name be related to "coordinator" but (else
I will change to use the name, suggested by Quigan


http://gerrit.cloudera.org:8080/#/c/16613/2/common/thrift/metrics.json@2734
PS2, Line 2734: admission-controller.total-dequeue-failed-cannot-scale"
> I had the same impression here as well. The new counter added counts the #
Thanks! I will use this name



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3456396ac139c562ad9cd3ac1a624d8f35487518
Gerrit-Change-Number: 16613
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Comment-Date: Thu, 22 Oct 2020 21:15:13 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10266: Identify FileSystem type based on the protocol scheme.

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16628 )

Change subject: IMPALA-10266: Identify FileSystem type based on the protocol 
scheme.
..


Patch Set 4:

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/16628/3/fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java@63
PS3, Line 63: Preconditions.checkNotNull(tableName);
> Maybe move this to FileSystemUtil? That way all these lists of capabilities
Done


http://gerrit.cloudera.org:8080/#/c/16628/3/fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java@161
PS3, Line 161:
> Can we generate this error message from the list of supported schemes? I th
Done


http://gerrit.cloudera.org:8080/#/c/16628/3/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
File fe/src/main/java/org/apache/impala/common/FileSystemUtil.java:

http://gerrit.cloudera.org:8080/#/c/16628/3/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java@72
PS3, Line 72:   ImmutableSet.builder()
> I like this pattern!
Agree, I feel it is easier to understand by grouping them like this.


http://gerrit.cloudera.org:8080/#/c/16628/2/fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java
File fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java:

http://gerrit.cloudera.org:8080/#/c/16628/2/fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java@100
PS2, Line 100: testFsType(
> line too long (91 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16628/2/fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java@186
PS2, Line 186: return 
"adl://dummy-account.azuredatalakestore.net/dummy-part-3";
> line too long (91 > 90)
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
Gerrit-Change-Number: 16628
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 22 Oct 2020 21:13:03 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10266: Identify FileSystem type based on the protocol scheme.

Hello Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-10266: Identify FileSystem type based on the protocol 
scheme.
..

IMPALA-10266: Identify FileSystem type based on the protocol scheme.

Frontend identifies the type of FileSystem in two ways. The first is
done using the instanceof operator with subclasses of
org.apache.hadoop.fs.FileSystem. The second is by checking the
FileSystem protocol scheme. This patch standardizes the FileSystem
identification based on the scheme only.

Testing:
- Add several tests in FileSystemUtilTest to check validity of some
  FileSystemUtil functions.
- Run and pass core tests.

Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
---
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java
5 files changed, 261 insertions(+), 67 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
Gerrit-Change-Number: 16628
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7532/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie075389b3ab65c612d64ba58e16a10b19bdf4d6f
Gerrit-Change-Number: 16630
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 22 Oct 2020 21:10:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16630/1/be/src/util/webserver.cc
File be/src/util/webserver.cc:

http://gerrit.cloudera.org:8080/#/c/16630/1/be/src/util/webserver.cc@400
PS1, Line 400: ss << "Webserver: Password file does not exist: " << 
FLAGS_webserver_password_file;
line too long (91 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie075389b3ab65c612d64ba58e16a10b19bdf4d6f
Gerrit-Change-Number: 16630
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 22 Oct 2020 20:55:42 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3816 Codegen perf critical loops in Sorter

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-3816 Codegen perf critical loops in Sorter
..


Patch Set 7:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7531/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie08137449d4a7b554ca8b8650260f8bd72e0a81b
Gerrit-Change-Number: 16621
Gerrit-PatchSet: 7
Gerrit-Owner: Qifan Chen 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 22 Oct 2020 20:55:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode

2020-10-22 Thread Wenzhe Zhou (Code Review)

Wenzhe Zhou has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/16630


Change subject: WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode
..

WIP IMPALA-10206: Avoid MD5 Digest Authorization in FIPS mode

To compliant with FIPS requirement, we should use OpenSSL libraries
for cryptographic hash functions, instead of own hash functions.
This patch replace MD5 and SHA1 functions in Squeasel Web server
with OpenSSL APIs. It also turn off HTTP Digest Authorization in
FIPS mode since Digest Authorization use MD5 hash.

Testing:
 - Passed core tests.
 - TODO: Verify HTTP Digest Authorization could not be enabled
   on FIPS enabled cluster.

Change-Id: Ie075389b3ab65c612d64ba58e16a10b19bdf4d6f
---
M be/src/thirdparty/squeasel/squeasel.c
M be/src/util/webserver.cc
2 files changed, 37 insertions(+), 11 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie075389b3ab65c612d64ba58e16a10b19bdf4d6f
Gerrit-Change-Number: 16630
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou

[Impala-ASF-CR] IMPALA-10244: Make non-scalable failures to dequeue observable.

Qifan Chen has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16613 )

Change subject: IMPALA-10244: Make non-scalable failures to dequeue observable.
..


Patch Set 3:

(1 comment)

Good good to me and just have a comment.

http://gerrit.cloudera.org:8080/#/c/16613/2/common/thrift/metrics.json
File common/thrift/metrics.json:

http://gerrit.cloudera.org:8080/#/c/16613/2/common/thrift/metrics.json@2734
PS2, Line 2734: admission-controller.total-dequeue-failed-cannot-scale"
I had the same impression here as well. The new counter added counts the # of 
failed dequeue events in coordinators. It seems to rename it to 
"admission-controller.total-dequeue-failed-in-coordinator" would be more 
precise.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3456396ac139c562ad9cd3ac1a624d8f35487518
Gerrit-Change-Number: 16613
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Comment-Date: Thu, 22 Oct 2020 20:54:50 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3816 Codegen perf critical loops in Sorter

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-3816 Codegen perf critical loops in Sorter
..


Patch Set 6:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7530/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie08137449d4a7b554ca8b8650260f8bd72e0a81b
Gerrit-Change-Number: 16621
Gerrit-PatchSet: 6
Gerrit-Owner: Qifan Chen 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 22 Oct 2020 20:42:43 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3816 Codegen perf critical loops in Sorter

Qifan Chen has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/16621 )

Change subject: IMPALA-3816 Codegen perf critical loops in Sorter
..

IMPALA-3816 Codegen perf critical loops in Sorter

This fix added the functionality to codegen method
Sorter::TupleSorter::SortHelper() in sorter, which improves the
performance for both the sort and the partial sort operators.

In one unit test to order 7300 rows from table functional.alltypes,
the speedup of the code-gen version over non-code-gen version of
the method is around 29%. In another unit test to partially
order 2880404 rows, the speedup is around 20%.

Testing:
1. Unit testing;
2. Ran Core tests successfully.

Change-Id: Ie08137449d4a7b554ca8b8650260f8bd72e0a81b
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/partial-sort-node.cc
M be/src/exec/partial-sort-node.h
M be/src/exec/sort-node.cc
M be/src/exec/sort-node.h
M be/src/runtime/sorter-internal.h
M be/src/runtime/sorter-ir.cc
M be/src/runtime/sorter.cc
M be/src/runtime/sorter.h
9 files changed, 122 insertions(+), 41 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie08137449d4a7b554ca8b8650260f8bd72e0a81b
Gerrit-Change-Number: 16621
Gerrit-PatchSet: 7
Gerrit-Owner: Qifan Chen 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-10244: Make non-scalable failures to dequeue observable.

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10244: Make non-scalable failures to dequeue observable.
..


Patch Set 3:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/7529/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3456396ac139c562ad9cd3ac1a624d8f35487518
Gerrit-Change-Number: 16613
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 22 Oct 2020 20:29:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3816 Codegen perf critical loops in Sorter

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-3816 Codegen perf critical loops in Sorter
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16621/6/be/src/runtime/sorter.h
File be/src/runtime/sorter.h:

http://gerrit.cloudera.org:8080/#/c/16621/6/be/src/runtime/sorter.h@106
PS6, Line 106:   /// codegend_sort_helper_fn is a reference to the code-gen 
version of
line has trailing whitespace



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie08137449d4a7b554ca8b8650260f8bd72e0a81b
Gerrit-Change-Number: 16621
Gerrit-PatchSet: 6
Gerrit-Owner: Qifan Chen 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 22 Oct 2020 20:25:15 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9990: Support SET OWNER for Kudu tables

2020-10-22 Thread Andrew Wong (Code Review)

Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/16273 )

Change subject: IMPALA-9990: Support SET OWNER for Kudu tables
..

Patch Set 6:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/16273/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3779
PS6, Line 3779: if (isSynchronizedKuduTable) {
> Thanks Vihang for the feedback!
Thinking through the options, a few come to mind:

1. Return an error
2. Change the Kudu table's owner _and_ the HMS external table's owner
3. Just change the HMS external table's owner

So here's some food for thought: Is the ownership of a table meant to be
consistent across all references to that table (i.e. all external tables)? I
think the answer is no. Take the case in which we have multiple, separate
Impala clusters pointing at a Kudu table in our local cluster via external
tables. If one of those Impala clusters sets a different owner for its external
table, the metadata change wouldn't be reflected on all of the other clusters'
HMSs' external tables, since none of the HMS tables are synchronized. In that
regard, I think it would be weird if the ownership change of an external table
resulted in a change in ownership of the underlying Kudu table, since the
ownership change may not be reflected by all of its references (including the
external table that we just changed the ownership of!). Based on this, I would
lean away from option 2.

Digging into the other two options, I'm trying to better understand what
ownership is in the context of an external table. Is it meant to be used as the
owner of the underlying table? Or the owner of the HMS metadata entry (e.g. the
owner of a symlink)? Also, are external tables even able to have owners that
are not the owner of the underlying table? I would think the answer is yes, and
that "ownership" refers to ownership of the external table only, in which case
I would lean towards option 3, especially if that's allowed behavior today.

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I29d641efc8db314964bc5ee9828a86d4a44ae95c
Gerrit-Change-Number: 16273
Gerrit-PatchSet: 6
Gerrit-Owner: Fang-Yu Rao
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Fang-Yu Rao
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Thu, 22 Oct 2020 20:25:52 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3816 Codegen perf critical loops in Sorter

Qifan Chen has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/16621 )

Change subject: IMPALA-3816 Codegen perf critical loops in Sorter
..

IMPALA-3816 Codegen perf critical loops in Sorter

This fix added the functionality to codegen method
Sorter::TupleSorter::SortHelper() in sorter, which improves the
performance for both the sort and the partial sort operators.

In one unit test to order 7300 rows from table functional.alltypes,
the speedup of the code-gen version over non-code-gen version of
the method is around 29%. In another unit test to partially
order 2880404 rows, the speedup is around 20%.

Testing:
1. Unit testing;
2. Ran Core tests successfully.

Change-Id: Ie08137449d4a7b554ca8b8650260f8bd72e0a81b
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/partial-sort-node.cc
M be/src/exec/partial-sort-node.h
M be/src/exec/sort-node.cc
M be/src/exec/sort-node.h
M be/src/runtime/sorter-internal.h
M be/src/runtime/sorter-ir.cc
M be/src/runtime/sorter.cc
M be/src/runtime/sorter.h
9 files changed, 122 insertions(+), 41 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie08137449d4a7b554ca8b8650260f8bd72e0a81b
Gerrit-Change-Number: 16621
Gerrit-PatchSet: 6
Gerrit-Owner: Qifan Chen 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-10244: Make non-scalable failures to dequeue observable.

Andrew Sherman has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/16613 )

Change subject: IMPALA-10244: Make non-scalable failures to dequeue observable.
..

IMPALA-10244: Make non-scalable failures to dequeue observable.

One of the important ways to observe Impala throughput is by looking at
when queries are queued. This can be an indication that more resources
should be added to the cluster by adding more executor groups. This is
only a good strategy if adding more resources will help with the current
workload. In some situations the head of the query queue cannot be
executed because of resource constraints on the coordinator. In these
cases the coordinator is the bottleneck so adding more executor groups
will not help. This change is to make these cases observable by adding a
new counter which is incremented when a dequeue fails because of
resource constraints that would not be resolved by adding more executor
groups.

The two cases that cause the counter to be incremented are:
- when there are not enough admission control slots on the coordinator
- when there is not enough memory on the coordinator
but it is possible that other conditions may be added in future.

TESTING:
  Added new unit tests.
  Ran all end-to-end tests.

Change-Id: I3456396ac139c562ad9cd3ac1a624d8f35487518
---
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M common/thrift/metrics.json
M tests/custom_cluster/test_admission_controller.py
5 files changed, 159 insertions(+), 55 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3456396ac139c562ad9cd3ac1a624d8f35487518
Gerrit-Change-Number: 16613
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-10266: Identify FileSystem type based on the protocol scheme.

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

Change subject: IMPALA-10266: Identify FileSystem type based on the protocol 
scheme.
..


Patch Set 3:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/16628/3/fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java@63
PS3, Line 63:   private static final Set VALID_SCHEME_FOR_INPATH =
Maybe move this to FileSystemUtil? That way all these lists of capabilities 
would be consolidated.


http://gerrit.cloudera.org:8080/#/c/16628/3/fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java@161
PS3, Line 161: + "must point to an HDFS, S3A, ADL, ABFS, or 
Ozone filesystem.",
Can we generate this error message from the list of supported schemes? I think 
the message would have to change a little to list the schemes instead of these 
ad-hoc names but it'd be less ambiguous in that case anyway.


http://gerrit.cloudera.org:8080/#/c/16628/3/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
File fe/src/main/java/org/apache/impala/common/FileSystemUtil.java:

http://gerrit.cloudera.org:8080/#/c/16628/3/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java@72
PS3, Line 72:   ImmutableSet.builder()
I like this pattern!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
Gerrit-Change-Number: 16628
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 22 Oct 2020 20:01:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10266: Identify FileSystem type based on the protocol scheme.

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10266: Identify FileSystem type based on the protocol 
scheme.
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7528/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
Gerrit-Change-Number: 16628
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 22 Oct 2020 20:00:58 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10210: Skip Authentication for connection from a trusted domain

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10210: Skip Authentication for connection from a trusted 
domain
..


Patch Set 3:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/7527/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09234078e2314dbc3177d0e869ae028e216ca699
Gerrit-Change-Number: 16542
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Thu, 22 Oct 2020 19:53:19 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10266: Identify FileSystem type based on the protocol scheme.

Hello Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-10266: Identify FileSystem type based on the protocol 
scheme.
..

IMPALA-10266: Identify FileSystem type based on the protocol scheme.

Frontend identifies the type of FileSystem in two ways. The first is
done using the instanceof operator with subclasses of
org.apache.hadoop.fs.FileSystem. The second is by checking the
FileSystem protocol scheme. This patch standardizes the FileSystem
identification based on the scheme only.

Testing:
- Add several tests in FileSystemUtilTest to check validity of some
  FileSystemUtil functions.
- Run and pass core tests.

Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
---
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java
5 files changed, 222 insertions(+), 65 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
Gerrit-Change-Number: 16628
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-10266: Identify FileSystem type based on the protocol scheme.

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10266: Identify FileSystem type based on the protocol 
scheme.
..


Patch Set 2:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7526/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
Gerrit-Change-Number: 16628
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 22 Oct 2020 19:46:36 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10210: Skip Authentication for connection from a trusted domain

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

Change subject: IMPALA-10210: Skip Authentication for connection from a trusted 
domain
..


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/16542/2/be/src/rpc/authentication-util.h
File be/src/rpc/authentication-util.h:

http://gerrit.cloudera.org:8080/#/c/16542/2/be/src/rpc/authentication-util.h@40
PS2, Line 40: a comm
> comma separated
Done


http://gerrit.cloudera.org:8080/#/c/16542/2/be/src/rpc/authentication-util.h@40
PS2, Line 40: without a port, picks the
: // left
> what's the rationale for only checking the first one?
added the rationale


http://gerrit.cloudera.org:8080/#/c/16542/2/be/src/rpc/authentication-util.h@48
PS2, Line 48: token
> nit: an error
Done


http://gerrit.cloudera.org:8080/#/c/16542/2/be/src/util/webserver.h
File be/src/util/webserver.h:

http://gerrit.cloudera.org:8080/#/c/16542/2/be/src/util/webserver.h@201
PS2, Line 201: const std::
> const&?
Done


http://gerrit.cloudera.org:8080/#/c/16542/2/be/src/util/webserver.cc
File be/src/util/webserver.cc:

http://gerrit.cloudera.org:8080/#/c/16542/2/be/src/util/webserver.cc@610
PS2, Line 610:   // checking for cookie to avoid subsequent reverse DNS lookups 
which can be
> Might be good to mention that we're doing this after checking cookies to av
Done


http://gerrit.cloudera.org:8080/#/c/16542/2/common/thrift/metrics.json
File common/thrift/metrics.json:

http://gerrit.cloudera.org:8080/#/c/16542/2/common/thrift/metrics.json@1299
PS2, Line 1299: ",
> I wonder if "Failure" is too strong of a word here and below, since this wi
Yea i agree with just removing the failure metric, its just adding noise



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09234078e2314dbc3177d0e869ae028e216ca699
Gerrit-Change-Number: 16542
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Thu, 22 Oct 2020 19:39:33 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10210: Skip Authentication for connection from a trusted domain

Hello Thomas Tauber-Marshall, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-10210: Skip Authentication for connection from a trusted 
domain
..

IMPALA-10210: Skip Authentication for connection from a trusted domain

Adds the ability to skip authentication for connection requests
originating from a trusted domain over the hs2 http endpoint and
the http webserver endpoint. The trusted domain can be specified
using the newly added "--trusted_domain" startup flag. Additionally,
if the startup flag "--trusted_domain_use_xff_header" is set to true,
impala will switch to using the 'X-Forwarded-For' HTML header to
extract the origin address while attempting to check if the connection
originated from a trusted domain.

Other highlights:
- This still requires the client to specify a username via a basic
  auth header.
- To avoid looking up hostname for every http request, a cookie is
  returned on the first auth attempt which will then be subsequently
  used for further communication on the same connection.

Testing:
Added tests for both the hs2 http endpoint and the webserver http endpoint

Change-Id: I09234078e2314dbc3177d0e869ae028e216ca699
---
M be/src/rpc/CMakeLists.txt
R be/src/rpc/authentication-util.cc
R be/src/rpc/authentication-util.h
M be/src/rpc/authentication.cc
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M be/src/util/webserver.cc
M be/src/util/webserver.h
M common/thrift/metrics.json
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
11 files changed, 421 insertions(+), 46 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I09234078e2314dbc3177d0e869ae028e216ca699
Gerrit-Change-Number: 16542
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall

[Impala-ASF-CR] IMPALA-10266: Identify FileSystem type based on the protocol scheme.

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10266: Identify FileSystem type based on the protocol 
scheme.
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16628/2/fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java
File fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java:

http://gerrit.cloudera.org:8080/#/c/16628/2/fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java@100
PS2, Line 100: testFsType(mockLocation(FileSystemUtil.SCHEME_ALLUXIO), 
FileSystemUtil.FsType.ALLUXIO);
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/16628/2/fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java@186
PS2, Line 186: FileSystemUtil.FsType type = 
FileSystemUtil.FsType.getFsType(path.toUri().getScheme());
line too long (91 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
Gerrit-Change-Number: 16628
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 22 Oct 2020 19:32:12 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10266: Identify FileSystem type based on the protocol scheme.

Hello Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-10266: Identify FileSystem type based on the protocol 
scheme.
..

IMPALA-10266: Identify FileSystem type based on the protocol scheme.

Frontend identifies the type of FileSystem in two ways. The first is
done using the instanceof operator with subclasses of
org.apache.hadoop.fs.FileSystem. The second is by checking the
FileSystem protocol scheme. This patch standardizes the FileSystem
identification based on the scheme only.

Testing:
- Add several tests in FileSystemUtilTest to check validity of some
  FileSystemUtil functions.
- Run and pass core tests.

Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
---
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java
5 files changed, 220 insertions(+), 60 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
Gerrit-Change-Number: 16628
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-9990: Support SET OWNER for Kudu tables

2020-10-22 Thread Fang-Yu Rao (Code Review)

Fang-Yu Rao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16273 )

Change subject: IMPALA-9990: Support SET OWNER for Kudu tables
..


Patch Set 6:

(2 comments)

Hi all, I have replied to Vihang's comments on my patch. But I also raised some 
follow-up questions and thus we might need some feedback from Andrew and Attila 
on the Kudu project as for how to proceed. Thank you very much for the help!

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

http://gerrit.cloudera.org:8080/#/c/16273/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3779
PS6, Line 3779: if (isSynchronizedKuduTable) {
> What does it mean to change owner of external kudu table? Is there a use-ca
Thanks Vihang for the feedback!

I do not have a concrete answer to the question that in what scenario a user 
only wants to update HMS on the owner change.

If we are sure that for an external and non-synchronized Kudu table (i.e., 
'isSynchronizedKuduTable' is false), there is some sort of mechanism that could 
update the Kudu server on the owner change, then it would be reasonable to just 
update HMS on the owner change to save one RPC to the Kudu server from Impala.

For example, the user issuing the ALTER TABLE SET OWNER statement always 
manually updates the Kudu server on the owner change, or the Kudu server is 
able to retrieve from the HMS the change to the owner of the Kudu table. 
Probably Andrew or Attila could offer some insight into it.

As for throwing an error in the analysis phase, it could be done by adding a 
statement of "table_ = tableRef.getTable()" after 
https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java#L66
 so that we are able to retrieve its corresponding 
org.apache.hadoop.hive.metastore.api.Table to determine whether the query  is 
an attempt to change the owner of an external Kudu table.

But I think a bigger question here is that do we really need to take into 
consideration the externality of a Kudu table. Can't we just simple update the 
Kudu server on the owner change unconditionally for a user executing a ALTER 
TABLE SET OWNER statement against a Kudu table? We probably need some 
suggestions from Andrew or Attila before I make any change to this patch set. 
Thanks!


http://gerrit.cloudera.org:8080/#/c/16273/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3781
PS6, Line 3781:   Preconditions.checkState(tbl instanceof  KuduTable);
> Not sure if this is really needed since the boolean is set above when this
Thanks Vihang for pointing this out! I will remove this since it looks like we 
don't need this check here.

I actually think the previous check is not required either since in 
KuduTable.isSynchronizedTable(msTbl) above 
(https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/KuduTable.java#L142),
 the same check has been done already.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I29d641efc8db314964bc5ee9828a86d4a44ae95c
Gerrit-Change-Number: 16273
Gerrit-PatchSet: 6
Gerrit-Owner: Fang-Yu Rao 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Thu, 22 Oct 2020 19:29:09 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10252: fix invalid runtime filters for outer joins

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10252: fix invalid runtime filters for outer joins
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7525/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I507af1cc8df15bca21e0d8555019997812087261
Gerrit-Change-Number: 16622
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Comment-Date: Thu, 22 Oct 2020 19:25:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10252: fix invalid runtime filters for outer joins

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10252: fix invalid runtime filters for outer joins
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7524/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I507af1cc8df15bca21e0d8555019997812087261
Gerrit-Change-Number: 16622
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Comment-Date: Thu, 22 Oct 2020 19:24:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10252: fix invalid runtime filters for outer joins

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10252: fix invalid runtime filters for outer joins
..


Patch Set 2:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7523/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I507af1cc8df15bca21e0d8555019997812087261
Gerrit-Change-Number: 16622
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Comment-Date: Thu, 22 Oct 2020 19:20:35 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3816 Codegen perf critical loops in Sorter

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-3816 Codegen perf critical loops in Sorter
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7522/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie08137449d4a7b554ca8b8650260f8bd72e0a81b
Gerrit-Change-Number: 16621
Gerrit-PatchSet: 4
Gerrit-Owner: Qifan Chen 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 22 Oct 2020 19:12:24 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10245: Disable test kudu scanner when run with erasure coding

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10245: Disable test_kudu_scanner when run with erasure 
coding
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia383c1a788c6c0c66e2ef7c6494fe5fe643956df
Gerrit-Change-Number: 16623
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 22 Oct 2020 19:11:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10245: Disable test kudu scanner when run with erasure coding

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10245: Disable test_kudu_scanner when run with erasure 
coding
..


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6599/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia383c1a788c6c0c66e2ef7c6494fe5fe643956df
Gerrit-Change-Number: 16623
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 22 Oct 2020 19:11:30 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10252: fix invalid runtime filters for outer joins

Hello Aman Sinha, Qifan Chen, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-10252: fix invalid runtime filters for outer joins
..

IMPALA-10252: fix invalid runtime filters for outer joins

The planner generates runtime filters for non-join conjuncts
assigned to LEFT OUTER and FULL OUTER JOIN nodes. This is
correct in many cases where NULLs stemming from unmatched rows
would result in the predicate evaluating to false. E.g.
x = y is always false is y is NULL.

However, it is incorrect if the NULL returned from the unmatched
row can result in the predicate evaluating to true. E.g.
x = isnull(y, 1) can return true even if y is NULL.

The fix is to detect cases when the source expression from the
left input of the join returns non-NULL for null inputs and then
skip generating the filter.

Testing:
Added regression tests:
* Planner tests for LEFT OUTER and FULL OUTER where the runtime
  filter was incorrectly generated before this patch.
* Enabled end-to-end test that was previously failing.
* Added a new runtime filter test that will execute on both
  Parquet and Kudu (which are subtly different because of nullability of
  slots).

Ran exhaustive tests.
Change-Id: I507af1cc8df15bca21e0d8555019997812087261
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-query/queries/QueryTest/runtime_filters.test
M testdata/workloads/functional-query/queries/QueryTest/subquery.test
M tests/query_test/test_queries.py
6 files changed, 144 insertions(+), 9 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I507af1cc8df15bca21e0d8555019997812087261
Gerrit-Change-Number: 16622
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen

[Impala-ASF-CR] IMPALA-10252: fix invalid runtime filters for outer joins

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10252: fix invalid runtime filters for outer joins
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16622/2/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:

http://gerrit.cloudera.org:8080/#/c/16622/2/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@379
PS2, Line 379: // input then it is not safe to generate a runtime 
filter from a LEFT OUTER JOIN
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/16622/2/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@382
PS2, Line 382: // E.g. If the source expression is zeroifnull(y), 
column y has values [1, 2, 3],
line too long (93 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I507af1cc8df15bca21e0d8555019997812087261
Gerrit-Change-Number: 16622
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Comment-Date: Thu, 22 Oct 2020 19:02:29 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10252: fix invalid runtime filters for outer joins

Hello Aman Sinha, Qifan Chen, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-10252: fix invalid runtime filters for outer joins
..

IMPALA-10252: fix invalid runtime filters for outer joins

The planner generates runtime filters for non-join conjuncts
assigned to LEFT OUTER and FULL OUTER JOIN nodes. This is
correct in many cases where NULLs stemming from unmatched rows
would result in the predicate evaluating to false. E.g.
x = y is always false is y is NULL.

However, it is incorrect if the NULL returned from the unmatched
row can result in the predicate evaluating to true. E.g.
x = isnull(y, 1) can return true even if y is NULL.

The fix is to detect cases when the source expression from the
left input of the join returns non-NULL for null inputs and then
skip generating the filter.

Testing:
Added regression tests:
* Planner tests for LEFT OUTER and FULL OUTER where the runtime
  filter was incorrectly generated before this patch.
* Enabled end-to-end test that was previously failing.
* Added a new runtime filter test that will execute on both
  Parquet and Kudu (which are subtly different because of nullability of
  slots).

Ran exhaustive tests.
Change-Id: I507af1cc8df15bca21e0d8555019997812087261
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-query/queries/QueryTest/runtime_filters.test
M testdata/workloads/functional-query/queries/QueryTest/subquery.test
5 files changed, 144 insertions(+), 7 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I507af1cc8df15bca21e0d8555019997812087261
Gerrit-Change-Number: 16622
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen

[Impala-ASF-CR] IMPALA-10252: fix invalid runtime filters for outer joins

Tim Armstrong has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/16622 )

Change subject: IMPALA-10252: fix invalid runtime filters for outer joins
..

IMPALA-10252: fix invalid runtime filters for outer joins

The planner generates runtime filters for non-join conjuncts
assigned to LEFT OUTER and FULL OUTER JOIN nodes. This is
correct in many cases where NULLs stemming from unmatched rows
would result in the predicate evaluating to false. E.g.
x = y is always false is y is NULL.

However, it is incorrect if the NULL returned from the unmatched
row can result in the predicate evaluating to true. E.g.
x = isnull(y, 1) can return true even if y is NULL.

The fix is to detect cases when the source expression from the
left input of the join returns non-NULL for null inputs and then
skip generating the filter.

Testing:
Added regression tests:
* Planner tests for LEFT OUTER and FULL OUTER where the runtime
  filter was incorrectly generated before this patch.
* Enabled end-to-end test that was previously failing.
* Added a new runtime filter test that will execute on both
  Parquet and Kudu (which are subtly different because of nullability of
  slots).

Ran exhaustive tests.
Change-Id: I507af1cc8df15bca21e0d8555019997812087261
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-query/queries/QueryTest/runtime_filters.test
M testdata/workloads/functional-query/queries/QueryTest/subquery.test
5 files changed, 144 insertions(+), 7 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I507af1cc8df15bca21e0d8555019997812087261
Gerrit-Change-Number: 16622
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong

[Impala-ASF-CR] IMPALA-3816 Codegen perf critical loops in Sorter

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-3816 Codegen perf critical loops in Sorter
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7521/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie08137449d4a7b554ca8b8650260f8bd72e0a81b
Gerrit-Change-Number: 16621
Gerrit-PatchSet: 3
Gerrit-Owner: Qifan Chen 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 22 Oct 2020 18:56:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3816 Codegen perf critical loops in Sorter

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-3816 Codegen perf critical loops in Sorter
..


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/16621/4/be/src/runtime/sorter-internal.h
File be/src/runtime/sorter-internal.h:

http://gerrit.cloudera.org:8080/#/c/16621/4/be/src/runtime/sorter-internal.h@487
PS4, Line 487:   Status IR_ALWAYS_INLINE InsertionSort(const TupleIterator& 
begin, const TupleIterator& end);
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/16621/4/be/src/runtime/sorter-ir.cc
File be/src/runtime/sorter-ir.cc:

http://gerrit.cloudera.org:8080/#/c/16621/4/be/src/runtime/sorter-ir.cc@68
PS4, Line 68: bool IR_ALWAYS_INLINE Sorter::TupleSorter::Less(const TupleRow* 
lhs, const TupleRow* rhs) {
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/16621/4/be/src/runtime/sorter-ir.cc@188
PS4, Line 188: Tuple* IR_ALWAYS_INLINE 
Sorter::TupleSorter::SelectPivot(TupleIterator begin, TupleIterator end) {
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/16621/4/be/src/runtime/sorter-ir.cc@212
PS4, Line 212: Tuple* IR_ALWAYS_INLINE 
Sorter::TupleSorter::MedianOfThree(Tuple* t1, Tuple* t2, Tuple* t3) {
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/16621/4/be/src/runtime/sorter-ir.cc@249
PS4, Line 249: void IR_ALWAYS_INLINE Sorter::TupleSorter::Swap(Tuple* RESTRICT 
left, Tuple* RESTRICT right,
line too long (92 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie08137449d4a7b554ca8b8650260f8bd72e0a81b
Gerrit-Change-Number: 16621
Gerrit-PatchSet: 4
Gerrit-Owner: Qifan Chen 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 22 Oct 2020 18:50:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3816 Codegen perf critical loops in Sorter

Qifan Chen has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/16621 )

Change subject: IMPALA-3816 Codegen perf critical loops in Sorter
..

IMPALA-3816 Codegen perf critical loops in Sorter

This fix added the functionality to codegen method
Sorter::TupleSorter::SortHelper() in sorter, which improves the
performance for both the sort and the partial sort operators.

In one unit testing to order 7300 rows from functional.alltypes,
the speedup of the code-gen version of sort node is 29%.

Testing:
1. Unit testing;
2. Ran Core tests successfully.

Change-Id: Ie08137449d4a7b554ca8b8650260f8bd72e0a81b
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/partial-sort-node.cc
M be/src/exec/partial-sort-node.h
M be/src/exec/sort-node.cc
M be/src/exec/sort-node.h
M be/src/runtime/sorter-internal.h
M be/src/runtime/sorter-ir.cc
M be/src/runtime/sorter.cc
M be/src/runtime/sorter.h
9 files changed, 112 insertions(+), 34 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie08137449d4a7b554ca8b8650260f8bd72e0a81b
Gerrit-Change-Number: 16621
Gerrit-PatchSet: 4
Gerrit-Owner: Qifan Chen 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-3816 Codegen perf critical loops in Sorter

Qifan Chen has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/16621 )

Change subject: IMPALA-3816 Codegen perf critical loops in Sorter
..

IMPALA-3816 Codegen perf critical loops in Sorter

This fix added the functionality to codegen method
Sorter::TupleSorter::SortHelper() in sorter, which improves the
performance for both the sort and the partial sort operators.

Testing:
1. Unit testing;
2. Ran Core tests successfully.

Change-Id: Ie08137449d4a7b554ca8b8650260f8bd72e0a81b
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/partial-sort-node.cc
M be/src/exec/partial-sort-node.h
M be/src/exec/sort-node.cc
M be/src/exec/sort-node.h
M be/src/runtime/sorter-internal.h
M be/src/runtime/sorter-ir.cc
M be/src/runtime/sorter.cc
M be/src/runtime/sorter.h
9 files changed, 112 insertions(+), 34 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie08137449d4a7b554ca8b8650260f8bd72e0a81b
Gerrit-Change-Number: 16621
Gerrit-PatchSet: 3
Gerrit-Owner: Qifan Chen 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-3816 Codegen perf critical loops in Sorter

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-3816 Codegen perf critical loops in Sorter
..


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/16621/3/be/src/runtime/sorter-internal.h
File be/src/runtime/sorter-internal.h:

http://gerrit.cloudera.org:8080/#/c/16621/3/be/src/runtime/sorter-internal.h@487
PS3, Line 487:   Status IR_ALWAYS_INLINE InsertionSort(const TupleIterator& 
begin, const TupleIterator& end);
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/16621/3/be/src/runtime/sorter-ir.cc
File be/src/runtime/sorter-ir.cc:

http://gerrit.cloudera.org:8080/#/c/16621/3/be/src/runtime/sorter-ir.cc@68
PS3, Line 68: bool IR_ALWAYS_INLINE Sorter::TupleSorter::Less(const TupleRow* 
lhs, const TupleRow* rhs) {
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/16621/3/be/src/runtime/sorter-ir.cc@188
PS3, Line 188: Tuple* IR_ALWAYS_INLINE 
Sorter::TupleSorter::SelectPivot(TupleIterator begin, TupleIterator end) {
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/16621/3/be/src/runtime/sorter-ir.cc@212
PS3, Line 212: Tuple* IR_ALWAYS_INLINE 
Sorter::TupleSorter::MedianOfThree(Tuple* t1, Tuple* t2, Tuple* t3) {
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/16621/3/be/src/runtime/sorter-ir.cc@249
PS3, Line 249: void IR_ALWAYS_INLINE Sorter::TupleSorter::Swap(Tuple* RESTRICT 
left, Tuple* RESTRICT right,
line too long (92 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie08137449d4a7b554ca8b8650260f8bd72e0a81b
Gerrit-Change-Number: 16621
Gerrit-PatchSet: 3
Gerrit-Owner: Qifan Chen 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 22 Oct 2020 18:41:58 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10216: add logging to help debug flaky test

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10216: add logging to help debug flaky test
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6598/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09984dd3cea686808115ca4cb8c88d24271d8cc1
Gerrit-Change-Number: 16620
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 22 Oct 2020 18:17:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10216: add logging to help debug flaky test

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10216: add logging to help debug flaky test
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09984dd3cea686808115ca4cb8c88d24271d8cc1
Gerrit-Change-Number: 16620
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 22 Oct 2020 18:17:41 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10266: Identify FileSystem type based on the protocol scheme.

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16628 )

Change subject: IMPALA-10266: Identify FileSystem type based on the protocol 
scheme.
..


Patch Set 1:

> Patch Set 1:
>
> It looks like there are some more instanceof usages sprinkled around the code:
>
> $ git grep 'instanceof.*FileSy'
> fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java:  if (!(fs 
> instanceof DistributedFileSystem) && !(fs instanceof S3AFileSystem) &&
> fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java:  !(fs 
> instanceof AzureBlobFileSystem) &&
> fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java:  !(fs 
> instanceof SecureAzureBlobFileSystem) &&
> fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java:  !(fs 
> instanceof AdlFileSystem) &&
> fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java:  !(fs 
> instanceof OzoneFileSystem)) {
> fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java:  boolean 
> shouldCheckPerms = !(fs instanceof AdlFileSystem ||
> fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java:fs 
> instanceof AzureBlobFileSystem || fs instanceof SecureAzureBlobFileSystem);
> fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:  
> if (FileSystemUtil.getDefaultFileSystem() instanceof DistributedFileSystem) {
> fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:  
> if (FileSystemUtil.getDefaultFileSystem() instanceof DistributedFileSystem) {
> fe/src/main/java/org/apache/impala/common/FileSystemUtil.java:
> Preconditions.checkState(fs instanceof DistributedFileSystem);
> fe/src/main/java/org/apache/impala/service/JniFrontend.java:  if (!(fs 
> instanceof DistributedFileSystem ||
> fe/src/main/java/org/apache/impala/service/JniFrontend.java:fs 
> instanceof S3AFileSystem ||
> fe/src/main/java/org/apache/impala/service/JniFrontend.java:fs 
> instanceof AzureBlobFileSystem ||
> fe/src/main/java/org/apache/impala/service/JniFrontend.java:fs 
> instanceof SecureAzureBlobFileSystem ||
> fe/src/main/java/org/apache/impala/service/JniFrontend.java:fs 
> instanceof AdlFileSystem)) {

Ack. Will look into these codes.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
Gerrit-Change-Number: 16628
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 22 Oct 2020 17:47:54 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10266: Identify FileSystem type based on the protocol scheme.

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

Change subject: IMPALA-10266: Identify FileSystem type based on the protocol 
scheme.
..


Patch Set 1:

It looks like there are some more instanceof usages sprinkled around the code:

$ git grep 'instanceof.*FileSy'
fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java:  if (!(fs 
instanceof DistributedFileSystem) && !(fs instanceof S3AFileSystem) &&
fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java:  !(fs 
instanceof AzureBlobFileSystem) &&
fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java:  !(fs 
instanceof SecureAzureBlobFileSystem) &&
fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java:  !(fs 
instanceof AdlFileSystem) &&
fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java:  !(fs 
instanceof OzoneFileSystem)) {
fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java:  boolean 
shouldCheckPerms = !(fs instanceof AdlFileSystem ||
fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java:fs 
instanceof AzureBlobFileSystem || fs instanceof SecureAzureBlobFileSystem);
fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:  if 
(FileSystemUtil.getDefaultFileSystem() instanceof DistributedFileSystem) {
fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:  if 
(FileSystemUtil.getDefaultFileSystem() instanceof DistributedFileSystem) {
fe/src/main/java/org/apache/impala/common/FileSystemUtil.java:
Preconditions.checkState(fs instanceof DistributedFileSystem);
fe/src/main/java/org/apache/impala/service/JniFrontend.java:  if (!(fs 
instanceof DistributedFileSystem ||
fe/src/main/java/org/apache/impala/service/JniFrontend.java:fs 
instanceof S3AFileSystem ||
fe/src/main/java/org/apache/impala/service/JniFrontend.java:fs 
instanceof AzureBlobFileSystem ||
fe/src/main/java/org/apache/impala/service/JniFrontend.java:fs 
instanceof SecureAzureBlobFileSystem ||
fe/src/main/java/org/apache/impala/service/JniFrontend.java:fs 
instanceof AdlFileSystem)) {


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
Gerrit-Change-Number: 16628
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 22 Oct 2020 17:35:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10266: Identify FileSystem type based on the protocol scheme.

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10266: Identify FileSystem type based on the protocol 
scheme.
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7520/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
Gerrit-Change-Number: 16628
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 22 Oct 2020 16:56:25 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10266: Identify FileSystem type based on the protocol scheme.

Riza Suminto has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/16628


Change subject: IMPALA-10266: Identify FileSystem type based on the protocol 
scheme.
..

IMPALA-10266: Identify FileSystem type based on the protocol scheme.

Frontend identifies the type of FileSystem in two ways. The first is
done using the instanceof operator with subclasses of
org.apache.hadoop.fs.FileSystem. The second is by checking the
FileSystem protocol scheme. This patch standardizes the FileSystem
identification based on the scheme only.

Testing:
- Add testSupportStorageIds and testWriteableByImpala in
  FileSystemUtilTest.
- Run and pass core tests.

Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
---
M fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
M fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java
2 files changed, 141 insertions(+), 36 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I04492326a6e84895eef369fc11a3ec11f1536b6b
Gerrit-Change-Number: 16628
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto

[Impala-ASF-CR] IMPALA-10215: Implement INSERT INTO for non-partitioned Iceberg tables (Parquet)

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10215: Implement INSERT INTO for non-partitioned Iceberg 
tables (Parquet)
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5690fb6c2cc51f0033fa26caf8597c80a11bcd8e
Gerrit-Change-Number: 16545
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng 
Gerrit-Comment-Date: Thu, 22 Oct 2020 16:03:00 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10215: Implement INSERT INTO for non-partitioned Iceberg tables (Parquet)

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10215: Implement INSERT INTO for non-partitioned Iceberg 
tables (Parquet)
..


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6597/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5690fb6c2cc51f0033fa26caf8597c80a11bcd8e
Gerrit-Change-Number: 16545
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng 
Gerrit-Comment-Date: Thu, 22 Oct 2020 16:03:01 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10215: Implement INSERT INTO for non-partitioned Iceberg tables (Parquet)

2020-10-22 Thread Csaba Ringhofer (Code Review)

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

Change subject: IMPALA-10215: Implement INSERT INTO for non-partitioned Iceberg 
tables (Parquet)
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5690fb6c2cc51f0033fa26caf8597c80a11bcd8e
Gerrit-Change-Number: 16545
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng 
Gerrit-Comment-Date: Thu, 22 Oct 2020 15:48:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
..


Patch Set 7:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7519/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 7
Gerrit-Owner: wangsheng 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng 
Gerrit-Comment-Date: Thu, 22 Oct 2020 15:31:59 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10215: Implement INSERT INTO for non-partitioned Iceberg tables (Parquet)

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10215: Implement INSERT INTO for non-partitioned Iceberg 
tables (Parquet)
..


Patch Set 5:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7518/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5690fb6c2cc51f0033fa26caf8597c80a11bcd8e
Gerrit-Change-Number: 16545
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng 
Gerrit-Comment-Date: Thu, 22 Oct 2020 15:21:52 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables

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

Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
..


Patch Set 7: Code-Review+1

Thanks!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 7
Gerrit-Owner: wangsheng 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng 
Gerrit-Comment-Date: Thu, 22 Oct 2020 15:17:44 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables

wangsheng has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16606 )

Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
..


Patch Set 7:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3388
PS4, Line 3388: EBERG_TA
> It was just a nitpick comment, but forgot to add 'nit'.
Done, rename 'oldMsTbl' to 'msTbl'



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 7
Gerrit-Owner: wangsheng 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng 
Gerrit-Comment-Date: Thu, 22 Oct 2020 15:16:25 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables

wangsheng has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/16606 )

Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
..

IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables

This patch mainly implement ALTER TABLE for Iceberg
tables, we currently support these statements:
  * ADD COLUMNS
  * RENAME TABLE
  * SET TBL_PROPERTIES
  * SET OWNER
We forbid DROP COLUMN/REPLACE COLUMNS/ALTER COLUMN in this
patch, since these statemens may make Iceberg tables unreadable.
We may support column resolution by field id in the near future,
after that, we will support COLUMN/REPLACE COLUMNS/ALTER COLUMN
for Iceberg tables.

Here something we still need to pay attention:
1.RENAME TABLE is not supported for HadoopCatalog/HadoopTables,
even if we already implement 'RENAME TABLE' statement, so we
only rename the table in the Hive Metastore for external table.
2.We cannot ADD/DROP PARTITION now since there is no API for that
in Iceberg, but related work is already in progess in Iceberg.

Testing:
- Iceberg table alter test in test_iceberg.py
- Iceberg table negative test in test_scanners.py

Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M 
fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.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/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
19 files changed, 493 insertions(+), 11 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 7
Gerrit-Owner: wangsheng 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng

[Impala-ASF-CR] IMPALA-10215: Implement INSERT INTO for non-partitioned Iceberg tables (Parquet)

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

Change subject: IMPALA-10215: Implement INSERT INTO for non-partitioned Iceberg 
tables (Parquet)
..


Patch Set 5:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/16545/4/be/src/runtime/coordinator.cc@772
PS4, Line 772: HdfsTableDescriptor* hdfs_table
> Uninitialized variable.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5690fb6c2cc51f0033fa26caf8597c80a11bcd8e
Gerrit-Change-Number: 16545
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng 
Gerrit-Comment-Date: Thu, 22 Oct 2020 15:02:16 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10215: Implement INSERT INTO for non-partitioned Iceberg tables (Parquet)

Hello Gabor Kaszab, wangsheng, Csaba Ringhofer, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-10215: Implement INSERT INTO for non-partitioned Iceberg 
tables (Parquet)
..

IMPALA-10215: Implement INSERT INTO for non-partitioned Iceberg tables (Parquet)

This commit adds support for INSERT INTO statements against Iceberg
tables when the table is non-partitioned and the underlying file format
is Parquet.

We still use Impala's HdfsParquetTableWriter to write the data files,
though they needed some modifications to conform to the Iceberg spec,
namely:
 * write Iceberg/Parquet 'field_id' for the columns
 * TIMESTAMPs are encoded as INT64 micros (without time zone)

We use DmlExecState to transfer information from the table sink
operators to the coordinator, then updateCatalog() invokes the
AppendFiles API to add files atomically. DmlExecState is encoded in
protobuf, communication with the Frontend uses Thrift. Therefore to
avoid defining Iceberg DataFile multiple times they are stored in
FlatBuffers.

The commit also does some corrections on Impala type <-> Iceberg type
mapping:
 * Impala TIMESTAMP is Iceberg TIMESTAMP (without time zone)
 * Impala CHAR is Iceberg FIXED

Testing:
 * Added INSERT tests to iceberg-insert.test
 * Added negative tests to iceberg-negative.test
 * I also did some manual testing with Spark. Spark is able to read
   Iceberg tables written by Impala until we use TIMESTAMPs. In that
   case Spark rejects the data files because it only accepts TIMESTAMPS
   with time zone.
 * Added concurrent INSERT tests to test_insert_stress.py

Change-Id: I5690fb6c2cc51f0033fa26caf8597c80a11bcd8e
---
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
A be/src/exec/output-partition.h
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.h
M be/src/exec/parquet/parquet-metadata-utils.cc
M be/src/exec/parquet/parquet-metadata-utils.h
M be/src/runtime/coordinator.cc
M be/src/runtime/descriptors.cc
M be/src/runtime/descriptors.h
M be/src/runtime/dml-exec-state.cc
M be/src/runtime/dml-exec-state.h
M be/src/service/client-request-state.cc
M common/fbs/CMakeLists.txt
A common/fbs/IcebergObjects.fbs
M common/protobuf/control_service.proto
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/Descriptors.thrift
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/catalog/Column.java
M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
A fe/src/main/java/org/apache/impala/catalog/IcebergColumn.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-insert.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/common/skip.py
M tests/query_test/test_iceberg.py
M tests/stress/test_insert_stress.py
41 files changed, 856 insertions(+), 192 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5690fb6c2cc51f0033fa26caf8597c80a11bcd8e
Gerrit-Change-Number: 16545
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng

[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables

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

Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
..


Patch Set 6: Code-Review+1

(1 comment)

Thanks for your response, LGTM! I'm only giving it +1 to let others take a look 
as well.

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

http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3388
PS4, Line 3388: .ICEBERG
> In fact, I don't quite understand where the problem is. I write this method
It was just a nitpick comment, but forgot to add 'nit'.

Nothing is wrong here, only the name doesn't really fit. And I think it also 
doesn't really fit for renameManagedKuduTable() (but no need to change it 
there).

My suggestion was only to rename the variable to 'msTbl'. I'm also OK with 
keeping the original name, I just noticed that there was no reply to this 
comment.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 6
Gerrit-Owner: wangsheng 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng 
Gerrit-Comment-Date: Thu, 22 Oct 2020 15:00:12 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
..


Patch Set 6:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7517/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 6
Gerrit-Owner: wangsheng 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng 
Gerrit-Comment-Date: Thu, 22 Oct 2020 14:57:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables

wangsheng has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16606 )

Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
..


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/16606/5/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
File 
fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java:

http://gerrit.cloudera.org:8080/#/c/16606/5/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java@87
PS5, Line 87: TableIdentifier oldTableId = 
IcebergUtil.getIcebergTableIdentifier(feTable);
: try {
> I think we should catch the UnsupportedException being thrown by Iceberg, a
Done


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

http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3388
PS4, Line 3388: .ICEBERG
> It's actually the 'new' metastore table. We have deep copied it to safely m
In fact, I don't quite understand where the problem is. I write this method 
according to renameManagedKuduTable. It seem that both these two methods only 
update 'oldMsTbl' properties.
Do you mean we cannot transfom new 'oldMsTbl' to HMS by 
getHiveClient().alter_table()?
Besides, we cannot rename managed table of HadoopTables and HadoopCatalog, so I 
cannot test this situation yet.


http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@112
PS4, Line 112:*   FLOAT -> DOUBLE
 :*   DECIMAL(s1,p1) -> DECIMAL(s1,p2), same scale, p1<=p2
 :*/
> I don't think we need to re-implement the checks, we just need to provide g
Ok, I would consider this when supporting alter column in a later patch!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 6
Gerrit-Owner: wangsheng 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng 
Gerrit-Comment-Date: Thu, 22 Oct 2020 14:42:10 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10132 Implement ds hll estimate bounds as string() function.

2020-10-22 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-10132 Implement ds_hll_estimate_bounds_as_string() 
function.
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/7516/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I46bf8263e8fd3877a087b9cb6f0d1a2392bb9153
Gerrit-Change-Number: 16626
Gerrit-PatchSet: 1
Gerrit-Owner: Fucun Chu 
Gerrit-Reviewer: Fucun Chu 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 22 Oct 2020 14:41:08 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables