date:20181019

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

Change subject: IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.
..


Patch Set 10: Code-Review+2

Fixed disk-io-mgr-test.cc and some typos in a couple tests. Carrying +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
Gerrit-Change-Number: 11630
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Sat, 20 Oct 2018 02:46:52 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.

Joe McDonnell has uploaded a new patch set (#10) to the change originally 
created by mackror...@apache.org. ( http://gerrit.cloudera.org:8080/11630 )

Change subject: IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.
..

IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.

HADOOP-15407 adds a new FileSystem implementation called "ABFS" for the
ADLS Gen2 service. It's in the hadoop-azure module as a replacement for
WASB. Filesystem semantics should be the same, so skipped tests and
other behavior changes have simply mirrored what is done for ADLS Gen1
by default. Tests skipped on ADLS Gen1 due to eventual consistency of
the Python client can be run against ADLS Gen2.

Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
---
M be/src/exec/hdfs-table-sink.cc
M be/src/runtime/io/disk-io-mgr-test.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
M be/src/runtime/io/scan-range.cc
M be/src/util/hdfs-util.cc
M be/src/util/hdfs-util.h
M bin/impala-config.sh
M fe/pom.xml
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.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/analysis/AnalyzeStmtsTest.java
M testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.tmpl
M tests/common/impala_test_suite.py
M tests/common/skip.py
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_hdfs_fd_caching.py
M tests/custom_cluster/test_insert_behaviour.py
M tests/custom_cluster/test_metadata_replicas.py
M tests/custom_cluster/test_parquet_max_page_header.py
M tests/custom_cluster/test_permanent_udfs.py
M tests/data_errors/test_data_errors.py
M tests/failure/test_failpoints.py
M tests/metadata/test_compute_stats.py
M tests/metadata/test_ddl.py
M tests/metadata/test_hdfs_encryption.py
M tests/metadata/test_hdfs_permissions.py
M tests/metadata/test_hms_integration.py
M tests/metadata/test_metadata_query_statements.py
M tests/metadata/test_partition_metadata.py
M tests/metadata/test_refresh_partition.py
M tests/metadata/test_views_compatibility.py
M tests/query_test/test_compressed_formats.py
M tests/query_test/test_hdfs_caching.py
M tests/query_test/test_insert.py
M tests/query_test/test_insert_behaviour.py
M tests/query_test/test_insert_parquet.py
M tests/query_test/test_join_queries.py
M tests/query_test/test_nested_types.py
M tests/query_test/test_observability.py
M tests/query_test/test_partitioning.py
M tests/query_test/test_resource_limits.py
M tests/query_test/test_scanners.py
M tests/stress/test_ddl_stress.py
A tests/util/abfs_util.py
M tests/util/filesystem_utils.py
48 files changed, 360 insertions(+), 32 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
Gerrit-Change-Number: 11630
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger

[Impala-ASF-CR] IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.
..


Patch Set 9: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
Gerrit-Change-Number: 11630
Gerrit-PatchSet: 9
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Sat, 20 Oct 2018 02:45:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use 
KRPC
..


Patch Set 19:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1121/ : 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/10855
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe
Gerrit-Change-Number: 10855
Gerrit-PatchSet: 19
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Michal Ostrowski 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 20 Oct 2018 02:26:24 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC

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

Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use 
KRPC
..


Patch Set 18:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10855/18/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:

http://gerrit.cloudera.org:8080/#/c/10855/18/be/src/runtime/query-state.cc@363
PS18, Line 363: ERROR
> I'd use DFATAL so that it would at least crash in debug builds. (DFATAL tur
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe
Gerrit-Change-Number: 10855
Gerrit-PatchSet: 18
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Michal Ostrowski 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 20 Oct 2018 01:52:28 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC

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

Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use 
KRPC
..


Patch Set 19: Code-Review+1

Carry +1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe
Gerrit-Change-Number: 10855
Gerrit-PatchSet: 19
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Michal Ostrowski 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 20 Oct 2018 01:52:39 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC

Hello Thomas Marshall, Todd Lipcon, Tim Armstrong, Bikramjeet Vig, Impala 
Public Jenkins, Michal Ostrowski,

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

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

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

Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use 
KRPC
..

IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC

This change converts ReportExecStatus() RPC from thrift
based RPC to KRPC. This is done in part of the preparation
for fixing IMPALA-2990 as we can take advantage of TCP connection
multiplexing in KRPC to avoid overwhelming the coordinator
with too many connections by reducing the number of TCP connection
to one for each executor.

This patch also introduces a new service pool for all query execution
control related RPCs in the future so that control commands from
coordinators aren't blocked by long-running DataStream services' RPCs.
To avoid unnecessary delays due to sharing the network connections
between DataStream service and Control service, this change added the
service name as part of the user credentials for the ConnectionId
so each service will use a separate connection.

The majority of this patch is mechanical conversion of some Thrift
structures used in ReportExecStatus() RPC to Protobuf. Note that the
runtime profile is still retained as a Thrift structure as Impala
clients will still fetch query profiles using Thrift RPCs. This also
avoids duplicating the serialization implementation in both Thrift
and Protobuf for the runtime profile. The Thrift runtime profiles
are serialized and sent as a sidecar in ReportExecStatus() RPC.

This patch also fixes IMPALA-7241 which may lead to duplicated
dml stats being applied. The fix is by adding a monotonically
increasing version number for fragment instances' reports. The
coordinator will ignore any report smaller than or equal to the
version in the last report.

Testing done:
1. Exhaustive build.
2. Added some targeted test cases for profile serialization failure
   and RPC retries/timeout.

Change-Id: I7638583b433dcac066b87198e448743d90415ebe
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/catalog/catalog-util.cc
M be/src/common/global-flags.cc
M be/src/exec/data-sink.cc
M be/src/exec/data-sink.h
M be/src/exec/hbase-table-sink.cc
M be/src/exec/hdfs-parquet-table-writer.cc
M be/src/exec/hdfs-parquet-table-writer.h
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-writer.cc
M be/src/exec/hdfs-table-writer.h
M be/src/rpc/CMakeLists.txt
M be/src/rpc/jni-thrift-util.h
M be/src/rpc/rpc-mgr-kerberized-test.cc
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr-test.h
M be/src/rpc/rpc-mgr.h
M be/src/rpc/thrift-util-test.cc
M be/src/rpc/thrift-util.h
M be/src/runtime/backend-client.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/runtime/dml-exec-state.cc
M be/src/runtime/dml-exec-state.h
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/fragment-instance-state.h
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/test-env.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/scheduler-test-util.cc
M be/src/service/CMakeLists.txt
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
A be/src/service/control-service.cc
A be/src/service/control-service.h
M be/src/service/data-stream-service.cc
M be/src/service/data-stream-service.h
M be/src/service/impala-internal-service.cc
M be/src/service/impala-internal-service.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/testutil/in-process-servers.cc
M be/src/util/container-util.h
A be/src/util/error-util-internal.h
M be/src/util/error-util-test.cc
M be/src/util/error-util.cc
M be/src/util/error-util.h
M be/src/util/runtime-profile.cc
M be/src/util/uid-util.h
M common/protobuf/CMakeLists.txt
M common/protobuf/common.proto
A common/protobuf/control_service.proto
M common/protobuf/data_stream_service.proto
M common/protobuf/row_batch.proto
M common/protobuf/rpc_test.proto
M common/thrift/ImpalaInternalService.thrift
M tests/custom_cluster/test_rpc_timeout.py
65 files changed, 1,298 insertions(+), 769 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe
Gerrit-Change-Number: 10855
Gerrit-PatchSet: 19
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impa

[Impala-ASF-CR] IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.
..


Patch Set 8:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1120/ : 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/11630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
Gerrit-Change-Number: 11630
Gerrit-PatchSet: 8
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Sat, 20 Oct 2018 00:48:50 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5031: make codegen ubsan available by environment variable

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-5031: make codegen ubsan available by environment 
variable
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1119/ : 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/11742
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd108f8a56158566d95f4769048bc9ab45bd3514
Gerrit-Change-Number: 11742
Gerrit-PatchSet: 1
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Sat, 20 Oct 2018 00:32:43 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..

IMPALA-7721: Fix broken /catalog_object web API when getting a privilege

Before this patch, /catalog_object web API was broken when getting a
privilege due to an incorrect way of getting a role ID. IMPALA-7616
broke this even more due to a lack of test coverage in /catalog_object
when authorization is enabled. This patch fixes the issue and makes the
/catalog_object web API usable again for getting a privilege.

Testing:
- Added a new BE test
- Added a new E2E test
- Ran all E2E authorization tests

Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Reviewed-on: http://gerrit.cloudera.org:8080/11721
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/catalog/catalog-util-test.cc
M be/src/catalog/catalog-util.cc
M be/src/catalog/catalog-util.h
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
M tests/authorization/test_authorization.py
6 files changed, 260 insertions(+), 13 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 12
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 11: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Sat, 20 Oct 2018 00:19:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.
..


Patch Set 9:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
Gerrit-Change-Number: 11630
Gerrit-PatchSet: 9
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Sat, 20 Oct 2018 00:13:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.
..


Patch Set 9:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
Gerrit-Change-Number: 11630
Gerrit-PatchSet: 9
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Sat, 20 Oct 2018 00:09:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.
..


Patch Set 9: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
Gerrit-Change-Number: 11630
Gerrit-PatchSet: 9
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Sat, 20 Oct 2018 00:09:50 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.

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

Change subject: IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.
..


Patch Set 8: Code-Review+2

Thanks!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
Gerrit-Change-Number: 11630
Gerrit-PatchSet: 8
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Sat, 20 Oct 2018 00:06:46 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.

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

Change subject: IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.
..


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11630/7/be/src/runtime/io/scan-range.cc
File be/src/runtime/io/scan-range.cc:

http://gerrit.cloudera.org:8080/#/c/11630/7/be/src/runtime/io/scan-range.cc@31
PS7, Line 31:
> Nit: stray line
Done


http://gerrit.cloudera.org:8080/#/c/11630/7/be/src/runtime/io/scan-range.cc@447
PS7, Line 447:   if (disk_id_ == io_mgr_->RemoteAdlsDiskId()) {
 : DCHECK(IsADLSPath(file()));
 : return FLAGS_adls_read_chunk_size;
 :   }
> What should we do for this commit regarding ABFS chunk size?
Yes, I agree that #3 is best.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
Gerrit-Change-Number: 11630
Gerrit-PatchSet: 7
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Fri, 19 Oct 2018 23:59:40 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.

Jim Apple has uploaded a new patch set (#8) to the change originally created by 
mackror...@apache.org. ( http://gerrit.cloudera.org:8080/11630 )

Change subject: IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.
..

IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.

HADOOP-15407 adds a new FileSystem implementation called "ABFS" for the
ADLS Gen2 service. It's in the hadoop-azure module as a replacement for
WASB. Filesystem semantics should be the same, so skipped tests and
other behavior changes have simply mirrored what is done for ADLS Gen1
by default. Tests skipped on ADLS Gen1 due to eventual consistency of
the Python client can be run against ADLS Gen2.

Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
---
M be/src/exec/hdfs-table-sink.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
M be/src/runtime/io/scan-range.cc
M be/src/util/hdfs-util.cc
M be/src/util/hdfs-util.h
M bin/impala-config.sh
M fe/pom.xml
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.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/analysis/AnalyzeStmtsTest.java
M testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.tmpl
M tests/common/impala_test_suite.py
M tests/common/skip.py
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_hdfs_fd_caching.py
M tests/custom_cluster/test_insert_behaviour.py
M tests/custom_cluster/test_metadata_replicas.py
M tests/custom_cluster/test_parquet_max_page_header.py
M tests/custom_cluster/test_permanent_udfs.py
M tests/data_errors/test_data_errors.py
M tests/failure/test_failpoints.py
M tests/metadata/test_compute_stats.py
M tests/metadata/test_ddl.py
M tests/metadata/test_hdfs_encryption.py
M tests/metadata/test_hdfs_permissions.py
M tests/metadata/test_hms_integration.py
M tests/metadata/test_metadata_query_statements.py
M tests/metadata/test_partition_metadata.py
M tests/metadata/test_refresh_partition.py
M tests/metadata/test_views_compatibility.py
M tests/query_test/test_compressed_formats.py
M tests/query_test/test_hdfs_caching.py
M tests/query_test/test_insert.py
M tests/query_test/test_insert_behaviour.py
M tests/query_test/test_insert_parquet.py
M tests/query_test/test_join_queries.py
M tests/query_test/test_nested_types.py
M tests/query_test/test_observability.py
M tests/query_test/test_partitioning.py
M tests/query_test/test_resource_limits.py
M tests/query_test/test_scanners.py
M tests/stress/test_ddl_stress.py
A tests/util/abfs_util.py
M tests/util/filesystem_utils.py
47 files changed, 357 insertions(+), 30 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
Gerrit-Change-Number: 11630
Gerrit-PatchSet: 8
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger

[Impala-ASF-CR] IMPALA-7697: Fix flakiness in test resource limits

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7697: Fix flakiness in test_resource_limits
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ba7080f62f0af3e16ef6c304463ebf78dec1b0c
Gerrit-Change-Number: 11741
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 23:44:14 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7697: Fix flakiness in test resource limits

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7697: Fix flakiness in test_resource_limits
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ba7080f62f0af3e16ef6c304463ebf78dec1b0c
Gerrit-Change-Number: 11741
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 23:44:13 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7697: Fix flakiness in test resource limits

2018-10-19 Thread Bikramjeet Vig (Code Review)

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

Change subject: IMPALA-7697: Fix flakiness in test_resource_limits
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11741/1/testdata/workloads/functional-query/queries/QueryTest/query-resource-limits.test
File 
testdata/workloads/functional-query/queries/QueryTest/query-resource-limits.test:

http://gerrit.cloudera.org:8080/#/c/11741/1/testdata/workloads/functional-query/queries/QueryTest/query-resource-limits.test@46
PS1, Line 46: select sleep(1)
> I like that the simplification addresses more directly what's being tested
i thought the same thing but unfortunately other checks need to run a working 
query that uses CPU time or scans data for it to work.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ba7080f62f0af3e16ef6c304463ebf78dec1b0c
Gerrit-Change-Number: 11741
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 23:43:37 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5031: make codegen ubsan available by environment variable

Jim Apple has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11742


Change subject: IMPALA-5031: make codegen ubsan available by environment 
variable
..

IMPALA-5031: make codegen ubsan available by environment variable

bin/jenkins/all-tests.sh does not support any flags when calling
bootstrap_development.sh, which eventually calls buildall.sh. Since
Jenkins scripts are called non-interactively, the type of build is
usually controlled by an environment variable, but that was not
supported for codegen ubsan. This patch makes that possible under the
name "UBSAN_FULL".

Change-Id: Ifd108f8a56158566d95f4769048bc9ab45bd3514
---
M CMakeLists.txt
M be/CMakeLists.txt
M bin/make_impala.sh
M buildall.sh
M tests/common/environ.py
5 files changed, 15 insertions(+), 13 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifd108f8a56158566d95f4769048bc9ab45bd3514
Gerrit-Change-Number: 11742
Gerrit-PatchSet: 1
Gerrit-Owner: Jim Apple

[Impala-ASF-CR] IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.

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

Change subject: IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.
..


Patch Set 7:

(2 comments)

I'm basically ready to +2 this. I'd like to settle what we are doing with the 
chunk size.

http://gerrit.cloudera.org:8080/#/c/11630/7/be/src/runtime/io/scan-range.cc
File be/src/runtime/io/scan-range.cc:

http://gerrit.cloudera.org:8080/#/c/11630/7/be/src/runtime/io/scan-range.cc@31
PS7, Line 31:
Nit: stray line


http://gerrit.cloudera.org:8080/#/c/11630/7/be/src/runtime/io/scan-range.cc@447
PS7, Line 447:   if (disk_id_ == io_mgr_->RemoteAdlsDiskId()) {
 : DCHECK(IsADLSPath(file()));
 : return FLAGS_adls_read_chunk_size;
 :   }
What should we do for this commit regarding ABFS chunk size?

Some options:
1. Don't customize for ABFS and add parameters later. File a JIRA to customize 
it.
2. Use adls_read_chunk_size for both ABFS and ADLS
3. Add a parameter now and set the default the same as adls_read_chunk_size
4. Hard code ABFS chunk size as 128k for now. File a JIRA to do perf tests and 
customize it.

If we use 128k for S3 and ADLS, it seems reasonable to start out using it for 
ABFS until we can do performance tests (or until we can convince ourselves that 
ABFS is different).

#3 seems reasonable to me. What do you think?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
Gerrit-Change-Number: 11630
Gerrit-PatchSet: 7
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Fri, 19 Oct 2018 23:38:46 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7699: Fix spilling test run with hdfs erasure coding turned on

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7699: Fix spilling test run with hdfs erasure coding 
turned on
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I207569822ba7388e78936d25e2311fa09c7a1b9a
Gerrit-Change-Number: 11740
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 23:36:44 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7699: Fix spilling test run with hdfs erasure coding turned on

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7699: Fix spilling test run with hdfs erasure coding 
turned on
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I207569822ba7388e78936d25e2311fa09c7a1b9a
Gerrit-Change-Number: 11740
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 23:36:43 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

2018-10-19 Thread Thomas Marshall (Code Review)

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

Change subject: IMPALA-5821: Add query with implicit casts to extended explain 
output.
..


Patch Set 2:

(3 comments)

> > It seems good to me, but I would ping Greg Rahn in the Jira,
 > maybe
 > > he has some ideas.
 >
 > I have pinged him
 >
 > > I have one issue with the current output: as I saw in the .test
 > > files, queries are printed in one line by default, which can make
 > > complex queries very difficult to read. It would be much nicer to
 > > break them at logical points, but I have no idea how to do it
 > > easily.
 >
 > I agree that a clever printing would be nice, but that sounds
 > tricky. I could wrap at say the last space before 80 columns with
 > only small effort, but this will make the header harder to parse in
 > tests. Should I try this?

Doing the simple thing (wrapping at 80) is probably fine.

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

http://gerrit.cloudera.org:8080/#/c/11719/2//COMMIT_MSG@32
PS2, Line 32: If a query in a .test file produces a diff when run by 
PlannerTest,
: then print the name of the .test file in the output.
Could you submit this as a standalone review?

If I understand correctly, it doesn't really have anything to do with the rest 
of the review, and its nice to keep changes relatively focused, esp. since this 
is already such a large patch.


http://gerrit.cloudera.org:8080/#/c/11719/2//COMMIT_MSG@67
PS2, Line 67: Is "Query with implicit casts:" a good description?
Any reason not to also show the rewritten sql here and call this something like 
"Analyzed query"


http://gerrit.cloudera.org:8080/#/c/11719/2/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test:

http://gerrit.cloudera.org:8080/#/c/11719/2/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test@59
PS2, Line 59: c_custkey > 10
not sure how difficult this would be, but it would be nice to include the 
implicit casts here too



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Fri, 19 Oct 2018 22:58:18 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7697: Fix flakiness in test resource limits

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7697: Fix flakiness in test_resource_limits
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1118/ : 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/11741
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ba7080f62f0af3e16ef6c304463ebf78dec1b0c
Gerrit-Change-Number: 11741
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 22:52:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.
..


Patch Set 7:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1117/ : 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/11630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
Gerrit-Change-Number: 11630
Gerrit-PatchSet: 7
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Fri, 19 Oct 2018 22:50:00 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7699: Fix spilling test run with hdfs erasure coding turned on

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7699: Fix spilling test run with hdfs erasure coding 
turned on
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1116/ : 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/11740
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I207569822ba7388e78936d25e2311fa09c7a1b9a
Gerrit-Change-Number: 11740
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 22:30:19 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7697: Fix flakiness in test resource limits

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

Change subject: IMPALA-7697: Fix flakiness in test_resource_limits
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11741/1/testdata/workloads/functional-query/queries/QueryTest/query-resource-limits.test
File 
testdata/workloads/functional-query/queries/QueryTest/query-resource-limits.test:

http://gerrit.cloudera.org:8080/#/c/11741/1/testdata/workloads/functional-query/queries/QueryTest/query-resource-limits.test@46
PS1, Line 46: select sleep(1)
I like that the simplification addresses more directly what's being tested 
here.  Can we do the same for the other time limit tests here?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ba7080f62f0af3e16ef6c304463ebf78dec1b0c
Gerrit-Change-Number: 11741
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 22:28:20 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7699: Fix spilling test run with hdfs erasure coding turned on

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

Change subject: IMPALA-7699: Fix spilling test run with hdfs erasure coding 
turned on
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I207569822ba7388e78936d25e2311fa09c7a1b9a
Gerrit-Change-Number: 11740
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 22:24:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7697: Fix flakiness in test resource limits

2018-10-19 Thread Bikramjeet Vig (Code Review)

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


Change subject: IMPALA-7697: Fix flakiness in test_resource_limits
..

IMPALA-7697: Fix flakiness in test_resource_limits

This patch fixes one of the tests in test_resource_limits that expects a
query to run for more than 2 seconds but currently fails because it
sometimes completes earlier than that.

Change-Id: I2ba7080f62f0af3e16ef6c304463ebf78dec1b0c
---
M 
testdata/workloads/functional-query/queries/QueryTest/query-resource-limits.test
1 file changed, 1 insertion(+), 6 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.

Jim Apple has uploaded a new patch set (#7) to the change originally created by 
mackror...@apache.org. ( http://gerrit.cloudera.org:8080/11630 )

Change subject: IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.
..

IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.

HADOOP-15407 adds a new FileSystem implementation called "ABFS" for the
ADLS Gen2 service. It's in the hadoop-azure module as a replacement for
WASB. Filesystem semantics should be the same, so skipped tests and
other behavior changes have simply mirrored what is done for ADLS Gen1
by default. Tests skipped on ADLS Gen1 due to eventual consistency of
the Python client can be run against ADLS Gen2.

Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
---
M be/src/exec/hdfs-table-sink.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
M be/src/runtime/io/scan-range.cc
M be/src/util/hdfs-util.cc
M be/src/util/hdfs-util.h
M bin/impala-config.sh
M fe/pom.xml
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.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/analysis/AnalyzeStmtsTest.java
M testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.tmpl
M tests/common/impala_test_suite.py
M tests/common/skip.py
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_hdfs_fd_caching.py
M tests/custom_cluster/test_insert_behaviour.py
M tests/custom_cluster/test_metadata_replicas.py
M tests/custom_cluster/test_parquet_max_page_header.py
M tests/custom_cluster/test_permanent_udfs.py
M tests/data_errors/test_data_errors.py
M tests/failure/test_failpoints.py
M tests/metadata/test_compute_stats.py
M tests/metadata/test_ddl.py
M tests/metadata/test_hdfs_encryption.py
M tests/metadata/test_hdfs_permissions.py
M tests/metadata/test_hms_integration.py
M tests/metadata/test_metadata_query_statements.py
M tests/metadata/test_partition_metadata.py
M tests/metadata/test_refresh_partition.py
M tests/metadata/test_views_compatibility.py
M tests/query_test/test_compressed_formats.py
M tests/query_test/test_hdfs_caching.py
M tests/query_test/test_insert.py
M tests/query_test/test_insert_behaviour.py
M tests/query_test/test_insert_parquet.py
M tests/query_test/test_join_queries.py
M tests/query_test/test_nested_types.py
M tests/query_test/test_observability.py
M tests/query_test/test_partitioning.py
M tests/query_test/test_resource_limits.py
M tests/query_test/test_scanners.py
M tests/stress/test_ddl_stress.py
A tests/util/abfs_util.py
M tests/util/filesystem_utils.py
47 files changed, 352 insertions(+), 29 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
Gerrit-Change-Number: 11630
Gerrit-PatchSet: 7
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger

[Impala-ASF-CR] IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.

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

Change subject: IMPALA-7681. Add Azure Blob File System (ADLS Gen2) support.
..


Patch Set 6: Code-Review+1

(2 comments)

Two nits. Joe, Phil, would appreciate you taking a quick look - I'm going to 
post a new patchset, since I know Sean is otherwise occupied.

http://gerrit.cloudera.org:8080/#/c/11630/6/tests/query_test/test_insert.py
File tests/query_test/test_insert.py:

http://gerrit.cloudera.org:8080/#/c/11630/6/tests/query_test/test_insert.py@112
PS6, Line 112:   # This fails against ABFS due HADOOP-15860. It succeeds if the 
partition name
Can you add a SkipIfABFS to this test, too?


http://gerrit.cloudera.org:8080/#/c/11630/6/tests/util/abfs_util.py
File tests/util/abfs_util.py:

http://gerrit.cloudera.org:8080/#/c/11630/6/tests/util/abfs_util.py@32
PS6, Line 32:   def hadoop_fs_shell(self, command):
Methods used only within the class are traditionally prefixed by an underscore; 
here and below.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5120b071760e7655e78902dce8483f8f54de445d
Gerrit-Change-Number: 11630
Gerrit-PatchSet: 6
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Fri, 19 Oct 2018 22:11:08 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP] IMPALA-6932: Speed up scans for sequence datasets with many files

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: [WIP] IMPALA-6932: Speed up scans for sequence datasets with 
many files
..


Patch Set 2:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1115/ : 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/11517
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I211e2511ea3bb5edea29f1bd63e6b1fa4c4b1965
Gerrit-Change-Number: 11517
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 19 Oct 2018 22:05:54 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7699: Fix spilling test run with hdfs erasure coding turned on

2018-10-19 Thread Bikramjeet Vig (Code Review)

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


Change subject: IMPALA-7699: Fix spilling test run with hdfs erasure coding 
turned on
..

IMPALA-7699: Fix spilling test run with hdfs erasure coding turned on

A spilling test when run on test build with hdfs erasure coding turned
on hits an out of memory error on the hdfs scan node. This happens
because the test is tuned for a regular 3 node minicluster without
hdfs erasure coding. Fix is to simply increase the memory limit on
the test to accommodate this difference yet keep it small enough to
achieve desired spilling on the hash join node.

Testing:
Ran it on an EC enabled minicluster to make sure it works

Change-Id: I207569822ba7388e78936d25e2311fa09c7a1b9a
---
M 
testdata/workloads/functional-query/queries/QueryTest/spilling-no-debug-action.test
1 file changed, 1 insertion(+), 1 deletion(-)



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

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

[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an 
upper case role name
..


Patch Set 9: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 21:48:27 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an 
upper case role name
..

IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

Sentry stores the role names in lower case and Impala stores the role
names based on the original input role names. IMPALA-7343 introduced
a new bulk API (listAllRolesPrivileges) from Sentry that returns a map
of role name to a set of privileges. Since Impala preserves the case
sensitivity of the role names based on the original input role names,
this causes an issue when trying to retrieve a set of privileges from
a role name that is stored in Impala, especially when the role names in
Impala differ than the ones returned by listAllRolesPrivileges. This
issue will then result in privileges with mismatch role names to never
get refreshed in the Catalogd, which causes Impalad to wait indefinitely
waiting for the privileges to be updated by Catalogd. The fix is to get
a set of privileges using the role names returned by Sentry's
listAllRoles instead of using the role names stored in Impala.

Testing:
- Added a new E2E test
- Ran all E2E authorization tests

Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Reviewed-on: http://gerrit.cloudera.org:8080/11734
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M fe/src/main/java/org/apache/impala/catalog/Principal.java
M fe/src/main/java/org/apache/impala/catalog/Role.java
M fe/src/main/java/org/apache/impala/catalog/User.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M tests/authorization/test_grant_revoke.py
5 files changed, 61 insertions(+), 7 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-6658: improve Parquet RLE for low bit widths

2018-10-19 Thread Thomas Marshall (Code Review)

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

Change subject: IMPALA-6658: improve Parquet RLE for low bit widths
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/11582/2/be/src/util/rle-encoding.h
File be/src/util/rle-encoding.h:

http://gerrit.cloudera.org:8080/#/c/11582/2/be/src/util/rle-encoding.h@203
PS2, Line 203: the previous run is flushed out
I think that it would be helpful to update this comment with a brief discussion 
of the use of the circular buffer and the way that you flush 8 values at a time 
when you have a not long enough repeated run.


http://gerrit.cloudera.org:8080/#/c/11582/2/be/src/util/rle-encoding.h@348
PS2, Line 348: most values
most MAX_RUN_LENGTH_BUFFER values


http://gerrit.cloudera.org:8080/#/c/11582/2/be/src/util/rle-encoding.h@350
PS2, Line 350: _
nit: period, here and elsewhere


http://gerrit.cloudera.org:8080/#/c/11582/2/be/src/util/rle-test.cc
File be/src/util/rle-test.cc:

http://gerrit.cloudera.org:8080/#/c/11582/2/be/src/util/rle-test.cc@148
PS2, Line 148: Rle
nit: RleTest



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I191a581d3f699b6669e48ac9dc39c76ed77c4a76
Gerrit-Change-Number: 11582
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Fri, 19 Oct 2018 21:45:33 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP] IMPALA-6932: Speed up scans for sequence datasets with many files

2018-10-19 Thread Pooja Nilangekar (Code Review)

Pooja Nilangekar has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/11517 )

Change subject: [WIP] IMPALA-6932: Speed up scans for sequence datasets with 
many files
..

[WIP] IMPALA-6932: Speed up scans for sequence datasets with many files

This change addresses the slow scans of sequence datasets with
many files by enqueueing the scan ranges to the head of the disk
IO queue instead of the tail. This ensures that the data ranges
get priority over headers of other files. Hence it produces
results earlier for limit queries [and dynamic filters?].

Testing:
Added logs to verify that the scan ranges for sequence files are
added to the head.
TODO: Verify that this patch solves the issue. [This can't be
tested on the minicluster]

Tested the patch with backend and end-to-end tests.
Single node performance test results:
+--++-++++
| Workload | File Format| Avg (s) | Delta(Avg) | GeoMean(s) | 
Delta(GeoMean) |
+--++-++++
| TPCH(50) | avro / none / none | 65.62   | -0.38% | 43.51  | -0.79%
 |
+--++-++++

Change-Id: I211e2511ea3bb5edea29f1bd63e6b1fa4c4b1965
---
M be/src/exec/base-sequence-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M be/src/runtime/io/request-context.cc
M be/src/runtime/io/request-context.h
M be/src/util/internal-queue-test.cc
M be/src/util/internal-queue.h
9 files changed, 140 insertions(+), 99 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I211e2511ea3bb5edea29f1bd63e6b1fa4c4b1965
Gerrit-Change-Number: 11517
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 10:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1114/ : 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/11721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:53:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 11: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:26:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 11:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:26:48 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

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

Change subject: IMPALA-5821: Add query with implicit casts to extended explain 
output.
..


Patch Set 2:

> > It seems good to me, but I would ping Greg Rahn in the Jira,
 > maybe
 > > he has some ideas.
 >
 > I have pinged him
 >
 > > I have one issue with the current output: as I saw in the .test
 > > files, queries are printed in one line by default, which can make
 > > complex queries very difficult to read. It would be much nicer to
 > > break them at logical points, but I have no idea how to do it
 > > easily.
 >
 > I agree that a clever printing would be nice, but that sounds
 > tricky. I could wrap at say the last space before 80 columns with
 > only small effort, but this will make the header harder to parse in
 > tests. Should I try this?

About good places to start new line: the enum could have a property whether to 
add new lines, and some ParseNodes could add new lines if it is true, e.g 
before joins and unions.

Another idea (of the "if you already touch it, why not rewrite the whole thing" 
type, so it is optional): if there is an object passed to toSql(), maybe it 
could be some kind of ToSqlBuilder instead of ToSqlOption. It could contain the 
implicit cast related information and could be also used as a string builder. 
The current implementation uses string builders in most nodes, but I think that 
toSql() can still be O(n^2) in some cases.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:22:41 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 10: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:21:44 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 8:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1113/ : 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/11721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:21:27 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..

IMPALA-7721: Fix broken /catalog_object web API when getting a privilege

Before this patch, /catalog_object web API was broken when getting a
privilege due to an incorrect way of getting a role ID. IMPALA-7616
broke this even more due to a lack of test coverage in /catalog_object
when authorization is enabled. This patch fixes the issue and makes the
/catalog_object web API usable again for getting a privilege.

Testing:
- Added a new BE test
- Added a new E2E test
- Ran all E2E authorization tests

Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
---
M be/src/catalog/catalog-util-test.cc
M be/src/catalog/catalog-util.cc
M be/src/catalog/catalog-util.h
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
M tests/authorization/test_authorization.py
6 files changed, 260 insertions(+), 13 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11721/8/be/src/catalog/catalog-util.cc
File be/src/catalog/catalog-util.cc:

http://gerrit.cloudera.org:8080/#/c/11721/8/be/src/catalog/catalog-util.cc@227
PS8, Line 227:
> lets use StringParser::StringToInt here (util/string-parser.h)
Ah I didn't know about that function. Done.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:20:03 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5050: Add support to read TIMESTAMP MILLIS and TIMESTAMP MICROS from Parquet

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-5050: Add support to read TIMESTAMP_MILLIS and 
TIMESTAMP_MICROS from Parquet
..


Patch Set 17:

Build Failed

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c7c01fffa31b3d2ca3480adf6ff851137dadac3
Gerrit-Change-Number: 11057
Gerrit-PatchSet: 17
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:03:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11721/8/be/src/catalog/catalog-util.cc
File be/src/catalog/catalog-util.cc:

http://gerrit.cloudera.org:8080/#/c/11721/8/be/src/catalog/catalog-util.cc@227
PS8, Line 227: stoi(principal_id))
lets use StringParser::StringToInt here (util/string-parser.h)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:02:48 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..

IMPALA-7721: Fix broken /catalog_object web API when getting a privilege

Before this patch, /catalog_object web API was broken when getting a
privilege due to an incorrect way of getting a role ID. IMPALA-7616
broke this even more due to a lack of test coverage in /catalog_object
when authorization is enabled. This patch fixes the issue and makes the
/catalog_object web API usable again for getting a privilege.

Testing:
- Added a new BE test
- Added a new E2E test
- Ran all E2E authorization tests

Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
---
M be/src/catalog/catalog-util-test.cc
M be/src/catalog/catalog-util.cc
M be/src/catalog/catalog-util.h
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
M tests/authorization/test_authorization.py
6 files changed, 257 insertions(+), 13 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 8:

Fixed clang-tidy.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:00:13 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5050: Add support to read TIMESTAMP MILLIS and TIMESTAMP MICROS from Parquet

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-5050: Add support to read TIMESTAMP_MILLIS and 
TIMESTAMP_MICROS from Parquet
..


Patch Set 18:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks// : 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/11057
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c7c01fffa31b3d2ca3480adf6ff851137dadac3
Gerrit-Change-Number: 11057
Gerrit-PatchSet: 18
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 19 Oct 2018 19:59:26 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 7:

Build Failed

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 19:48:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..

IMPALA-7721: Fix broken /catalog_object web API when getting a privilege

Before this patch, /catalog_object web API was broken when getting a
privilege due to an incorrect way of getting a role ID. IMPALA-7616
broke this even more due to a lack of test coverage in /catalog_object
when authorization is enabled. This patch fixes the issue and makes the
/catalog_object web API usable again for getting a privilege.

Testing:
- Added a new BE test
- Added a new E2E test
- Ran all E2E authorization tests

Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
---
M be/src/catalog/catalog-util-test.cc
M be/src/catalog/catalog-util.cc
M be/src/catalog/catalog-util.h
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
M tests/authorization/test_authorization.py
6 files changed, 257 insertions(+), 13 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 7:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util-test.cc
File be/src/catalog/catalog-util-test.cc:

http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util-test.cc@122
PS6, Line 122:   TPrivilege privilege;
> add more edge cases for these negative tests. for example, empty string, un
Done. More tests added. The string comes from: 
https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java#L56-L127


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

http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@210
PS6, Line 210: principal
> principal
Done


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@210
PS6, Line 210: principal
> principal
Done


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@226
PS6, Line 226:
> what does this do for a malformed id?
atoi usally returns 0 (not guaranteed) for a malformed ID, which is pretty bad: 
https://stackoverflow.com/questions/8871711/atoi-how-to-identify-the-difference-between-zero-and-error.
 I changed it to use std::stoi instead.


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@227
PS6, Line 227: 
catalog_object->privilege.__set_principal_id(stoi(principal_id));
> guaranteed to be uppercase?
Yeah we control this: 
https://gerrit.cloudera.org/c/11721/6/fe/src/main/java/org/apache/impala/catalog/Catalog.java#570


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@248
PS6, Line 248: e;
> dcheck not null for this. its a publicly exposed method (for test only?)
It's for exposed for test only. DCHECK added.


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@250
PS6, Line 250:   }
> add a comment about the expected format and an example.
Done


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@253
PS6, Line 253:
 : Status TPrivilegeFro
> check the len... valid?
Oops. Forgot to add the check. Done.


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@254
PS6, Line 254: ectName(
> all guaranteed to be lowercase?
Yeah, we control this: 
https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java#L56-L127



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 19:13:07 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 6:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util-test.cc
File be/src/catalog/catalog-util-test.cc:

http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util-test.cc@122
PS6, Line 122:   TPrivilege privilege;
add more edge cases for these negative tests. for example, empty string, 
unexpected lengths, mixed case. as it stands, I don't trust that string 
(where's it come from?)


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

http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@210
PS6, Line 210: privilege
principal


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@210
PS6, Line 210: privilege
principal


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@226
PS6, Line 226: atoi(principal_id.c_str())
what does this do for a malformed id?


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@227
PS6, Line 227:   if (principal_type == "ROLE") {
guaranteed to be uppercase?


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@248
PS6, Line 248: TPrivilege* privilege
dcheck not null for this. its a publicly exposed method (for test only?)


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@250
PS6, Line 250:   boost::algorithm::split_regex(split, object_name, 
boost::regex("->"));
add a comment about the expected format and an example.


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@253
PS6, Line 253: key_value, s, [](char c){ return c == '='; });
 : if (key_value[0]
check the len... valid?


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@254
PS6, Line 254: "server"
all guaranteed to be lowercase?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 18:38:25 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an 
upper case role name
..


Patch Set 8:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1109/ : 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/11734
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 18:36:59 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6661 Make NaN values equal for grouping purposes.

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

Change subject: IMPALA-6661 Make NaN values equal for grouping purposes.
..


Patch Set 16:

(3 comments)

Hey sorry for not getting back earlier. Bogged down again with some users' 
issues which I have to help with.

http://gerrit.cloudera.org:8080/#/c/11535/16/testdata/workloads/functional-query/queries/QueryTest/aggregation.test
File testdata/workloads/functional-query/queries/QueryTest/aggregation.test:

http://gerrit.cloudera.org:8080/#/c/11535/16/testdata/workloads/functional-query/queries/QueryTest/aggregation.test@1384
PS16, Line 1384: (VALUES((1.6 x, 2 y), (3.2, 4), (5.4,6))) T
Please see comments in joins.test


http://gerrit.cloudera.org:8080/#/c/11535/16/testdata/workloads/functional-query/queries/QueryTest/joins.test
File testdata/workloads/functional-query/queries/QueryTest/joins.test:

http://gerrit.cloudera.org:8080/#/c/11535/16/testdata/workloads/functional-query/queries/QueryTest/joins.test@801
PS16, Line 801: (VALUES((1.6 x, 0 y), (3.2, 1), (5.4,2), (0.5, 3), (0.5, 4), 
(-0.5, 5))) XX),
The problem with this kind of query with few number VALUES() is that codegen 
will be disabled as the planner knows the number of rows will be small. I think 
it may make sense to have another test cases to scan some sizable table. This 
is also a good test case to keep as this exercises the interpretation path.

Of course, one can also set the query option DISABLE_CODEGEN_ROWS_THRESHOLD to 
a small value but it seems better to have a more realistic test query with 
scans in there instead of joining two union nodes of constants.

You can check the query profile to see codegen is enabled in the HASH JOIN node.


http://gerrit.cloudera.org:8080/#/c/11535/16/testdata/workloads/functional-query/queries/QueryTest/joins.test@818
PS16, Line 818: with q as (VALUES((cast(1.0 as FLOAT) x), (2.0))),
  :  r as (select t1.x from q t1, q t2 where sqrt(1.0-t1.x) <=> 
sqrt(1.0-t2.x))
  :  select * from r
If you plan to keep this test case, this can be simplified as:
  with q as (VALUES((cast(1.0 as FLOAT) x), (2.0)))
  select t1.x from q t1, q t2 where sqrt(1.0-t1.x) <=> sqrt(1.0-t2.x)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I996c4a2e1934fd887046ed0c55457b7285375086
Gerrit-Change-Number: 11535
Gerrit-PatchSet: 16
Gerrit-Owner: Michal Ostrowski 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Michal Ostrowski 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 19 Oct 2018 18:18:15 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an 
upper case role name
..


Patch Set 9:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 18:01:23 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an 
upper case role name
..


Patch Set 9: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 18:01:22 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

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

Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an 
upper case role name
..


Patch Set 8: Code-Review+2

thx for the fix!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:59:43 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5050: Add support to read TIMESTAMP MILLIS and TIMESTAMP MICROS from Parquet

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-5050: Add support to read TIMESTAMP_MILLIS and 
TIMESTAMP_MICROS from Parquet
..


Patch Set 18:

Build Failed

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c7c01fffa31b3d2ca3480adf6ff851137dadac3
Gerrit-Change-Number: 11057
Gerrit-PatchSet: 18
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:54:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

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

Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an 
upper case role name
..


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11734/7/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11734/7/fe/src/main/java/org/apache/impala/util/SentryProxy.java@199
PS7, Line 199: // allRolesPrivileges keys and sentryRole.getName() are used 
here since they both
 : // come from Sentry so they agree in case.
 : refreshPrivilegesInCatalog(sentryRole.getRoleName(), 
role, allRolesPrivileges);
 :   }
 :   return rolesToRemove;
 : }
 :
> terser at the call-sites: allRolesPrivileges keys and sentryRole are used h
Done. Comment updated.


http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@388
PS4, Line 388: tadata won't hang due
> what was done?
I added a comment.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:45:29 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

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

Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an 
upper case role name
..

IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

Sentry stores the role names in lower case and Impala stores the role
names based on the original input role names. IMPALA-7343 introduced
a new bulk API (listAllRolesPrivileges) from Sentry that returns a map
of role name to a set of privileges. Since Impala preserves the case
sensitivity of the role names based on the original input role names,
this causes an issue when trying to retrieve a set of privileges from
a role name that is stored in Impala, especially when the role names in
Impala differ than the ones returned by listAllRolesPrivileges. This
issue will then result in privileges with mismatch role names to never
get refreshed in the Catalogd, which causes Impalad to wait indefinitely
waiting for the privileges to be updated by Catalogd. The fix is to get
a set of privileges using the role names returned by Sentry's
listAllRoles instead of using the role names stored in Impala.

Testing:
- Added a new E2E test
- Ran all E2E authorization tests

Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
---
M fe/src/main/java/org/apache/impala/catalog/Principal.java
M fe/src/main/java/org/apache/impala/catalog/Role.java
M fe/src/main/java/org/apache/impala/catalog/User.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M tests/authorization/test_grant_revoke.py
5 files changed, 61 insertions(+), 7 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an 
upper case role name
..


Patch Set 7:

Build Failed

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:44:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

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

Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an 
upper case role name
..


Patch Set 7:

looks fine.. just one more clarification for the test.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:27:57 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

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

Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an 
upper case role name
..


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11734/7/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11734/7/fe/src/main/java/org/apache/impala/util/SentryProxy.java@199
PS7, Line 199: // sentryRole.getRoleName() always returns the role name in 
lower case.
 : // However role.getName() preserves the case sensitivity 
of the role name.
 : // It is important to get the set of privileges from 
allRolesPrivileges using
 : // sentryRole.getRoleName() instead of role.getName(). 
If Sentry changes
 : // the role names to be upper case or case sensitive, we 
don't need to
 : // change this particular code since the case for 
allRolesPrivileges' keys
 : // will always match sentryRole.getRoleName().
terser at the call-sites: allRolesPrivileges keys and sentryRole are used here 
since they both come from Sentry so agree in case.


http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@388
PS4, Line 388: tadata won't hang due
> Done
what was done?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:27:14 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5050: Add support to read TIMESTAMP MILLIS and TIMESTAMP MICROS from Parquet

Hello Zoltan Borok-Nagy, Attila Jeges, Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-5050: Add support to read TIMESTAMP_MILLIS and 
TIMESTAMP_MICROS from Parquet
..

IMPALA-5050: Add support to read TIMESTAMP_MILLIS and TIMESTAMP_MICROS from 
Parquet

Changes:
- parquet.thrift is updated to a newer version which contains the
  timestamp logical type.
- INT64 columns with converted types TIMESTAMP_MILLIS and
  TIMESTAMP_MICROS can be read as TIMESTAMP.
- If the logical type is timestamp, then the type will contain the
  information whether the UTC->local conversion is necessary. This
  feature is only supported for the new timestamp types, so INT96
  timestamps must still use flag
  convert_legacy_hive_parquet_utc_timestamps.
- Min/max stat filtering is enabled again for columns that need
  UTC->local conversion. This was disabled in IMPALA-7559 because
  it could incorrectly drop column chunks.
- CREATE TABLE LIKE PARQUET converts these columns to
  TIMESTAMP - before the change, an error was returned instead.

Testing:
- Added unit tests for timezone conversion (this needed a new public
  function in timezone_db.h and adding CET to tzdb_tiny).
- Added parquet files (created with parquet-mr) with int64 timestamp
  columns.

Change-Id: I4c7c01fffa31b3d2ca3480adf6ff851137dadac3
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-parquet-scanner.h
M be/src/exec/parquet-column-readers.cc
M be/src/exec/parquet-column-readers.h
M be/src/exec/parquet-column-stats.cc
M be/src/exec/parquet-column-stats.h
M be/src/exec/parquet-column-stats.inline.h
M be/src/exec/parquet-common.cc
M be/src/exec/parquet-common.h
M be/src/exec/parquet-metadata-utils.cc
M be/src/exprs/timezone_db.h
M be/src/runtime/timestamp-test.cc
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/util/dict-encoding.h
M common/thrift/parquet.thrift
M fe/src/main/java/org/apache/impala/analysis/ParquetHelper.java
M testdata/data/README
A testdata/data/int64_timestamps_at_dst_changes.parquet
A testdata/data/int64_timestamps_dict.parquet
A testdata/data/int64_timestamps_plain.parquet
A testdata/tzdb_tiny/CET
A 
testdata/workloads/functional-query/queries/QueryTest/parquet-int64-timestamps.test
M tests/query_test/test_scanners.py
24 files changed, 892 insertions(+), 169 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c7c01fffa31b3d2ca3480adf6ff851137dadac3
Gerrit-Change-Number: 11057
Gerrit-PatchSet: 18
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy

[Impala-ASF-CR] IMPALA-5050: Add support to read TIMESTAMP MILLIS and TIMESTAMP MICROS from Parquet

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-5050: Add support to read TIMESTAMP_MILLIS and 
TIMESTAMP_MICROS from Parquet
..


Patch Set 17:

Build Failed

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c7c01fffa31b3d2ca3480adf6ff851137dadac3
Gerrit-Change-Number: 11057
Gerrit-PatchSet: 17
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:21:01 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

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

Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an 
upper case role name
..


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@172
PS4, Line 172: he key
> got it. keeping track of what's from sentry and what isn't and how that may
If for some reason, Sentry decides to make principal names to be case 
sensitive, I don't think we want to lower case the strings. Obviously when they 
do that, we also need to CatalogObjectCache to be case sensitive. Transforming 
the Sentry data structures into Impala data structures can be an expensive 
operations since we have to go through N privileges. I'll add more comments.


http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@388
PS4, Line 388: tadata won't hang due
> sorry, meant role. this is an e2e test so if could be done here? anyways, i
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:14:22 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

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

Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an 
upper case role name
..

IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

Sentry stores the role names in lower case and Impala stores the role
names based on the original input role names. IMPALA-7343 introduced
a new bulk API (listAllRolesPrivileges) from Sentry that returns a map
of role name to a set of privileges. Since Impala preserves the case
sensitivity of the role names based on the original input role names,
this causes an issue when trying to retrieve a set of privileges from
a role name that is stored in Impala, especially when the role names in
Impala differ than the ones returned by listAllRolesPrivileges. This
issue will then result in privileges with mismatch role names to never
get refreshed in the Catalogd, which causes Impalad to wait indefinitely
waiting for the privileges to be updated by Catalogd. The fix is to get
a set of privileges using the role names returned by Sentry's
listAllRoles instead of using the role names stored in Impala.

Testing:
- Added a new E2E test
- Ran all E2E authorization tests

Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
---
M fe/src/main/java/org/apache/impala/catalog/Principal.java
M fe/src/main/java/org/apache/impala/catalog/Role.java
M fe/src/main/java/org/apache/impala/catalog/User.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M tests/authorization/test_grant_revoke.py
5 files changed, 71 insertions(+), 7 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

2018-10-19 Thread Andrew Sherman (Code Review)

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

Change subject: IMPALA-5821: Add query with implicit casts to extended explain 
output.
..


Patch Set 2:

> It seems good to me, but I would ping Greg Rahn in the Jira, maybe
 > he has some ideas.

I have pinged him

 > I have one issue with the current output: as I saw in the .test
 > files, queries are printed in one line by default, which can make
 > complex queries very difficult to read. It would be much nicer to
 > break them at logical points, but I have no idea how to do it
 > easily.

I agree that a clever printing would be nice, but that sounds tricky. I could 
wrap at say the last space before 80 columns with only small effort, but this 
will make the header harder to parse in tests. Should I try this?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:04:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5050: Add support to read TIMESTAMP MILLIS and TIMESTAMP MICROS from Parquet

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

Change subject: IMPALA-5050: Add support to read TIMESTAMP_MILLIS and 
TIMESTAMP_MICROS from Parquet
..


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/11057/15//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11057/15//COMMIT_MSG@10
PS15, Line 10: - parquet.thrift is updated to a newer version which contains 
the timestamp
> nit: too long lines
Done


http://gerrit.cloudera.org:8080/#/c/11057/15/be/src/exec/hdfs-parquet-scanner.h
File be/src/exec/hdfs-parquet-scanner.h:

http://gerrit.cloudera.org:8080/#/c/11057/15/be/src/exec/hdfs-parquet-scanner.h@654
PS15, Line 654:
> Instead of having this function maybe it'd be better to extend ColumnStatsR
Done


http://gerrit.cloudera.org:8080/#/c/11057/16/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/11057/16/be/src/exec/hdfs-parquet-scanner.cc@518
PS16, Line 518: ip_row_group = true;
  :   break;
  : }
  :
  : if (stats_read) {
  :   TupleRow row;
  :   row.SetTuple(0, min_max_tuple_);
  :   i
> For readability, I'd prefer this logic to be hidden in ReadFromThrift().
I see what you mean, but I did not want add TimestampHelper to the simple 
ReadTimestampStatFromThrift() call. I have removed the duplicated code on 
MIN/MAX to make it more readable.


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

http://gerrit.cloudera.org:8080/#/c/11057/4/be/src/exec/parquet-column-readers.cc@1686
PS4, Line 1686:   return parent->obj_pool_.Add(reader);
> Oops, this is still not done yet. I will return to this issue. This affects
I have moved parent->obj_pool_.Add(); to the constructor of 
ParquetColumnReader, so now it always has an owner.


http://gerrit.cloudera.org:8080/#/c/11057/4/tests/query_test/test_scanners.py
File tests/query_test/test_scanners.py:

http://gerrit.cloudera.org:8080/#/c/11057/4/tests/query_test/test_scanners.py@759
PS4, Line 759: self.client.execute(("""CREATE TABLE {0}.int64_timestamps (
> It is now tested by using create_table_from_parquet.
I did some back and forth here:
I thought for some reason that CREATE TABLE LIKE PARQUET used to convert these 
columns to BIGINT, and changing this to TIMESTAMP would be breaking change, and 
I created IMPALA-7723 about this.
Then I realized that CREATE TABLE LIKE PARQUET returned an error for the 
columns, so converting to TIMESTAMP cannot be a breaking change.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c7c01fffa31b3d2ca3480adf6ff851137dadac3
Gerrit-Change-Number: 11057
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:05:03 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7698: Add centos support to bootstrap system.

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

Change subject: IMPALA-7698: Add centos support to bootstrap_system.
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11731/1//COMMIT_MSG@29
PS1, Line 29: I'm not yet clear on why the ORC library is choosing to open that 
time zone
: file (which doesn't exist), but this change seems workable even 
with the test
: failures.
> For this particular change, I'm planning on not doing anything with Orc. I
I see, if these tests do not have to pass, then it is better not to jump into 
this too deeply. I just wanted to dump my thoughts about this when a I saw the 
review.

Thanks for creating the follow up Jira!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id54294d7607f51de87a9de373dcfc4a33f4bedf5
Gerrit-Change-Number: 11731
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Fri, 19 Oct 2018 16:55:58 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7698: Add centos support to bootstrap system.

2018-10-19 Thread Philip Zeyliger (Code Review)

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

Change subject: IMPALA-7698: Add centos support to bootstrap_system.
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11731/1//COMMIT_MSG@29
PS1, Line 29: I'm not yet clear on why the ORC library is choosing to open that 
time zone
: file (which doesn't exist), but this change seems workable even 
with the test
: failures.
> About the ORC timezone issues:
For this particular change, I'm planning on not doing anything with Orc. I 
think it's useful to add centos6 support. If only data load goes through (which 
it does), we can catch a bunch of py2.6/py2.7 compatibility things during GVO, 
which is extremely handy.

I'm still puzzled about why physical machines that we use with CentOS 6 have 
different results. It might be as simple as non-determinism in file listing 
order or something. I think the next thing to do to figure it out might be 
either (a) debugger or (b) taking the files generated within the Docker context 
and copying them to the hardware context, or vice-versa, but I'm inclined to 
ignore that for now.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id54294d7607f51de87a9de373dcfc4a33f4bedf5
Gerrit-Change-Number: 11731
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Fri, 19 Oct 2018 16:43:58 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5050: Add support to read TIMESTAMP MILLIS and TIMESTAMP MICROS from Parquet

Hello Zoltan Borok-Nagy, Attila Jeges, Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-5050: Add support to read TIMESTAMP_MILLIS and 
TIMESTAMP_MICROS from Parquet
..

IMPALA-5050: Add support to read TIMESTAMP_MILLIS and TIMESTAMP_MICROS from 
Parquet

Changes:
- parquet.thrift is updated to a newer version which contains the
  timestamp logical type.
- INT64 columns with converted types TIMESTAMP_MILLIS and
  TIMESTAMP_MICROS can be read as TIMESTAMP.
- If the logical type is timestamp, then the type will contain the
  information whether the UTC->local conversion is necessary. This
  feature is only supported for the new timestamp types, so INT96
  timestamps must still use flag
  convert_legacy_hive_parquet_utc_timestamps.
- Min/max stat filtering is enabled again for columns that need
  UTC->local conversion. This was disabled in IMPALA-7559 because
  it could incorrectly drop column chunks.
- CREATE TABLE LIKE PARQUET converts these columns to
  TIMESTAMP - before the change, an error was returned instead.

Testing:
- Added unit tests for timezone conversion (this needed a new public
  function in timezone_db.h and adding CET to tzdb_tiny).
- Added parquet files (created with parquet-mr) with int64 timestamp
  columns.

Change-Id: I4c7c01fffa31b3d2ca3480adf6ff851137dadac3
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-parquet-scanner.h
M be/src/exec/parquet-column-readers.cc
M be/src/exec/parquet-column-readers.h
M be/src/exec/parquet-column-stats.cc
M be/src/exec/parquet-column-stats.h
M be/src/exec/parquet-column-stats.inline.h
M be/src/exec/parquet-common.cc
M be/src/exec/parquet-common.h
M be/src/exec/parquet-metadata-utils.cc
M be/src/exprs/timezone_db.h
M be/src/runtime/timestamp-test.cc
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/util/dict-encoding.h
M common/thrift/parquet.thrift
M fe/src/main/java/org/apache/impala/analysis/ParquetHelper.java
M testdata/data/README
A testdata/data/int64_timestamps_at_dst_changes.parquet
A testdata/data/int64_timestamps_dict.parquet
A testdata/data/int64_timestamps_plain.parquet
A testdata/tzdb_tiny/CET
A 
testdata/workloads/functional-query/queries/QueryTest/parquet-int64-timestamps.test
M tests/query_test/test_scanners.py
24 files changed, 891 insertions(+), 169 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c7c01fffa31b3d2ca3480adf6ff851137dadac3
Gerrit-Change-Number: 11057
Gerrit-PatchSet: 17
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy

[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

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

Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an
upper case role name
..

Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@172
PS4, Line 172: String
> This is lower case as of now. However, it's probably a good idea to not ass
got it. keeping track of what's from sentry and what isn't and how that may or
may not change over time is error prone. how about we just canonicalize
up-front here to lower-case strings? perhaps a cleaner option is just have
stuff that comes back from Sentry conform to data structures used in Impala. So
in this case, that Map could be a CatalogObjectCache. If you canonicalize the
map strings, then you'd need to lower-case in the look-up; if that cache is
used, then it would just fall-out.

either way, but please add a comment in each of the places I raised a question
since the assumptions are currently unclear and error-prone.

http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@386
PS4, Line 386: grp.getgrnam(
> It's to get a group name.
oh... its from the import.

http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@388
PS4, Line 388: "invalidate metadata"
> The issue is with role names having different cases and not with the privil
sorry, meant role. this is an e2e test so if could be done here? anyways, if
its not possible or extremely difficult, pls add a comment/perhaps a todo. this
seems a bit circuitous to debug a case issue.

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2
Gerrit-Change-Number: 11734
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Fri, 19 Oct 2018 16:32:28 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7698: Add centos support to bootstrap system.

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

Change subject: IMPALA-7698: Add centos support to bootstrap_system.
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/11731/1//COMMIT_MSG@29
PS1, Line 29: I'm not yet clear on why the ORC library is choosing to open that 
time zone
: file (which doesn't exist), but this change seems workable even 
with the test
: failures.
About the ORC timezone issues:
tldr: ORC's timezone handling is likely to be broken in Impala so we should 
patch it in the toolchain

The ORC library implements its own IANA timezone handling to convert stored 
timestamps from UTC to local time + do something similar for min/max stats. The 
writer's timezone can be also stored in .orc files and used instead of local 
timezone.

Impala's and ORC library's timezone can be different because of several reasons:
- ORC's timezone is not overridden by env var TZ and query option timezone
- ORC uses a simpler way to detect the local timezone which may not work on 
some Linux distros (see TimezoneDatabase::LocalZoneName in Impala vs 
LOCAL_TIMEZONE in Orc)
- .orc files can use any time zone as writer's timezone and we cannot be sure 
that it will exist on the reader machine

My suggestion is to patch the ORC library in the toolchain and remove timezone 
handling (e.g. by always using UTC, maybe depending on a flag), as the way it 
is currently working is likely to be broken and is surely not consistent with 
the rest of Impala.

I am not sure how timezones could be handled correctly in Orc + Impala. If 
someone plans to work on it, I would gladly help in the integration to Impala.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id54294d7607f51de87a9de373dcfc4a33f4bedf5
Gerrit-Change-Number: 11731
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Fri, 19 Oct 2018 16:03:08 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7710: test owner privileges with grant failed with AuthorizationException

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

Change subject: IMPALA-7710: test_owner_privileges_with_grant failed with 
AuthorizationException
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11723/1/tests/authorization/test_owner_privileges.py
File tests/authorization/test_owner_privileges.py:

http://gerrit.cloudera.org:8080/#/c/11723/1/tests/authorization/test_owner_privileges.py@229
PS1, Line 229: test_obj.obj_name), user="oo_user1", 
delay_s=sentry_refresh_timeout_s)
> I was not able to reproduce manually but reviewing the failed builds, this
Since this is a flaky test, let's spend some time figuring out the root case 
instead of fixing something that we may think is the fix, but end up having to 
fix it again.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If8bb2e1fba334ba26e6ae25fbb04c1a5785ac677
Gerrit-Change-Number: 11723
Gerrit-PatchSet: 1
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Fri, 19 Oct 2018 15:43:18 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name

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

Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an
upper case role name
..

Patch Set 4:

(8 comments)

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

http://gerrit.cloudera.org:8080/#/c/11734/4//COMMIT_MSG@10
PS4, Line 10: original input role names
> I saw CatalogObjectCache is where users/roles are stored from Authorization
That's correct. The CatalogObjectCache stores can store they key in a
case-sensitive way or case-insensitive way depending on the
caseInsensitiveKeys_ flag. See:
https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/CatalogObjectCache.java#L74.
However, we store the TPrincipal name
(https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/Principal.java#L47)
in a case-preserving way. I also checked tested the code against the commit
prior to the introduction of Principal and it looks like that's been the
behavior for quite some time.

http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@172
PS4, Line 172: String
> so this is lowercase or can it have casing that does not match the name tha
This is lower case as of now. However, it's probably a good idea to not assume
Sentry will always send this in a lower case.

http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@185
PS4, Line 185: existingRole
> ... and this contains a name that is not lower-cased?
Role.getName() will return the role name in a case-preserving way based on the
original input role name.

http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@186
PS4, Line 186: sentryRole.getRoleName()
> afaict, this will get lowercased when performing the lookup.
That's correct if we're talking about getting the role name form the catalog.

http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@198
PS4, Line 198: getRoleName
> ... and this is lowercased?
Yes, this is lower case as of now. Since both listAllRoles and
listAllRolesPriivleges come from Sentry, I would expect they will return the
role names in a case-consistent manner.

http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@249
PS4, Line 249: sentryPrincipalName
This is pretty much the problem.

http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@386
PS4, Line 386: grp.getgrnam(
> what's this?
It's to get a group name.

http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@388
PS4, Line 388: "invalidate metadata"
> I think its good to have this test, but is there a more direct way to test
The issue is with role names having different cases and not with the privilege
names. I can't think of an easy way of testing it without doing E2E test.

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

[Impala-ASF-CR] IMPALA-5050: Add support to read TIMESTAMP MILLIS and TIMESTAMP MICROS from Parquet

2018-10-19 Thread Zoltan Borok-Nagy (Code Review)

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

Change subject: IMPALA-5050: Add support to read TIMESTAMP_MILLIS and 
TIMESTAMP_MICROS from Parquet
..


Patch Set 16:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11057/16/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/11057/16/be/src/exec/hdfs-parquet-scanner.cc@518
PS16, Line 518: (col_type.IsTimestampType()) {
  : stats_read = 
ColumnStatsReader::ReadTimestampStatFromThrift(col_chunk, col_order,
  : *node->element, ColumnStatsReader::StatsField::MIN,
  : CreateTimestampHelper(*node->element), 
static_cast(slot));
  :   } else {
  : stats_read = ColumnStatsReader::ReadFromThrift(
  : col_chunk, col_type, col_order, 
ColumnStatsReader::StatsField::MIN, slot);
  :   }
For readability, I'd prefer this logic to be hidden in ReadFromThrift().



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c7c01fffa31b3d2ca3480adf6ff851137dadac3
Gerrit-Change-Number: 11057
Gerrit-PatchSet: 16
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 19 Oct 2018 13:43:52 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7717: Handle concurrent partition changes in local 
catalog mode
..


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 7
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 09:22:03 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

2018-10-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7717: Handle concurrent partition changes in local 
catalog mode
..

IMPALA-7717: Handle concurrent partition changes in local catalog mode

Current code throws a RuntimeException (RTE) when partial fetch RPCs
looking up partition metadata and the corresponding partition ID is
missing on the Catalog server. There are a couple of cases here.

1. The partition could be genuinely missing as it was dropped by a
   concurrent operation.
2. Partial fetch RPCs lookup partitions by IDs instead of names. This is
   problematic since the IDs can change over the lifetime of a table.

In both the cases, throwing a RTE is not the right approach and for (2)
we need to transparently retry the fetch with the new partition ID.

We eventually need to fix (2) as looking up by partition ID is not the
right approach.

Testing: Updated an e-e test which fails without the patch.

Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Reviewed-on: http://gerrit.cloudera.org:8080/11732
Reviewed-by: Bharath Vissapragada 
Tested-by: Impala Public Jenkins 
---
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
M tests/custom_cluster/test_local_catalog.py
5 files changed, 27 insertions(+), 58 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 8
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name