[Impala-ASF-CR] IMPALA-7698: Add centos support to bootstrap system.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/11731 ) Change subject: IMPALA-7698: Add centos support to bootstrap_system. .. Patch Set 4: Awesome! Good to know the ORC issue is resolved. -- To view, visit http://gerrit.cloudera.org:8080/11731 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id54294d7607f51de87a9de373dcfc4a33f4bedf5 Gerrit-Change-Number: 11731 Gerrit-PatchSet: 4 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Thu, 25 Oct 2018 04:53:10 + Gerrit-HasComments: No
[Impala-ASF-CR] test-with-docker: decrease image size by "de-duping" HDFS.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11782 ) Change subject: test-with-docker: decrease image size by "de-duping" HDFS. .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1153/ : 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/11782 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a13910ba5e873c31893dbb810a8410547adb2f1 Gerrit-Change-Number: 11782 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 25 Oct 2018 04:28:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5004: Switch to sorting node for large TopN queries
Vuk Ercegovac has posted comments on this change. (
http://gerrit.cloudera.org:8080/11698 )
Change subject: IMPALA-5004: Switch to sorting node for large TopN queries
..
Patch Set 5: Code-Review+1
(2 comments)
one question about tests, but otherwise, lgtm. will let others take another
pass.
http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@334
PS4, Line 334: then it is likely we are sor
> Yeah, but that doesn't seem ideal. So I the most recent patch allows settin
makes sense, that seems easier.
http://gerrit.cloudera.org:8080/#/c/11698/5/fe/src/test/java/org/apache/impala/planner/PlannerTest.java
File fe/src/test/java/org/apache/impala/planner/PlannerTest.java:
http://gerrit.cloudera.org:8080/#/c/11698/5/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@214
PS5, Line 214: public void testTopNBytesLimitDisabled() {
perhaps merge this with the test on L195? settings are the same so unclear why
these are separated.
--
To view, visit http://gerrit.cloudera.org:8080/11698
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34c9db33c9302b55e9978f53f9c7061f2806c8a9
Gerrit-Change-Number: 11698
Gerrit-PatchSet: 5
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Sahil Takiar
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Thu, 25 Oct 2018 04:20:32 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] test-with-docker: allow built images to be used with "docker run" easily.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11781 ) Change subject: test-with-docker: allow built images to be used with "docker run" easily. .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1152/ : 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/11781 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib8d6a28d4cb4ab019cd72415024b23374a6d9e2f Gerrit-Change-Number: 11781 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Laszlo Gaal Gerrit-Comment-Date: Thu, 25 Oct 2018 04:20:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7698: Add centos support to bootstrap system.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11731 ) Change subject: IMPALA-7698: Add centos support to bootstrap_system. .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1151/ : 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/11731 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id54294d7607f51de87a9de373dcfc4a33f4bedf5 Gerrit-Change-Number: 11731 Gerrit-PatchSet: 4 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Thu, 25 Oct 2018 04:13:13 + Gerrit-HasComments: No
[Impala-ASF-CR] test-with-docker: decrease image size by "de-duping" HDFS.
Philip Zeyliger has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11782 Change subject: test-with-docker: decrease image size by "de-duping" HDFS. .. test-with-docker: decrease image size by "de-duping" HDFS. This change shaves about 20GB of the (uncompressed) Docker image for test-with-docker, taking it from ~60GB to ~40GB. Compressed, the image ends up being about 14GB. To do this, we cheat: HDFS represents every block three times, so we have three copies of every block. Before committing the image, we simply hard-link the blocks together, which happens to work. It's an implementation detail of HDFS that these blocks aren't, say, appended to, but I think the trade-off in time and disk space saved is worth it. Because the image is smaller, it takes less time to "docker commit" it. Change-Id: I4a13910ba5e873c31893dbb810a8410547adb2f1 --- M docker/entrypoint.sh 1 file changed, 18 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/11782/1 -- To view, visit http://gerrit.cloudera.org:8080/11782 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4a13910ba5e873c31893dbb810a8410547adb2f1 Gerrit-Change-Number: 11782 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger
[Impala-ASF-CR] test-with-docker: allow built images to be used with "docker run" easily.
Philip Zeyliger has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11781 Change subject: test-with-docker: allow built images to be used with "docker run" easily. .. test-with-docker: allow built images to be used with "docker run" easily. Configures the built container to enter into a script that starts the minicluster. As a result, "docker run -ti " will launch the user into a shell with the Impala minicluster and the impala development cluster running. To handle cases where users don't specify --privileged, we skip Kudu if it NTP seems unavailable. Change-Id: Ib8d6a28d4cb4ab019cd72415024b23374a6d9e2f --- M docker/entrypoint.sh M docker/test-with-docker.py 2 files changed, 58 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/11781/1 -- To view, visit http://gerrit.cloudera.org:8080/11781 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib8d6a28d4cb4ab019cd72415024b23374a6d9e2f Gerrit-Change-Number: 11781 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7698: Add centos support to bootstrap system.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11731 ) Change subject: IMPALA-7698: Add centos support to bootstrap_system. .. Patch Set 4: (6 comments) Good news--the ORC / timestamp stuff is figured out. Attila--this changes a line you and I talked about in https://gerrit.cloudera.org/#/c/11730/ back to readlink() from realpath(). Definitely a curiosity I would not have predicted! I'm comfortable carrying the +2, but I'll let this linger for a bit to see if anyone wants to make more comments. I'm waiting on the review for the parent change, anyway. Thanks Quanlong for hinting me to the conclusion of the timezone stuff. The update there is in the commit message. http://gerrit.cloudera.org:8080/#/c/11731/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11731/1//COMMIT_MSG@29 PS1, Line 29: : With Quanlong's help, I learned what was happening. test-with-docker was : translati > The error is due to hive writes a wrong timezone name into the ORC files. I Thank you! You lead me on the right path. The updated commit message explains what happens, but basically the tz database for Java and Linux are different here, and Java is missing some entries. http://gerrit.cloudera.org:8080/#/c/11731/2/bin/bootstrap_system.sh File bin/bootstrap_system.sh: http://gerrit.cloudera.org:8080/#/c/11731/2/bin/bootstrap_system.sh@145 PS2, Line 145: https://www-us.apache.org/dist/ant/binaries/apache-ant-1.9.13-bin.tar.gz > line too long (93 > 90) ignored. http://gerrit.cloudera.org:8080/#/c/11731/2/bin/bootstrap_system.sh@147 PS2, Line 147: redhat sha512sum -c - <<< 'c8321aa223f70d7e64d3d0274263000cfffb46fbea61488534e26f9f0245d99e9872d0888e35cd3274416392a13f80c748c07750caaeffa5f9cae1220020715f apache-ant-1.9.13-bin.tar.gz' > line too long (187 > 90) ignored http://gerrit.cloudera.org:8080/#/c/11731/2/bin/bootstrap_system.sh@148 PS2, Line 148: redhat sudo tar -C /usr/local -xzf apache-maven-3.5.4-bin.tar.gz > line too long (186 > 90) ignored http://gerrit.cloudera.org:8080/#/c/11731/2/bin/bootstrap_system.sh@196 PS2, Line 196: # widely. > "localhost" Done http://gerrit.cloudera.org:8080/#/c/11731/3/bin/bootstrap_system.sh File bin/bootstrap_system.sh: http://gerrit.cloudera.org:8080/#/c/11731/3/bin/bootstrap_system.sh@139 PS3, Line 139: # Clean up yum caches : redhat sudo yum clean all > Since you install EPEL above, you may want to install these packages for Ce I removed this entirely. They don't seem to be strictly necessary at the moment. -- To view, visit http://gerrit.cloudera.org:8080/11731 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id54294d7607f51de87a9de373dcfc4a33f4bedf5 Gerrit-Change-Number: 11731 Gerrit-PatchSet: 4 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Thu, 25 Oct 2018 03:39:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7698: Add centos support to bootstrap system.
Hello Quanlong Huang, Laszlo Gaal, Jim Apple, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11731 to look at the new patch set (#4). Change subject: IMPALA-7698: Add centos support to bootstrap_system. .. IMPALA-7698: Add centos support to bootstrap_system. Largely, the changes involve conditionalizing some invocations to account for differences between RH and Ubuntu. The trickiest bits were timezone-related test errors (see below), postgresql permissions (need to accept md5 passwords from localhost) and default ulimits (1024 user processes/threads is not enough). To test this, I built using test-with-docker. In additional to the ulimit issue, I ran into the fact that /tmp needed 1777 permissions for the postgresql socket, and entrypoint.sh had a few places that needed special cases. At the moment, the data load ran fine, as did most of the tests. I observed a test that relied on a python2.7-ism fail, which is part of the point of this. In the course of development, I encountered a handful of tests fail with "Encounter parse error: failed to open /usr/share/zoneinfo/GMT-08:00 - No such file or directory.", which was reproduced as follows: [localhost:21000] default> use functional_orc_def; select * from alltypes; ... WARNINGS: Encounter parse error: failed to open /usr/share/zoneinfo/GMT-08:00 - No such file or directory. With Quanlong's help, I learned what was happening. test-with-docker was translating my time zone (America/Los_Angeles) to US/Pacific-New, because realpath(/etc/localtime) = US/Pacific-New. This timezone exists in centos:6, so that wasn't a problem. However, this timezone does not exist in the package "tzdata-java", which is the copy of the timezone information used by Java. (There are bugs here that may have been fixed in centos:7.) As a result, when ORC asks (by using TimeZone.getDefault().getID()) the JDK (src/solaris/native/java/util/TimeZone_md.c) for the default timezone, it can't find the same name as /etc/localtime points to in its repository and defaults to "GMT-08:00". This string then gets written into the ORC files generated by Hive as part of data load, and then the C++ library can't read them. This is fixed by changing "realpath" to "readlink" in test-with-docker.py. centos:7 is not addressed by this change. The move to systemd makes "service sshd start" (and the same for postgresql) not work, and additional care needs to be done to work around that. This change is a joint effort with Laszlo Gaal. Change-Id: Id54294d7607f51de87a9de373dcfc4a33f4bedf5 --- M bin/bootstrap_system.sh M docker/entrypoint.sh M docker/test-with-docker.py 3 files changed, 167 insertions(+), 61 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/11731/4 -- To view, visit http://gerrit.cloudera.org:8080/11731 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id54294d7607f51de87a9de373dcfc4a33f4bedf5 Gerrit-Change-Number: 11731 Gerrit-PatchSet: 4 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-6323 Allow constant analytic window expressions.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11556 ) Change subject: IMPALA-6323 Allow constant analytic window expressions. .. Patch Set 6: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3350/ -- To view, visit http://gerrit.cloudera.org:8080/11556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf129026c45120e9470df601268863634037908c Gerrit-Change-Number: 11556 Gerrit-PatchSet: 6 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 25 Oct 2018 03:07:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. Patch Set 20: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1149/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 20 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 25 Oct 2018 03:00:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351: Add estimates to Exchange node
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11692 ) Change subject: IMPALA-7351: Add estimates to Exchange node .. Patch Set 5: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3349/ -- To view, visit http://gerrit.cloudera.org:8080/11692 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5b577f9511abc48b992e814d50bba4959f23f7fd Gerrit-Change-Number: 11692 Gerrit-PatchSet: 5 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 25 Oct 2018 02:59:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4063: Merge report of query fragment instances per executor
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11615 ) Change subject: IMPALA-4063: Merge report of query fragment instances per executor .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1150/ : 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/11615 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f95e026ba05631f33f48ce32da6db39c6f421fa Gerrit-Change-Number: 11615 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 25 Oct 2018 02:53:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5004: Switch to sorting node for large TopN queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11698 ) Change subject: IMPALA-5004: Switch to sorting node for large TopN queries .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1148/ : 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/11698 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I34c9db33c9302b55e9978f53f9c7061f2806c8a9 Gerrit-Change-Number: 11698 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 25 Oct 2018 02:45:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4063: Merge report of query fragment instances per executor
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11615 ) Change subject: IMPALA-4063: Merge report of query fragment instances per executor .. Patch Set 6: Code-Review+1 Carry +1 -- To view, visit http://gerrit.cloudera.org:8080/11615 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f95e026ba05631f33f48ce32da6db39c6f421fa Gerrit-Change-Number: 11615 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 25 Oct 2018 02:18:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6661 Make NaN values equal for grouping purposes.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11535 ) Change subject: IMPALA-6661 Make NaN values equal for grouping purposes. .. Patch Set 18: (1 comment) http://gerrit.cloudera.org:8080/#/c/11535/18/testdata/workloads/functional-query/queries/QueryTest/aggregation.test File testdata/workloads/functional-query/queries/QueryTest/aggregation.test: http://gerrit.cloudera.org:8080/#/c/11535/18/testdata/workloads/functional-query/queries/QueryTest/aggregation.test@1402 PS18, Line 1402: bigint, float You forgot to update result type for the extra column in the select list. -- To view, visit http://gerrit.cloudera.org:8080/11535 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I996c4a2e1934fd887046ed0c55457b7285375086 Gerrit-Change-Number: 11535 Gerrit-PatchSet: 18 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 25 Oct 2018 02:17:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4063: Merge report of query fragment instances per executor
Michael Ho has posted comments on this change. (
http://gerrit.cloudera.org:8080/11615 )
Change subject: IMPALA-4063: Merge report of query fragment instances per
executor
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11615/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11615/5//COMMIT_MSG@14
PS5, Line 14: coordinator backend states. In addition, due to the lack
: of coordinator between query fragment instances, a query may end
: without collecting the profiles from all fragment instances when
: one of them hits an error before some other fragment instance
manages
: to finish calling Prepare(), leading to missing
> Does this mean that we've fixed the problem in IMPALA-7148 and can re-enabl
Yes. Uncommented some of the tests in bloom_filters.test.
http://gerrit.cloudera.org:8080/#/c/11615/1/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:
http://gerrit.cloudera.org:8080/#/c/11615/1/be/src/runtime/query-state.cc@236
PS1, Line 236: if (overall_status_.IsCancelled()) {
> Right, I was thinking that CANCELLED_INTERNALLY shouldn't leak out of fragm
Keep it as-is for now.
http://gerrit.cloudera.org:8080/#/c/11615/5/testdata/workloads/functional-query/queries/QueryTest/udf-non-deterministic.test
File
testdata/workloads/functional-query/queries/QueryTest/udf-non-deterministic.test:
http://gerrit.cloudera.org:8080/#/c/11615/5/testdata/workloads/functional-query/queries/QueryTest/udf-non-deterministic.test@18
PS5, Line 18: QUERY
> This test doesn't really have anything to do with nondeterminism. Would you
Done
--
To view, visit http://gerrit.cloudera.org:8080/11615
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f95e026ba05631f33f48ce32da6db39c6f421fa
Gerrit-Change-Number: 11615
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Thomas Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Thu, 25 Oct 2018 02:13:45 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5004: Switch to sorting node for large TopN queries
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/11698 ) Change subject: IMPALA-5004: Switch to sorting node for large TopN queries .. Patch Set 5: (8 comments) Addressed comments and made a few more changes: * Changed query-options.cc to use ParseMemValue to parse the value of topn_bytes_limit since it is expressed in bytes * Re-organized the tests slightly into three distinct tests: (1) test with the default value of topn_bytes_limit, (2) test with a small value of topn_bytes_limit, and (3) test with topn_bytes_limit = 0 which disables the changes * A value of 0 or -1 for topn_bytes_limit now disables the changes http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/analysis/SortInfo.java File fe/src/main/java/org/apache/impala/analysis/SortInfo.java: http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/analysis/SortInfo.java@236 PS4, Line 236: th > the Done http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/analysis/SortInfo.java@238 PS4, Line 238:* average tuple serialized size. 'cardinality' is the cardinality of the TopN > offset is not mentioned so either include it or remove it since the arg des The "estimated # of rows in memory" refers to the sum of the cardinality and offset. I don't explicitly mention the offset, because one can infer from the method body that "estimated # of rows in memory" = "cardinality + offset". If you still think the formula is redundant, I can remove it. http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/analysis/SortInfo.java@240 PS4, Line 240: > Even though this style of commenting is widely used, Impala tries not to us Done http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java: http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@325 PS4, Line 325: rdinality = > as pointed out in the header for SortInfo, the abstractions are not quite r Moved the tuple handling into estimateTopNMaterializedSize. I couldn't make it static since it needs to reference the tuple descriptors of the current SortInfo. estimateTopNMaterializedSize is called by the SingleNodePlanner and the SortNode now. http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@326 PS4, Line 326: th.min(root.cardinalit > this looks correct and consistent with the avgRowSize_ computation for Plan Done http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@334 PS4, Line 334: then it is likely we are sor > if we get this wrong, perhaps due to a mistaken many-to-many join cardinali Yeah, but that doesn't seem ideal. So I the most recent patch allows settings topn_bytes_limit to 0 or -1 which disables the decision entirely. http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/planner/SortNode.java File fe/src/main/java/org/apache/impala/planner/SortNode.java: http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/planner/SortNode.java@260 PS4, Line 260: nodeResourceProfile_ = ResourceProfile.noReservation( > where'd the return go? pls look into why tests did not get bothered by this I didn't run the full suite of tests, only the topn functional-query tests, which don't validate memory estimates by default. http://gerrit.cloudera.org:8080/#/c/11698/4/testdata/workloads/functional-planner/queries/PlannerTest/topn-default-limit-bytes.test File testdata/workloads/functional-planner/queries/PlannerTest/topn-default-limit-bytes.test: http://gerrit.cloudera.org:8080/#/c/11698/4/testdata/workloads/functional-planner/queries/PlannerTest/topn-default-limit-bytes.test@1 PS4, Line 1: > seems to apply to the second test, not the first, so got confused by this. Done -- To view, visit http://gerrit.cloudera.org:8080/11698 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I34c9db33c9302b55e9978f53f9c7061f2806c8a9 Gerrit-Change-Number: 11698 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 25 Oct 2018 02:13:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. Patch Set 20: Code-Review+2 Carry +2 -- 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: 20 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 25 Oct 2018 02:12:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. Patch Set 19: (1 comment) http://gerrit.cloudera.org:8080/#/c/10855/19/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/19/be/src/runtime/query-state.cc@363 PS19, Line 363: LOG(DFATAL) << FromKuduStatus(sidecar_status, "Failed to add sidecar").GetDetail(); > line too long (91 > 90) Done -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 19 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 25 Oct 2018 02:12:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4063: Merge report of query fragment instances per executor
Hello Thomas Marshall, Tim Armstrong, Joe McDonnell, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11615 to look at the new patch set (#6). Change subject: IMPALA-4063: Merge report of query fragment instances per executor .. IMPALA-4063: Merge report of query fragment instances per executor Previously, each fragment instance executing on an executor will independently report its status to the coordinator periodically. This creates a huge amount of RPCs to the coordinator under highly concurrent workloads, causing lock contention in the coordinator's backend states when multiple fragment instances send them at the same time. In addition, due to the lack of coordination between query fragment instances, a query may end without collecting the profiles from all fragment instances when one of them hits an error before another fragment instance manages to finish Prepare(), leading to missing profiles for certain fragment instances. This change fixes the problem above by making a thread per QueryState (started by QueryExecMgr) to be responsible for periodically reporting the status and profiles of all fragment instances of a query running on a backend. As part of this refactoring, each query fragment instance will not report their errors individually. Instead, there is a cumulative status maintained per QueryState. It's set to the error status of the first fragment instance which hits an error or any general error (e.g. failure to start a thread) when starting fragment instances. With this change, the status reporting threads are also removed. Testing done: exhaustive tests This patch is based on a patch by Sailesh Mukil Change-Id: I5f95e026ba05631f33f48ce32da6db39c6f421fa --- M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-exec-mgr.h M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/control-service.cc M be/src/service/control-service.h M common/protobuf/control_service.proto M common/thrift/RuntimeProfile.thrift M testdata/workloads/functional-query/queries/QueryTest/bloom_filters.test A testdata/workloads/functional-query/queries/QueryTest/udf-no-expr-rewrite.test D testdata/workloads/functional-query/queries/QueryTest/udf-non-deterministic.test M testdata/workloads/functional-query/queries/QueryTest/udf.test M tests/query_test/test_udfs.py 21 files changed, 419 insertions(+), 473 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/11615/6 -- To view, visit http://gerrit.cloudera.org:8080/11615 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5f95e026ba05631f33f48ce32da6db39c6f421fa Gerrit-Change-Number: 11615 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Hello Thomas Marshall, Todd Lipcon, Tim Armstrong, Bikramjeet Vig, Impala Public Jenkins, Michal Ostrowski, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10855 to look at the new patch set (#20). Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC This change converts ReportExecStatus() RPC from thrift based RPC to KRPC. This is done in part of the preparation for fixing IMPALA-2990 as we can take advantage of TCP connection multiplexing in KRPC to avoid overwhelming the coordinator with too many connections by reducing the number of TCP connection to one for each executor. This patch also introduces a new service pool for all query execution control related RPCs in the future so that control commands from coordinators aren't blocked by long-running DataStream services' RPCs. To avoid unnecessary delays due to sharing the network connections between DataStream service and Control service, this change added the service name as part of the user credentials for the ConnectionId so each service will use a separate connection. The majority of this patch is mechanical conversion of some Thrift structures used in ReportExecStatus() RPC to Protobuf. Note that the runtime profile is still retained as a Thrift structure as Impala clients will still fetch query profiles using Thrift RPCs. This also avoids duplicating the serialization implementation in both Thrift and Protobuf for the runtime profile. The Thrift runtime profiles are serialized and sent as a sidecar in ReportExecStatus() RPC. This patch also fixes IMPALA-7241 which may lead to duplicated dml stats being applied. The fix is by adding a monotonically increasing version number for fragment instances' reports. The coordinator will ignore any report smaller than or equal to the version in the last report. Testing done: 1. Exhaustive build. 2. Added some targeted test cases for profile serialization failure and RPC retries/timeout. Change-Id: I7638583b433dcac066b87198e448743d90415ebe --- M be/src/benchmarks/expr-benchmark.cc M be/src/catalog/catalog-util.cc M be/src/common/global-flags.cc M be/src/exec/data-sink.cc M be/src/exec/data-sink.h M be/src/exec/hbase-table-sink.cc M be/src/exec/hdfs-parquet-table-writer.cc M be/src/exec/hdfs-parquet-table-writer.h M be/src/exec/hdfs-table-sink.cc M be/src/exec/hdfs-table-writer.cc M be/src/exec/hdfs-table-writer.h M be/src/rpc/CMakeLists.txt M be/src/rpc/jni-thrift-util.h M be/src/rpc/rpc-mgr-kerberized-test.cc M be/src/rpc/rpc-mgr-test.cc M be/src/rpc/rpc-mgr-test.h M be/src/rpc/rpc-mgr.h M be/src/rpc/thrift-util-test.cc M be/src/rpc/thrift-util.h M be/src/runtime/backend-client.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/dml-exec-state.cc M be/src/runtime/dml-exec-state.h M be/src/runtime/exec-env.cc M be/src/runtime/exec-env.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M be/src/runtime/krpc-data-stream-sender.cc M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/runtime/test-env.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/scheduler-test-util.cc M be/src/service/CMakeLists.txt M be/src/service/client-request-state.cc M be/src/service/client-request-state.h A be/src/service/control-service.cc A be/src/service/control-service.h M be/src/service/data-stream-service.cc M be/src/service/data-stream-service.h M be/src/service/impala-internal-service.cc M be/src/service/impala-internal-service.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/testutil/in-process-servers.cc M be/src/util/container-util.h A be/src/util/error-util-internal.h M be/src/util/error-util-test.cc M be/src/util/error-util.cc M be/src/util/error-util.h M be/src/util/runtime-profile.cc M be/src/util/uid-util.h M common/protobuf/CMakeLists.txt M common/protobuf/common.proto A common/protobuf/control_service.proto M common/protobuf/data_stream_service.proto M common/protobuf/row_batch.proto M common/protobuf/rpc_test.proto M common/thrift/ImpalaInternalService.thrift M tests/custom_cluster/test_rpc_timeout.py 65 files changed, 1,299 insertions(+), 769 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/10855/20 -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 20 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer:
[Impala-ASF-CR] IMPALA-5004: Switch to sorting node for large TopN queries
Hello Lars Volker, Paul Rogers, Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11698 to look at the new patch set (#5). Change subject: IMPALA-5004: Switch to sorting node for large TopN queries .. IMPALA-5004: Switch to sorting node for large TopN queries Adds a new query option 'topn_bytes_limit' that places a limit on the number of estimated bytes that a TopN operator can process. If the Impala planner estimates that a TopN operator will process more bytes than this limit, it will replace the TopN operator with a sort operator. Since the TopN operator cannot spill to disk, it has to buffer everything in memory. This can cause frequent OOM issues when running with a large limit + offset. Switching to a sort operator allows Impala to spill to disk. We prefer to use the TopN operator when possible as it has better performance than the sort operator for 'order by limit [offset]' queries. The default limit is set to 512MB and is based on micro-benchmarking the topn vs. sort operator for various limits (see the JIRA for full details). The default is set to an intentionally high value in order to avoid performance regressions. If the planner thinks the TopN operator will end up processing the entire input dataset, it falls back to a Sort operator. Since TopN will just end up sorting the entire dataset, there is no point in using it vs. the Sort operator (which has the advantage that it can spill to disk). Testing: * Added a new planner test to fuctional-planner/ to validate that 'topn_bytes_limit' properly switches between topn and sort operators. * Added a new planner test to functional-planner/ to validate that running an 'order by limit x' where x is the size of the input, triggers a sort instead of a TopN (even if the 'topn_bytes_limit' threshold has not been exceeded) Change-Id: I34c9db33c9302b55e9978f53f9c7061f2806c8a9 --- M be/src/service/query-options-test.cc M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M fe/src/main/java/org/apache/impala/analysis/SortInfo.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java M fe/src/main/java/org/apache/impala/planner/SortNode.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java A testdata/workloads/functional-planner/queries/PlannerTest/topn-bytes-limit-disabled.test A testdata/workloads/functional-planner/queries/PlannerTest/topn-bytes-limit-small.test A testdata/workloads/functional-planner/queries/PlannerTest/topn-bytes-limit.test 12 files changed, 250 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/11698/5 -- To view, visit http://gerrit.cloudera.org:8080/11698 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I34c9db33c9302b55e9978f53f9c7061f2806c8a9 Gerrit-Change-Number: 11698 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6661 Make NaN values equal for grouping purposes.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11535 ) Change subject: IMPALA-6661 Make NaN values equal for grouping purposes. .. Patch Set 18: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3348/ -- To view, visit http://gerrit.cloudera.org:8080/11535 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I996c4a2e1934fd887046ed0c55457b7285375086 Gerrit-Change-Number: 11535 Gerrit-PatchSet: 18 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 25 Oct 2018 01:39:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351: Add estimates to Exchange node
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11692 ) Change subject: IMPALA-7351: Add estimates to Exchange node .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1147/ : 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/11692 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5b577f9511abc48b992e814d50bba4959f23f7fd Gerrit-Change-Number: 11692 Gerrit-PatchSet: 4 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 24 Oct 2018 23:47:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6323 Allow constant analytic window expressions.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11556 ) Change subject: IMPALA-6323 Allow constant analytic window expressions. .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1146/ : 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/11556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf129026c45120e9470df601268863634037908c Gerrit-Change-Number: 11556 Gerrit-PatchSet: 6 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 24 Oct 2018 23:35:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7662: fix error race when scanner open fails
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11596 ) Change subject: IMPALA-7662: fix error race when scanner open fails .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1145/ : 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/11596 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45a61210ca7d057b048c77d9f2f2695ec450f19b Gerrit-Change-Number: 11596 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 24 Oct 2018 23:15:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6323 Allow constant analytic window expressions.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11556 ) Change subject: IMPALA-6323 Allow constant analytic window expressions. .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3350/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf129026c45120e9470df601268863634037908c Gerrit-Change-Number: 11556 Gerrit-PatchSet: 6 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 24 Oct 2018 23:13:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6323 Allow constant analytic window expressions.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11556 ) Change subject: IMPALA-6323 Allow constant analytic window expressions. .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf129026c45120e9470df601268863634037908c Gerrit-Change-Number: 11556 Gerrit-PatchSet: 6 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 24 Oct 2018 23:13:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351: Add estimates to Exchange node
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11692 ) Change subject: IMPALA-7351: Add estimates to Exchange node .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11692 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5b577f9511abc48b992e814d50bba4959f23f7fd Gerrit-Change-Number: 11692 Gerrit-PatchSet: 5 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 24 Oct 2018 23:07:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351: Add estimates to Exchange node
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11692 ) Change subject: IMPALA-7351: Add estimates to Exchange node .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3349/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11692 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5b577f9511abc48b992e814d50bba4959f23f7fd Gerrit-Change-Number: 11692 Gerrit-PatchSet: 5 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 24 Oct 2018 23:07:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351: Add estimates to Exchange node
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11692 ) Change subject: IMPALA-7351: Add estimates to Exchange node .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11692 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5b577f9511abc48b992e814d50bba4959f23f7fd Gerrit-Change-Number: 11692 Gerrit-PatchSet: 4 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 24 Oct 2018 23:06:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351: Add estimates to Exchange node
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/11692 to look at the new patch set (#4). Change subject: IMPALA-7351: Add estimates to Exchange node .. IMPALA-7351: Add estimates to Exchange node Added rough estimates for exchange node and a justification of the method in the in-line comments. Testing: Updated Planner tests. Change-Id: I5b577f9511abc48b992e814d50bba4959f23f7fd --- M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java M fe/src/main/java/org/apache/impala/planner/PlanNode.java M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test M testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test M testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test 11 files changed, 900 insertions(+), 811 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/92/11692/4 -- To view, visit http://gerrit.cloudera.org:8080/11692 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5b577f9511abc48b992e814d50bba4959f23f7fd Gerrit-Change-Number: 11692 Gerrit-PatchSet: 4 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6323 Allow constant analytic window expressions.
Michal Ostrowski has posted comments on this change. ( http://gerrit.cloudera.org:8080/11556 ) Change subject: IMPALA-6323 Allow constant analytic window expressions. .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/11556/5/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java File fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java: http://gerrit.cloudera.org:8080/#/c/11556/5/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java@308 PS5, Line 308: /** > spurious newline Done http://gerrit.cloudera.org:8080/#/c/11556/5/testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test File testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test: http://gerrit.cloudera.org:8080/#/c/11556/5/testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test@2473 PS5, Line 2473: Order by > order/ordering Done -- To view, visit http://gerrit.cloudera.org:8080/11556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf129026c45120e9470df601268863634037908c Gerrit-Change-Number: 11556 Gerrit-PatchSet: 6 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 24 Oct 2018 23:04:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6323 Allow constant analytic window expressions.
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/11556 to look at the new patch set (#6). Change subject: IMPALA-6323 Allow constant analytic window expressions. .. IMPALA-6323 Allow constant analytic window expressions. The constraint imposed by IMPALA-1354 was artificial. If there are constant "partition by" expressions, simply drop them, they are no-ops. Constant "order by" expressions can be ignored as well, though in effect they should be accounted for as null expressions in the backend, with the effect that combine all rows in the same window (i.e. no window breaks). Change-Id: Idf129026c45120e9470df601268863634037908c --- M be/src/exec/analytic-eval-node.cc M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test 5 files changed, 53 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/56/11556/6 -- To view, visit http://gerrit.cloudera.org:8080/11556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idf129026c45120e9470df601268863634037908c Gerrit-Change-Number: 11556 Gerrit-PatchSet: 6 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7662: fix error race when scanner open fails
Hello Pooja Nilangekar, Lars Volker, Csaba Ringhofer, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11596 to look at the new patch set (#8). Change subject: IMPALA-7662: fix error race when scanner open fails .. IMPALA-7662: fix error race when scanner open fails This is very similar to IMPALA-7335, except happens when 'progress_' is incremented in the call chain HdfsScanNode::ProcessSplit -> HdfsScanNodeBase::CreateAndOpenScanner() -> HdfsScanner::Close() The fix required restructuring the code so that SetDoneInternal() is called with the error *before* HdfsScanner::Close(). This required a refactoring because HdfsScanNodeBase doesn't actually know about SetDoneInternal(). My fix is to put the common logic between HdfsScanNode and HdfsScanNodeMt into a helper in HdfsScanNodeBase, then in HdfsScanNode, make sure to call SetDoneInternal() before closing the scanner. I also reworked HdfsScanNode::ProcessSplit() to handle error propagation internally. I think the joint responsibility between ProcessSplit() and its caller for handling errors made things harder than necessary. Testing: Added a debug action and test that reproduced the race before the fix. Change-Id: I45a61210ca7d057b048c77d9f2f2695ec450f19b --- M be/src/exec/exec-node.cc M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/hdfs-scan-node-base.h M be/src/exec/hdfs-scan-node-mt.cc M be/src/exec/hdfs-scan-node-mt.h M be/src/exec/hdfs-scan-node.cc M be/src/exec/hdfs-scan-node.h M common/thrift/PlanNodes.thrift A testdata/workloads/functional-query/queries/QueryTest/parquet-error-propagation-race.test M tests/failure/test_failpoints.py M tests/query_test/test_scanners.py 11 files changed, 102 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/11596/8 -- To view, visit http://gerrit.cloudera.org:8080/11596 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I45a61210ca7d057b048c77d9f2f2695ec450f19b Gerrit-Change-Number: 11596 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7662: fix error race when scanner open fails
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/11596 )
Change subject: IMPALA-7662: fix error race when scanner open fails
..
Patch Set 7:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11596/7/be/src/exec/hdfs-scan-node-base.h
File be/src/exec/hdfs-scan-node-base.h:
http://gerrit.cloudera.org:8080/#/c/11596/7/be/src/exec/hdfs-scan-node-base.h@564
PS7, Line 564: /// - If the scanner is created but opening fails, returns an
error sets *scanner. The
> nit: missing and?
Done
http://gerrit.cloudera.org:8080/#/c/11596/7/be/src/exec/hdfs-scan-node-base.h@565
PS7, Line 565: /// caller is then responsible for closing the scanner
> nit: missing .
Done
http://gerrit.cloudera.org:8080/#/c/11596/7/tests/query_test/test_scanners.py
File tests/query_test/test_scanners.py:
http://gerrit.cloudera.org:8080/#/c/11596/7/tests/query_test/test_scanners.py@732
PS7, Line 732: del vector.get_value('exec_option')['debug_action']
> Can you explain why this line is needed?
Done
--
To view, visit http://gerrit.cloudera.org:8080/11596
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45a61210ca7d057b048c77d9f2f2695ec450f19b
Gerrit-Change-Number: 11596
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 24 Oct 2018 22:44:28 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6323 Allow constant analytic window expressions.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11556 ) Change subject: IMPALA-6323 Allow constant analytic window expressions. .. Patch Set 5: Code-Review+2 LGTM once my two style comments are addressed -- To view, visit http://gerrit.cloudera.org:8080/11556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf129026c45120e9470df601268863634037908c Gerrit-Change-Number: 11556 Gerrit-PatchSet: 5 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 24 Oct 2018 22:40:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6661 Make NaN values equal for grouping purposes.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11535 ) Change subject: IMPALA-6661 Make NaN values equal for grouping purposes. .. Patch Set 18: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3348/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11535 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I996c4a2e1934fd887046ed0c55457b7285375086 Gerrit-Change-Number: 11535 Gerrit-PatchSet: 18 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 24 Oct 2018 21:45:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6661 Make NaN values equal for grouping purposes.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11535 ) Change subject: IMPALA-6661 Make NaN values equal for grouping purposes. .. Patch Set 18: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11535 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I996c4a2e1934fd887046ed0c55457b7285375086 Gerrit-Change-Number: 11535 Gerrit-PatchSet: 18 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 24 Oct 2018 21:45:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6661 Make NaN values equal for grouping purposes.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11535 ) Change subject: IMPALA-6661 Make NaN values equal for grouping purposes. .. Patch Set 17: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11535 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I996c4a2e1934fd887046ed0c55457b7285375086 Gerrit-Change-Number: 11535 Gerrit-PatchSet: 17 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 24 Oct 2018 21:44:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7655: Rewrite if, isnull, coalesce to use CASE
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11760 ) Change subject: IMPALA-7655: Rewrite if, isnull, coalesce to use CASE .. Patch Set 4: (3 comments) main question from my end is to consider what happens when rewrites are disabled. http://gerrit.cloudera.org:8080/#/c/11760/4/common/function-registry/impala_functions.py File common/function-registry/impala_functions.py: http://gerrit.cloudera.org:8080/#/c/11760/4/common/function-registry/impala_functions.py@531 PS4, Line 531: # Rewritten away in the FE can you explain why this is needed in this change? do we assert anywhere that these fns are never seen by the backend? http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java: http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@a92 PS4, Line 92: rewrites can be disabled via a query option so cannot be assumed to run. it cannot be assumed to transform an invalid/unknown stmt to a valid one. its purpose is only to transform valid statements to other equivalent valid statements. what happens when you do: set enable_expr_rewrites = false ? http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java File fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java: http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java@112 PS4, Line 112: to be consistent with the style in the frontend Java code, pls remove these javadoc markups. I think the idea is that the code is read through a variety of editors and since the javadocs are not used, simplify writing/reading the comments by omitting the markup. -- To view, visit http://gerrit.cloudera.org:8080/11760 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I526654d8546e17b2545c42cc59dab66d9fe1b163 Gerrit-Change-Number: 11760 Gerrit-PatchSet: 4 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 24 Oct 2018 21:13:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7351: Add estimates to Exchange node
Bikramjeet Vig has posted comments on this change. (
http://gerrit.cloudera.org:8080/11692 )
Change subject: IMPALA-7351: Add estimates to Exchange node
..
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11692/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11692/3//COMMIT_MSG@10
PS3, Line 10: method in the in-line comments.
> Do we know what the likely upper bound is for the memory estimates on large
Yes, the worst case is with a merging exchange, large sized rows, large number
of rows and a huge cluster such that the number of queues is alot too. In that
case the mem estimate would be => (18MB * num_senders)
{18MB comes from default values of exchg_node_buffer_size_bytes +
ROWBATCH_MAX_MEM_USAGE = 10 + 8)
In this case it does however seem likely that the overestimation might not be
by a huge factor, since large rows and this many number of senders can
overwhelm an exchange node
http://gerrit.cloudera.org:8080/#/c/11692/3/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
File fe/src/main/java/org/apache/impala/planner/ExchangeNode.java:
http://gerrit.cloudera.org:8080/#/c/11692/3/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java@77
PS3, Line 77: if (!streamSink.getOutputPartition().isPartitioned() &&
fragment_.isPartitioned()) {
> nit: can just return this expression
Done
http://gerrit.cloudera.org:8080/#/c/11692/3/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java@241
PS3, Line 241: long deferredBatchQueueSize = avgRowBatchByteSize *
numSenders;
> nit: unnecessary intermediate variable
Done
--
To view, visit http://gerrit.cloudera.org:8080/11692
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b577f9511abc48b992e814d50bba4959f23f7fd
Gerrit-Change-Number: 11692
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Wed, 24 Oct 2018 21:02:19 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5031: Increase backend test timeout under UBSAN FULL
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11764 ) Change subject: IMPALA-5031: Increase backend test timeout under UBSAN_FULL .. IMPALA-5031: Increase backend test timeout under UBSAN_FULL When codegen is run with ubsan, backend tests slow down dramatically. This patch increases the timeout to four hours in when UBSAN_FULL is the build type. This limit is approached by expr-test, which takes almost three hours under UBSAN_FULL. Change-Id: I3eee4c2b3affc9d65d86c043fcc382d7248adf3e Reviewed-on: http://gerrit.cloudera.org:8080/11764 Reviewed-by: Jim Apple Tested-by: Impala Public Jenkins --- M be/CMakeLists.txt M be/src/common/init.cc 2 files changed, 5 insertions(+), 0 deletions(-) Approvals: Jim Apple: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3eee4c2b3affc9d65d86c043fcc382d7248adf3e Gerrit-Change-Number: 11764 Gerrit-PatchSet: 3 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5031: Increase backend test timeout under UBSAN FULL
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11764 ) Change subject: IMPALA-5031: Increase backend test timeout under UBSAN_FULL .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3eee4c2b3affc9d65d86c043fcc382d7248adf3e Gerrit-Change-Number: 11764 Gerrit-PatchSet: 2 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 24 Oct 2018 20:27:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7655: Rewrite if, isnull, coalesce to use CASE
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/11760 )
Change subject: IMPALA-7655: Rewrite if, isnull, coalesce to use CASE
..
Patch Set 4:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java
File fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java:
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@66
PS4, Line 66: public Expr RewritesOk(String tableName, String exprStr,
ExprRewriteRule rule, String expectedExprStr)
line too long (104 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@71
PS4, Line 71: public Expr RewritesOk(String exprStr, List
rules, String expectedExprStr)
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@88
PS4, Line 88: public Expr RewritesOkWhereExpr(String exprStr, ExprRewriteRule
rule, String expectedExprStr)
line too long (95 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@90
PS4, Line 90: return RewritesOkWhereExpr("functional.alltypessmall",
exprStr, rule, expectedExprStr);
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@93
PS4, Line 93: public Expr RewritesOkWhereExpr(String tableName, String
exprStr, ExprRewriteRule rule, String expectedExprStr)
line too long (113 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@95
PS4, Line 95: return RewritesOkWhereExpr(tableName, exprStr,
Lists.newArrayList(rule), expectedExprStr);
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@98
PS4, Line 98: public Expr RewritesOkWhereExpr(String tableName, String
exprStr, List rules,
line too long (96 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/11760
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I526654d8546e17b2545c42cc59dab66d9fe1b163
Gerrit-Change-Number: 11760
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 24 Oct 2018 19:48:22 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6661 Make NaN values equal for grouping purposes.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11535 ) Change subject: IMPALA-6661 Make NaN values equal for grouping purposes. .. Patch Set 17: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1144/ : 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/11535 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I996c4a2e1934fd887046ed0c55457b7285375086 Gerrit-Change-Number: 11535 Gerrit-PatchSet: 17 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 24 Oct 2018 19:42:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6661 Make NaN values equal for grouping purposes.
Michal Ostrowski has posted comments on this change. ( http://gerrit.cloudera.org:8080/11535 ) Change subject: IMPALA-6661 Make NaN values equal for grouping purposes. .. Patch Set 17: (2 comments) http://gerrit.cloudera.org:8080/#/c/11535/16/testdata/workloads/functional-query/queries/QueryTest/joins.test File testdata/workloads/functional-query/queries/QueryTest/joins.test: http://gerrit.cloudera.org:8080/#/c/11535/16/testdata/workloads/functional-query/queries/QueryTest/joins.test@801 PS16, Line 801: (VALUES((1.6 x, 0 y), (3.2, 1), (5.4,2), (0.5, 3), (0.5, 4), (-0.5, 5))) XX), > It looks like we should be running test_basic_join_queries with disable_cod Done http://gerrit.cloudera.org:8080/#/c/11535/16/testdata/workloads/functional-query/queries/QueryTest/joins.test@818 PS16, Line 818: select t1.float_col as v : from functional.alltypessmall t1, functional.alltypessmall t2 : where sqrt(0.5-t > Should this be addressed ? Done -- To view, visit http://gerrit.cloudera.org:8080/11535 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I996c4a2e1934fd887046ed0c55457b7285375086 Gerrit-Change-Number: 11535 Gerrit-PatchSet: 17 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 24 Oct 2018 19:06:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6661 Make NaN values equal for grouping purposes.
Hello Michael Ho, Thomas Marshall, Paul Rogers, Tim Armstrong, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11535 to look at the new patch set (#17). Change subject: IMPALA-6661 Make NaN values equal for grouping purposes. .. IMPALA-6661 Make NaN values equal for grouping purposes. Similar to the treatment of NULLs, we want to consider NaN values as equal when grouping. - When detecting a NaN in a set of row values, the NaN value must be converted to a canonical value - so that all NaN values have the same bit-pattern for hashing purposes. - When doing equality evaluation, floating point types must have additional logic to consider NaN values as equal. - Existing logic for handling NULLs in this way is appropriate for triggering this behavior for NaN values. - Relabel "force null equality" as "inclusive equality" to expand the scope of the concept to a more generic form that includes NaN. Change-Id: I996c4a2e1934fd887046ed0c55457b7285375086 --- M be/src/codegen/codegen-anyval.cc M be/src/codegen/codegen-anyval.h M be/src/exec/hash-table.cc M be/src/exec/hash-table.h M be/src/exec/hash-table.inline.h M be/src/runtime/raw-value.cc M be/src/runtime/raw-value.h M be/src/runtime/raw-value.inline.h M testdata/workloads/functional-query/queries/QueryTest/aggregation.test M testdata/workloads/functional-query/queries/QueryTest/exprs.test M testdata/workloads/functional-query/queries/QueryTest/joins.test 11 files changed, 202 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/11535/17 -- To view, visit http://gerrit.cloudera.org:8080/11535 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I996c4a2e1934fd887046ed0c55457b7285375086 Gerrit-Change-Number: 11535 Gerrit-PatchSet: 17 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR](asf-site) Fix name of 3.0.1 in link title
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/11767 ) Change subject: Fix name of 3.0.1 in link title .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11767 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: comment Gerrit-Change-Id: I46ee692cd0925d6a72613ed152110d6d1eb34d6a Gerrit-Change-Number: 11767 Gerrit-PatchSet: 2 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 24 Oct 2018 19:00:22 + Gerrit-HasComments: No
[Impala-ASF-CR](asf-site) Fix name of 3.0.1 in link title
Jim Apple has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11767 ) Change subject: Fix name of 3.0.1 in link title .. Fix name of 3.0.1 in link title Change-Id: I46ee692cd0925d6a72613ed152110d6d1eb34d6a Reviewed-on: http://gerrit.cloudera.org:8080/11767 Reviewed-by: Tim Armstrong Tested-by: Jim Apple --- M downloads.html 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Tim Armstrong: Looks good to me, approved Jim Apple: Verified -- To view, visit http://gerrit.cloudera.org:8080/11767 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: merged Gerrit-Change-Id: I46ee692cd0925d6a72613ed152110d6d1eb34d6a Gerrit-Change-Number: 11767 Gerrit-PatchSet: 3 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR](asf-site) Fix name of 3.0.1 in link title
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11767 ) Change subject: Fix name of 3.0.1 in link title .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11767 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: comment Gerrit-Change-Id: I46ee692cd0925d6a72613ed152110d6d1eb34d6a Gerrit-Change-Number: 11767 Gerrit-PatchSet: 2 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 24 Oct 2018 18:19:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351: Add estimates to Exchange node
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/11692 )
Change subject: IMPALA-7351: Add estimates to Exchange node
..
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11692/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11692/3//COMMIT_MSG@10
PS3, Line 10: method in the in-line comments.
Do we know what the likely upper bound is for the memory estimates on larger
clusters and data sets?
It seems like the worst case is probably a merging exchange on a large cluster
right, since you have a large number of queues?
My understanding is that in the other cases the queue size provides a
reasonable cap because there is only one queue.
http://gerrit.cloudera.org:8080/#/c/11692/3/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
File fe/src/main/java/org/apache/impala/planner/ExchangeNode.java:
http://gerrit.cloudera.org:8080/#/c/11692/3/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java@77
PS3, Line 77: if (!streamSink.getOutputPartition().isPartitioned() &&
fragment_.isPartitioned()) {
nit: can just return this expression
http://gerrit.cloudera.org:8080/#/c/11692/3/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java@241
PS3, Line 241: long deferredBatchQueueSize = avgRowBatchByteSize *
numSenders;
nit: unnecessary intermediate variable
--
To view, visit http://gerrit.cloudera.org:8080/11692
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b577f9511abc48b992e814d50bba4959f23f7fd
Gerrit-Change-Number: 11692
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Wed, 24 Oct 2018 18:15:50 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7655: Rewrite if, isnull, coalesce to use CASE
Philip Zeyliger has posted comments on this change. (
http://gerrit.cloudera.org:8080/11760 )
Change subject: IMPALA-7655: Rewrite if, isnull, coalesce to use CASE
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11760/2/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java
File fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java:
http://gerrit.cloudera.org:8080/#/c/11760/2/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java@89
PS2, Line 89:* IF(TRUE, then, else)
then
:* IF(FALSE|NULL, then, else)
else
> Further testing revealed complexities. First, the rewrite engine does not a
The following code looks to me like it'll do re-writes until we're "stuck". It
may be that "does not apply the same rule multiple times" is a test artifact,
in that in the tests, we explicitly specify which rules to apply.
public Expr rewrite(Expr expr, Analyzer analyzer) throws AnalysisException {
// Keep applying the rule list until no rule has made any changes.
int oldNumChanges;
Expr rewrittenExpr = expr;
do {
oldNumChanges = numChanges_;
for (ExprRewriteRule rule: rules_) {
rewrittenExpr = applyRuleRepeatedly(rewrittenExpr, rule, analyzer);
}
} while (oldNumChanges != numChanges_);
return rewrittenExpr;
}
http://gerrit.cloudera.org:8080/#/c/11760/2/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java@122
PS2, Line 122:* Rewrites IFNULL(a, x):
> Tried removing the optimizations. Turns out that the CASE simplification di
Is the issue that "null is null" is not getting constant optimization? In my
quick test, it looks like it does. Note the "predicates" in the explain plan
below. Again, the question is whether or not the test case is taking advantage
of constant folding.
[localhost:21000] default> explain select * from tpch.orders where o_orderkey
is null limit 1;
Query: explain select * from tpch.orders where o_orderkey is null limit 1
++
| Explain String |
++
| Max Per-Host Resource Reservation: Memory=8.00MB Threads=3 |
| Per-Host Resource Estimates: Memory=176MB |
||
| PLAN-ROOT SINK |
| | |
| 01:EXCHANGE [UNPARTITIONED]|
| | limit: 1|
| | |
| 00:SCAN HDFS [tpch.orders] |
|partitions=1/1 files=1 size=162.56MB|
|predicates: o_orderkey IS NULL |
|limit: 1|
++
Fetched 12 row(s) in 0.03s
[localhost:21000] default> explain select * from tpch.orders where null is null
limit 1;
Query: explain select * from tpch.orders where null is null limit 1
++
| Explain String |
++
| Max Per-Host Resource Reservation: Memory=8.00MB Threads=2 |
| Per-Host Resource Estimates: Memory=88MB |
| Codegen disabled by planner|
||
| PLAN-ROOT SINK |
| | |
| 00:SCAN HDFS [tpch.orders] |
|partitions=1/1 files=1 size=162.56MB|
|limit: 1|
++
Fetched 9 row(s) in 0.01s
--
To view, visit http://gerrit.cloudera.org:8080/11760
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I526654d8546e17b2545c42cc59dab66d9fe1b163
Gerrit-Change-Number: 11760
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 24 Oct 2018 18:09:51 +
Gerrit-HasComments: Yes
[Impala-ASF-CR](asf-site) Fix name of 3.0.1 in link title
Jim Apple has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11767 Change subject: Fix name of 3.0.1 in link title .. Fix name of 3.0.1 in link title Change-Id: I46ee692cd0925d6a72613ed152110d6d1eb34d6a --- M downloads.html 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/11767/1 -- To view, visit http://gerrit.cloudera.org:8080/11767 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: newchange Gerrit-Change-Id: I46ee692cd0925d6a72613ed152110d6d1eb34d6a Gerrit-Change-Number: 11767 Gerrit-PatchSet: 1 Gerrit-Owner: Jim Apple
[Impala-ASF-CR] IMPALA-7655: Rewrite if, isnull, coalesce to use CASE
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/11760 )
Change subject: IMPALA-7655: Rewrite if, isnull, coalesce to use CASE
..
Patch Set 3:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11760/3/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java
File fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java:
http://gerrit.cloudera.org:8080/#/c/11760/3/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@49
PS3, Line 49: public Expr RewritesOk(String tableName, String exprStr,
ExprRewriteRule rule, String expectedExprStr)
line too long (104 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/3/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@54
PS3, Line 54: public Expr RewritesOk(String exprStr, List
rules, String expectedExprStr)
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/3/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@71
PS3, Line 71: public Expr RewritesOkWhereExpr(String exprStr, ExprRewriteRule
rule, String expectedExprStr)
line too long (95 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/3/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@73
PS3, Line 73: return RewritesOkWhereExpr("functional.alltypessmall",
exprStr, rule, expectedExprStr);
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/3/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@76
PS3, Line 76: public Expr RewritesOkWhereExpr(String tableName, String
exprStr, ExprRewriteRule rule, String expectedExprStr)
line too long (113 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/3/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@78
PS3, Line 78: return RewritesOkWhereExpr(tableName, exprStr,
Lists.newArrayList(rule), expectedExprStr);
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/3/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@81
PS3, Line 81: public Expr RewritesOkWhereExpr(String tableName, String
exprStr, List rules,
line too long (96 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/11760
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I526654d8546e17b2545c42cc59dab66d9fe1b163
Gerrit-Change-Number: 11760
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 24 Oct 2018 18:02:21 +
Gerrit-HasComments: Yes
[Impala-ASF-CR](asf-site) Download 3.0.1
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11766 ) Change subject: Download 3.0.1 .. Patch Set 1: Code-Review+2 Thanks for doing it. -- To view, visit http://gerrit.cloudera.org:8080/11766 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: comment Gerrit-Change-Id: I044f119788e5c228bf41ed241ae6d23e5eb00a25 Gerrit-Change-Number: 11766 Gerrit-PatchSet: 1 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Wed, 24 Oct 2018 17:56:28 + Gerrit-HasComments: No
[Impala-ASF-CR](asf-site) Download 3.0.1
Jim Apple has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11766 ) Change subject: Download 3.0.1 .. Download 3.0.1 This removes download links for old versions that don't have the security patches 3.0.1 was released for. Change-Id: I044f119788e5c228bf41ed241ae6d23e5eb00a25 Reviewed-on: http://gerrit.cloudera.org:8080/11766 Reviewed-by: Michael Ho Tested-by: Jim Apple --- M downloads.html 1 file changed, 3 insertions(+), 75 deletions(-) Approvals: Michael Ho: Looks good to me, approved Jim Apple: Verified -- To view, visit http://gerrit.cloudera.org:8080/11766 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: merged Gerrit-Change-Id: I044f119788e5c228bf41ed241ae6d23e5eb00a25 Gerrit-Change-Number: 11766 Gerrit-PatchSet: 2 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR](asf-site) Download 3.0.1
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/11766 ) Change subject: Download 3.0.1 .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11766 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: comment Gerrit-Change-Id: I044f119788e5c228bf41ed241ae6d23e5eb00a25 Gerrit-Change-Number: 11766 Gerrit-PatchSet: 1 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Wed, 24 Oct 2018 17:56:45 + Gerrit-HasComments: No
[Impala-ASF-CR](asf-site) Download 3.0.1
Jim Apple has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11766 Change subject: Download 3.0.1 .. Download 3.0.1 This removes download links for old versions that don't have the security patches 3.0.1 was released for. Change-Id: I044f119788e5c228bf41ed241ae6d23e5eb00a25 --- M downloads.html 1 file changed, 3 insertions(+), 75 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/66/11766/1 -- To view, visit http://gerrit.cloudera.org:8080/11766 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: newchange Gerrit-Change-Id: I044f119788e5c228bf41ed241ae6d23e5eb00a25 Gerrit-Change-Number: 11766 Gerrit-PatchSet: 1 Gerrit-Owner: Jim Apple
[Impala-ASF-CR] IMPALA-7698: Add centos support to bootstrap system.
Laszlo Gaal has posted comments on this change. ( http://gerrit.cloudera.org:8080/11731 ) Change subject: IMPALA-7698: Add centos support to bootstrap_system. .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/11731/3/bin/bootstrap_system.sh File bin/bootstrap_system.sh: http://gerrit.cloudera.org:8080/#/c/11731/3/bin/bootstrap_system.sh@139 PS3, Line 139: if lsb_release -r -s | grep '^7'; then : redhat sudo yum install -y libev-devel pigz python-virtualenv Since you install EPEL above, you may want to install these packages for CentOS 6 from there. EPEL offers the following versions: - pigz-2.3.4-1.el6.x86_64 - libev-devel-4.03-3.el6.x86_64 - python-virtualenv-12.0.7-1.el6.noarch.rpm - python-setuptools-0.6.10-4.el6_9.noarch.rpm -- To view, visit http://gerrit.cloudera.org:8080/11731 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id54294d7607f51de87a9de373dcfc4a33f4bedf5 Gerrit-Change-Number: 11731 Gerrit-PatchSet: 3 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Wed, 24 Oct 2018 17:36:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5004: Switch to sorting node for large TopN queries
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11698 ) Change subject: IMPALA-5004: Switch to sorting node for large TopN queries .. Patch Set 4: (8 comments) http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/analysis/SortInfo.java File fe/src/main/java/org/apache/impala/analysis/SortInfo.java: http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/analysis/SortInfo.java@236 PS4, Line 236: is the http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/analysis/SortInfo.java@238 PS4, Line 238:* average tuple serialized size. offset is not mentioned so either include it or remove it since the arg descriptions and the one-liner definition make things pretty clear already. http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/analysis/SortInfo.java@240 PS4, Line 240: @param Even though this style of commenting is widely used, Impala tries not to use it (yes, there are exceptions in the code base, but its on the rare side). If a file already uses them, then pls stick with the style of that file, but in this case, lets stick with the default style so remove the '@param'. http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java: http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@325 PS4, Line 325: estimateTopNMaterializedSize as pointed out in the header for SortInfo, the abstractions are not quite right. however, this call-site looks fairly complicated. compueMemLayout does not redo if its already been called, so perhaps simplify this to let SortInfo deal with its descriptors and calling computeMemLayout? You can have a static method do what estimateTopNMaterializedSize does now so that it can be used in SortInfo as well as from the SortNode. http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@326 PS4, Line 326: getAvgSerializedSize() this looks correct and consistent with the avgRowSize_ computation for PlanNodes. http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@334 PS4, Line 334: estimatedTopNMaterializedSize if we get this wrong, perhaps due to a mistaken many-to-many join cardinality estimate in some descendant child, what is the easiest way to turn this decision off? set topn_bytes_limit to max long? http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/planner/SortNode.java File fe/src/main/java/org/apache/impala/planner/SortNode.java: http://gerrit.cloudera.org:8080/#/c/11698/4/fe/src/main/java/org/apache/impala/planner/SortNode.java@260 PS4, Line 260: nodeResourceProfile_ = ResourceProfile.noReservation( where'd the return go? pls look into why tests did not get bothered by this... the profile could have been overwritten down below, in which case we'd get possibly different estimates. http://gerrit.cloudera.org:8080/#/c/11698/4/testdata/workloads/functional-planner/queries/PlannerTest/topn-default-limit-bytes.test File testdata/workloads/functional-planner/queries/PlannerTest/topn-default-limit-bytes.test: http://gerrit.cloudera.org:8080/#/c/11698/4/testdata/workloads/functional-planner/queries/PlannerTest/topn-default-limit-bytes.test@1 PS4, Line 1: triggers sort seems to apply to the second test, not the first, so got confused by this. -- To view, visit http://gerrit.cloudera.org:8080/11698 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I34c9db33c9302b55e9978f53f9c7061f2806c8a9 Gerrit-Change-Number: 11698 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 24 Oct 2018 17:19:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5031: Increase backend test timeout under UBSAN FULL
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11764 ) Change subject: IMPALA-5031: Increase backend test timeout under UBSAN_FULL .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3347/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3eee4c2b3affc9d65d86c043fcc382d7248adf3e Gerrit-Change-Number: 11764 Gerrit-PatchSet: 2 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 24 Oct 2018 16:27:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7655: Rewrite if, isnull, coalesce to use CASE
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/11760 ) Change subject: IMPALA-7655: Rewrite if, isnull, coalesce to use CASE .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11760/2/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java File fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java: http://gerrit.cloudera.org:8080/#/c/11760/2/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java@89 PS2, Line 89:* code gen for , avoiding the need for ad-hoc implementations :* of each function. > Removing optimizations worked here. Further testing revealed complexities. First, the rewrite engine does not apply the same rule multiple times (rewrite function, then rewrite resulting case.) If the code is changed to apply CASE rewrite to the result, additional problems result. One of which is that the CASE rewrites will revert to the original expression if all aggregates are removed. But, we can't do that here: we no longer have function implementations. So, we would need the aggregate fallback to fallback to the CASE rewritten from the function. The resulting code gets pretty complex and looks hard to maintain. So, I'm thinking to revert to the code in the first patch, with the added subtlety of handling aggregates in the function rewrites. Stay tuned for a revised version. -- To view, visit http://gerrit.cloudera.org:8080/11760 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I526654d8546e17b2545c42cc59dab66d9fe1b163 Gerrit-Change-Number: 11760 Gerrit-PatchSet: 4 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 24 Oct 2018 16:28:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5031: Increase backend test timeout under UBSAN FULL
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/11764 ) Change subject: IMPALA-5031: Increase backend test timeout under UBSAN_FULL .. Patch Set 2: Code-Review+2 carry Tim's +2 -- To view, visit http://gerrit.cloudera.org:8080/11764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3eee4c2b3affc9d65d86c043fcc382d7248adf3e Gerrit-Change-Number: 11764 Gerrit-PatchSet: 2 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 24 Oct 2018 16:27:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7698: Add centos support to bootstrap system.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11731 ) Change subject: IMPALA-7698: Add centos support to bootstrap_system. .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/11731/3/bin/bootstrap_system.sh File bin/bootstrap_system.sh: http://gerrit.cloudera.org:8080/#/c/11731/3/bin/bootstrap_system.sh@148 PS3, Line 148: https://www-us.apache.org/dist/maven/maven-3/3.5.4/binaries/apache-maven-3.5.4-bin.tar.gz \ line too long (93 > 90) http://gerrit.cloudera.org:8080/#/c/11731/3/bin/bootstrap_system.sh@150 PS3, Line 150: redhat sha512sum -c - <<< '2a803f578f341e164f6753e410413d16ab60fabe31dc491d1fe35c984a5cce696bc71f57757d4538fe7738be04065a216f3ebad4ef7e0ce1bb4c51bc36d6be86 apache-maven-3.5.4-bin.tar.gz' line too long (187 > 90) http://gerrit.cloudera.org:8080/#/c/11731/3/bin/bootstrap_system.sh@151 PS3, Line 151: redhat sha512sum -c - <<< 'c8321aa223f70d7e64d3d0274263000cfffb46fbea61488534e26f9f0245d99e9872d0888e35cd3274416392a13f80c748c07750caaeffa5f9cae1220020715f apache-ant-1.9.13-bin.tar.gz' line too long (186 > 90) -- To view, visit http://gerrit.cloudera.org:8080/11731 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id54294d7607f51de87a9de373dcfc4a33f4bedf5 Gerrit-Change-Number: 11731 Gerrit-PatchSet: 3 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Wed, 24 Oct 2018 16:17:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5031: Increase backend test timeout under UBSAN FULL
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11764 ) Change subject: IMPALA-5031: Increase backend test timeout under UBSAN_FULL .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3eee4c2b3affc9d65d86c043fcc382d7248adf3e Gerrit-Change-Number: 11764 Gerrit-PatchSet: 1 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 24 Oct 2018 16:11:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6323 Allow constant analytic window expressions.
Michal Ostrowski has posted comments on this change. ( http://gerrit.cloudera.org:8080/11556 ) Change subject: IMPALA-6323 Allow constant analytic window expressions. .. Patch Set 5: (1 comment) > Patch Set 5: > > (1 comment) The http://gerrit.cloudera.org:8080/#/c/11556/5/be/src/exec/analytic-eval-node.cc File be/src/exec/analytic-eval-node.cc: http://gerrit.cloudera.org:8080/#/c/11556/5/be/src/exec/analytic-eval-node.cc@397 PS5, Line 397: && !(order_by_eq_expr_eval_ == nullptr || > I'd think of it as normalising the ways we can represent equivalent operati The tricky part here may be that "window_end" isn't actually a local value -- it's "window.__isset.window_end". I don't want to artificially set that to false, as I can't/don't want to modify "window". So you end up needing to have another local member that sort of shadows "window.__isset.window_end". -- To view, visit http://gerrit.cloudera.org:8080/11556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf129026c45120e9470df601268863634037908c Gerrit-Change-Number: 11556 Gerrit-PatchSet: 5 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 24 Oct 2018 15:54:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7662: fix error race when scanner open fails
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/11596 )
Change subject: IMPALA-7662: fix error race when scanner open fails
..
Patch Set 7: Code-Review+1
(3 comments)
Just some nits.
http://gerrit.cloudera.org:8080/#/c/11596/7/be/src/exec/hdfs-scan-node-base.h
File be/src/exec/hdfs-scan-node-base.h:
http://gerrit.cloudera.org:8080/#/c/11596/7/be/src/exec/hdfs-scan-node-base.h@564
PS7, Line 564: /// - If the scanner is created but opening fails, returns an
error sets *scanner. The
nit: missing and?
http://gerrit.cloudera.org:8080/#/c/11596/7/be/src/exec/hdfs-scan-node-base.h@565
PS7, Line 565: /// caller is then responsible for closing the scanner
nit: missing .
http://gerrit.cloudera.org:8080/#/c/11596/7/tests/query_test/test_scanners.py
File tests/query_test/test_scanners.py:
http://gerrit.cloudera.org:8080/#/c/11596/7/tests/query_test/test_scanners.py@732
PS7, Line 732: del vector.get_value('exec_option')['debug_action']
Can you explain why this line is needed?
--
To view, visit http://gerrit.cloudera.org:8080/11596
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45a61210ca7d057b048c77d9f2f2695ec450f19b
Gerrit-Change-Number: 11596
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 24 Oct 2018 15:38:17 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7698: Add centos support to bootstrap system.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11731 ) Change subject: IMPALA-7698: Add centos support to bootstrap_system. .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/11731/2/bin/bootstrap_system.sh File bin/bootstrap_system.sh: http://gerrit.cloudera.org:8080/#/c/11731/2/bin/bootstrap_system.sh@145 PS2, Line 145: https://www-us.apache.org/dist/maven/maven-3/3.5.4/binaries/apache-maven-3.5.4-bin.tar.gz \ line too long (93 > 90) http://gerrit.cloudera.org:8080/#/c/11731/2/bin/bootstrap_system.sh@147 PS2, Line 147: redhat sha512sum -c - <<< '2a803f578f341e164f6753e410413d16ab60fabe31dc491d1fe35c984a5cce696bc71f57757d4538fe7738be04065a216f3ebad4ef7e0ce1bb4c51bc36d6be86 apache-maven-3.5.4-bin.tar.gz' line too long (187 > 90) http://gerrit.cloudera.org:8080/#/c/11731/2/bin/bootstrap_system.sh@148 PS2, Line 148: redhat sha512sum -c - <<< 'c8321aa223f70d7e64d3d0274263000cfffb46fbea61488534e26f9f0245d99e9872d0888e35cd3274416392a13f80c748c07750caaeffa5f9cae1220020715f apache-ant-1.9.13-bin.tar.gz' line too long (186 > 90) -- To view, visit http://gerrit.cloudera.org:8080/11731 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id54294d7607f51de87a9de373dcfc4a33f4bedf5 Gerrit-Change-Number: 11731 Gerrit-PatchSet: 2 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Wed, 24 Oct 2018 14:33:01 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7655: Rewrite if, isnull, coalesce to use CASE
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/11760 )
Change subject: IMPALA-7655: Rewrite if, isnull, coalesce to use CASE
..
Patch Set 4:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/11760/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11760/4//COMMIT_MSG@20
PS4, Line 20: k
nit: please wrap at 72 chars
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java
File fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java:
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java@144
PS4, Line 144: instanceof NullLiteral)
I would prefer .isNullLiteral() as other functions use that.
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java@183
PS4, Line 183: revised.size() - 1;
Can you add a comment about not adding the last child here? It took me some
time until I understood that it only added in CaseExpr's default branch.
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java@328
PS4, Line 328: isNull
typo: ifNull
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java@335
PS4, Line 335:* nullif(x, null)
NULL
Is this really the way it should behave?
Currently select nullif(1,NULL) returns 1.
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java@345
PS4, Line 345: tail
I think that we should return head in this case (if head is not null, then head
and tail are distinct, so head should be returned).
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/test/java/org/apache/impala/analysis/SimplifyConditionalRulesTest.java
File
fe/src/test/java/org/apache/impala/analysis/SimplifyConditionalRulesTest.java:
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/test/java/org/apache/impala/analysis/SimplifyConditionalRulesTest.java@252
PS4, Line 252: // If non-leading parameter is a non-NULL constant, drop
other terms
nit: missing .
http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/test/java/org/apache/impala/analysis/SimplifyConditionalRulesTest.java@291
PS4, Line 291: NULL
This should be 'id', see my comment at SimplifyConditionalsRule.java:335
expr-test should catch this - be/src/exprs/expr-test.cc contains:
TestValue("nullif(TRUE, NULL)", TYPE_BOOLEAN, true);
Can you verify that it indeed catches this issue?
--
To view, visit http://gerrit.cloudera.org:8080/11760
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I526654d8546e17b2545c42cc59dab66d9fe1b163
Gerrit-Change-Number: 11760
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 24 Oct 2018 13:27:42 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5031: Increase backend test timeout under UBSAN FULL
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11764 ) Change subject: IMPALA-5031: Increase backend test timeout under UBSAN_FULL .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1143/ : 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/11764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3eee4c2b3affc9d65d86c043fcc382d7248adf3e Gerrit-Change-Number: 11764 Gerrit-PatchSet: 1 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 24 Oct 2018 13:03:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7655: Rewrite if, isnull, coalesce to use CASE
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/11760 )
Change subject: IMPALA-7655: Rewrite if, isnull, coalesce to use CASE
..
Patch Set 2:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11760/2/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java
File fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java:
http://gerrit.cloudera.org:8080/#/c/11760/2/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@49
PS2, Line 49: public Expr RewritesOk(String tableName, String exprStr,
ExprRewriteRule rule, String expectedExprStr)
line too long (104 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/2/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@54
PS2, Line 54: public Expr RewritesOk(String exprStr, List
rules, String expectedExprStr)
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/2/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@71
PS2, Line 71: public Expr RewritesOkWhereExpr(String exprStr, ExprRewriteRule
rule, String expectedExprStr)
line too long (95 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/2/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@73
PS2, Line 73: return RewritesOkWhereExpr("functional.alltypessmall",
exprStr, rule, expectedExprStr);
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/2/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@76
PS2, Line 76: public Expr RewritesOkWhereExpr(String tableName, String
exprStr, ExprRewriteRule rule, String expectedExprStr)
line too long (113 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/2/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@78
PS2, Line 78: return RewritesOkWhereExpr(tableName, exprStr,
Lists.newArrayList(rule), expectedExprStr);
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/11760/2/fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java@81
PS2, Line 81: public Expr RewritesOkWhereExpr(String tableName, String
exprStr, List rules,
line too long (96 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/11760
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I526654d8546e17b2545c42cc59dab66d9fe1b163
Gerrit-Change-Number: 11760
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 24 Oct 2018 12:49:00 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5031: Increase backend test timeout under UBSAN FULL
Jim Apple has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11764 Change subject: IMPALA-5031: Increase backend test timeout under UBSAN_FULL .. IMPALA-5031: Increase backend test timeout under UBSAN_FULL When codegen is run with ubsan, backend tests slow down dramatically. This patch increases the timeout to four hours in when UBSAN_FULL is the build type. This limit is approached by expr-test, which takes almost three hours under UBSAN_FULL. Change-Id: I3eee4c2b3affc9d65d86c043fcc382d7248adf3e --- M be/CMakeLists.txt M be/src/common/init.cc 2 files changed, 5 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11764/1 -- To view, visit http://gerrit.cloudera.org:8080/11764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3eee4c2b3affc9d65d86c043fcc382d7248adf3e Gerrit-Change-Number: 11764 Gerrit-PatchSet: 1 Gerrit-Owner: Jim Apple
[Impala-ASF-CR] IMPALA-6658: improve Parquet RLE for low bit widths
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11582 ) Change subject: IMPALA-6658: improve Parquet RLE for low bit widths .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/11582/3/be/src/util/rle-test.cc File be/src/util/rle-test.cc: http://gerrit.cloudera.org:8080/#/c/11582/3/be/src/util/rle-test.cc@216 PS3, Line 216: c Oops, I missed this one: nit: capital C -- To view, visit http://gerrit.cloudera.org:8080/11582 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I191a581d3f699b6669e48ac9dc39c76ed77c4a76 Gerrit-Change-Number: 11582 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 24 Oct 2018 11:51:06 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6658: improve Parquet RLE for low bit widths
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11582 ) Change subject: IMPALA-6658: improve Parquet RLE for low bit widths .. Patch Set 3: Code-Review+1 (3 comments) If you do not want the implement the "use shorter min_repeated_run_length if the last run was a repeated run" logic then lgtm. http://gerrit.cloudera.org:8080/#/c/11582/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11582/2//COMMIT_MSG@20 PS2, Line 20: length 16 for single bit values. All other bit widths will use the : existing length 8 runs. : > I agree with your calculations. I think sometimes min_run_length=24 is bett >This could be avoided by using smaller >min_repeated_run_length if the last run was a repeated run: Did you think about the logic above? I think that it would be very close to optimal encoding, and would not need extreme code changes (e.g. there could be a min_repeated_run_length_after_literal_run_, and a min_repeated_run_length_after_repeated_run_, and FlushLiteralRun / FlushRepeatedRun could switch between them). About the "typical use case": I agree that interrupting repeated runs should be more common (but we shouldn't cause a regression in other cases). I think that the most typical use case where we can win a few bytes is when one value is much less frequent than another (e.g. most values in a column are NULL, while some are not), which leads to alternating short literal runs and longer repeated runs. If the ratio is around 1/8 - 1/24, most of the repeated runs will be short enough to win with encoding as literal. http://gerrit.cloudera.org:8080/#/c/11582/3/be/src/util/rle-encoding.h File be/src/util/rle-encoding.h: http://gerrit.cloudera.org:8080/#/c/11582/3/be/src/util/rle-encoding.h@266 PS3, Line 266: // Using min_repeated_run_length=16 is always better than 8. I prefer to phrase it as "never worse than 8". For alternating long repeated runs and runs of 8 it is the same. http://gerrit.cloudera.org:8080/#/c/11582/3/be/src/util/rle-encoding.h@267 PS3, Line 267: better than 8 Maybe "better than 16" would be better. -- To view, visit http://gerrit.cloudera.org:8080/11582 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I191a581d3f699b6669e48ac9dc39c76ed77c4a76 Gerrit-Change-Number: 11582 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 24 Oct 2018 11:18:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6661 Make NaN values equal for grouping purposes.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11535 ) Change subject: IMPALA-6661 Make NaN values equal for grouping purposes. .. Patch Set 16: Code-Review+2 (2 comments) LGTM. Two minor comments which need to be addressed. http://gerrit.cloudera.org:8080/#/c/11535/16/be/src/runtime/raw-value.h File be/src/runtime/raw-value.h: http://gerrit.cloudera.org:8080/#/c/11535/16/be/src/runtime/raw-value.h@40 PS16, Line 40: nit: blank space http://gerrit.cloudera.org:8080/#/c/11535/16/testdata/workloads/functional-query/queries/QueryTest/joins.test File testdata/workloads/functional-query/queries/QueryTest/joins.test: http://gerrit.cloudera.org:8080/#/c/11535/16/testdata/workloads/functional-query/queries/QueryTest/joins.test@818 PS16, Line 818: with q as (VALUES((cast(1.0 as FLOAT) x), (2.0))), : r as (select t1.x from q t1, q t2 where sqrt(1.0-t1.x) <=> sqrt(1.0-t2.x)) : select * from r > If you plan to keep this test case, this can be simplified as: Should this be addressed ? -- To view, visit http://gerrit.cloudera.org:8080/11535 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I996c4a2e1934fd887046ed0c55457b7285375086 Gerrit-Change-Number: 11535 Gerrit-PatchSet: 16 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 24 Oct 2018 06:41:34 + Gerrit-HasComments: Yes
