[Impala-ASF-CR] IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext()
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11296 ) Change subject: IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext() .. Patch Set 3: (5 comments) looks good to me, just some nits http://gerrit.cloudera.org:8080/#/c/11296/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11296/3//COMMIT_MSG@10 PS3, Line 10: status_ nit: HdfsScanNode::status_ http://gerrit.cloudera.org:8080/#/c/11296/3//COMMIT_MSG@10 PS3, Line 10: It made : the fragment lose track of the error set by the scanner thread. nit: this resulted an error status being lost in case .happened. http://gerrit.cloudera.org:8080/#/c/11296/3//COMMIT_MSG@12 PS3, Line 12: This change ensures that GetNext returns the first non-ok status : set in the HdfsScanNode. Does this always ensure that GetNext returns the first non-ok status? is there a possibility that GetNextInternal() might not http://gerrit.cloudera.org:8080/#/c/11296/3/be/src/exec/hdfs-scan-node-base.h File be/src/exec/hdfs-scan-node-base.h: http://gerrit.cloudera.org:8080/#/c/11296/3/be/src/exec/hdfs-scan-node-base.h@544 PS3, Line 544: It returns Status::OK() if it successfully issues all scan ranges corresponding to : /// the scan node. nit: superflous comment. Can say: A non-OK status is returned only if.. http://gerrit.cloudera.org:8080/#/c/11296/3/be/src/exec/hdfs-scan-node-base.h@545 PS3, Line 545: A non-ok status is returned if ValidateScanRange() fails or the : /// reader_context_ is cancelled before all scan ranges are added. nit: avoid reference to private members, how about: "A non-ok status is returned only if it encounters an invalid scan range or if a scanner thread cancels the scan when it runs into an error." -- To view, visit http://gerrit.cloudera.org:8080/11296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4569cc7b0843a29c617a094e590c31f7c648ff45 Gerrit-Change-Number: 11296 Gerrit-PatchSet: 3 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 23 Aug 2018 03:01:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7399: Emit a junit xml report when trapping errors
David Knupp has posted comments on this change. (
http://gerrit.cloudera.org:8080/11257 )
Change subject: IMPALA-7399: Emit a junit xml report when trapping errors
..
Patch Set 10:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11257/9/lib/python/impala_py_lib/jenkins/generate_junitxml.py
File lib/python/impala_py_lib/jenkins/generate_junitxml.py:
http://gerrit.cloudera.org:8080/#/c/11257/9/lib/python/impala_py_lib/jenkins/generate_junitxml.py@164
PS9, Line 164: g_
> Not python2.6-compatible. Use {0}
Done.
I actually waffled on this a bit. I think that it makes more sense to return
the file name after all, and have the caller print (or not) as necessary. But I
fixed it there.
http://gerrit.cloudera.org:8080/#/c/11257/9/testdata/bin/check-schema-diff.sh
File testdata/bin/check-schema-diff.sh:
http://gerrit.cloudera.org:8080/#/c/11257/9/testdata/bin/check-schema-diff.sh@30
PS9, Line 30: t, we don't call setup_report_build_error.
> just say "we don't call setup_report_build_error".
Done
--
To view, visit http://gerrit.cloudera.org:8080/11257
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd62045bb43357abc2b89a78afff499149d3c3fc
Gerrit-Change-Number: 11257
Gerrit-PatchSet: 10
Gerrit-Owner: David Knupp
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Michael Brown
Gerrit-Reviewer: Nithya Janarthanan
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Thu, 23 Aug 2018 00:52:14 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7399: Emit a junit xml report when trapping errors
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/11257 ) Change subject: IMPALA-7399: Emit a junit xml report when trapping errors .. Patch Set 10: All outstanding comments have been addressed. -- To view, visit http://gerrit.cloudera.org:8080/11257 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idd62045bb43357abc2b89a78afff499149d3c3fc Gerrit-Change-Number: 11257 Gerrit-PatchSet: 10 Gerrit-Owner: David Knupp Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Nithya Janarthanan Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 23 Aug 2018 00:52:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7460 part 1: require user to install Paramiko and Fabric
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11264 ) Change subject: IMPALA-7460 part 1: require user to install Paramiko and Fabric .. IMPALA-7460 part 1: require user to install Paramiko and Fabric - Remove Fabric and Paramiko as requirements. They aren't needed by anything in buildall.sh. - Add a means to install into the impala-python virtual environment by hand. impala-pip is fine for this. - Add another requirements file for extended testing. The dependency situation is messy and untangling that out of impala-python and into lib/python should be out of the scope of IMPALA-7460. - Update core tests, which cover real regressions that have happened in the past, to run against locations that don't require a Paramiko import. This moves some logic out of concurrent_select.py into a thinner module. - Insulate ssh_util from globally-scoped import so that it only imports when needed. Testing: - This works in my development environment. - This works in my downstream stress and query gen environments. - This works when doing a full data load. - Impala still builds on a variety of OSs. Todo: - A subsequent review will update the versions. Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65 Reviewed-on: http://gerrit.cloudera.org:8080/11264 Reviewed-by: Michael Brown Tested-by: Impala Public Jenkins --- A bin/impala-pip M infra/python/deps/compiled-requirements.txt A infra/python/deps/extended-test-requirements.txt M tests/comparison/cluster.py M tests/comparison/leopard/impala_docker_env.py M tests/infra/test_stress_infra.py M tests/stress/concurrent_select.py M tests/util/cluster_controller.py M tests/util/parse_util.py M tests/util/ssh_util.py M tests/util/test_file_parser.py 11 files changed, 157 insertions(+), 82 deletions(-) Approvals: Michael Brown: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11264 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65 Gerrit-Change-Number: 11264 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7460 part 1: require user to install Paramiko and Fabric
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11264 ) Change subject: IMPALA-7460 part 1: require user to install Paramiko and Fabric .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11264 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65 Gerrit-Change-Number: 11264 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 23 Aug 2018 00:20:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7399: Emit a junit xml report when trapping errors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11257 ) Change subject: IMPALA-7399: Emit a junit xml report when trapping errors .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/460/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11257 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idd62045bb43357abc2b89a78afff499149d3c3fc Gerrit-Change-Number: 11257 Gerrit-PatchSet: 10 Gerrit-Owner: David Knupp Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Nithya Janarthanan Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 23 Aug 2018 00:06:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11296 ) Change subject: IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext() .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/459/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4569cc7b0843a29c617a094e590c31f7c648ff45 Gerrit-Change-Number: 11296 Gerrit-PatchSet: 3 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 22 Aug 2018 23:39:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7447. Evict LocalCatalog cache entries based on size
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11231 ) Change subject: IMPALA-7447. Evict LocalCatalog cache entries based on size .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/11231/6/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: http://gerrit.cloudera.org:8080/#/c/11231/6/be/src/runtime/exec-env.cc@86 PS6, Line 86: DEFINE_int32_hidden(local_catalog_cache_mb, -1, : "If --use_local_catalog is enabled, configures the size of the catalog " : "cache within each impalad. If this is set to -1, the cache is auto-" : "configured to 60% of the configured Java heap size. Note that the Java " : "heap size is distinct from and typically smaller than the overall " : "Impala memory limit.") > IMO, from a end user POV, it is less confusing if this directly exposes the oops, its hidden, nvm. -- To view, visit http://gerrit.cloudera.org:8080/11231 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia96af49b35c17e505b7b6785e78d140939085d91 Gerrit-Change-Number: 11231 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 22 Aug 2018 23:32:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7399: Emit a junit xml report when trapping errors
Hello Lars Volker, Michael Brown, Nithya Janarthanan, Philip Zeyliger, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11257 to look at the new patch set (#10). Change subject: IMPALA-7399: Emit a junit xml report when trapping errors .. IMPALA-7399: Emit a junit xml report when trapping errors This patch will cause a junitxml file to be emitted in the case of errors in build scripts. Instead of simply echoing a message to the console, we set up a trap function that also writes out to a junit xml report that can be consumed by jenkins.impala.io. Main things to pay attention to: - New file that gets sourced by all bash scripts when trapping within bash scripts: https://gerrit.cloudera.org/c/11257/1/bin/report_build_error.sh - Installation of the python lib into impala-python venv for use from within python files: https://gerrit.cloudera.org/c/11257/1/bin/impala-python-common.sh - Change to the generate_junitxml.py file itself, for ease of https://gerrit.cloudera.org/c/11257/1/lib/python/impala_py_lib/jenkins/generate_junitxml.py Most of the other changes are to source the new report_build_error.sh script to set up the trap function. Change-Id: Idd62045bb43357abc2b89a78afff499149d3c3fc --- M bin/clean-cmake.sh M bin/clean.sh M bin/create-test-configuration.sh M bin/create_testdata.sh M bin/distcc/distcc_server_setup.sh M bin/impala-python-common.sh M bin/jenkins/all-tests.sh M bin/jenkins/build-all-flag-combinations.sh M bin/jenkins/build-only.sh M bin/make_impala.sh A bin/report_build_error.sh M bin/run-all-tests.sh M bin/run-backend-tests.sh M bin/start-catalogd.sh M bin/start-impalad.sh M bin/start-statestored.sh M buildall.sh M infra/python/bootstrap_virtualenv.py M lib/python/impala_py_lib/jenkins/generate_junitxml.py M shell/make_shell_tarball.sh M testdata/bin/check-schema-diff.sh M testdata/bin/compute-table-stats.sh M testdata/bin/copy-data-sources.sh M testdata/bin/copy-udfs-udas.sh M testdata/bin/create-load-data.sh M testdata/bin/create-table-many-blocks.sh M testdata/bin/generate-load-nested.sh M testdata/bin/kill-all.sh M testdata/bin/kill-hbase.sh M testdata/bin/kill-hive-server.sh M testdata/bin/kill-java-service.sh M testdata/bin/kill-sentry-service.sh M testdata/bin/load-hive-builtins.sh M testdata/bin/load-metastore-snapshot.sh M testdata/bin/load-test-warehouse-snapshot.sh M testdata/bin/run-all.sh M testdata/bin/run-hbase.sh M testdata/bin/run-hive-server.sh M testdata/bin/run-mini-dfs.sh M testdata/bin/run-sentry-service.sh M testdata/bin/setup-hdfs-env.sh M testdata/cluster/admin M testdata/datasets/tpcds/preload M testdata/datasets/tpch/preload M tests/run-custom-cluster-tests.sh M tests/run-process-failure-tests.sh 46 files changed, 159 insertions(+), 68 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/11257/10 -- To view, visit http://gerrit.cloudera.org:8080/11257 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idd62045bb43357abc2b89a78afff499149d3c3fc Gerrit-Change-Number: 11257 Gerrit-PatchSet: 10 Gerrit-Owner: David Knupp Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Nithya Janarthanan Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7447. Evict LocalCatalog cache entries based on size
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11231 ) Change subject: IMPALA-7447. Evict LocalCatalog cache entries based on size .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/11231/6/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: http://gerrit.cloudera.org:8080/#/c/11231/6/be/src/runtime/exec-env.cc@86 PS6, Line 86: DEFINE_int32_hidden(local_catalog_cache_mb, -1, : "If --use_local_catalog is enabled, configures the size of the catalog " : "cache within each impalad. If this is set to -1, the cache is auto-" : "configured to 60% of the configured Java heap size. Note that the Java " : "heap size is distinct from and typically smaller than the overall " : "Impala memory limit.") IMO, from a end user POV, it is less confusing if this directly exposes the % configuration. Something like local_catalog_heap_max_percent... Given we already have a max_heap (-Xmx), this would confuse users as to why they need to configure two separate values. Anyway we probably don't expect the customers to change this often, so feel free to ignore if that is the case. -- To view, visit http://gerrit.cloudera.org:8080/11231 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia96af49b35c17e505b7b6785e78d140939085d91 Gerrit-Change-Number: 11231 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 22 Aug 2018 23:27:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext()
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11296 ) Change subject: IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext() .. Patch Set 3: Code-Review+1 Will let bikram look too -- To view, visit http://gerrit.cloudera.org:8080/11296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4569cc7b0843a29c617a094e590c31f7c648ff45 Gerrit-Change-Number: 11296 Gerrit-PatchSet: 3 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 22 Aug 2018 23:19:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-110: Support for multiple DISTINCT
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10771 ) Change subject: IMPALA-110: Support for multiple DISTINCT .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/458/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/10771 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I055402eaef6d81e5f70e850d9f8a621e766830a4 Gerrit-Change-Number: 10771 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 22 Aug 2018 23:12:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext()
Pooja Nilangekar has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/11296 ) Change subject: IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext() .. IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext() Previously, HdfsScanNode::GetNext passed the status returned by IssueInitialScanRanges() without inspecting the status_. It made the fragment lose track of the error set by the scanner thread. This change ensures that GetNext returns the first non-ok status set in the HdfsScanNode. Testing: Added sleeps to the IssueInitialRanges() to cause deterministic failures of test_udf_errors and then applied this patch to it. It passes all the tests despite the sleeps. Change-Id: I4569cc7b0843a29c617a094e590c31f7c648ff45 --- M be/src/exec/hdfs-scan-node-base.h M be/src/exec/hdfs-scan-node.cc 2 files changed, 15 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/11296/3 -- To view, visit http://gerrit.cloudera.org:8080/11296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4569cc7b0843a29c617a094e590c31f7c648ff45 Gerrit-Change-Number: 11296 Gerrit-PatchSet: 3 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext()
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/11296 ) Change subject: IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext() .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/11296/1/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/11296/1/be/src/exec/hdfs-scan-node.cc@90 PS1, Line 90: eader_context_ was cance > Ahh ok, I see your point - I think those errors from IssueInitialScanRanges Done http://gerrit.cloudera.org:8080/#/c/11296/2/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/11296/2/be/src/exec/hdfs-scan-node.cc@87 PS2, Line 87: Status status = IssueInitialScanRanges(state); > I think we want the comment in the header where IssueInitialScanRanges() is Done -- To view, visit http://gerrit.cloudera.org:8080/11296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4569cc7b0843a29c617a094e590c31f7c648ff45 Gerrit-Change-Number: 11296 Gerrit-PatchSet: 3 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 22 Aug 2018 23:08:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11296 ) Change subject: IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext() .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/457/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4569cc7b0843a29c617a094e590c31f7c648ff45 Gerrit-Change-Number: 11296 Gerrit-PatchSet: 2 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 22 Aug 2018 23:04:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-110: Support for multiple DISTINCT
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10771 ) Change subject: IMPALA-110: Support for multiple DISTINCT .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/10771/7/be/src/codegen/gen_ir_descriptions.py File be/src/codegen/gen_ir_descriptions.py: http://gerrit.cloudera.org:8080/#/c/10771/7/be/src/codegen/gen_ir_descriptions.py@55 PS7, Line 55: o flake8: E501 line too long (125 > 90 characters) -- To view, visit http://gerrit.cloudera.org:8080/10771 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I055402eaef6d81e5f70e850d9f8a621e766830a4 Gerrit-Change-Number: 10771 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 22 Aug 2018 22:42:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext()
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/11296 )
Change subject: IMPALA-7418: Return first non-ok status from
HdfsScanNode::GetNext()
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11296/1/be/src/exec/hdfs-scan-node.cc
File be/src/exec/hdfs-scan-node.cc:
http://gerrit.cloudera.org:8080/#/c/11296/1/be/src/exec/hdfs-scan-node.cc@90
PS1, Line 90: tatus.ok()) {
> Apart from reader_context_ being cancelled, there are failure modes for Iss
Ahh ok, I see your point - I think those errors from IssueInitialScanRanges()
should probably take precedence. The IsCancelled() check makes sense then, but
maybe the comment should explain this.
http://gerrit.cloudera.org:8080/#/c/11296/2/be/src/exec/hdfs-scan-node.cc
File be/src/exec/hdfs-scan-node.cc:
http://gerrit.cloudera.org:8080/#/c/11296/2/be/src/exec/hdfs-scan-node.cc@87
PS2, Line 87: // IssueInitialScanRanges() returns a non-ok status if
ValidateScanRange() fails or
I think we want the comment in the header where IssueInitialScanRanges() is
declared.
--
To view, visit http://gerrit.cloudera.org:8080/11296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4569cc7b0843a29c617a094e590c31f7c648ff45
Gerrit-Change-Number: 11296
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 22 Aug 2018 22:42:25 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-110: Support for multiple DISTINCT
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/10771 ) Change subject: IMPALA-110: Support for multiple DISTINCT .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/10771/4/testdata/workloads/functional-query/queries/QueryTest/multiple-distinct-aggs.test File testdata/workloads/functional-query/queries/QueryTest/multiple-distinct-aggs.test: PS4: > I should probably think about the testing matrix further. The random query Done -- To view, visit http://gerrit.cloudera.org:8080/10771 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I055402eaef6d81e5f70e850d9f8a621e766830a4 Gerrit-Change-Number: 10771 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 22 Aug 2018 22:41:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-110: Support for multiple DISTINCT
Hello Michael Ho, Tim Armstrong, Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10771 to look at the new patch set (#7). Change subject: IMPALA-110: Support for multiple DISTINCT .. IMPALA-110: Support for multiple DISTINCT This patch adds support for having multiple aggregate functions in a single SELECT block that use DISTINCT over different sets of columns. Planner design: - The existing tree-based plan shape with a two-phased aggregation is maintained. - Existing plans are not changed. - Aggregates are grouped into 'aggregation classes' based on their expressions in the distinct portion which may be empty for non-distinct aggregates. - The aggregation framework is generalized to simultaneously process multiple aggregation classes within the tree-based plan. This process splits the results of different aggregation classes into separate rows, so a final aggregation is needed to transpose the results into the desired form. - Main challenge: Each aggregation class consumes and produces different tuples, so conceptually a union-type of tuples flows through the runtime. The tuple union is represented by a TupleRow with one tuple per aggregation class. Only one tuple in such a TupleRow is non-NULL. - Backend exec nodes in the aggregation plan will be aware of this tuple-union either explicitly in their implementation or by relying on expressions that distinguish the aggregation classes. - To distinguish the aggregation classes, e.g. in hash exchanges, CASE expressions are crafted to hash/group on the appropriate slots. Deferred FE work: - Beautify/condense the long CASE exprs - Push applicable conjuncts into individual aggregators before the transposition step - Added a few testing TODOs to reduce the size of this patch - Decide whether we want to change existing plans to the new model Execution design: - Previous patches separated out aggregation logic from the exec node into Aggregators. This is extended to support multiple Aggregators per node, with different grouping and aggregating functions. - There is a fast path for aggregations with only one aggregator, which leaves the execution essentially unchanged from before. - When there are multiple aggregators, the first aggregation node in the plan replicates its input to each aggregator. The output of this step is rows where only a single tuple is non-null, corresponding to the aggregator that produced the row. - A new expr is introduced, ValidTupleId, which takes one of these rows and returns which tuple is non-null. - For additional aggregation nodes, the input is split apart into 'mini-batches' according to which aggregator the row corresponds to. Testing: - Added analyzer and planner tests - Added end-to-end queries tests - Ran hdfs/core tests Change-Id: I055402eaef6d81e5f70e850d9f8a621e766830a4 --- M be/src/codegen/gen_ir_descriptions.py M be/src/exec/CMakeLists.txt A be/src/exec/aggregation-node-base.cc A be/src/exec/aggregation-node-base.h M be/src/exec/aggregation-node.cc M be/src/exec/aggregation-node.h M be/src/exec/aggregator.cc M be/src/exec/aggregator.h M be/src/exec/exec-node.cc M be/src/exec/grouping-aggregator-ir.cc M be/src/exec/grouping-aggregator.cc M be/src/exec/grouping-aggregator.h M be/src/exec/non-grouping-aggregator.cc M be/src/exec/non-grouping-aggregator.h M be/src/exec/streaming-aggregation-node.cc M be/src/exec/streaming-aggregation-node.h M be/src/exprs/CMakeLists.txt M be/src/exprs/aggregate-functions-ir.cc M be/src/exprs/aggregate-functions.h M be/src/exprs/scalar-expr.cc A be/src/exprs/valid-tuple-id.cc A be/src/exprs/valid-tuple-id.h M be/src/runtime/row-batch.h M common/thrift/Exprs.thrift M common/thrift/PlanNodes.thrift M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java M fe/src/main/java/org/apache/impala/analysis/AggregateInfoBase.java A fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java A fe/src/main/java/org/apache/impala/analysis/ValidTupleIdExpr.java M fe/src/main/java/org/apache/impala/catalog/AggregateFunction.java M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java M fe/src/main/java/org/apache/impala/planner/AggregationNode.java M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M test
[Impala-ASF-CR] IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext()
Pooja Nilangekar has posted comments on this change. (
http://gerrit.cloudera.org:8080/11296 )
Change subject: IMPALA-7418: Return first non-ok status from
HdfsScanNode::GetNext()
..
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11296/1/be/src/exec/hdfs-scan-node.cc
File be/src/exec/hdfs-scan-node.cc:
http://gerrit.cloudera.org:8080/#/c/11296/1/be/src/exec/hdfs-scan-node.cc@87
PS1, Line 87: // IssueInitialScanRanges() returns a non-ok status if
ValidateScanRange() fails or
> Let's update the IssueInitialScanRanges() comment to explain the cases when
Done
http://gerrit.cloudera.org:8080/#/c/11296/1/be/src/exec/hdfs-scan-node.cc@88
PS1, Line 88: // the request_context_ is cancelled before all scan ranges
are added.
> I'd suggest adding a brief comment explaining this. E.g.
Done
http://gerrit.cloudera.org:8080/#/c/11296/1/be/src/exec/hdfs-scan-node.cc@90
PS1, Line 90: tatus.ok()) {
> I think we can remove this IsCancelled() check. It seems ok if status_ alwa
Apart from reader_context_ being cancelled, there are failure modes for
IssueInitialScanRanges(). For example, invalid file length for Parquet and Orc
files, unsupported compression formats for text files or if ValidateScanRange
fails. In those cases, shouldn't the status returned take precedence over
status_?
--
To view, visit http://gerrit.cloudera.org:8080/11296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4569cc7b0843a29c617a094e590c31f7c648ff45
Gerrit-Change-Number: 11296
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 22 Aug 2018 22:31:32 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext()
Pooja Nilangekar has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/11296 ) Change subject: IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext() .. IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext() Previously, HdfsScanNode::GetNext passed the status returned by IssueInitialScanRanges() without inspecting the status_. It made the fragment lose track of the error set by the scanner thread. This change ensures that GetNext returns the first non-ok status set in the HdfsScanNode. Testing: Added sleeps to the IssueInitialRanges() to cause deterministic failures of test_udf_errors and then applied this patch to it. It passes all the tests despite the sleeps. Change-Id: I4569cc7b0843a29c617a094e590c31f7c648ff45 --- M be/src/exec/hdfs-scan-node.cc 1 file changed, 11 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/11296/2 -- To view, visit http://gerrit.cloudera.org:8080/11296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4569cc7b0843a29c617a094e590c31f7c648ff45 Gerrit-Change-Number: 11296 Gerrit-PatchSet: 2 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11296 ) Change subject: IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext() .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/456/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4569cc7b0843a29c617a094e590c31f7c648ff45 Gerrit-Change-Number: 11296 Gerrit-PatchSet: 1 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 22 Aug 2018 22:04:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7399: Emit a junit xml report when trapping errors
Michael Brown has posted comments on this change. (
http://gerrit.cloudera.org:8080/11257 )
Change subject: IMPALA-7399: Emit a junit xml report when trapping errors
..
Patch Set 9: Code-Review+1
(2 comments)
Phil had good comments. I had looked at an earlier patch set and didn't have
much more to say. This looks good to me now though.
http://gerrit.cloudera.org:8080/#/c/11257/9/lib/python/impala_py_lib/jenkins/generate_junitxml.py
File lib/python/impala_py_lib/jenkins/generate_junitxml.py:
http://gerrit.cloudera.org:8080/#/c/11257/9/lib/python/impala_py_lib/jenkins/generate_junitxml.py@164
PS9, Line 164: {}
Not python2.6-compatible. Use {0}
http://gerrit.cloudera.org:8080/#/c/11257/9/testdata/bin/check-schema-diff.sh
File testdata/bin/check-schema-diff.sh:
http://gerrit.cloudera.org:8080/#/c/11257/9/testdata/bin/check-schema-diff.sh@30
PS9, Line 30: the same trap function used by other shell scripts
just say "we don't call setup_report_build_error".
--
To view, visit http://gerrit.cloudera.org:8080/11257
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd62045bb43357abc2b89a78afff499149d3c3fc
Gerrit-Change-Number: 11257
Gerrit-PatchSet: 9
Gerrit-Owner: David Knupp
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Michael Brown
Gerrit-Reviewer: Nithya Janarthanan
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Wed, 22 Aug 2018 22:00:43 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext()
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/11296 )
Change subject: IMPALA-7418: Return first non-ok status from
HdfsScanNode::GetNext()
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11296/1/be/src/exec/hdfs-scan-node.cc
File be/src/exec/hdfs-scan-node.cc:
http://gerrit.cloudera.org:8080/#/c/11296/1/be/src/exec/hdfs-scan-node.cc@87
PS1, Line 87: Status status = IssueInitialScanRanges(state);
Let's update the IssueInitialScanRanges() comment to explain the cases when it
returns a non-OK status (i.e. an error issuing the ranges or the
reader_context_ being cancelled).
http://gerrit.cloudera.org:8080/#/c/11296/1/be/src/exec/hdfs-scan-node.cc@88
PS1, Line 88: if (!status.ok()) {
I'd suggest adding a brief comment explaining this. E.g.
"If a scanner thread hit an error and cancelled reader_context_, the scanner
thread's error must take precedence."
http://gerrit.cloudera.org:8080/#/c/11296/1/be/src/exec/hdfs-scan-node.cc@90
PS1, Line 90: status.IsCancelled() &&
I think we can remove this IsCancelled() check. It seems ok if status_ always
takes precedence.
--
To view, visit http://gerrit.cloudera.org:8080/11296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4569cc7b0843a29c617a094e590c31f7c648ff45
Gerrit-Change-Number: 11296
Gerrit-PatchSet: 1
Gerrit-Owner: Pooja Nilangekar
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 22 Aug 2018 21:43:22 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext()
Pooja Nilangekar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11296 Change subject: IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext() .. IMPALA-7418: Return first non-ok status from HdfsScanNode::GetNext() Previously, HdfsScanNode::GetNext passed the status returned by IssueInitialScanRanges() without inspecting the status_. It made the fragment lose track of the error set by the scanner thread. This change ensures that GetNext returns the first non-ok status set in the HdfsScanNode. Testing: Added sleeps to the IssueInitialRanges() to cause deterministic failures of test_udf_errors and then applied this patch to it. It passes all the tests despite the sleeps. Change-Id: I4569cc7b0843a29c617a094e590c31f7c648ff45 --- M be/src/exec/hdfs-scan-node.cc 1 file changed, 6 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/11296/1 -- To view, visit http://gerrit.cloudera.org:8080/11296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4569cc7b0843a29c617a094e590c31f7c648ff45 Gerrit-Change-Number: 11296 Gerrit-PatchSet: 1 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11279 ) Change subject: IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements .. IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements Prior to this patch, any user with ALTER privilege could alter the database/table ownership from one user/role to another user/role. This is undesirable because altering an object ownership means giving a full access to that object. This patch restricts the ALTER DATABASE/TABLE SET OWNER statements to require ALL/OWNER with GRANT OPTION when authorization is enabled. Testing: - Added FE authorization tests - Ran all FE tests - Ran core tests Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 Reviewed-on: http://gerrit.cloudera.org:8080/11279 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M bin/impala-config.sh M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java M fe/src/main/java/org/apache/impala/analysis/TableRef.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequest.java M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java 15 files changed, 238 insertions(+), 86 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11279 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 Gerrit-Change-Number: 11279 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11279 ) Change subject: IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11279 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 Gerrit-Change-Number: 11279 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, 22 Aug 2018 21:27:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7437. LRU caching of partitions in impalad
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/11208 )
Change subject: IMPALA-7437. LRU caching of partitions in impalad
..
Patch Set 8:
(8 comments)
Just a bunch of nits I found skimming through the change. Feel free to address
in the following changes. Change lgtm.
http://gerrit.cloudera.org:8080/#/c/11208/8/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/11208/8/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@281
PS8, Line 281: if (!missingCols.isEmpty()) {
nit: May be easier to read if we invert the condition.
if (missintCols.isEmpty()) return emptyList;
http://gerrit.cloudera.org:8080/#/c/11208/8/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@286
PS8, Line 286: ");
Add table name?
http://gerrit.cloudera.org:8080/#/c/11208/8/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@324
PS8, Line 324: ");
table name
http://gerrit.cloudera.org:8080/#/c/11208/8/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@329
PS8, Line 329: );
table
http://gerrit.cloudera.org:8080/#/c/11208/8/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@415
PS8, Line 415: ");
table + part name
http://gerrit.cloudera.org:8080/#/c/11208/8/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@435
PS8, Line 435: );
, tablename?
http://gerrit.cloudera.org:8080/#/c/11208/8/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@492
PS8, Line 492: throw new TException(String.format("Invalid response from
catalogd for request " +
Should we log the exception stack here instead of logging it at all the callers?
http://gerrit.cloudera.org:8080/#/c/11208/8/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@689
PS8, Line 689:* Cache key for the partition list of a table.
nit:For a moment I got confused if there is duplication between this and
PartitionCacheKey. May be good to clarify that they cache different forms of
Partition metadata
--
To view, visit http://gerrit.cloudera.org:8080/11208
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a57521ad851da605604a1e7c48d3d6627da5df5
Gerrit-Change-Number: 11208
Gerrit-PatchSet: 8
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Wed, 22 Aug 2018 21:20:35 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7460 part 1: require user to install Paramiko and Fabric
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11264 ) Change subject: IMPALA-7460 part 1: require user to install Paramiko and Fabric .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3072/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11264 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65 Gerrit-Change-Number: 11264 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 22 Aug 2018 21:05:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7460 part 1: require user to install Paramiko and Fabric
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11264 ) Change subject: IMPALA-7460 part 1: require user to install Paramiko and Fabric .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/455/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11264 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65 Gerrit-Change-Number: 11264 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 22 Aug 2018 20:47:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7460 part 1: require user to install Paramiko and Fabric
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/11264 ) Change subject: IMPALA-7460 part 1: require user to install Paramiko and Fabric .. Patch Set 5: Code-Review+2 rebase/commit message fix -- To view, visit http://gerrit.cloudera.org:8080/11264 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65 Gerrit-Change-Number: 11264 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 22 Aug 2018 20:13:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7460 part 1: require user to install Paramiko and Fabric
Hello Lars Volker, Philip Zeyliger, David Knupp, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11264 to look at the new patch set (#5). Change subject: IMPALA-7460 part 1: require user to install Paramiko and Fabric .. IMPALA-7460 part 1: require user to install Paramiko and Fabric - Remove Fabric and Paramiko as requirements. They aren't needed by anything in buildall.sh. - Add a means to install into the impala-python virtual environment by hand. impala-pip is fine for this. - Add another requirements file for extended testing. The dependency situation is messy and untangling that out of impala-python and into lib/python should be out of the scope of IMPALA-7460. - Update core tests, which cover real regressions that have happened in the past, to run against locations that don't require a Paramiko import. This moves some logic out of concurrent_select.py into a thinner module. - Insulate ssh_util from globally-scoped import so that it only imports when needed. Testing: - This works in my development environment. - This works in my downstream stress and query gen environments. - This works when doing a full data load. - Impala still builds on a variety of OSs. Todo: - A subsequent review will update the versions. Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65 --- A bin/impala-pip M infra/python/deps/compiled-requirements.txt A infra/python/deps/extended-test-requirements.txt M tests/comparison/cluster.py M tests/comparison/leopard/impala_docker_env.py M tests/infra/test_stress_infra.py M tests/stress/concurrent_select.py M tests/util/cluster_controller.py M tests/util/parse_util.py M tests/util/ssh_util.py M tests/util/test_file_parser.py 11 files changed, 157 insertions(+), 82 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11264/5 -- To view, visit http://gerrit.cloudera.org:8080/11264 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65 Gerrit-Change-Number: 11264 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/11264 ) Change subject: WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11264 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65 Gerrit-Change-Number: 11264 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 22 Aug 2018 20:12:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6373: Allow primitive type widening on parquet tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11268 ) Change subject: IMPALA-6373: Allow primitive type widening on parquet tables .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/454/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11268 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If93394b035c64cf6fc5f37b54d29c034cc1f86e4 Gerrit-Change-Number: 11268 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 22 Aug 2018 20:02:53 + Gerrit-HasComments: No
[Impala-ASF-CR] WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11264 ) Change subject: WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/453/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11264 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65 Gerrit-Change-Number: 11264 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 22 Aug 2018 19:56:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6373: Allow primitive type widening on parquet tables
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/11268 )
Change subject: IMPALA-6373: Allow primitive type widening on parquet tables
..
Patch Set 3:
(3 comments)
Rebased the CR. Csaba/Zoltan, do you want to take another look for the +2?
http://gerrit.cloudera.org:8080/#/c/11268/2/be/src/exec/parquet-common.h
File be/src/exec/parquet-common.h:
http://gerrit.cloudera.org:8080/#/c/11268/2/be/src/exec/parquet-common.h@253
PS2, Line 253: inline int DecodeWithConversion(const uint8_t* buffer, const
uint8_t* buffer_end, To* v) {
> optional: I would prefer to merge the common functionality somehow.
Thanks for the suggestion. I updated the code to remove code duplication as per
your first suggestion. I didn't go further as to create a type mapper, but I
can do that in a different patch :)
http://gerrit.cloudera.org:8080/#/c/11268/2/be/src/exec/parquet-plain-test.cc
File be/src/exec/parquet-plain-test.cc:
http://gerrit.cloudera.org:8080/#/c/11268/2/be/src/exec/parquet-plain-test.cc@138
PS2, Line 138: TestType
> nit: I'd choose a different name, e.g. 'TestTypeWidening'
Done
http://gerrit.cloudera.org:8080/#/c/11268/2/tests/query_test/test_scanners.py
File tests/query_test/test_scanners.py:
http://gerrit.cloudera.org:8080/#/c/11268/2/tests/query_test/test_scanners.py@758
PS2, Line 758: self.run_test_case('QueryTest/hdfs-text-scan', vector)
:
: # Test various escape char cases. We have to check the
count(*) result against
: # the count(col) result because if the scan range is split
right after the escape
: # char, the escape char has no effect because we cannot scan
backwards to the
: # previous scan range.
: for t in self.ESCAPE_TABLE_LIST:
: expected_result = self.client.execute("select count(col)
from " + t)
> It seems https://gerrit.cloudera.org/#/c/11127/ is about to go in, so you c
Thanks. It looks a lot nicer. Done.
--
To view, visit http://gerrit.cloudera.org:8080/11268
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If93394b035c64cf6fc5f37b54d29c034cc1f86e4
Gerrit-Change-Number: 11268
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Wed, 22 Aug 2018 19:28:32 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6373: Allow primitive type widening on parquet tables
Fredy Wijaya has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/11268 ) Change subject: IMPALA-6373: Allow primitive type widening on parquet tables .. IMPALA-6373: Allow primitive type widening on parquet tables This patch implements support for primitive type widening on parquet tables. It only supports conversion to those types without any loss of precision. - tinyint (INT32) -> smallint (INT32), int (INT32), bigint (INT64), double (DOUBLE) - smallint (INT32) -> int (INT32), bigint (INT64), double (DOUBLE) - int (INT32) -> bigint (INT64), double (DOUBLE) - float (FLOAT) -> double (DOUBLE) Testing: - Added BE test - Added E2E test - Ran core tests Change-Id: If93394b035c64cf6fc5f37b54d29c034cc1f86e4 --- M be/src/exec/parquet-column-readers.cc M be/src/exec/parquet-column-readers.h M be/src/exec/parquet-common.h M be/src/exec/parquet-metadata-utils.cc M be/src/exec/parquet-plain-test.cc M testdata/data/README A testdata/data/primitive_type_widening.parquet A testdata/workloads/functional-query/queries/QueryTest/parquet-type-widening.test M tests/query_test/test_scanners.py 9 files changed, 139 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/11268/3 -- To view, visit http://gerrit.cloudera.org:8080/11268 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If93394b035c64cf6fc5f37b54d29c034cc1f86e4 Gerrit-Change-Number: 11268 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/11264 ) Change subject: WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric .. Patch Set 4: > Expect another non-trivial update. It turns out to fix data load I just had to move an import to be more localized. I also updated the testing section of the commit message. -- To view, visit http://gerrit.cloudera.org:8080/11264 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65 Gerrit-Change-Number: 11264 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 22 Aug 2018 19:22:27 + Gerrit-HasComments: No
[Impala-ASF-CR] WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric
Hello Lars Volker, Philip Zeyliger, David Knupp, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11264 to look at the new patch set (#4). Change subject: WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric .. WIP: IMPALA-7460 part 1: require user to install Paramiko and Fabric - Remove Fabric and Paramiko as requirements. They aren't needed by anything in buildall.sh. - Add a means to install into the impala-python virtual environment by hand. impala-pip is fine for this. - Add another requirements file for extended testing. The dependency situation is messy and untangling that out of impala-python and into lib/python should be out of the scope of IMPALA-7460. - Update core tests, which cover real regressions that have happened in the past, to run against locations that don't require a Paramiko import. This moves some logic out of concurrent_select.py into a thinner module. - Insulate ssh_util from globally-scoped import so that it only imports when needed. Testing: - This works in my development environment. - This works in my downstream stress and query gen environments. - This works when doing a full data load. - Impala still builds on a variety of OSs. Todo: - A subsequent review will update the versions. Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65 --- A bin/impala-pip M infra/python/deps/compiled-requirements.txt A infra/python/deps/extended-test-requirements.txt M tests/comparison/cluster.py M tests/comparison/leopard/impala_docker_env.py M tests/infra/test_stress_infra.py M tests/stress/concurrent_select.py M tests/util/cluster_controller.py M tests/util/parse_util.py M tests/util/ssh_util.py M tests/util/test_file_parser.py 11 files changed, 157 insertions(+), 82 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11264/4 -- To view, visit http://gerrit.cloudera.org:8080/11264 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibf9010a0387b52c95b7bda5d1d4606eba1008b65 Gerrit-Change-Number: 11264 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7399: Emit a junit xml report when trapping errors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11257 ) Change subject: IMPALA-7399: Emit a junit xml report when trapping errors .. Patch Set 9: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11257 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idd62045bb43357abc2b89a78afff499149d3c3fc Gerrit-Change-Number: 11257 Gerrit-PatchSet: 9 Gerrit-Owner: David Knupp Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Nithya Janarthanan Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 22 Aug 2018 19:10:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/10855/10/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/10/be/src/runtime/query-state.cc@287 PS10, Line 287: > To answer the question about your second race, I think it's impossible toda Ah, I missed that we join on the reporter thread first. Given that, does the report sequence number generation need to be atomic? It seems you expect only a single thread to be reporting at once, in which case non-atomic int would be fine. On a similar note, the interaction between 'done' and the sequence numbers is a bit strange to me. It seems like, if we trust our sequence number generation is free of the above race, then we dont need the special handling where "done" always wins out over the sequence-number checks, right? i.e we already guarantee that the "done" message has a higher sequence than any prior report? I also wonder whether it is feasible to add an assertion for non-concurrency of report-sending by using a DFAKE_MUTEX inside the ReportExecStatusAux function. Then if someone accidentally breaks these assumptions we'll get a clear failure in debug builds instead of some more rare hard-to-debug stuck query. -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 11 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 22 Aug 2018 18:47:07 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11279 ) Change subject: IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/452/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11279 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 Gerrit-Change-Number: 11279 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, 22 Aug 2018 18:36:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7412: width bucket() function overflows too easily
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11282 ) Change subject: IMPALA-7412: width_bucket() function overflows too easily .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11282 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I68262698144029ef7f54e027e586eaf105f36ab3 Gerrit-Change-Number: 11282 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Wed, 22 Aug 2018 18:26:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7412: width bucket() function overflows too easily
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11282 ) Change subject: IMPALA-7412: width_bucket() function overflows too easily .. IMPALA-7412: width_bucket() function overflows too easily Running the tests of https://gerrit.cloudera.org/#/c/10859/ it turned out that the width_bucket() function overflows very often. A common problem is that the function tries to cast the 'num_buckets' parameter to the decimal determined by the Frontend. When the Frontend determined the precision and scale of this decimal it only considered the decimal arguments and ignored everything else. Therefore the determined precision and scale is often not suitable for the 'num_buckets' parameter. WidthBucketImpl() has three decimal arguments, all of them have the same byte size, precision, and scale. So it is possible to interpret them as plain integers and still calculate the proper bucket. I included the python test cases from IMPALA-7202 developed by Taras Bobrovytsky. I also extended the backend tests with new test cases. For performance test I used the following query: SELECT sum(width_bucket(cast(l_orderkey AS DECIMAL(30, 10)), 0, 550, 100)) FROM tpch_parquet.lineitem; The new implementation executed it in ~0.3 seconds. The old implementation executed it in ~0.8 seconds. Change-Id: I68262698144029ef7f54e027e586eaf105f36ab3 Reviewed-on: http://gerrit.cloudera.org:8080/11282 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/exprs/expr-test.cc M be/src/exprs/math-functions-ir.cc M be/src/util/bit-util.h M tests/query_test/test_decimal_fuzz.py 4 files changed, 200 insertions(+), 124 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11282 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I68262698144029ef7f54e027e586eaf105f36ab3 Gerrit-Change-Number: 11282 Gerrit-PatchSet: 3 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: anujphadke
[Impala-ASF-CR] IMPALA-6709: Simplify tests that copy local files to tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11127 ) Change subject: IMPALA-6709: Simplify tests that copy local files to tables .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11127 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie00a4561825facf8abe2e8e74a6b6e93194f416f Gerrit-Change-Number: 11127 Gerrit-PatchSet: 5 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 22 Aug 2018 18:08:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6709: Simplify tests that copy local files to tables
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11127 ) Change subject: IMPALA-6709: Simplify tests that copy local files to tables .. IMPALA-6709: Simplify tests that copy local files to tables We had quite a few tests that created a table and used "hdfs dfs -copyFromLocal" to copy data files to the warehouse directory for this table. This operation needs some boilerplate code that I refactored to the new functions called create_table_from_parquet() and create_table_and_copy_files(). Change-Id: Ie00a4561825facf8abe2e8e74a6b6e93194f416f Reviewed-on: http://gerrit.cloudera.org:8080/11127 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M testdata/workloads/functional-query/queries/QueryTest/parquet-def-levels.test A tests/common/file_utils.py M tests/query_test/test_parquet_stats.py M tests/query_test/test_scanners.py 4 files changed, 114 insertions(+), 160 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11127 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ie00a4561825facf8abe2e8e74a6b6e93194f416f Gerrit-Change-Number: 11127 Gerrit-PatchSet: 6 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11279 ) Change subject: IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11279 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 Gerrit-Change-Number: 11279 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, 22 Aug 2018 18:05:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11279 ) Change subject: IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3071/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11279 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 Gerrit-Change-Number: 11279 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, 22 Aug 2018 18:05:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11279 ) Change subject: IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11279 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 Gerrit-Change-Number: 11279 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, 22 Aug 2018 18:03:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7349: Add Admission control support for automatically setting mem limit
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11157 ) Change subject: IMPALA-7349: Add Admission control support for automatically setting mem_limit .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/11157/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11157/7//COMMIT_MSG@10 PS7, Line 10: With this patch the mem_limit of a query is automatically set based on : the mem_limit set in the query options and the mem_estimate calculated : by the planner. > In the "else" clause, I understand why we have the min/max clamping values the reasoning behind clamping mem_limit is to prevent a case where a user can set a really high mem_limit and the admin wants to avoid that unpredictability by clamping the max mem a query in the pool can get. On the other hand, if the admin trusts the users enough to make the right decision for mem_limit, then you can set strict-min-max-query-mem-limit to false. until the final limit is set, the estimates are still named mem_estimate, but like you mentioned, it will be a bit difficult to change the 'mem_limit' term in the query options. -- To view, visit http://gerrit.cloudera.org:8080/11157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifec00141651982f5975803c2165b7d7a10ebeaa6 Gerrit-Change-Number: 11157 Gerrit-PatchSet: 7 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 22 Aug 2018 18:01:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements
Fredy Wijaya has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/11279 ) Change subject: IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements .. IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements Prior to this patch, any user with ALTER privilege could alter the database/table ownership from one user/role to another user/role. This is undesirable because altering an object ownership means giving a full access to that object. This patch restricts the ALTER DATABASE/TABLE SET OWNER statements to require ALL/OWNER with GRANT OPTION when authorization is enabled. Testing: - Added FE authorization tests - Ran all FE tests - Ran core tests Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 --- M bin/impala-config.sh M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java M fe/src/main/java/org/apache/impala/analysis/TableRef.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequest.java M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java 15 files changed, 238 insertions(+), 86 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/79/11279/4 -- To view, visit http://gerrit.cloudera.org:8080/11279 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 Gerrit-Change-Number: 11279 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/11279 )
Change subject: IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements
..
Patch Set 4:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java:
http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java@43
PS3, Line 43: analyzer.getDb(dbName_, Privilege.ALL, /* throw if does not
exist */ true,
> add /* parameter name */ before these bools to make the call more readable.
Done
http://gerrit.cloudera.org:8080/#/c/11279/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/11279/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2472
PS3, Line 2472: }
> I see its duplicated in three places now: method before, here, and next met
Done
http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2636
PS3, Line 2636: / Add access event for auditing.
: if (table instanceof FeView) {
: FeView view = (FeView) table;
: Pre
> remove similar redundancy here with the method on L2651.
Done
http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
File
fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java:
http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@38
PS3, Line 38: boolean grantOption_ = false;
> explicitly set to false for default?
Done
http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java:
http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3785
PS3, Line 3785: testNumberOfMembers(TableRef.class, 21);
> why did this change?
Because of an additional member variable "requireGrantOption_" in TableRef.
--
To view, visit http://gerrit.cloudera.org:8080/11279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3
Gerrit-Change-Number: 11279
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, 22 Aug 2018 18:00:39 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11183 ) Change subject: IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions .. Patch Set 10: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3067/ -- To view, visit http://gerrit.cloudera.org:8080/11183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I572b5876b979ddae58165bd40d5b008ce9d7a4aa Gerrit-Change-Number: 11183 Gerrit-PatchSet: 10 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 22 Aug 2018 17:58:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11279 ) Change subject: IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements .. Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java File fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java: http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java@43 PS3, Line 43: analyzer.getDb(dbName_, Privilege.ALL, true, true); add /* parameter name */ before these bools to make the call more readable. right now, its unclear how the GRANT OPTION is specified. http://gerrit.cloudera.org:8080/#/c/11279/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/11279/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2472 PS3, Line 2472: /** I see its duplicated in three places now: method before, here, and next method-- this all can go stale easily. would be good to have the detail once and explain the others as just setting defaults for the more general method. http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2636 PS3, Line 2636: Registers a table-level privilege request and an access event for auditing :* for the given table and privilege. The table must be a base table or a :* catalog view (not a local view). This method does not require the grant :* opti remove similar redundancy here with the method on L2651. http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java File fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java: http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@38 PS3, Line 38: boolean grantOption_; explicitly set to false for default? http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java: http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3785 PS3, Line 3785: testNumberOfMembers(TableRef.class, 21); why did this change? -- To view, visit http://gerrit.cloudera.org:8080/11279 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 Gerrit-Change-Number: 11279 Gerrit-PatchSet: 3 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, 22 Aug 2018 17:27:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7349: Add Admission control support for automatically setting mem limit
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/11157 ) Change subject: IMPALA-7349: Add Admission control support for automatically setting mem_limit .. Patch Set 7: (3 comments) http://gerrit.cloudera.org:8080/#/c/11157/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11157/7//COMMIT_MSG@10 PS7, Line 10: With this patch the mem_limit of a query is automatically set based on : the mem_limit set in the query options and the mem_estimate calculated : by the planner. > For the new behavior (Assuming either min or max memlimit is set in the poo In the "else" clause, I understand why we have the min/max clamping values (because we're using heuristics to cook up a mem_limit). But in the "then" clause, why does it make sense to override the mem_limit if it was explicitly set by the user? Also, in terms of terminology in this part, my suggestion would be to come up with a different term rather than mem_limit for the thing before it gets clamped, and keep using mem_limit to mean only the final value used to limit memory consumption. But it's difficult to do that given the "then" clause and the pre-existing mem_limit query option name, which is another reason for my question above. http://gerrit.cloudera.org:8080/#/c/11157/7//COMMIT_MSG@15 PS7, Line 15: in-query-mem-limit-bytes, max-query-mem-limit-bytes these names seem okay. http://gerrit.cloudera.org:8080/#/c/11157/7//COMMIT_MSG@18 PS7, Line 18: strict-min-max-query-mem-limit this is a bit unclear, but not sure I have a better suggestion. In any case, let's finish the discussion above first anyway. -- To view, visit http://gerrit.cloudera.org:8080/11157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifec00141651982f5975803c2165b7d7a10ebeaa6 Gerrit-Change-Number: 11157 Gerrit-PatchSet: 7 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 22 Aug 2018 17:09:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions
Attila Jeges has posted comments on this change. (
http://gerrit.cloudera.org:8080/11183 )
Change subject: IMPALA-7417: Speed up sub-second unix time->TimestampValue
conversions
..
Patch Set 9:
(18 comments)
Did a quick first-round review. I will look at the tests the next time.
http://gerrit.cloudera.org:8080/#/c/11183/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11183/9//COMMIT_MSG@35
PS9, Line 35: This should have
: no visible side-effects.
Why? Is it used for testing only? If so, please add a sentence explaining it.
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/benchmarks/convert-timestamp-benchmark.cc
File be/src/benchmarks/convert-timestamp-benchmark.cc:
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/benchmarks/convert-timestamp-benchmark.cc@258
PS9, Line 258: <<
nit: space is missing before '<<' operator
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/benchmarks/convert-timestamp-benchmark.cc@561
PS9, Line 561: GRANURALITY
typo: GRANULARITY.
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/benchmarks/convert-timestamp-benchmark.cc@574
PS9, Line 574:
nit: remove extra spaces.
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/exec/hdfs-orc-scanner.cc
File be/src/exec/hdfs-orc-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/exec/hdfs-orc-scanner.cc@27
PS9, Line 27: #include "runtime/tuple-row.h"
: #include "runtime/timestamp-value.inline.h"
nit: order alphabetically.
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/runtime/timestamp-value.h@229
PS9, Line 229: set_time
You should validate the time value here (call Validate() or whatever function
time-validation ends up in)
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/runtime/timestamp-value.h@295
PS9, Line 295: /// Sets both date and time to invalid if date is outside the
valid range.
Fix the comment or move the time-validation logic below to a separate function.
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/runtime/timestamp-value.h@298
PS9, Line 298: if
else if
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/runtime/timestamp-value.h@303
PS9, Line 303: DCHECK_GE(time_.total_nanoseconds(), 0);
: DCHECK_LT(time_.total_nanoseconds(), NANOS_PER_DAY);
Maybe instead of DCHECK calls, you should call SetToInvalidDateTime() here?
Again, it might make sense to move time-validation code to a separate function.
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/runtime/timestamp-value.h@314
PS9, Line 314: GRANURALITY
typo: GRANULARITY
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/runtime/timestamp-value.h@327
PS9, Line 327: unix_time
I think, this should be called 'unix_time_ticks'
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/runtime/timestamp-value.h@327
PS9, Line 327: UtcFromUnixTime
Maybe this should be called UtcFromUnixTimeTicks().
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/runtime/timestamp-value.cc
File be/src/runtime/timestamp-value.cc:
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/runtime/timestamp-value.cc@153
PS9, Line 153: /// Return a ptime representation of the given Unix time
(seconds since the Unix epoch).
: /// The time zone of the resulting ptime is 'local_tz'. This is
called by UnixTimeToPtime.
Fix comment.
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/runtime/timestamp-value.cc@168
PS9, Line 168: }
nit: insert empty line after }
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/runtime/timestamp-value.cc@169
PS9, Line 169: ){
nit: missing space between ) and {
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/runtime/timestamp-value.inline.h
File be/src/runtime/timestamp-value.inline.h:
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/runtime/timestamp-value.inline.h@34
PS9, Line 34: UtcFromUnixTime
should be called UtcFromUnixTimeTicks().
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/runtime/timestamp-value.inline.h@34
PS9, Line 34: unix_time
should be called 'unix_time_ticks'
http://gerrit.cloudera.org:8080/#/c/11183/9/be/src/runtime/timestamp-value.inline.h@61
PS9, Line 61: round
Is calling round() here and below intentional? The previous version of
FromSubsecondUnixTime() just casts unix_time to int64_t.
Is this change addressed in the commit message?
--
To view, visit http://gerrit.cloudera.org:8080/11183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I572b5876b979ddae58165bd40d5b008ce9d7a4aa
Gerrit-Change-Number: 11183
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Wed, 22 Aug 2018 16:23:17 +
Gerrit
[Impala-ASF-CR] IMPALA-7457. statestore: allow filtering by key prefix
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11253 ) Change subject: IMPALA-7457. statestore: allow filtering by key prefix .. IMPALA-7457. statestore: allow filtering by key prefix This adds the ability for a statestore subscriber to specify a key prefix which acts as a filter. Only topic entries which match the specified prefix are transmitted to the subscriber. This patch makes use of the new feature for a small optimization: the catalogd subscribes to the catalog topic with a key prefix "!" which we know doesn't match any actual topic items. This avoids the statestore having to reflect back the catalog contents to the catalogd, since the catalogd ignored this info anyway. A later patch will make use of this to publish lightweight catalog object version numbers in the same topic as the catalog objects themselves. The modification to catalogd's topic subscription is covered by existing tests. A new specific test is added to verify the filtering mechanism. Change-Id: I6ddcf3bfaf16bc3cd1ba01100e948ff142a67620 Reviewed-on: http://gerrit.cloudera.org:8080/11253 Tested-by: Impala Public Jenkins Reviewed-by: Todd Lipcon --- M be/src/catalog/catalog-server.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/scheduler.cc M be/src/service/impala-server.cc M be/src/statestore/statestore-subscriber.cc M be/src/statestore/statestore-subscriber.h M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M common/thrift/StatestoreService.thrift M tests/statestore/test_statestore.py 10 files changed, 115 insertions(+), 16 deletions(-) Approvals: Impala Public Jenkins: Verified Todd Lipcon: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I6ddcf3bfaf16bc3cd1ba01100e948ff142a67620 Gerrit-Change-Number: 11253 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6758: Add metric for current catalog version to catalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11293 ) Change subject: IMPALA-6758: Add metric for current catalog version to catalog .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/451/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11293 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff7e158292ca9e5a17663e5bfc74931cc57c0328 Gerrit-Change-Number: 11293 Gerrit-PatchSet: 1 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 22 Aug 2018 16:06:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7447. Evict LocalCatalog cache entries based on size
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11231 ) Change subject: IMPALA-7447. Evict LocalCatalog cache entries based on size .. IMPALA-7447. Evict LocalCatalog cache entries based on size This pulls in the 'sizeof' library from ehcache (Apache-licensed) and uses it to implement size-based eviction of cache entries in LocalCatalog. This is difficult to test without being quite fragile to small changes in the cached structures. However, I added a simple unit test as a general sanity-check that it is computing some reasonable result. Change-Id: Ia96af49b35c17e505b7b6785e78d140939085d91 Reviewed-on: http://gerrit.cloudera.org:8080/11231 Tested-by: Impala Public Jenkins Reviewed-by: Todd Lipcon --- M be/src/runtime/exec-env.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/pom.xml M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java 7 files changed, 113 insertions(+), 12 deletions(-) Approvals: Impala Public Jenkins: Verified Todd Lipcon: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11231 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ia96af49b35c17e505b7b6785e78d140939085d91 Gerrit-Change-Number: 11231 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7447. Evict LocalCatalog cache entries based on size
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11231 ) Change subject: IMPALA-7447. Evict LocalCatalog cache entries based on size .. Patch Set 6: Code-Review+2 carrying +2 from Vuk -- To view, visit http://gerrit.cloudera.org:8080/11231 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia96af49b35c17e505b7b6785e78d140939085d91 Gerrit-Change-Number: 11231 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 22 Aug 2018 16:05:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7457. statestore: allow filtering by key prefix
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11253 ) Change subject: IMPALA-7457. statestore: allow filtering by key prefix .. Patch Set 4: Code-Review+2 Carrying +2 from Vuk and Tim -- To view, visit http://gerrit.cloudera.org:8080/11253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ddcf3bfaf16bc3cd1ba01100e948ff142a67620 Gerrit-Change-Number: 11253 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 22 Aug 2018 16:05:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Impala Public Jenkins 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 11: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/450/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 11 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Wed, 22 Aug 2018 15:58:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7399: Emit a junit xml report when trapping errors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11257 ) Change subject: IMPALA-7399: Emit a junit xml report when trapping errors .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3070/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11257 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idd62045bb43357abc2b89a78afff499149d3c3fc Gerrit-Change-Number: 11257 Gerrit-PatchSet: 9 Gerrit-Owner: David Knupp Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Nithya Janarthanan Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 22 Aug 2018 15:50:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6758: Add metric for current catalog version to catalog
Vincent Tran has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11293 Change subject: IMPALA-6758: Add metric for current catalog version to catalog .. IMPALA-6758: Add metric for current catalog version to catalog This change adds a new metric to the catalogd web UI: - catalog-server.current-topic-version As per its namesake, it tracks the current topic version as per the catalogd. Testing: Manually verified that the metric displays in the catalogd web UI and that it displays the expected version number. Change-Id: Iff7e158292ca9e5a17663e5bfc74931cc57c0328 --- M be/src/catalog/catalog-server.cc M be/src/catalog/catalog-server.h M common/thrift/metrics.json 3 files changed, 17 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/11293/1 -- To view, visit http://gerrit.cloudera.org:8080/11293 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iff7e158292ca9e5a17663e5bfc74931cc57c0328 Gerrit-Change-Number: 11293 Gerrit-PatchSet: 1 Gerrit-Owner: Vincent Tran
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Hello Andrew Sherman, Lars Volker, Zoram Thanga, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10847 to look at the new patch set (#11). Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. IMPALA-6271: Impala daemon should log a message when it's being shut down Currently Impalad does not log any message when SIGTERM is sent to impalad to terminate or to do a graceful shut down. This change logs a message when SIGTERM is received by impalad/catalogd/statestored. This logging will assist in debugging the issues seen in the field where impalad was not gracefully shut down (some other signal was generated that led to impalad/catalogd/statestored crash). Testing: --- a) Used kill to send signals to impalad/catalogd/statestored `kill -s SIGTERM ` and see the log message is being logged in impalad/catalogd/statestored.INFO. b) Ran test_breakpad.py to check that existing breakpad functionalities are not affected. c) Ran exhaustive tests without failure. d) Added new test in test_breakpad.py to handle SIGTERM for impalad/statestored/catalogd. Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 --- M be/src/catalog/catalogd-main.cc M be/src/common/init.cc M be/src/util/minidump.cc M tests/custom_cluster/test_breakpad.py 4 files changed, 60 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10847/11 -- 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: newpatchset Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 11 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6373: Allow primitive type widening on parquet tables
Zoltan Borok-Nagy has posted comments on this change. (
http://gerrit.cloudera.org:8080/11268 )
Change subject: IMPALA-6373: Allow primitive type widening on parquet tables
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11268/2/tests/query_test/test_scanners.py
File tests/query_test/test_scanners.py:
http://gerrit.cloudera.org:8080/#/c/11268/2/tests/query_test/test_scanners.py@758
PS2, Line 758: TABLE_NAME = "primitive_type_widening"
: self.client.execute("""CREATE TABLE {0}.{1} (a smallint, b
int, c bigint, d double,
: e int, f bigint, g double, h int, i double, j double)
: STORED AS PARQUET""".format(unique_database, TABLE_NAME))
: table_loc = get_fs_path(
: "/test-warehouse/{0}.db/{1}".format(unique_database,
TABLE_NAME))
: check_call(["hdfs", "dfs", "-copyFromLocal",
os.environ["IMPALA_HOME"] +
: "/testdata/data/{0}.parquet".format(TABLE_NAME),
table_loc])
It seems https://gerrit.cloudera.org/#/c/11127/ is about to go in, so you can
use the utility function create_table_and_copy_files().
--
To view, visit http://gerrit.cloudera.org:8080/11268
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If93394b035c64cf6fc5f37b54d29c034cc1f86e4
Gerrit-Change-Number: 11268
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Wed, 22 Aug 2018 14:53:14 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7412: width bucket() function overflows too easily
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11282 ) Change subject: IMPALA-7412: width_bucket() function overflows too easily .. Patch Set 2: Yeah, I've ran it around 300 times without seeing any errors. Still, fingers crossed... :) -- To view, visit http://gerrit.cloudera.org:8080/11282 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I68262698144029ef7f54e027e586eaf105f36ab3 Gerrit-Change-Number: 11282 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Wed, 22 Aug 2018 14:46:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7412: width bucket() function overflows too easily
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11282 ) Change subject: IMPALA-7412: width_bucket() function overflows too easily .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3069/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11282 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I68262698144029ef7f54e027e586eaf105f36ab3 Gerrit-Change-Number: 11282 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Wed, 22 Aug 2018 14:43:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7412: width bucket() function overflows too easily
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11282 ) Change subject: IMPALA-7412: width_bucket() function overflows too easily .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11282 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I68262698144029ef7f54e027e586eaf105f36ab3 Gerrit-Change-Number: 11282 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Wed, 22 Aug 2018 14:43:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11183 ) Change subject: IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions .. Patch Set 10: The test has hit IMPALA-7459 - the commit that causes it was reverted, so a rebase should help. -- To view, visit http://gerrit.cloudera.org:8080/11183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I572b5876b979ddae58165bd40d5b008ce9d7a4aa Gerrit-Change-Number: 11183 Gerrit-PatchSet: 10 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 22 Aug 2018 14:43:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6709: Simplify tests that copy local files to tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11127 ) Change subject: IMPALA-6709: Simplify tests that copy local files to tables .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3068/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11127 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie00a4561825facf8abe2e8e74a6b6e93194f416f Gerrit-Change-Number: 11127 Gerrit-PatchSet: 5 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 22 Aug 2018 14:42:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6709: Simplify tests that copy local files to tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11127 ) Change subject: IMPALA-6709: Simplify tests that copy local files to tables .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11127 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie00a4561825facf8abe2e8e74a6b6e93194f416f Gerrit-Change-Number: 11127 Gerrit-PatchSet: 5 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 22 Aug 2018 14:42:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11183 ) Change subject: IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions .. Patch Set 10: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3067/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I572b5876b979ddae58165bd40d5b008ce9d7a4aa Gerrit-Change-Number: 11183 Gerrit-PatchSet: 10 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 22 Aug 2018 14:41:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6709: Simplify tests that copy local files to tables
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11127 ) Change subject: IMPALA-6709: Simplify tests that copy local files to tables .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11127 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie00a4561825facf8abe2e8e74a6b6e93194f416f Gerrit-Change-Number: 11127 Gerrit-PatchSet: 4 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 22 Aug 2018 14:40:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-376: add built-in functions for parsing JSON
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10950 ) Change subject: IMPALA-376: add built-in functions for parsing JSON .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/449/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/10950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a9d3598cb3beca0865a7edb094f3a5b602dbd2f Gerrit-Change-Number: 10950 Gerrit-PatchSet: 11 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 22 Aug 2018 14:17:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-376: add built-in functions for parsing JSON
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10950 ) Change subject: IMPALA-376: add built-in functions for parsing JSON .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/448/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/10950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a9d3598cb3beca0865a7edb094f3a5b602dbd2f Gerrit-Change-Number: 10950 Gerrit-PatchSet: 10 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 22 Aug 2018 14:10:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11183 ) Change subject: IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions .. Patch Set 9: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3066/ -- To view, visit http://gerrit.cloudera.org:8080/11183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I572b5876b979ddae58165bd40d5b008ce9d7a4aa Gerrit-Change-Number: 11183 Gerrit-PatchSet: 9 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 22 Aug 2018 14:01:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-376: add built-in functions for parsing JSON
Hello Zoltan Borok-Nagy, Attila Jeges, Todd Lipcon, Tim Armstrong, Impala
Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10950
to look at the new patch set (#11).
Change subject: IMPALA-376: add built-in functions for parsing JSON
..
IMPALA-376: add built-in functions for parsing JSON
This patch implements the same function as Hive UDF get_json_object.
We reuse RapidJson to parse the json string. In order to track the
memory used in RapidJson, we wrap FunctionContext into an allocator.
get_json_object accepts two parameters: a json string and a selector
(json path). We parse the json string into a Document tree and then
perform BFS according to the selector. For example, to process
get_json_object('[{\"a\":1}, {\"a\":2}, {\"a\":3}]', '$[*].a'),
we first perform '$[*]' to extract all the items in the root array.
Then we get a queue consists of {a:1},{a:2},{a:3} and perform '.a'
selector on all values in the queue. The final results is 1,2,3 in the
queue. As there're multiple results, they should be encapsulated into
an array. The output results is a string of '[1,2,3]'.
More examples can be found in expr-test.cc.
Test:
* Add unit tests in expr-test
* Add e2e tests in exprs.test
Change-Id: I6a9d3598cb3beca0865a7edb094f3a5b602dbd2f
---
M be/src/exprs/CMakeLists.txt
M be/src/exprs/expr-test.cc
M be/src/exprs/string-functions-ir.cc
A be/src/exprs/string-functions.cc
M be/src/exprs/string-functions.h
M be/src/util/string-util.cc
M be/src/util/string-util.h
M common/function-registry/impala_functions.py
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
9 files changed, 432 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/10950/11
--
To view, visit http://gerrit.cloudera.org:8080/10950
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6a9d3598cb3beca0865a7edb094f3a5b602dbd2f
Gerrit-Change-Number: 10950
Gerrit-PatchSet: 11
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-376: add built-in functions for parsing JSON
Quanlong Huang has posted comments on this change. (
http://gerrit.cloudera.org:8080/10950 )
Change subject: IMPALA-376: add built-in functions for parsing JSON
..
Patch Set 10:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10950/9/be/src/exprs/string-functions.cc
File be/src/exprs/string-functions.cc:
http://gerrit.cloudera.org:8080/#/c/10950/9/be/src/exprs/string-functions.cc@51
PS9, Line 51: JsonUdfAllocator(FunctionContext* ctx): ctx_(ctx) {}
> I'm ok since this patch is for 3.x. We're still using the 2.x branch in pro
Done
http://gerrit.cloudera.org:8080/#/c/10950/9/be/src/exprs/string-functions.cc@96
PS9, Line 96: in()
> yeah, 'pos_' is also a good name for it.
Done
http://gerrit.cloudera.org:8080/#/c/10950/9/be/src/exprs/string-functions.cc@194
PS9, Line 194:
> Sure. Will add test coverage for this.
Done
http://gerrit.cloudera.org:8080/#/c/10950/9/be/src/exprs/string-functions.cc@195
PS9, Line 195: ocess numb
> Good point! I'll add more error messages.
Done
http://gerrit.cloudera.org:8080/#/c/10950/9/be/src/util/string-util.h
File be/src/util/string-util.h:
http://gerrit.cloudera.org:8080/#/c/10950/9/be/src/util/string-util.h@53
PS9, Line 53: FindEndOfIdentifi
> Oh I see, I still think it's not a good name for either functions unless it
Done
--
To view, visit http://gerrit.cloudera.org:8080/10950
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a9d3598cb3beca0865a7edb094f3a5b602dbd2f
Gerrit-Change-Number: 10950
Gerrit-PatchSet: 10
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Wed, 22 Aug 2018 13:38:21 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-376: add built-in functions for parsing JSON
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10950 ) Change subject: IMPALA-376: add built-in functions for parsing JSON .. Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/10950/10/common/function-registry/impala_functions.py File common/function-registry/impala_functions.py: http://gerrit.cloudera.org:8080/#/c/10950/10/common/function-registry/impala_functions.py@514 PS10, Line 514: b flake8: E501 line too long (98 > 90 characters) -- To view, visit http://gerrit.cloudera.org:8080/10950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a9d3598cb3beca0865a7edb094f3a5b602dbd2f Gerrit-Change-Number: 10950 Gerrit-PatchSet: 10 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 22 Aug 2018 13:37:23 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-376: add built-in functions for parsing JSON
Hello Zoltan Borok-Nagy, Attila Jeges, Todd Lipcon, Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10950
to look at the new patch set (#10).
Change subject: IMPALA-376: add built-in functions for parsing JSON
..
IMPALA-376: add built-in functions for parsing JSON
This patch implements the same function as Hive UDF get_json_object.
We reuse RapidJson to parse the json string. In order to track the
memory used in RapidJson, we wrap FunctionContext into an allocator.
get_json_object accepts two parameters: a json string and a selector
(json path). We parse the json string into a Document tree and then
perform BFS according to the selector. For example, to process
get_json_object('[{\"a\":1}, {\"a\":2}, {\"a\":3}]', '$[*].a'),
we first perform '$[*]' to extract all the items in the root array.
Then we get a queue consists of {a:1},{a:2},{a:3} and perform '.a'
selector on all values in the queue. The final results is 1,2,3 in the
queue. As there're multiple results, they should be encapsulated into
an array. The output results is a string of '[1,2,3]'.
More examples can be found in expr-test.cc.
Test:
* Add unit tests in expr-test
* Add e2e tests in exprs.test
Change-Id: I6a9d3598cb3beca0865a7edb094f3a5b602dbd2f
---
M be/src/exprs/CMakeLists.txt
M be/src/exprs/expr-test.cc
M be/src/exprs/string-functions-ir.cc
A be/src/exprs/string-functions.cc
M be/src/exprs/string-functions.h
M be/src/util/string-util.cc
M be/src/util/string-util.h
M common/function-registry/impala_functions.py
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
9 files changed, 431 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/10950/10
--
To view, visit http://gerrit.cloudera.org:8080/10950
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6a9d3598cb3beca0865a7edb094f3a5b602dbd2f
Gerrit-Change-Number: 10950
Gerrit-PatchSet: 10
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11183 ) Change subject: IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/447/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I572b5876b979ddae58165bd40d5b008ce9d7a4aa Gerrit-Change-Number: 11183 Gerrit-PatchSet: 9 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 22 Aug 2018 11:22:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/11183 )
Change subject: IMPALA-7417: Speed up sub-second unix time->TimestampValue
conversions
..
Patch Set 9: -Code-Review
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11183/8/be/src/runtime/timestamp-value.inline.h
File be/src/runtime/timestamp-value.inline.h:
http://gerrit.cloudera.org:8080/#/c/11183/8/be/src/runtime/timestamp-value.inline.h@67
PS8, Line 67: const Timezone& local_tz) {
The test failure in patch set 8 was caused by hitting this DCHECK for very
small (<<1 nanosec) negative unix times. floor() truncated these values to -1,
but unix_time - unix_time_whole was 1.0 (instead of 1-very_small_fractional
_number) due the to the decreased preciison at 1.0 compared to 0.0. This would
also be a problem when adding the sub-second value to time_, because adding a
whole sec can step to a different day/timezone rule.
The solution was to use rounding to get seconds, calculate nanoseconds from
unix_time-unix_time_whole without any expectations, and give the results to
FromUnixTimeNanos() , which can handle negative or >=1 sec values. This is
probably a bit sub-optimal, but I do not want to put too much effort to this
function. A benchmark was added that shows that this function is still faster
than it used to be.
--
To view, visit http://gerrit.cloudera.org:8080/11183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I572b5876b979ddae58165bd40d5b008ce9d7a4aa
Gerrit-Change-Number: 11183
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Wed, 22 Aug 2018 11:12:14 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11183 ) Change subject: IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3066/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I572b5876b979ddae58165bd40d5b008ce9d7a4aa Gerrit-Change-Number: 11183 Gerrit-PatchSet: 9 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 22 Aug 2018 10:49:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions
Hello Attila Jeges, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11183 to look at the new patch set (#9). Change subject: IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions .. IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions Impala used to convert from sub-second unix time to TimestampValue (which is split do date_ and time_ similarly to boost::posix_time::ptime) by first splitting the input into seconds and sub-seconds part, converting the seconds part with boost::posix_time::from_time_t(), and then adding the sub-seconds part to this timestamp. Different tricks are used to speed up different functions: - UTC functions that expect a single integer as input can split it into date_ and time_ directly. - Non-UTC functions need seconds for timezone conversion, because CCTZ expects time points as seconds. These were optimized by adding the subsecond part to time_ instead of adding it to a ptime. This can be done safely because the sub-second part is between [0, 1 sec), so it cannot overflow into a different day or timezone. The main motivation is IMPALA-5050: "Add support to read TIMESTAMP_MILLIS and TIMESTAMP_MICROS to the parquet scanner" - reading these types will run micro/milli->TimestampValue conversion for every row. Other changes: - TimestampValue::UtcFromUnixTimeMillis was added - currently this is only used in tests but it will be useful for IMPALA-5050 - Some functions were moved from .h to .inline.h. - FromUnixTimeMicros was changed to do the utc->local conversion depending on flag use_local_tz_for_unix_timestamp_conversions to be consistent with other similar functions. This should have no visible side-effects. - When a result mismatch is detected in convert-timestamp-benchmark.cc it now prints non-equal values. - Benchmarks were added for micro + nano conversions. Note that only single threaded benchmarks were added because I do not expect any difference in thw multi threaded case. - DCHECKs were added to TimeStampValue::Validate to ensure that time_ is between [0, 24 hour). Testing: - timestamp-test.cc was extended to give better coverage for sub-second conversions. Edge cases were already covered pretty well. Change-Id: I572b5876b979ddae58165bd40d5b008ce9d7a4aa --- M be/src/benchmarks/convert-timestamp-benchmark.cc M be/src/exec/data-source-scan-node.cc M be/src/exec/hdfs-orc-scanner.cc M be/src/exprs/decimal-operators-ir.cc M be/src/exprs/expr-test.cc M be/src/runtime/timestamp-test.cc M be/src/runtime/timestamp-value.cc M be/src/runtime/timestamp-value.h M be/src/runtime/timestamp-value.inline.h 9 files changed, 365 insertions(+), 104 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/11183/9 -- To view, visit http://gerrit.cloudera.org:8080/11183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I572b5876b979ddae58165bd40d5b008ce9d7a4aa Gerrit-Change-Number: 11183 Gerrit-PatchSet: 9 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-6373: Allow primitive type widening on parquet tables
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11268 ) Change subject: IMPALA-6373: Allow primitive type widening on parquet tables .. Patch Set 2: Code-Review+1 (1 comment) lgtm! http://gerrit.cloudera.org:8080/#/c/11268/2/be/src/exec/parquet-plain-test.cc File be/src/exec/parquet-plain-test.cc: http://gerrit.cloudera.org:8080/#/c/11268/2/be/src/exec/parquet-plain-test.cc@138 PS2, Line 138: TestType nit: I'd choose a different name, e.g. 'TestTypeWidening' -- To view, visit http://gerrit.cloudera.org:8080/11268 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If93394b035c64cf6fc5f37b54d29c034cc1f86e4 Gerrit-Change-Number: 11268 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 22 Aug 2018 10:38:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7466: Improve readability of describe authorization tests
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11278 ) Change subject: IMPALA-7466: Improve readability of describe authorization tests .. IMPALA-7466: Improve readability of describe authorization tests This patch improves the readability and usability of the describe authorization tests. It removes the ambiguity of the function parameters required for validating the describe output. Testing: - Refactored describe authorization tests - Ran AuthorizationStmtTests Change-Id: I6244b6dbafbd3827d488588a16e66dbf15d0e115 Reviewed-on: http://gerrit.cloudera.org:8080/11278 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java 1 file changed, 115 insertions(+), 62 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11278 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I6244b6dbafbd3827d488588a16e66dbf15d0e115 Gerrit-Change-Number: 11278 Gerrit-PatchSet: 5 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7466: Improve readability of describe authorization tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11278 ) Change subject: IMPALA-7466: Improve readability of describe authorization tests .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11278 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6244b6dbafbd3827d488588a16e66dbf15d0e115 Gerrit-Change-Number: 11278 Gerrit-PatchSet: 4 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 22 Aug 2018 09:06:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7469. Invalidate LocalCatalog cache based on topic updates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11280 ) Change subject: IMPALA-7469. Invalidate LocalCatalog cache based on topic updates .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11280 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I615f9e6bd167b36cd8d93da59426dd6813ae4984 Gerrit-Change-Number: 11280 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 22 Aug 2018 08:16:48 + Gerrit-HasComments: No
