[Impala-ASF-CR] IMPALA-7212: Removes --use krpc flag and remove old DataStream services
Hello Sailesh Mukil, Joe McDonnell, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10835 to look at the new patch set (#4). Change subject: IMPALA-7212: Removes --use_krpc flag and remove old DataStream services .. IMPALA-7212: Removes --use_krpc flag and remove old DataStream services This change removes the flag --use_krpc which allows users to fall back to using Thrift based implementation of DataStream services. This flag was originally added during development of IMPALA-2567. It has served its purpose. As we port more ImpalaInternalServices to use KRPC, it's becoming increasingly burdensome to maintain parallel implementation of the RPC handlers. Therefore, going forward, KRPC is always enabled. This change removes the Thrift based implemenation of DataStreamServices and also simplifies some of the tests which were skipped when KRPC is disabled. Testing done: core debug build. Change-Id: Icfed200751508478a3d728a917448f2dabfc67c3 --- M be/src/common/global-flags.cc M be/src/exec/data-sink.cc M be/src/exec/exchange-node.cc M be/src/exec/exchange-node.h M be/src/rpc/authentication.cc M be/src/rpc/rpc-mgr-kerberized-test.cc M be/src/rpc/thrift-server-test.cc M be/src/runtime/CMakeLists.txt M be/src/runtime/backend-client.h D be/src/runtime/data-stream-mgr-base.h D be/src/runtime/data-stream-mgr.h D be/src/runtime/data-stream-recvr-base.h D be/src/runtime/data-stream-recvr.cc D be/src/runtime/data-stream-recvr.h D be/src/runtime/data-stream-sender.cc D be/src/runtime/data-stream-sender.h M be/src/runtime/data-stream-test.cc 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/krpc-data-stream-mgr.cc M be/src/runtime/krpc-data-stream-mgr.h M be/src/runtime/krpc-data-stream-recvr.cc M be/src/runtime/krpc-data-stream-recvr.h M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/scheduling/scheduler.cc M be/src/service/data-stream-service.cc 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 bin/run-all-tests.sh M bin/start-impala-cluster.py M common/thrift/ImpalaInternalService.thrift M tests/common/custom_cluster_test_suite.py M tests/common/skip.py D tests/common/test_skip.py M tests/conftest.py M tests/custom_cluster/test_krpc_mem_usage.py M tests/custom_cluster/test_krpc_metrics.py M tests/custom_cluster/test_rpc_exception.py M tests/query_test/test_codegen.py M tests/webserver/test_web_pages.py 44 files changed, 88 insertions(+), 2,143 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/10835/4 -- To view, visit http://gerrit.cloudera.org:8080/10835 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icfed200751508478a3d728a917448f2dabfc67c3 Gerrit-Change-Number: 10835 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-7212: Removes --use krpc flag and remove old DataStream services
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10835 ) Change subject: IMPALA-7212: Removes --use_krpc flag and remove old DataStream services .. Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/10835/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10835/3//COMMIT_MSG@7 PS3, Line 7: Deprecate > Nit: I think it would be clearer to say "Remove" rather than "Deprecate". Done http://gerrit.cloudera.org:8080/#/c/10835/3//COMMIT_MSG@9 PS3, Line 9: deprecates > Same here. Done http://gerrit.cloudera.org:8080/#/c/10835/3/be/src/common/global-flags.cc File be/src/common/global-flags.cc: http://gerrit.cloudera.org:8080/#/c/10835/3/be/src/common/global-flags.cc@40 PS3, Line 40: DEFINE_int32_hidden(krpc_port, 27000, : "port on which KRPC based ImpalaInternalService is exported"); > Now that we're exposing this, we should probably document this (if we're no Internal doc bug filed. http://gerrit.cloudera.org:8080/#/c/10835/3/be/src/common/global-flags.cc@260 PS3, Line 260: REMOVED_FLAG(disable_mem_pools); > What are our rules about removing startup flags? Should use_krpc be here? ( Done http://gerrit.cloudera.org:8080/#/c/10835/1/tests/custom_cluster/test_rpc_exception.py File tests/custom_cluster/test_rpc_exception.py: http://gerrit.cloudera.org:8080/#/c/10835/1/tests/custom_cluster/test_rpc_exception.py@a73 PS1, Line 73: > So that goes to say that we won't benefit much from running with this for R Yes -- To view, visit http://gerrit.cloudera.org:8080/10835 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icfed200751508478a3d728a917448f2dabfc67c3 Gerrit-Change-Number: 10835 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Wed, 18 Jul 2018 05:33:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ ONLY
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10974 ) Change subject: IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ_ONLY .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2831/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/10974 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1dd81100ae73fcabdbfaf679c20cea7dc102cd13 Gerrit-Change-Number: 10974 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 18 Jul 2018 04:59:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7314: Doc generation should fail on error
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10976 Change subject: IMPALA-7314: Doc generation should fail on error .. IMPALA-7314: Doc generation should fail on error This patch updates the doc generation to fail when there is an error. dita does not exit with non-zero exit code when there is an error. The patch checks for [ERROR] in the dita output and fails if it encounters one. Testing: - Manually tested by injecting failures Change-Id: Ic452aa282a3f2a761e3b04a7460e0d86bc51d721 --- M docs/.gitignore M docs/Makefile A docs/build-doc.sh 3 files changed, 39 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/10976/1 -- To view, visit http://gerrit.cloudera.org:8080/10976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ic452aa282a3f2a761e3b04a7460e0d86bc51d721 Gerrit-Change-Number: 10976 Gerrit-PatchSet: 1 Gerrit-Owner: Fredy Wijaya
[Impala-ASF-CR] IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10918 ) Change subject: IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10918 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icead43e689404a286859344e309427eb6c68646a Gerrit-Change-Number: 10918 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 18 Jul 2018 03:33:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10923 ) Change subject: IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f Gerrit-Change-Number: 10923 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 18 Jul 2018 03:14:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10923 ) Change subject: IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE .. IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE In DESCRIBE DATABASE, having VIEW_METADATA privilege allows seeing the metadata information on the target database. Similarly, other SQL show commands require VIEW_METADATA privilege on the target database/table. In the prior code, DESCRIBE requires SELECT privilege on the target table and is inconsistent with the rest of other SQL metadata commands. The patch fixes the inconsistency by requiring DESCRIBE to use VIEW_METADATA privilege. Testing: - Updated authorization tests - Ran all FE tests Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f Reviewed-on: http://gerrit.cloudera.org:8080/10923 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java 2 files changed, 26 insertions(+), 34 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/10923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f Gerrit-Change-Number: 10923 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] [DOCS] Fix UPDATE/UPSERT/DELETE authorization doc
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10975 Change subject: [DOCS] Fix UPDATE/UPSERT/DELETE authorization doc .. [DOCS] Fix UPDATE/UPSERT/DELETE authorization doc The patch also fixes broken links in the authorization doc. Change-Id: I9bf6109636e44ca514cfe74fb565f7c506ec0708 --- M docs/shared/impala_common.xml M docs/topics/impala_authorization.xml 2 files changed, 8 insertions(+), 249 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/10975/1 -- To view, visit http://gerrit.cloudera.org:8080/10975 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I9bf6109636e44ca514cfe74fb565f7c506ec0708 Gerrit-Change-Number: 10975 Gerrit-PatchSet: 1 Gerrit-Owner: Fredy Wijaya
[Impala-ASF-CR] IMPALA-7014: Disable stacktrace symbolisation by default
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10964 ) Change subject: IMPALA-7014: Disable stacktrace symbolisation by default .. IMPALA-7014: Disable stacktrace symbolisation by default Stacktrace symbolization has been shown to be 2500x slower compared to just printing the un-symbolized one. This has burned us a few times now, so let's disable it by default. Change-Id: If3af209890ccc242beb742145c63eb6836d4bfbb Reviewed-on: http://gerrit.cloudera.org:8080/10964 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/common/init.cc 1 file changed, 2 insertions(+), 0 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/10964 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: If3af209890ccc242beb742145c63eb6836d4bfbb Gerrit-Change-Number: 10964 Gerrit-PatchSet: 3 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7014: Disable stacktrace symbolisation by default
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10964 ) Change subject: IMPALA-7014: Disable stacktrace symbolisation by default .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10964 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If3af209890ccc242beb742145c63eb6836d4bfbb Gerrit-Change-Number: 10964 Gerrit-PatchSet: 2 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 18 Jul 2018 02:43:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
Hello Tianyi Wang, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10914 to look at the new patch set (#3). Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog .. IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog This adds support for INSERT and LOAD DATA statements when LocalCatalog is enabled by fixing the following items: * Remove some downcasts to HdfsTable in various Stmt classes and replace them with casts to FeFsTable. * Stub out the "write access" checks to fake always being writable. Left a TODO to properly implement these. * Implemented various 'getPartition()' calls which take a user-provided partition specification, used by INSERT and LOAD statements that specify an explicit target partition. * Fixed the "prototype partition" created by LocalFsTable to not include a location field. This fixed insertion into dynamic partitions. Additionally fixed a couple other issues which were exercised by the e2e test coverage for load/insert: * The LocalCatalog getDb() and getTable() calls were previously assuming that all callers passed pre-canonicalized table names, but that wasn't the case. This adds the necessary toLower() calls so that statements referencing capitalized table names work. With this patch, most of the associated e2e tests pass, with the exception of those that check permissions levels of the target directories. Those calls are still stubbed out. Overall, running 'run-tests.py -k "not hbase and not avro"' results in about a 90% pass rate after this patch: = 186 failed, 1657 passed, 56 skipped, 33 xfailed, 1 xpassed in 512.75 seconds = Remaining test failures seem mostly unrelated to the above changes and will be addressed in continuing patches. Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3 --- M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetCachedStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java M fe/src/main/java/org/apache/impala/analysis/TableRef.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java 19 files changed, 149 insertions(+), 69 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/10914/3 -- To view, visit http://gerrit.cloudera.org:8080/10914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3 Gerrit-Change-Number: 10914 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7140 (part 9): add support for SHOW FILES in LocalFsTable
Hello Tianyi Wang, Vuk Ercegovac, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/10973 to review the following change. Change subject: IMPALA-7140 (part 9): add support for SHOW FILES in LocalFsTable .. IMPALA-7140 (part 9): add support for SHOW FILES in LocalFsTable This adds support for the SHOW FILES IN ... command for LocalFsTable instances. Tested manually against functional.alltypesagg and also covered by existing e2e tests. Change-Id: I143bea9f72b6a25ae2545663e06f4849b58533ba --- M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M fe/src/main/java/org/apache/impala/service/Frontend.java 4 files changed, 26 insertions(+), 28 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/10973/1 -- To view, visit http://gerrit.cloudera.org:8080/10973 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I143bea9f72b6a25ae2545663e06f4849b58533ba Gerrit-Change-Number: 10973 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7307 (part 2). Support TABLESAMPLE in LocalCatalog
Hello Tianyi Wang, Vuk Ercegovac, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/10972 to review the following change. Change subject: IMPALA-7307 (part 2). Support TABLESAMPLE in LocalCatalog .. IMPALA-7307 (part 2). Support TABLESAMPLE in LocalCatalog Tested with 'run-tests.py -k tablesample' and the tests passed. Change-Id: I2f7baf05f16c6389ed900e0459708005ab44491e --- M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java 6 files changed, 109 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/10972/1 -- To view, visit http://gerrit.cloudera.org:8080/10972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I2f7baf05f16c6389ed900e0459708005ab44491e Gerrit-Change-Number: 10972 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7307 (part 1). Support stats extrapolation in LocalCatalog
Hello Tianyi Wang, Vuk Ercegovac, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/10971 to review the following change. Change subject: IMPALA-7307 (part 1). Support stats extrapolation in LocalCatalog .. IMPALA-7307 (part 1). Support stats extrapolation in LocalCatalog Tested by running 'run-tests.py -k stats_extrap' and the tests passed. Change-Id: I479b7f517091dd558768601e1e0704a1902b78a5 --- M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/test/java/org/apache/impala/planner/StatsExtrapolationTest.java 7 files changed, 55 insertions(+), 69 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/10971/1 -- To view, visit http://gerrit.cloudera.org:8080/10971 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I479b7f517091dd558768601e1e0704a1902b78a5 Gerrit-Change-Number: 10971 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7276. Support CREATE TABLE AS SELECT with LocalCatalog
Hello Tianyi Wang, Csaba Ringhofer, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10913 to look at the new patch set (#3). Change subject: IMPALA-7276. Support CREATE TABLE AS SELECT with LocalCatalog .. IMPALA-7276. Support CREATE TABLE AS SELECT with LocalCatalog This fixed most of the remaining Kudu tests which relied on CTAS. Now only a few Kudu tests fail: FAIL query_test/test_kudu.py::TestKuduOperations::()::test_kudu_col_changed FAIL query_test/test_kudu.py::TestKuduOperations::()::test_kudu_col_null_changed FAIL query_test/test_kudu.py::TestKuduOperations::()::test_kudu_col_not_null_changed FAIL query_test/test_kudu.py::TestKuduOperations::()::test_kudu_col_added The above 4 fail because they are asserting something about the caching behavior of the old catalog implementation. FAIL query_test/test_kudu.py::TestImpalaKuduIntegration::()::test_delete_external_kudu_table FAIL query_test/test_kudu.py::TestImpalaKuduIntegration::()::test_delete_managed_kudu_table These fail due to attempting to load non-existent tables referred to by a DELETE statement. Need to investigate these further, but not related to CTAS. Change-Id: I93937aed9b76ef6a62b1c588c59c34d3d6831a46 --- M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/catalog/Db.java M fe/src/main/java/org/apache/impala/catalog/FeDb.java M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalKuduTable.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/TableSink.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java 10 files changed, 134 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/10913/3 -- To view, visit http://gerrit.cloudera.org:8080/10913 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I93937aed9b76ef6a62b1c588c59c34d3d6831a46 Gerrit-Change-Number: 10913 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ ONLY
Hello Tianyi Wang, Vuk Ercegovac, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/10974 to review the following change. Change subject: IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ_ONLY .. IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ_ONLY This changes the permissions-checking of INSERT so that, if a partition is specified, we only verify writability of the specific explicit partition. This allows insertion into a table even if it contains one or more read-only partitions. This matches the existing behavior of LOAD DATA. A regression test is added which failed prior to the fix. Change-Id: I1dd81100ae73fcabdbfaf679c20cea7dc102cd13 --- M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M tests/metadata/test_hdfs_permissions.py M tests/query_test/test_insert_behaviour.py 8 files changed, 175 insertions(+), 77 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/10974/1 -- To view, visit http://gerrit.cloudera.org:8080/10974 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1dd81100ae73fcabdbfaf679c20cea7dc102cd13 Gerrit-Change-Number: 10974 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 ) Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java File fe/src/main/java/org/apache/impala/catalog/FeFsTable.java: http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@111 PS2, Line 111: first > how is first defined (e.g., what determines the ordering of locations)? Done http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java File fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java: http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java@157 PS2, Line 157: String partitionNotFoundMsg = : > put this in a function so that this cost is paid only when there's an error Done http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java@160 PS2, Line 160: an Hdfs > nit: an fs Done -- To view, visit http://gerrit.cloudera.org:8080/10914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3 Gerrit-Change-Number: 10914 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 18 Jul 2018 01:10:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7276. Support CREATE TABLE AS SELECT with LocalCatalog
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/10913 )
Change subject: IMPALA-7276. Support CREATE TABLE AS SELECT with LocalCatalog
..
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/10913/2/fe/src/main/java/org/apache/impala/catalog/Db.java
File fe/src/main/java/org/apache/impala/catalog/Db.java:
http://gerrit.cloudera.org:8080/#/c/10913/2/fe/src/main/java/org/apache/impala/catalog/Db.java@176
PS2, Line 176: public FeKuduTable
createKuduCtasTarget(org.apache.hadoop.hive.metastore.api.Table msTbl,
> nit: long line
Done
http://gerrit.cloudera.org:8080/#/c/10913/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java:
http://gerrit.cloudera.org:8080/#/c/10913/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@57
PS2, Line 57: import
org.apache.sentry.hdfs.service.thrift.SentryHDFSService.get_all_related_paths_result;
> sorry, Eclipse went nuts on autocomplete here apparently. Will remove in ne
Done
http://gerrit.cloudera.org:8080/#/c/10913/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@211
PS2, Line 211: public TTableDescriptor toThriftDescriptor(int tableId,
Set referencedPartitions) {
> nit: long line
Done
http://gerrit.cloudera.org:8080/#/c/10913/2/fe/src/main/java/org/apache/impala/catalog/local/LocalKuduTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalKuduTable.java:
http://gerrit.cloudera.org:8080/#/c/10913/2/fe/src/main/java/org/apache/impala/catalog/local/LocalKuduTable.java@83
PS2, Line 83: List partitionBy =
KuduTable.loadPartitionByParams(
> nit: this could be one line
Done
--
To view, visit http://gerrit.cloudera.org:8080/10913
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93937aed9b76ef6a62b1c588c59c34d3d6831a46
Gerrit-Change-Number: 10913
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Tianyi Wang
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Wed, 18 Jul 2018 00:59:42 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 ) Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog .. Patch Set 2: Code-Review+2 remaining comments are primarily nits/clarifications for comments. -- To view, visit http://gerrit.cloudera.org:8080/10914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3 Gerrit-Change-Number: 10914 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 18 Jul 2018 00:05:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10918 ) Change subject: IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2830/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10918 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icead43e689404a286859344e309427eb6c68646a Gerrit-Change-Number: 10918 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 18 Jul 2018 00:04:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10918 ) Change subject: IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10918 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icead43e689404a286859344e309427eb6c68646a Gerrit-Change-Number: 10918 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 18 Jul 2018 00:04:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10918 ) Change subject: IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis .. Patch Set 4: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/10918/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java: http://gerrit.cloudera.org:8080/#/c/10918/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1166 PS3, Line 1166: TPrivilegeLevel.ALTER, TPrivilegeLevel.SELECT > There are test cases that are not applicable to "drop stats" if we don't un that would be ALL + testPair.second but ok, not too worried about this except that there is far more repetition than real difference. -- To view, visit http://gerrit.cloudera.org:8080/10918 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icead43e689404a286859344e309427eb6c68646a Gerrit-Change-Number: 10918 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 18 Jul 2018 00:01:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7217: Incorrect UPDATE/DELETE authorization privilege
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10966 Change subject: IMPALA-7217: Incorrect UPDATE/DELETE authorization privilege .. IMPALA-7217: Incorrect UPDATE/DELETE authorization privilege UPDATE and DELETE statements require ALL privilege on the target table. In the prior code, UPDATE and DELETE statements use the default FROM clause which requires SELECT privilege on the target table. This causes an issue where if a user executes an UPDATE/DELETE statement with only a SELECT privilege on SERVER or DATABASE, an AnalysisException may be thrown instead of an AuthorizationException, which may reveal potentially sensitive information. This patch fixes the issue by requiring the FROM clause to also require ALL privilege on the target table to be consistent with the UPDATE/DELETE authorization privilege requirement. Testing: - Updated authorization tests - Ran all FE tests Change-Id: I69d451f727a7df6c41166a15cf1ed6f5334dc739 --- M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java M fe/src/main/java/org/apache/impala/analysis/TableRef.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java 6 files changed, 74 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/66/10966/3 -- To view, visit http://gerrit.cloudera.org:8080/10966 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I69d451f727a7df6c41166a15cf1ed6f5334dc739 Gerrit-Change-Number: 10966 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: memset NULL
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/10948 ) Change subject: IMPALA-5031: Fix undefined behavior: memset NULL .. Patch Set 3: > (1 comment) Another wrinkle here is that this doesn't actually provide a memory-safe memset. It prevents trying to write to nullptr when n > 0 but p = nullptr, but that's incidental, and that would segfault anyway, right? What it's really preventing is the undefined behavior, not the dangers we normally associate with memory unsafety like double frees, writing beyond bounds, slicing, and so on. -- To view, visit http://gerrit.cloudera.org:8080/10948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18fa02dc887a42a94c6f81e4923d17568f2184f2 Gerrit-Change-Number: 10948 Gerrit-PatchSet: 3 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 17 Jul 2018 23:57:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10923 ) Change subject: IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f Gerrit-Change-Number: 10923 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 17 Jul 2018 23:55:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10923 ) Change subject: IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2829/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f Gerrit-Change-Number: 10923 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 17 Jul 2018 23:55:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10923 ) Change subject: IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f Gerrit-Change-Number: 10923 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 17 Jul 2018 23:52:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE
Fredy Wijaya has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/10923 ) Change subject: IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE .. IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE In DESCRIBE DATABASE, having VIEW_METADATA privilege allows seeing the metadata information on the target database. Similarly, other SQL show commands require VIEW_METADATA privilege on the target database/table. In the prior code, DESCRIBE requires SELECT privilege on the target table and is inconsistent with the rest of other SQL metadata commands. The patch fixes the inconsistency by requiring DESCRIBE to use VIEW_METADATA privilege. Testing: - Updated authorization tests - Ran all FE tests Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f --- M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java 2 files changed, 26 insertions(+), 34 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/10923/3 -- To view, visit http://gerrit.cloudera.org:8080/10923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f Gerrit-Change-Number: 10923 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10923 ) Change subject: IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/10923/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java: http://gerrit.cloudera.org:8080/#/c/10923/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1065 PS2, Line 1065: checkStrings = > doesn't look like this is used any longer except for L1066 (next line). inl Done http://gerrit.cloudera.org:8080/#/c/10923/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1076 PS2, Line 1076: > why is this null now? perhaps clarify with a comment. Done -- To view, visit http://gerrit.cloudera.org:8080/10923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f Gerrit-Change-Number: 10923 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 17 Jul 2018 23:48:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6034: Add CPU and scanned bytes limits per query
Bikramjeet Vig has posted comments on this change. (
http://gerrit.cloudera.org:8080/10415 )
Change subject: IMPALA-6034: Add CPU and scanned bytes limits per query
..
Patch Set 11: Code-Review+1
(3 comments)
http://gerrit.cloudera.org:8080/#/c/10415/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10415/9//COMMIT_MSG@14
PS9, Line 14:
> nit:long line
can you include an example too?
http://gerrit.cloudera.org:8080/#/c/10415/9/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:
http://gerrit.cloudera.org:8080/#/c/10415/9/be/src/runtime/coordinator-backend-state.cc@600
PS9, Line 600: avg_profile_->AddInfoString("num instances",
lexical_cast(num_instances_));
: }
> maybe add other members of resource_utilization_ as well here
can you address this too?
http://gerrit.cloudera.org:8080/#/c/10415/9/be/src/service/query-options-test.cc
File be/src/service/query-options-test.cc:
http://gerrit.cloudera.org:8080/#/c/10415/9/be/src/service/query-options-test.cc@146
PS9, Line 146: {-1, I64_MAX}},
> I just ran it through clang-format :).
i agree, looks more consistent now
--
To view, visit http://gerrit.cloudera.org:8080/10415
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20
Gerrit-Change-Number: 10415
Gerrit-PatchSet: 11
Gerrit-Owner: Mostafa Mokhtar
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Mostafa Mokhtar
Gerrit-Reviewer: Sailesh Mukil
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Tue, 17 Jul 2018 23:39:04 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/10918 )
Change subject: IMPALA-6881: COMPUTE STATS should require SELECT privilege at
analysis
..
Patch Set 4:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10918/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10918/3//COMMIT_MSG@10
PS3, Line 10: check fo
> the word "register" stuck out. seems like an implementation detail. would i
Makes sense. Done.
http://gerrit.cloudera.org:8080/#/c/10918/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/10918/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2389
PS3, Line 2389:
> add a comment clarifying whether all or some privilege is required when mul
Done
http://gerrit.cloudera.org:8080/#/c/10918/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2393
PS3, Line 2393: Preconditions.checkNotNull(privilege);
> nit: space before ":" (for consistency with this file)
Done
http://gerrit.cloudera.org:8080/#/c/10918/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2408
PS3, Line 2408: TCatalogObjectType objectType = TCatalogObjectType.TABLE;
> nit: space before ":"
Done
http://gerrit.cloudera.org:8080/#/c/10918/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:
http://gerrit.cloudera.org:8080/#/c/10918/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1166
PS3, Line 1166: TPrivilegeLevel.ALTER, TPrivilegeLevel.SELECT
> to avoid unrolling the loop over compute/drop, would it be straightforward
There are test cases that are not applicable to "drop stats" if we don't unroll
the loop. For example:
.error(alterError("functional.alltypes"),
onServer(allExcept(TPrivilegeLevel.ALL, TPrivilegeLevel.ALTER,
TPrivilegeLevel.SELECT))
--
To view, visit http://gerrit.cloudera.org:8080/10918
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icead43e689404a286859344e309427eb6c68646a
Gerrit-Change-Number: 10918
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Adam Holley
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Tue, 17 Jul 2018 23:38:03 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis
Fredy Wijaya has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/10918 ) Change subject: IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis .. IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis In order to do COMPUTE STATS, Impala performs several SELECT queries. However, in the COMPUTE STATS analysis phase, we only check for the ALTER privilege. Although the SELECT privilege is eventually checked in the target table by the SELECT child queries, it is better to check the SELECT privilege in the COMPUTE STATS analysis phase and fail early if the privilege requirements are not met instead of failing later in the SELECT child queries due to insufficient privileges. Testing: - Updated the authorization tests - Ran all FE tests Change-Id: Icead43e689404a286859344e309427eb6c68646a --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java 7 files changed, 93 insertions(+), 51 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/10918/4 -- To view, visit http://gerrit.cloudera.org:8080/10918 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icead43e689404a286859344e309427eb6c68646a Gerrit-Change-Number: 10918 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10923 ) Change subject: IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/10923/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java: http://gerrit.cloudera.org:8080/#/c/10923/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1065 PS2, Line 1065: locationString doesn't look like this is used any longer except for L1066 (next line). inline? http://gerrit.cloudera.org:8080/#/c/10923/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1076 PS2, Line 1076: null why is this null now? perhaps clarify with a comment. -- To view, visit http://gerrit.cloudera.org:8080/10923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f Gerrit-Change-Number: 10923 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 17 Jul 2018 23:36:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7014: Disable stacktrace symbolisation by default
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10964 ) Change subject: IMPALA-7014: Disable stacktrace symbolisation by default .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10964 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If3af209890ccc242beb742145c63eb6836d4bfbb Gerrit-Change-Number: 10964 Gerrit-PatchSet: 2 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 17 Jul 2018 23:29:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7014: Disable stacktrace symbolisation by default
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10964 ) Change subject: IMPALA-7014: Disable stacktrace symbolisation by default .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2828/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10964 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If3af209890ccc242beb742145c63eb6836d4bfbb Gerrit-Change-Number: 10964 Gerrit-PatchSet: 2 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 17 Jul 2018 23:30:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7294. TABLESAMPLE should not allocate array based on total table file count
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10936 ) Change subject: IMPALA-7294. TABLESAMPLE should not allocate array based on total table file count .. IMPALA-7294. TABLESAMPLE should not allocate array based on total table file count This changes HdfsTable.getFilesSample() to allocate its intermediate sampling array based on the number of files in the selected (post-pruning) partitions, rather than the total number of files in the table. While the former behavior was correct (the total file count is of course an upper bound on the pruned file count), it was an unnecessarily large allocation, which has some downsides around garbage collection. In addition, this is important for the LocalCatalog implementation of table sampling, since we do not want to have to load all partition file lists in order to compute a sample over a pruned subset of partitions. The original code indicated that this was an optimization to avoid looping over the partition list an extra time. However, typical partition lists are relatively small even in the worst case (order of 100k) and looping over 100k in-memory Java objects is not likely to be the bottleneck in planning any query. This is especially true considering that we loop over that same list later in the function anyway, so we probably aren't saving page faults or LLC cache misses either. In testing this change I noticed that the existing test for TABLESAMPLE didn't test TABLESAMPLE when applied in conjunction with a predicate. I added a new dimension to the test which employs a predicate which prunes some partitions to ensure that the code works in that case. I also added coverage of the "100%" sampling parameter as a sanity check that it returns the same results as a non-sampled query. Change-Id: I0248d89bcd9dd4ff8b4b85fef282c19e3fe9bdd5 Reviewed-on: http://gerrit.cloudera.org:8080/10936 Reviewed-by: Philip Zeyliger Reviewed-by: Vuk Ercegovac Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M tests/query_test/test_tablesample.py 2 files changed, 29 insertions(+), 13 deletions(-) Approvals: Philip Zeyliger: Looks good to me, approved Vuk Ercegovac: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10936 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I0248d89bcd9dd4ff8b4b85fef282c19e3fe9bdd5 Gerrit-Change-Number: 10936 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: memset NULL
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/10948 )
Change subject: IMPALA-5031: Fix undefined behavior: memset NULL
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10948/3/be/src/util/ubsan.h
File be/src/util/ubsan.h:
http://gerrit.cloudera.org:8080/#/c/10948/3/be/src/util/ubsan.h@26
PS3, Line 26: class Ubsan {
> I had intended for this class to include safe versions of other undefined o
I think in those cases I'd probably put each of the methods into wherever they
make sense, since these aren't workarounds for UBSAN so much as safe variants
of utility code. eg this one would be in memory.h, and overflow-safe math code
could be in safe_math.h (we have such a header in kudu/util fwiw). Bit-shift
behavior probably belongs in bit-util.h etc.
If others disagree with me though I'm not gonna be too much of a stickler.
--
To view, visit http://gerrit.cloudera.org:8080/10948
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I18fa02dc887a42a94c6f81e4923d17568f2184f2
Gerrit-Change-Number: 10948
Gerrit-PatchSet: 3
Gerrit-Owner: Jim Apple
Gerrit-Reviewer: Jim Apple
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Tue, 17 Jul 2018 22:25:19 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7294. TABLESAMPLE should not allocate array based on total table file count
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10936 ) Change subject: IMPALA-7294. TABLESAMPLE should not allocate array based on total table file count .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10936 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0248d89bcd9dd4ff8b4b85fef282c19e3fe9bdd5 Gerrit-Change-Number: 10936 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 17 Jul 2018 22:17:30 + Gerrit-HasComments: No
[Impala-ASF-CR] cleanup: extract RowBatchQueue into its own file
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10943 ) Change subject: cleanup: extract RowBatchQueue into its own file .. cleanup: extract RowBatchQueue into its own file While looking at IMPALA-7096, I noticed that RowBatchQueue was implemented in a strange place. Change-Id: I3577c1c6920b8cf858c8d49f8812ccc305d833f6 Reviewed-on: http://gerrit.cloudera.org:8080/10943 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/exec/blocking-join-node.h M be/src/exec/exec-node.cc M be/src/exec/exec-node.h M be/src/exec/hdfs-scan-node.cc M be/src/exec/kudu-scan-node.cc M be/src/exec/scan-node.cc M be/src/exec/scan-node.h M be/src/exec/scanner-context.cc M be/src/runtime/CMakeLists.txt M be/src/runtime/data-stream-recvr.cc M be/src/runtime/krpc-data-stream-recvr.cc A be/src/runtime/row-batch-queue.cc A be/src/runtime/row-batch-queue.h 13 files changed, 129 insertions(+), 66 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/10943 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3577c1c6920b8cf858c8d49f8812ccc305d833f6 Gerrit-Change-Number: 10943 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] cleanup: extract RowBatchQueue into its own file
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10943 ) Change subject: cleanup: extract RowBatchQueue into its own file .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10943 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3577c1c6920b8cf858c8d49f8812ccc305d833f6 Gerrit-Change-Number: 10943 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 17 Jul 2018 22:06:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7014: Disable stacktrace symbolisation by default
Zoram Thanga has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10964 Change subject: IMPALA-7014: Disable stacktrace symbolisation by default .. IMPALA-7014: Disable stacktrace symbolisation by default Stacktrace symbolization has been shown to be 2500x slower compared to just printing the un-symbolized one. This has burned us a few times now, so let's disable it by default. Change-Id: If3af209890ccc242beb742145c63eb6836d4bfbb --- M be/src/common/init.cc 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/10964/1 -- To view, visit http://gerrit.cloudera.org:8080/10964 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: If3af209890ccc242beb742145c63eb6836d4bfbb Gerrit-Change-Number: 10964 Gerrit-PatchSet: 1 Gerrit-Owner: Zoram Thanga
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: memset NULL
Jim Apple has posted comments on this change. (
http://gerrit.cloudera.org:8080/10948 )
Change subject: IMPALA-5031: Fix undefined behavior: memset NULL
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10948/3/be/src/util/ubsan.h
File be/src/util/ubsan.h:
http://gerrit.cloudera.org:8080/#/c/10948/3/be/src/util/ubsan.h@26
PS3, Line 26: class Ubsan {
> bikeshedding opportunity here: when I saw the name 'ubsan.h' and the name o
I had intended for this class to include safe versions of other undefined
operations, not all of which are memory:
https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html
Thoughts?
--
To view, visit http://gerrit.cloudera.org:8080/10948
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I18fa02dc887a42a94c6f81e4923d17568f2184f2
Gerrit-Change-Number: 10948
Gerrit-PatchSet: 3
Gerrit-Owner: Jim Apple
Gerrit-Reviewer: Jim Apple
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Tue, 17 Jul 2018 21:48:50 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7186: [DOCS] Documented the KUDU READ MODE query option
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10897 ) Change subject: IMPALA-7186: [DOCS] Documented the KUDU_READ_MODE query option .. IMPALA-7186: [DOCS] Documented the KUDU_READ_MODE query option Change-Id: I49b4ec29ae8cdbee8b3d38bdf2e678b4e9560952 Reviewed-on: http://gerrit.cloudera.org:8080/10897 Reviewed-by: Alex Rodoni Tested-by: Impala Public Jenkins --- M docs/impala.ditamap M docs/impala_keydefs.ditamap A docs/topics/impala_kudu_read_mode.xml 3 files changed, 92 insertions(+), 0 deletions(-) Approvals: Alex Rodoni: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10897 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I49b4ec29ae8cdbee8b3d38bdf2e678b4e9560952 Gerrit-Change-Number: 10897 Gerrit-PatchSet: 5 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-7186: [DOCS] Documented the KUDU READ MODE query option
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10897 ) Change subject: IMPALA-7186: [DOCS] Documented the KUDU_READ_MODE query option .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10897 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I49b4ec29ae8cdbee8b3d38bdf2e678b4e9560952 Gerrit-Change-Number: 10897 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Tue, 17 Jul 2018 21:12:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7186: [DOCS] Documented the KUDU READ MODE query option
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10897 ) Change subject: IMPALA-7186: [DOCS] Documented the KUDU_READ_MODE query option .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-docs-submit/347/ -- To view, visit http://gerrit.cloudera.org:8080/10897 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I49b4ec29ae8cdbee8b3d38bdf2e678b4e9560952 Gerrit-Change-Number: 10897 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Tue, 17 Jul 2018 21:03:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7186: [DOCS] Documented the KUDU READ MODE query option
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/10897 ) Change subject: IMPALA-7186: [DOCS] Documented the KUDU_READ_MODE query option .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10897 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I49b4ec29ae8cdbee8b3d38bdf2e678b4e9560952 Gerrit-Change-Number: 10897 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Tue, 17 Jul 2018 21:03:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7186: [DOCS] Documented the KUDU READ MODE query option
Hello Thomas Marshall, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10897 to look at the new patch set (#4). Change subject: IMPALA-7186: [DOCS] Documented the KUDU_READ_MODE query option .. IMPALA-7186: [DOCS] Documented the KUDU_READ_MODE query option Change-Id: I49b4ec29ae8cdbee8b3d38bdf2e678b4e9560952 --- M docs/impala.ditamap M docs/impala_keydefs.ditamap A docs/topics/impala_kudu_read_mode.xml 3 files changed, 92 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/10897/4 -- To view, visit http://gerrit.cloudera.org:8080/10897 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I49b4ec29ae8cdbee8b3d38bdf2e678b4e9560952 Gerrit-Change-Number: 10897 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-7186: [DOCS] Documented the KUDU READ MODE query option
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/10897 ) Change subject: IMPALA-7186: [DOCS] Documented the KUDU_READ_MODE query option .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/10897/3/docs/topics/impala_kudu_read_mode.xml File docs/topics/impala_kudu_read_mode.xml: http://gerrit.cloudera.org:8080/#/c/10897/3/docs/topics/impala_kudu_read_mode.xml@72 PS3, Line 72: expecting > except Done http://gerrit.cloudera.org:8080/#/c/10897/3/docs/topics/impala_kudu_read_mode.xml@73 PS3, Line 73: . > typo Done -- To view, visit http://gerrit.cloudera.org:8080/10897 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I49b4ec29ae8cdbee8b3d38bdf2e678b4e9560952 Gerrit-Change-Number: 10897 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Tue, 17 Jul 2018 21:01:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/10908 ) Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements .. IMPALA-7209: Disallow self referencing in ALTER VIEW statements Previously, ALTER VIEW queries did not carry out reference checks in the analysis phase. This allowed the DDL operation to succeed but subsequent queries to the view threw StackOverflowError because the catalog was unable to resolve the reference. With this change, the AlterViewStmt checks for direct and in-direct self references before altering a view. Testing: Added tests to AnalyzeDDLTest to verify it. Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 --- M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java 3 files changed, 85 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/10908/6 -- To view, visit http://gerrit.cloudera.org:8080/10908 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 Gerrit-Change-Number: 10908 Gerrit-PatchSet: 6 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Pooja Nilangekar
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/10908 ) Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/10908/4/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java File fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java: http://gerrit.cloudera.org:8080/#/c/10908/4/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java@72 PS4, Line 72: ((SelectStmt) stmt).getTableRefs(); > This only gets you fromClause_'s TableRefs. For example something like Done. I have added test cases for where clause and with clause. Other classes like "GROUP BY", "ORDER BY" and "HAVING" don't support sub-queries. http://gerrit.cloudera.org:8080/#/c/10908/4/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java@75 PS4, Line 75: InlineViewRef fromViewRef = (InlineViewRef) fromTblRef; > While I see the intention here, a cleaner way for this is to do something l That was my initial approach. However, I faced the issue that while getting the TableRefs from the viewDefStmt_, the function resolves it to the underlying table definition. Since at this point, the view definition is still not altered. Yet, the SQL statement contains references to the view itself. So this approach ensures that we get all the view references that need to be resolved instead of getting just the eventual base table references. -- To view, visit http://gerrit.cloudera.org:8080/10908 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 Gerrit-Change-Number: 10908 Gerrit-PatchSet: 6 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Tue, 17 Jul 2018 20:56:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7186: [DOCS] Documented the KUDU READ MODE query option
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/10897 ) Change subject: IMPALA-7186: [DOCS] Documented the KUDU_READ_MODE query option .. Patch Set 3: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/10897/3/docs/topics/impala_kudu_read_mode.xml File docs/topics/impala_kudu_read_mode.xml: http://gerrit.cloudera.org:8080/#/c/10897/3/docs/topics/impala_kudu_read_mode.xml@72 PS3, Line 72: expecting except http://gerrit.cloudera.org:8080/#/c/10897/3/docs/topics/impala_kudu_read_mode.xml@73 PS3, Line 73: . typo -- To view, visit http://gerrit.cloudera.org:8080/10897 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I49b4ec29ae8cdbee8b3d38bdf2e678b4e9560952 Gerrit-Change-Number: 10897 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Tue, 17 Jul 2018 20:16:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7294. TABLESAMPLE should not allocate array based on total table file count
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10936 ) Change subject: IMPALA-7294. TABLESAMPLE should not allocate array based on total table file count .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10936 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0248d89bcd9dd4ff8b4b85fef282c19e3fe9bdd5 Gerrit-Change-Number: 10936 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 17 Jul 2018 19:03:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7294. TABLESAMPLE should not allocate array based on total table file count
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10936 ) Change subject: IMPALA-7294. TABLESAMPLE should not allocate array based on total table file count .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2827/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/10936 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0248d89bcd9dd4ff8b4b85fef282c19e3fe9bdd5 Gerrit-Change-Number: 10936 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 17 Jul 2018 18:54:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7294. TABLESAMPLE should not allocate array based on total table file count
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/10936 ) Change subject: IMPALA-7294. TABLESAMPLE should not allocate array based on total table file count .. Patch Set 1: Code-Review+2 Yep, this seems fine. Thanks for the detailed commit message. -- To view, visit http://gerrit.cloudera.org:8080/10936 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0248d89bcd9dd4ff8b4b85fef282c19e3fe9bdd5 Gerrit-Change-Number: 10936 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 17 Jul 2018 18:47:16 + Gerrit-HasComments: No
[Impala-ASF-CR] cleanup: extract RowBatchQueue into its own file
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10943 ) Change subject: cleanup: extract RowBatchQueue into its own file .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10943 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3577c1c6920b8cf858c8d49f8812ccc305d833f6 Gerrit-Change-Number: 10943 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 17 Jul 2018 18:47:26 + Gerrit-HasComments: No
[Impala-ASF-CR] cleanup: extract RowBatchQueue into its own file
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10943 ) Change subject: cleanup: extract RowBatchQueue into its own file .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2826/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10943 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3577c1c6920b8cf858c8d49f8812ccc305d833f6 Gerrit-Change-Number: 10943 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 17 Jul 2018 18:47:27 + Gerrit-HasComments: No
[Impala-ASF-CR] cleanup: extract RowBatchQueue into its own file
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10943 ) Change subject: cleanup: extract RowBatchQueue into its own file .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10943 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3577c1c6920b8cf858c8d49f8812ccc305d833f6 Gerrit-Change-Number: 10943 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 17 Jul 2018 18:46:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: memset NULL
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/10948 )
Change subject: IMPALA-5031: Fix undefined behavior: memset NULL
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10948/3/be/src/util/ubsan.h
File be/src/util/ubsan.h:
http://gerrit.cloudera.org:8080/#/c/10948/3/be/src/util/ubsan.h@26
PS3, Line 26: class Ubsan {
bikeshedding opportunity here: when I saw the name 'ubsan.h' and the name of
this class, I thought this would have various utility code to deal with
interfacing with ubsan itself (eg util/asan.h has macros for interfacing with
ASAN)
In fact this is more like a "safe memory utilities" class. Consider a different
name?
--
To view, visit http://gerrit.cloudera.org:8080/10948
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I18fa02dc887a42a94c6f81e4923d17568f2184f2
Gerrit-Change-Number: 10948
Gerrit-PatchSet: 3
Gerrit-Owner: Jim Apple
Gerrit-Reviewer: Jim Apple
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Tue, 17 Jul 2018 18:44:04 +
Gerrit-HasComments: Yes
[Impala-ASF-CR](2.x) IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10960 ) Change subject: IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved. .. Patch Set 1: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2825/ -- To view, visit http://gerrit.cloudera.org:8080/10960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I50aa2e6607de6a8943eb068b8162b0506763078b Gerrit-Change-Number: 10960 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 17 Jul 2018 18:36:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 ) Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java File fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java: http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java@113 PS2, Line 113: HdfsTable.getPartition(table_, partitionSpec_) > because the implementation of this method only uses public methods that are thx, makes sense. -- To view, visit http://gerrit.cloudera.org:8080/10914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3 Gerrit-Change-Number: 10914 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 17 Jul 2018 18:28:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 ) Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java File fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java: http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java@113 PS2, Line 113: HdfsTable.getPartition(table_, partitionSpec_) > lost track of why we're going with the static accessor vs the instance-leve because the implementation of this method only uses public methods that are also part of the interface. In Java 8 we could use "default methods" in the interface, but otherwise we'd have to copy-paste the code into both implementations. Even though Impala 3.x can depend on Java 8, I think we want to preserve the ability to backport this to 2.x branch in case some users are interested in the feature, so I was trying to avoid Java 8-isms. -- To view, visit http://gerrit.cloudera.org:8080/10914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3 Gerrit-Change-Number: 10914 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 17 Jul 2018 18:26:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10914 ) Change subject: IMPALA-7277. Support INSERT and LOAD DATA statements in LocalCatalog .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java File fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java: http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java@113 PS2, Line 113: HdfsTable.getPartition(table_, partitionSpec_) lost track of why we're going with the static accessor vs the instance-level accessor? http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java File fe/src/main/java/org/apache/impala/catalog/FeFsTable.java: http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@111 PS2, Line 111: first how is first defined (e.g., what determines the ordering of locations)? what's returned if all locations have write access? http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java File fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java: http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java@157 PS2, Line 157: String partitionNotFoundMsg = : put this in a function so that this cost is paid only when there's an error. http://gerrit.cloudera.org:8080/#/c/10914/2/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java@160 PS2, Line 160: an Hdfs nit: an fs -- To view, visit http://gerrit.cloudera.org:8080/10914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4ae47a0b58022ed77abe51d2596c2b1d0111fae3 Gerrit-Change-Number: 10914 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 17 Jul 2018 17:39:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Zoram Thanga has posted comments on this change. ( http://gerrit.cloudera.org:8080/10847 ) Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. Patch Set 5: Code-Review+1 LGTM -- To view, visit http://gerrit.cloudera.org:8080/10847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 5 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 17 Jul 2018 17:15:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: memset NULL
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/10948 ) Change subject: IMPALA-5031: Fix undefined behavior: memset NULL .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/10948/2/be/src/util/ubsan.h File be/src/util/ubsan.h: http://gerrit.cloudera.org:8080/#/c/10948/2/be/src/util/ubsan.h@29 PS2, Line 29: if (s == nullptr) return s; > n > 0 && s == nullptr sorry I picked the first one, because in a release build, the calling MemSet(NULL, 'q', 10) has incorrect (but defined) behavior. In the latter, it has undefined behavior. -- To view, visit http://gerrit.cloudera.org:8080/10948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18fa02dc887a42a94c6f81e4923d17568f2184f2 Gerrit-Change-Number: 10948 Gerrit-PatchSet: 2 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 17 Jul 2018 16:57:34 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: memset NULL
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10948 ) Change subject: IMPALA-5031: Fix undefined behavior: memset NULL .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/10948/2/be/src/util/ubsan.h File be/src/util/ubsan.h: http://gerrit.cloudera.org:8080/#/c/10948/2/be/src/util/ubsan.h@29 PS2, Line 29: if (s == nullptr) return s; > Which suggestion are you referring to? I don't think that problem applies t n > 0 && s == nullptr sorry -- To view, visit http://gerrit.cloudera.org:8080/10948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18fa02dc887a42a94c6f81e4923d17568f2184f2 Gerrit-Change-Number: 10948 Gerrit-PatchSet: 2 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 17 Jul 2018 16:55:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7199: Add scripts to create code coverage reports
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10791 ) Change subject: IMPALA-7199: Add scripts to create code coverage reports .. IMPALA-7199: Add scripts to create code coverage reports gcovr is a python library that uses gcov to generate code coverage reports. This adds gcovr to the python dependencies and adds bin/impala-gcovr to provide easy access to gcovr's command line. gcovr 3.4 supports python 2.6+. This also adds bin/coverage_helper.sh to provide a simplified interface to generate reports and zero coverage counters. Code coverage data is written out when a program exits, so it is important to avoid hard kills to shut down the impalads when generating coverage. This modifies testdata/bin/kill-all.sh to call start-impala-cluster.py --kill when shutting down the minicluster to try to avoid doing a hard kill. It will still do a hard kill if impala is still running after the softer kill. Change-Id: I5b2e0b794c64f9343ec976de7a3f235e54d2badd Reviewed-on: http://gerrit.cloudera.org:8080/10791 Reviewed-by: Joe McDonnell Tested-by: Impala Public Jenkins --- A bin/coverage_helper.sh A bin/impala-gcovr M infra/python/deps/requirements.txt M testdata/bin/kill-all.sh 4 files changed, 111 insertions(+), 0 deletions(-) Approvals: Joe McDonnell: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I5b2e0b794c64f9343ec976de7a3f235e54d2badd Gerrit-Change-Number: 10791 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7238: Use custom timeout for create unique database
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10862 ) Change subject: IMPALA-7238: Use custom timeout for create unique database .. IMPALA-7238: Use custom timeout for create unique database test_kudu.TestCreateExternalTables() saw a timeout when creating the unique database for its tests. __unique_conn() opens a connection, creates a unique database, then returns another connection in that database. It takes a custom timeout argument, but the timeout is only for the returned connection. The first connection to create the unique database uses the default timeout of 45 seconds. This patch changes the first connection to use the custom timeout. For Kudu tests, this is 5 minutes rather than 45 seconds. Change-Id: I4f2beb5bc027a4bb44e854bf1dd8919807a92ea0 Reviewed-on: http://gerrit.cloudera.org:8080/10862 Reviewed-by: Joe McDonnell Tested-by: Impala Public Jenkins --- M tests/conftest.py 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: Joe McDonnell: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10862 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I4f2beb5bc027a4bb44e854bf1dd8919807a92ea0 Gerrit-Change-Number: 10862 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Remove dead multiple filesystems member from THdfsTable
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10935 ) Change subject: Remove dead multiple_filesystems member from THdfsTable .. Remove dead multiple_filesystems member from THdfsTable This member was added long ago with a TODO to remove it. The member has not been in use for quite some time, so this patch removes the member as well as its assignment. Change-Id: I33197a52e05d0c9647f5ce64a77c59950d9a1b94 Reviewed-on: http://gerrit.cloudera.org:8080/10935 Reviewed-by: Vuk Ercegovac Tested-by: Impala Public Jenkins --- M common/thrift/CatalogObjects.thrift M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java 3 files changed, 1 insertion(+), 11 deletions(-) Approvals: Vuk Ercegovac: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10935 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I33197a52e05d0c9647f5ce64a77c59950d9a1b94 Gerrit-Change-Number: 10935 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: memset NULL
Jim Apple has posted comments on this change. (
http://gerrit.cloudera.org:8080/10948 )
Change subject: IMPALA-5031: Fix undefined behavior: memset NULL
..
Patch Set 2:
(1 comment)
> (1 comment)
>
> Yeah I generally disagree with the idea of adding NULL checks to
> every invocation of memset() - I think it makes the invariants of
> the code harder to understand and adds runtime overhead. In
> practice I think all of the callsites pass n == 0 when there's a
> null pointer and glibc memset() won't dereference the pointer in
> that case.
>
> There are more theoretical possibilities if the compiler decides to
> inline a custom memset implementation but I find it unlikely in
> practice that that would be compiled to anything strange since that
> code still has to handle the n == 0 case correctly by not
> dereferencing the pointer. You could have an implementation like
> below
>
> if (p == NULL) DoSomethingWild();
> if (n >= ...) {
> }
> if (n >= ...) {
> }
>
> But something like below makes way more sense.
>
> if (n >= ...) {
> }
> if (n >= ...) {
> }
https://blog.regehr.org/archives/1292
shows
https://goo.gl/h7K89h
which shows
int set(char *p, int c, size_t n) {
memset(p, c, n);
return p == 0;
}
which compiles to
set:
subq$8, %rsp
callmemset
xorl%eax, %eax
addq$8, %rsp
ret
which uses the fact that the first argument can't be NULL to optimize away the
comparison in the return statement.
http://gerrit.cloudera.org:8080/#/c/10948/2/be/src/util/ubsan.h
File be/src/util/ubsan.h:
http://gerrit.cloudera.org:8080/#/c/10948/2/be/src/util/ubsan.h@29
PS2, Line 29: if (s == nullptr) return s;
> We should DCHECK that n == 0 in this case since otherwise it's a bug.
If DCHECKs are no-ops in release mode, the NULL pointer check will be missing,
which allows the compiler to have demons fly out of my nose.
https://groups.google.com/d/msg/comp.std.c/ycpVKxTZkgw/S2hHdTbv4d8J
--
To view, visit http://gerrit.cloudera.org:8080/10948
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I18fa02dc887a42a94c6f81e4923d17568f2184f2
Gerrit-Change-Number: 10948
Gerrit-PatchSet: 2
Gerrit-Owner: Jim Apple
Gerrit-Reviewer: Jim Apple
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Tue, 17 Jul 2018 16:11:07 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: memset NULL
Hello Zoltan Borok-Nagy, Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10948 to look at the new patch set (#3). Change subject: IMPALA-5031: Fix undefined behavior: memset NULL .. IMPALA-5031: Fix undefined behavior: memset NULL memset has undefined behavior when its first argument is NULL. The instance fixed here was found by Clang's undefined behavior sanitizer. It was found in the end-to-end tests. The interesting part of the stack trace is: /exec/data-source-scan-node.cc:152:10: runtime error: null pointer passed as argument 1, which is declared to never be null /usr/include/string.h:62:79: note: nonnull attribute specified here #0 0x482fd8e in DataSourceScanNode::GetNextInputBatch() /exec/data-source-scan-node.cc:152:3 #1 0x482fb40 in DataSourceScanNode::Open(RuntimeState*) /exec/data-source-scan-node.cc:124:10 #2 0x47ef854 in AggregationNode::Open(RuntimeState*) /exec/aggregation-node.cc:71:49 #3 0x23506a4 in FragmentInstanceState::Open() /runtime/fragment-instance-state.cc:266:53 #4 0x234b6a8 in FragmentInstanceState::Exec() /runtime/fragment-instance-state.cc:81:12 #5 0x236ee52 in QueryState::ExecFInstance(FragmentInstanceState*) /runtime/query-state.cc:401:24 #6 0x237093e in QueryState::StartFInstances()::$_0::operator()() const /runtime/query-state.cc:341:44 Change-Id: I18fa02dc887a42a94c6f81e4923d17568f2184f2 --- M be/src/exec/data-source-scan-node.cc A be/src/util/ubsan.h 2 files changed, 39 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/10948/3 -- To view, visit http://gerrit.cloudera.org:8080/10948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I18fa02dc887a42a94c6f81e4923d17568f2184f2 Gerrit-Change-Number: 10948 Gerrit-PatchSet: 3 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: memset NULL
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/10948 )
Change subject: IMPALA-5031: Fix undefined behavior: memset NULL
..
Patch Set 2:
(1 comment)
Yeah I generally disagree with the idea of adding NULL checks to every
invocation of memset() - I think it makes the invariants of the code harder to
understand and adds runtime overhead. In practice I think all of the callsites
pass n == 0 when there's a null pointer and glibc memset() won't dereference
the pointer in that case.
There are more theoretical possibilities if the compiler decides to inline a
custom memset implementation but I find it unlikely in practice that that would
be compiled to anything strange since that code still has to handle the n == 0
case correctly by not dereferencing the pointer. You could have an
implementation like below
if (p == NULL) DoSomethingWild();
if (n >= ...) {
}
if (n >= ...) {
}
But something like below makes way more sense.
if (n >= ...) {
}
if (n >= ...) {
}
http://gerrit.cloudera.org:8080/#/c/10948/2/be/src/util/ubsan.h
File be/src/util/ubsan.h:
http://gerrit.cloudera.org:8080/#/c/10948/2/be/src/util/ubsan.h@29
PS2, Line 29: if (s == nullptr) return s;
We should DCHECK that n == 0 in this case since otherwise it's a bug.
Or actually, this check could be if (n == 0) and we could DCHECK != NULL - I
think that's closer to the intent.
--
To view, visit http://gerrit.cloudera.org:8080/10948
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I18fa02dc887a42a94c6f81e4923d17568f2184f2
Gerrit-Change-Number: 10948
Gerrit-PatchSet: 2
Gerrit-Owner: Jim Apple
Gerrit-Reviewer: Jim Apple
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Tue, 17 Jul 2018 15:56:27 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] [experimental] Clang Tidy Diff trial balloon
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/9751 ) Change subject: [experimental] Clang Tidy Diff trial balloon .. Patch Set 2: Now that the clang-tidy quiet patch is in, is this unblocked? -- To view, visit http://gerrit.cloudera.org:8080/9751 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2fb6a13400367fd3d12a4738bbb2dfc944466a7 Gerrit-Change-Number: 9751 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 17 Jul 2018 15:31:38 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10960 ) Change subject: IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved. .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2825/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I50aa2e6607de6a8943eb068b8162b0506763078b Gerrit-Change-Number: 10960 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 17 Jul 2018 14:50:59 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10960 ) Change subject: IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved. .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I50aa2e6607de6a8943eb068b8162b0506763078b Gerrit-Change-Number: 10960 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 17 Jul 2018 14:49:36 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved.
Hello Impala Public Jenkins, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/10960 to review the following change. Change subject: IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved. .. IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved. Impala master branch can already write the Parquet page index. However, we still don't have a well-defined ordering for floating-point numbers in Parquet, see PARQUET-1222 Currently impala writes the page index with fmax()/fmin() semantics, but it might contradicts the future semantics that will be defined once PARQUET-1222 is resolved. >From this patch Impala won't write the column index for floating-point columns until PARQUET-1222 is resolved and implemented. I updated the python test accordingly. Change-Id: I50aa2e6607de6a8943eb068b8162b0506763078b Reviewed-on: http://gerrit.cloudera.org:8080/10951 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins (cherry picked from commit 041197444d2a73bc3e3da4c6dbfdf1d63c236fbf) --- M be/src/exec/hdfs-parquet-table-writer.cc M tests/query_test/test_parquet_page_index.py 2 files changed, 11 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/10960/1 -- To view, visit http://gerrit.cloudera.org:8080/10960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: newchange Gerrit-Change-Id: I50aa2e6607de6a8943eb068b8162b0506763078b Gerrit-Change-Number: 10960 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: memset NULL
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10948 ) Change subject: IMPALA-5031: Fix undefined behavior: memset NULL .. Patch Set 2: Code-Review+1 Thank you for your answers, lgtm. -- To view, visit http://gerrit.cloudera.org:8080/10948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18fa02dc887a42a94c6f81e4923d17568f2184f2 Gerrit-Change-Number: 10948 Gerrit-PatchSet: 2 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 17 Jul 2018 13:19:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: memset NULL
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/10948 ) Change subject: IMPALA-5031: Fix undefined behavior: memset NULL .. Patch Set 2: > Do we want to update the other call sites of std::memset? Not yet. First, when I last submitted a patch with many memset fixes some months ago,it had trouble getting through review. Second, not every call needs to be fixed, since some don't call it with a NULL parameter and so do not induce undefined behavior. > IMPALA-5031 is quite general, while this patch set is very > specific. Is IMPALA-5031 some kind of umbrella Jira for all the > ubsan-related issues? Yes. -- To view, visit http://gerrit.cloudera.org:8080/10948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18fa02dc887a42a94c6f81e4923d17568f2184f2 Gerrit-Change-Number: 10948 Gerrit-PatchSet: 2 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 17 Jul 2018 13:02:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10951 ) Change subject: IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved. .. IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved. Impala master branch can already write the Parquet page index. However, we still don't have a well-defined ordering for floating-point numbers in Parquet, see PARQUET-1222 Currently impala writes the page index with fmax()/fmin() semantics, but it might contradicts the future semantics that will be defined once PARQUET-1222 is resolved. >From this patch Impala won't write the column index for floating-point columns until PARQUET-1222 is resolved and implemented. I updated the python test accordingly. Change-Id: I50aa2e6607de6a8943eb068b8162b0506763078b Reviewed-on: http://gerrit.cloudera.org:8080/10951 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/exec/hdfs-parquet-table-writer.cc M tests/query_test/test_parquet_page_index.py 2 files changed, 11 insertions(+), 0 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/10951 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I50aa2e6607de6a8943eb068b8162b0506763078b Gerrit-Change-Number: 10951 Gerrit-PatchSet: 3 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10951 ) Change subject: IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved. .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10951 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I50aa2e6607de6a8943eb068b8162b0506763078b Gerrit-Change-Number: 10951 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 17 Jul 2018 12:27:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: memset NULL
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10948 ) Change subject: IMPALA-5031: Fix undefined behavior: memset NULL .. Patch Set 2: Do we want to update the other call sites of std::memset? IMPALA-5031 is quite general, while this patch set is very specific. Is IMPALA-5031 some kind of umbrella Jira for all the ubsan-related issues? Or, this is the last issue related to undefined behavior, and after this ubsan builds will be error-free? -- To view, visit http://gerrit.cloudera.org:8080/10948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18fa02dc887a42a94c6f81e4923d17568f2184f2 Gerrit-Change-Number: 10948 Gerrit-PatchSet: 2 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 17 Jul 2018 10:10:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10951 ) Change subject: IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved. .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2824/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10951 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I50aa2e6607de6a8943eb068b8162b0506763078b Gerrit-Change-Number: 10951 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 17 Jul 2018 09:14:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10951 ) Change subject: IMPALA-7304: Don't write floating column index until PARQUET-1222 is resolved. .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10951 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I50aa2e6607de6a8943eb068b8162b0506763078b Gerrit-Change-Number: 10951 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 17 Jul 2018 09:14:10 + Gerrit-HasComments: No
[Impala-ASF-CR] Remove dead multiple filesystems member from THdfsTable
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10935 ) Change subject: Remove dead multiple_filesystems member from THdfsTable .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10935 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I33197a52e05d0c9647f5ce64a77c59950d9a1b94 Gerrit-Change-Number: 10935 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 17 Jul 2018 06:29:20 + Gerrit-HasComments: No
