[Impala-ASF-CR] IMPALA-110 (part 2): Refactor PartitionedAggregationNode
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10394 ) Change subject: IMPALA-110 (part 2): Refactor PartitionedAggregationNode .. Patch Set 9: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2755/ -- To view, visit http://gerrit.cloudera.org:8080/10394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e Gerrit-Change-Number: 10394 Gerrit-PatchSet: 9 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 29 Jun 2018 03:07:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3040: Remove cache directive before dropping a table
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/10792 ) Change subject: IMPALA-3040: Remove cache directive before dropping a table .. Patch Set 2: > Do you know what in test_caching_ddl() is calling this drop (drop db > cascade/drop table etc.) ? https://github.com/apache/impala/blob/master/tests/query_test/test_hdfs_caching.py#L207 BTW, The specific case I looked into failed in test_caching_ddl_drop_database. > Also, thinking a bit more about your theory, are you able to reproduce it by > adding Thread.sleep() s in the required places? The tricky part is that reloadTable() calls into HMS to get the table before loading partitions and we need to let that one succeed. Plus load() is called multiple times and the exact timing becomes unclear. I spent some time on it but no luck so far. The fix in this patch should still work. -- To view, visit http://gerrit.cloudera.org:8080/10792 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id7701a499405e961456adea63f3592b43bd69170 Gerrit-Change-Number: 10792 Gerrit-PatchSet: 2 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Tianyi Wang Gerrit-Comment-Date: Fri, 29 Jun 2018 02:14:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6883: [DOCS] Refactor impala authorization doc
Hello Fredy Wijaya, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10786 to look at the new patch set (#3). Change subject: IMPALA-6883: [DOCS] Refactor impala_authorization doc .. IMPALA-6883: [DOCS] Refactor impala_authorization doc Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d --- M docs/shared/impala_common.xml M docs/topics/impala_authorization.xml M docs/topics/impala_grant.xml 3 files changed, 474 insertions(+), 270 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/10786/3 -- To view, visit http://gerrit.cloudera.org:8080/10786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d Gerrit-Change-Number: 10786 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function
anujphadke has posted comments on this change. (
http://gerrit.cloudera.org:8080/6023 )
Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..
Patch Set 20:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6023/20/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:
http://gerrit.cloudera.org:8080/#/c/6023/20/be/src/exprs/math-functions-ir.cc@565
PS20, Line 565: ctx->SetError("Encountered division by 0");
clang fails with division by 0. Since we check for
We have a check above min_range >= max_range on line 519. Can't think of a case
to hit this.
I added this to get pass the clang failure.
@taras - Can you take an another look? Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 20
Gerrit-Owner: anujphadke
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown
Gerrit-Reviewer: Taras Bobrovytsky
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: anujphadke
Gerrit-Comment-Date: Fri, 29 Jun 2018 00:30:15 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6802 (part 6): Clean up authorization tests
Fredy Wijaya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/10841 ) Change subject: IMPALA-6802 (part 6): Clean up authorization tests .. IMPALA-6802 (part 6): Clean up authorization tests This is the last part of the authorization test clean up. This patch rewrites the following tests: - alter database - explain - comment on - function - alter table/view This patch also adds the following authorization tests: - update - upsert - delete The tests in AuthorizationTest.java that have been rewritten into AuthorizationStmtTest.java are removed. Cherry-picks: not for 2.x Change-Id: Id594ce09a821aef4a1debfdd61569a11defd1c55 --- M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java 2 files changed, 554 insertions(+), 2,057 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/10841/5 -- To view, visit http://gerrit.cloudera.org:8080/10841 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id594ce09a821aef4a1debfdd61569a11defd1c55 Gerrit-Change-Number: 10841 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function
Hello Taras Bobrovytsky, Michael Brown, Tim Armstrong, Alex Behm, Dan Hecht, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6023 to look at the new patch set (#20). Change subject: IMPALA-4848: Add WIDTH_BUCKET() function .. IMPALA-4848: Add WIDTH_BUCKET() function Syntax : width_bucket(expr decimal, min_val decimal, max_val decimal, num_buckets int) This function creates equiwidth histograms , where the histogram range is divided into num_buckets buckets having identical sizes. This function returns the bucket in which the expr value would fall. min_val and max_val are the minimum and maximum value of the histogram range respectively. -> This function returns NULL if expr is a NULL. -> This function returns 0 if expr < min_val -> This function returns num_buckets + 1 if expr > max_val E.g. [localhost:21000] > select width_bucket(8, 1, 20, 3); +---+ | width_bucket(8, 1, 20, 3) | +---+ | 2 | +---+ Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f --- M be/src/exprs/expr-test.cc M be/src/exprs/math-functions-ir.cc M be/src/exprs/math-functions.h M be/src/util/string-util.cc M be/src/util/string-util.h M common/function-registry/impala_functions.py M fe/src/main/java/org/apache/impala/analysis/Expr.java 7 files changed, 254 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/6023/20 -- To view, visit http://gerrit.cloudera.org:8080/6023 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f Gerrit-Change-Number: 6023 Gerrit-PatchSet: 20 Gerrit-Owner: anujphadke Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke
[Impala-ASF-CR] IMPALA-110 (part 2): Refactor PartitionedAggregationNode
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10394 ) Change subject: IMPALA-110 (part 2): Refactor PartitionedAggregationNode .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/10394/4/be/src/exec/grouping-aggregator.cc File be/src/exec/grouping-aggregator.cc: http://gerrit.cloudera.org:8080/#/c/10394/4/be/src/exec/grouping-aggregator.cc@91 PS4, Line 91: is_streaming_preagg_(tnode.agg_node.use_streaming_preaggregation), > I'm not sure having a TAggregationNode and a TStreamingAggregationNode is t Okay, leaving it works for me. -- To view, visit http://gerrit.cloudera.org:8080/10394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e Gerrit-Change-Number: 10394 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 29 Jun 2018 00:22:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function
Hello Taras Bobrovytsky, Michael Brown, Tim Armstrong, Alex Behm, Dan Hecht, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6023 to look at the new patch set (#19). Change subject: IMPALA-4848: Add WIDTH_BUCKET() function .. IMPALA-4848: Add WIDTH_BUCKET() function Syntax : width_bucket(expr decimal, min_val decimal, max_val decimal, num_buckets int) This function creates equiwidth histograms , where the histogram range is divided into num_buckets buckets having identical sizes. This function returns the bucket in which the expr value would fall. min_val and max_val are the minimum and maximum value of the histogram range respectively. -> This function returns NULL if expr is a NULL. -> This function returns 0 if expr < min_val -> This function returns num_buckets + 1 if expr > max_val E.g. [localhost:21000] > select width_bucket(8, 1, 20, 3); +---+ | width_bucket(8, 1, 20, 3) | +---+ | 2 | +---+ Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f --- M be/src/exprs/expr-test.cc M be/src/exprs/math-functions-ir.cc M be/src/exprs/math-functions.h M be/src/util/string-util.cc M be/src/util/string-util.h M common/function-registry/impala_functions.py M fe/src/main/java/org/apache/impala/analysis/Expr.java 7 files changed, 254 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/6023/19 -- To view, visit http://gerrit.cloudera.org:8080/6023 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f Gerrit-Change-Number: 6023 Gerrit-PatchSet: 19 Gerrit-Owner: anujphadke Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke
[Impala-ASF-CR] IMPALA-110 (part 2): Refactor PartitionedAggregationNode
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10394 ) Change subject: IMPALA-110 (part 2): Refactor PartitionedAggregationNode .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2755/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e Gerrit-Change-Number: 10394 Gerrit-PatchSet: 9 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 28 Jun 2018 23:47:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-110 (part 2): Refactor PartitionedAggregationNode
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/10394 ) Change subject: IMPALA-110 (part 2): Refactor PartitionedAggregationNode .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/10394/6/be/src/exec/aggregator.h File be/src/exec/aggregator.h: http://gerrit.cloudera.org:8080/#/c/10394/6/be/src/exec/aggregator.h@26 PS6, Line 26: #include "util/runtime-profile.h" > I think you might be able to avoid this problem if you declare the destruct Done -- To view, visit http://gerrit.cloudera.org:8080/10394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e Gerrit-Change-Number: 10394 Gerrit-PatchSet: 8 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 28 Jun 2018 23:47:07 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-110 (part 2): Refactor PartitionedAggregationNode
Hello Tim Armstrong, Alex Behm, Vuk Ercegovac, Dan Hecht, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10394 to look at the new patch set (#8). Change subject: IMPALA-110 (part 2): Refactor PartitionedAggregationNode .. IMPALA-110 (part 2): Refactor PartitionedAggregationNode This patch refactors PartitionedAggregationNode in preparation for supporting multiple distinct operators in a query. The primary goal of the refactor is to separate out the core aggregation functionality into a new type of object called an Aggregator. For now, each aggregation ExecNode will contain a single Aggregator. Then, future patches will extend the aggregation ExecNode to support taking a single input and processing it with multiple Aggregators, allowing us to support more exotic combinations of aggregate functions and groupings. Specifically, this patch splits PartitionedAggregationNode into five new classes: - Aggregator: a superclass containing the functionality that's shared between GroupingAggregator and NonGroupingAggregator. - GroupingAggregator: this class contains the bulk of the interesting aggregation code, including everything related to creating and updating partitions and hash tables, spilling, etc. - NonGroupingAggregator: this class handles the case of aggregations that don't have grouping exprs. Since these aggregations always result in just a single output row, the functionality here is relatively simple (eg. no spilling or streaming). - StreamingAggregationNode: this node performs a streaming preaggregation, where the input is retrieved from the child during GetNext() and passed to the GroupingAggregator (non-grouping do not support streaming) Eventually, we'll support a list of GroupingAggregators. - AggregationNode: this node performs a final aggregation, where the input is retrieved from the child during Open() and passed to the Aggregator. Currently the Aggregator can be either grouping or non-grouping. Eventually we'll support a list of GroupingAggregator and/or a single NonGroupingAggregator. Testing: - Passed a full exhaustive run. Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e --- M be/src/codegen/gen_ir_descriptions.py M be/src/codegen/impala-ir.cc M be/src/exec/CMakeLists.txt A be/src/exec/aggregation-node.cc A be/src/exec/aggregation-node.h A be/src/exec/aggregator.cc A be/src/exec/aggregator.h M be/src/exec/exec-node.cc M be/src/exec/exec-node.h R be/src/exec/grouping-aggregator-ir.cc A be/src/exec/grouping-aggregator-partition.cc A be/src/exec/grouping-aggregator.cc R be/src/exec/grouping-aggregator.h A be/src/exec/non-grouping-aggregator-ir.cc A be/src/exec/non-grouping-aggregator.cc A be/src/exec/non-grouping-aggregator.h D be/src/exec/partitioned-aggregation-node.cc A be/src/exec/streaming-aggregation-node.cc A be/src/exec/streaming-aggregation-node.h 19 files changed, 3,054 insertions(+), 2,239 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/10394/8 -- To view, visit http://gerrit.cloudera.org:8080/10394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e Gerrit-Change-Number: 10394 Gerrit-PatchSet: 8 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-110 (part 2): Refactor PartitionedAggregationNode
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10394 ) Change subject: IMPALA-110 (part 2): Refactor PartitionedAggregationNode .. Patch Set 7: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2754/ -- To view, visit http://gerrit.cloudera.org:8080/10394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e Gerrit-Change-Number: 10394 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 28 Jun 2018 23:44:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-110 (part 2): Refactor PartitionedAggregationNode
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10394 ) Change subject: IMPALA-110 (part 2): Refactor PartitionedAggregationNode .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/10394/6/be/src/exec/aggregator.h File be/src/exec/aggregator.h: http://gerrit.cloudera.org:8080/#/c/10394/6/be/src/exec/aggregator.h@26 PS6, Line 26: #include "runtime/mem-tracker.h" > No, unique_ptr requires a full definition to determine the size I think you might be able to avoid this problem if you declare the destructor and put the definition in the .cc file. IIRC the usual issue is with the auto-generated default destructor. -- To view, visit http://gerrit.cloudera.org:8080/10394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e Gerrit-Change-Number: 10394 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 28 Jun 2018 23:36:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6086: Use of permanent function should require SELECT privilege on DB
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/10842 )
Change subject: IMPALA-6086: Use of permanent function should require SELECT
privilege on DB
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/10842/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java:
http://gerrit.cloudera.org:8080/#/c/10842/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@2576
PS1, Line 2576: AuthzOk(ctx, "create function default.to_lower(string)
returns string " +
This doesn't actually create a function in the catalog. You need to do the
following to be able to use it in other statements.
ScalarFunction fn = ScalarFunction.createForTesting("functional", "to_upper",
argTypes, Type.STRING, "/test-warehouse/libTestUdfs.so",
"_Z7ToUpperPN10impala_udf15FunctionContextERKNS_9StringValE", null,
null,
TFunctionBinaryType.NATIVE);
authzCatalog_.addFunction(fn);
http://gerrit.cloudera.org:8080/#/c/10842/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@2578
PS1, Line 2578: ToUpper
nit: it's probably better to call the function to_upper since the actual
function is to convert the string to upper case.
http://gerrit.cloudera.org:8080/#/c/10842/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@2596
PS1, Line 2596: AuthzError(ctx1, "select default.to_lower('ABCDEF')",
Due to the way authorization works to prefer AuthorizationException over
AnalysisException in order to not leak potentially sensitive data, I believe
the actual exception is AnalysisException because the function created in L2576
doesn't actually exist in the catalog. Calling ScalarFunction.createForTesting
as what I suggested above should fix the issue.
--
To view, visit http://gerrit.cloudera.org:8080/10842
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45f5b51363afbe46653b131fad9bf68e3e3ce71f
Gerrit-Change-Number: 10842
Gerrit-PatchSet: 1
Gerrit-Owner: Zoram Thanga
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Thu, 28 Jun 2018 23:32:22 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-110 (part 2): Refactor PartitionedAggregationNode
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10394 ) Change subject: IMPALA-110 (part 2): Refactor PartitionedAggregationNode .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e Gerrit-Change-Number: 10394 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 28 Jun 2018 23:27:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-110 (part 2): Refactor PartitionedAggregationNode
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10394 ) Change subject: IMPALA-110 (part 2): Refactor PartitionedAggregationNode .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2754/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e Gerrit-Change-Number: 10394 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 28 Jun 2018 23:27:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-110 (part 2): Refactor PartitionedAggregationNode
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/10394 ) Change subject: IMPALA-110 (part 2): Refactor PartitionedAggregationNode .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/10394/6/be/src/exec/aggregator.h File be/src/exec/aggregator.h: http://gerrit.cloudera.org:8080/#/c/10394/6/be/src/exec/aggregator.h@26 PS6, Line 26: #include "runtime/mem-tracker.h" > Nit: Can we forward-declare MemTracker instead of pulling in the whole head No, unique_ptr requires a full definition to determine the size http://gerrit.cloudera.org:8080/#/c/10394/4/be/src/exec/grouping-aggregator.cc File be/src/exec/grouping-aggregator.cc: http://gerrit.cloudera.org:8080/#/c/10394/4/be/src/exec/grouping-aggregator.cc@91 PS4, Line 91: is_streaming_preagg_(tnode.agg_node.use_streaming_preaggregation), > Right, I was imaging that eventually we'll have separate thrift nodes for A I'm not sure having a TAggregationNode and a TStreamingAggregationNode is the right way to go, since they would have identical fields. And of course we already handle other nodes this way, eg. there's a TSortNode that could produce either a SortNode, PartialSortNode, or TopNNode depending on a flag in the TSortNode -- To view, visit http://gerrit.cloudera.org:8080/10394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e Gerrit-Change-Number: 10394 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 28 Jun 2018 23:26:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6802 (part 6): Clean up authorization tests
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/10841 )
Change subject: IMPALA-6802 (part 6): Clean up authorization tests
..
Patch Set 4:
(1 comment)
Carry +1
http://gerrit.cloudera.org:8080/#/c/10841/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:
http://gerrit.cloudera.org:8080/#/c/10841/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2081
PS3, Line 2081: private static String accessError(String object) {
> Don't we need a similar handler for function use error?
Yup, good one. I'll add another case of select that uses a UDF.
--
To view, visit http://gerrit.cloudera.org:8080/10841
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id594ce09a821aef4a1debfdd61569a11defd1c55
Gerrit-Change-Number: 10841
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Adam Holley
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Reviewer: Zoram Thanga
Gerrit-Comment-Date: Thu, 28 Jun 2018 22:36:25 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6802 (part 6): Clean up authorization tests
Fredy Wijaya has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/10841 ) Change subject: IMPALA-6802 (part 6): Clean up authorization tests .. IMPALA-6802 (part 6): Clean up authorization tests This is the last part of the authorization test clean up. This patch rewrites the following tests: - alter database - explain - comment on - function - alter table/view This patch also adds the following authorization tests: - update - upsert - delete The tests in AuthorizationTest.java that have been rewritten into AuthorizationStmtTest.java are removed. Cherry-picks: not for 2.x Change-Id: Id594ce09a821aef4a1debfdd61569a11defd1c55 --- M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java 2 files changed, 534 insertions(+), 2,057 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/10841/4 -- To view, visit http://gerrit.cloudera.org:8080/10841 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id594ce09a821aef4a1debfdd61569a11defd1c55 Gerrit-Change-Number: 10841 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6883: [DOCS] Refactor impala authorization doc
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/10786 ) Change subject: IMPALA-6883: [DOCS] Refactor impala_authorization doc .. Patch Set 1: Added CTAS to privilege table -- To view, visit http://gerrit.cloudera.org:8080/10786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d Gerrit-Change-Number: 10786 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 28 Jun 2018 22:30:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6883: [DOCS] Refactor impala authorization doc
Hello Fredy Wijaya, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10786 to look at the new patch set (#2). Change subject: IMPALA-6883: [DOCS] Refactor impala_authorization doc .. IMPALA-6883: [DOCS] Refactor impala_authorization doc Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d --- M docs/shared/impala_common.xml M docs/topics/impala_authorization.xml M docs/topics/impala_grant.xml 3 files changed, 230 insertions(+), 129 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/10786/2 -- To view, visit http://gerrit.cloudera.org:8080/10786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d Gerrit-Change-Number: 10786 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10813 ) Change subject: IMPALA-7163: Implement a state machine for the QueryState class .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/10813/2/be/src/runtime/query-state.h File be/src/runtime/query-state.h: http://gerrit.cloudera.org:8080/#/c/10813/2/be/src/runtime/query-state.h@300 PS2, Line 300: The QueryState is ready for tear-down at this state > I've removed that line since we don't really do any tear down in this patch Actually, to reiterate, we may need to add a new state if we're going to manage ExecResources with this state machine as well. In that case, we'd need to add a new STOPPED state, since on ERROR and CANCELLED, we may have fragment instances still running. -- To view, visit http://gerrit.cloudera.org:8080/10813 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe Gerrit-Change-Number: 10813 Gerrit-PatchSet: 2 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Thu, 28 Jun 2018 22:26:34 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Hello Michael Ho, Joe McDonnell, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10813 to look at the new patch set (#3). Change subject: IMPALA-7163: Implement a state machine for the QueryState class .. IMPALA-7163: Implement a state machine for the QueryState class This patch adds a state machine for the QueryState class. The motivation behind this patch is to make the query lifecycle from the point of view of an executor much easier to reason about and this patch is key for a follow on patch for IMPALA-2990 where the status reporting will be per-query rather than per-fragment-instance. Currently, the state machine provides no other purpose, and it will mostly be used for IMPALA-2990. We introduce 7 possible states for the QueryState which include 3 terminal states (FINISHED, CANCELLED and ERROR) and 4 non-terminal states (INITIALIZED, PREPARED). The transition from one state to the next is always handled by a single thread which is also the QueryState thread. This thread will additionally bear the purpose of sending periodic updates after IMPALA-2990, which is the primary reason behind having only this thread modify the state of the query. 2 counting barriers are introduced which are used to indicate how many fragment instances have finished Preparing and Executing. We use CountingBarriers to allow fragment instance threads to communicate their completion of their respective states or errors with the QueryState thread. Due to this design, it's possible for the fragment instances to have collectively moved on to future states while the query state thread lags behind in realizing that. However, that's not a concern for us since we only care about showing a unified view of what's happening on this executor to the coordinator (post IMPALA-2990). The status reporting protocol has not been changed and remains exactly as it was. Testing: Ran 'core' and 'exhaustive' tests. TODO: Run stress tests. Future related work: 1) IMPALA-2990: Make status reporting per-query. 2) Try to logically align the FIS states with the QueryState states. 3) Consider mirroring the QueryState state machine to CoordinatorBackendState Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe --- M be/src/runtime/coordinator.cc M be/src/runtime/fragment-instance-state.cc M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-state.cc M be/src/runtime/query-state.h 5 files changed, 343 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/10813/3 -- To view, visit http://gerrit.cloudera.org:8080/10813 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe Gerrit-Change-Number: 10813 Gerrit-PatchSet: 3 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10813 ) Change subject: IMPALA-7163: Implement a state machine for the QueryState class .. Patch Set 2: (13 comments) http://gerrit.cloudera.org:8080/#/c/10813/2/be/src/runtime/query-state.h File be/src/runtime/query-state.h: http://gerrit.cloudera.org:8080/#/c/10813/2/be/src/runtime/query-state.h@66 PS2, Line 66: f any fragment instance hits an error or cancellation, then we immediately change the : /// state of the query to the corresponding error state. > but won't we need to wait until all FIS have completed (even if it's becaus Yes, once IMPALA-2990 is implemented, the invariant we'll maintain is that a final profile can be sent at any time after the PREPARED state. In the ERROR and CANCELLED states, that means we can send the final profile immediately without having to wait for the remaining fragment instances to complete, and since all the FISes will have been PREPARED, there will be mention of all fragment instances in the final report. In the FINISHED state, all the fragment instances will have completed anyway, so we can send the final profile then. http://gerrit.cloudera.org:8080/#/c/10813/2/be/src/runtime/query-state.h@194 PS2, Line 194: pair > rather than using a pair<>, how about defining a struct so that the fields Done http://gerrit.cloudera.org:8080/#/c/10813/2/be/src/runtime/query-state.h@245 PS2, Line 245: 2& > we generally only use const references. For things that will be modified, u Got rid of this. http://gerrit.cloudera.org:8080/#/c/10813/2/be/src/runtime/query-state.h@245 PS2, Line 245: pair > Use FInstanceStatus Got rid of this. http://gerrit.cloudera.org:8080/#/c/10813/2/be/src/runtime/query-state.h@262 PS2, Line 262: to another thread that : // called this function. > That doesn't seem right in the "less than 0" case, since this function only The less than 0 case is specifically handled because this can race with DoneWithState(). I just found that the same is done in CountingBarrier::NotifyRemaining() for the same reason (since it can race with CountingBarrier::Notify()). http://gerrit.cloudera.org:8080/#/c/10813/2/be/src/runtime/query-state.h@289 PS2, Line 289: INITIALIZED, : /// STARTED: All fragment instances have been started. Implies that the query is : /// being prepared. : STARTED, : /// PREPARED: All fragment instances managed by this QueryState have successfully : /// completed Prepare(). Implies that the query is Opening. : PREPARED, : /// OPENED: All fragment instances managed by this QueryState have successfully : /// completed Open(). Implies that the query is executing. : OPENED, > why do we need all of these states? it's a bit hard to see how they will be I didn't want to write too much in the header comments since that would end up explaining the implementation itself. I'll write down what the different states mean here in this comment and if you think it's reasonable to add them to the header comments, then I'll do that in the next patchset. INITIALIZED is the first state. The reason I added a separate state for this and not start with STARTED or PREPARED directly is because there are distinct failures that could happen before Prepare(). The only failures possible in this state today are: 1) If we're unable to create a descriptor table (PS2: L473). 2) If we're unable to create a thread for a FIS (PS2: L534) STARTED basically means that we were able to create a descriptor table and start all the threads for the FISes. It doesn't serve any special purpose other than marking a logical step which can help with debugging issues and providing observability once we're able to reflect these states in the RuntimeProfile. If a query fails in the STARTED state, then we know that some FIS failed to Prepare(). It's also lightweight to have this since we don't add any counting barriers or such for this. It basically means that StartFInstances() has completed successfully. I've removed this as well though, since there's been more push back on this. PREPARED means that all the FISes have finished Prepare(). This state is important as we expose FISes only after this state, for Cancel(), PublishFilter(), etc. We will also start status reporting only after reaching this state. OPENED doesn't serve any special purpose other than act as a logical step in the query lifecycle. Again, it could help with observability and debugging, but the benefit we get from this doesn't justify the cost of all the added synchronization, so I've removed it. FINISHED, CANCELLED and ERROR are fairly obvious as to why they're needed and important.
[Impala-ASF-CR] [DRAFT] IMPALA-6189: Add thread watchdogs for HDFS IO calls
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/10696 ) Change subject: [DRAFT] IMPALA-6189: Add thread watchdogs for HDFS IO calls .. Patch Set 4: Working on rebasing this patch on to an upcoming Kudu rebase. The Kudu code, in this case, changed quite a bit. Also looking into making minimal changes to the kudu-util code so that it helps future rebases. -- To view, visit http://gerrit.cloudera.org:8080/10696 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28e918761c120043d332b034450208eaf34e3e2b Gerrit-Change-Number: 10696 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Thu, 28 Jun 2018 22:09:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Michael Ho has posted comments on this change. (
http://gerrit.cloudera.org:8080/10813 )
Change subject: IMPALA-7163: Implement a state machine for the QueryState class
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10813/2/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:
http://gerrit.cloudera.org:8080/#/c/10813/2/be/src/runtime/query-state.cc@209
PS2, Line 209: {BackendExecState::INITIALIZED, "INITIALIZED"},
: {BackendExecState::STARTED, "STARTED"},
: {BackendExecState::PREPARED, "PREPARED"},
: {BackendExecState::OPENED, "OPENED"},
: {BackendExecState::FINISHED, "FINISHED"},
: {BackendExecState::CANCELLED, "CANCELLED"},
: {BackendExecState::ERROR, "ERROR"}};
: return exec_state_to_str.at(state);
As discussed offline, it seems we can simplify this a bit by merging
INITIALIZED and STARTED states and get rid of OPENED state altogether.
--
To view, visit http://gerrit.cloudera.org:8080/10813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe
Gerrit-Change-Number: 10813
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Sailesh Mukil
Gerrit-Comment-Date: Thu, 28 Jun 2018 22:08:12 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] [DOCS] Clarification on admission control and DDL statements
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/10829 ) Change subject: [DOCS] Clarification on admission control and DDL statements .. Patch Set 2: (6 comments) Removed the confusing examples and paragraphs. Added what are affected by admission control at the top. http://gerrit.cloudera.org:8080/#/c/10829/2/docs/topics/impala_admission.xml File docs/topics/impala_admission.xml: http://gerrit.cloudera.org:8080/#/c/10829/2/docs/topics/impala_admission.xml@822 PS2, Line 822: Most write operations in Impala are not resource-intensive, but > This bit about "most write operations" feels very tangential, not sure how Removed http://gerrit.cloudera.org:8080/#/c/10829/2/docs/topics/impala_admission.xml@847 PS2, Line 847: With or without admission control, all queries submitted in a session > Unfortunately this isn't even true - it depends on the driver, the APIs giv Removed http://gerrit.cloudera.org:8080/#/c/10829/2/docs/topics/impala_admission.xml@853 PS2, Line 853: Except for the CREATE TABLE AS SELECT statements, > And compute stats Done http://gerrit.cloudera.org:8080/#/c/10829/2/docs/topics/impala_admission.xml@855 PS2, Line 855: those DDL statements > which does "those DDL statements" mean? It's a little unclear. I think it's Done http://gerrit.cloudera.org:8080/#/c/10829/2/docs/topics/impala_admission.xml@855 PS2, Line 855: for serial execution > remove "for serial execution" - AC doesn't guarantee anything whether state Removed http://gerrit.cloudera.org:8080/#/c/10829/2/docs/topics/impala_admission.xml@859 PS2, Line 859: -- This query is queued by admission control to avoid out-of-memory at times of heavy load. : SELECT * FROM huge_table JOIN enormous_table USING (id); : : -- This subsequent statement in the same session is also queued, but for serial execution and : -- not for admission control, until the previous statement completes. : DROP TABLE huge_table; : > I think this example is still misleading. I'm not sure what it's demonstrat Done -- To view, visit http://gerrit.cloudera.org:8080/10829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e3e82bd34e88e7a13de1864aeb97f01023bc715 Gerrit-Change-Number: 10829 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 28 Jun 2018 21:54:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [DOCS] Clarification on admission control and DDL statements
Hello Balazs Jeszenszky, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10829 to look at the new patch set (#3). Change subject: [DOCS] Clarification on admission control and DDL statements .. [DOCS] Clarification on admission control and DDL statements Removed the confusing example and paragraphs. Change-Id: I2e3e82bd34e88e7a13de1864aeb97f01023bc715 --- M docs/topics/impala_admission.xml 1 file changed, 61 insertions(+), 85 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/10829/3 -- To view, visit http://gerrit.cloudera.org:8080/10829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2e3e82bd34e88e7a13de1864aeb97f01023bc715 Gerrit-Change-Number: 10829 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6802 (part 6): Clean up authorization tests
Zoram Thanga has posted comments on this change. (
http://gerrit.cloudera.org:8080/10841 )
Change subject: IMPALA-6802 (part 6): Clean up authorization tests
..
Patch Set 3: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10841/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:
http://gerrit.cloudera.org:8080/#/c/10841/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2081
PS3, Line 2081: private static String dropFunctionError(String object) {
Don't we need a similar handler for function use error?
--
To view, visit http://gerrit.cloudera.org:8080/10841
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id594ce09a821aef4a1debfdd61569a11defd1c55
Gerrit-Change-Number: 10841
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Adam Holley
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Reviewer: Zoram Thanga
Gerrit-Comment-Date: Thu, 28 Jun 2018 21:29:01 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7215: Implement a templatized CountingBarrier
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10827 ) Change subject: IMPALA-7215: Implement a templatized CountingBarrier .. IMPALA-7215: Implement a templatized CountingBarrier Currently, our CountingBarrier util only notifies a 'bool' value and uses an underlying Promise. We're seeing cases in code where we might want to be notified of a different kind of Promise (other than bool). We add a templatized class TypedCountingBarrier and convert CountingBarrier to use the TypedCountingBarrier internally. This was identified while working on IMPALA-7163. Testing: Ran 'core' tests. Change-Id: I05fc79228250408ae16481ae7ff3491a90d26b8e Reviewed-on: http://gerrit.cloudera.org:8080/10827 Reviewed-by: Sailesh Mukil Tested-by: Impala Public Jenkins --- M be/src/exec/hdfs-scan-node.cc M be/src/rpc/rpc-mgr-test.cc M be/src/runtime/coordinator.cc M be/src/util/counting-barrier.h M be/src/util/hdfs-bulk-ops.cc 5 files changed, 62 insertions(+), 25 deletions(-) Approvals: Sailesh Mukil: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10827 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I05fc79228250408ae16481ae7ff3491a90d26b8e Gerrit-Change-Number: 10827 Gerrit-PatchSet: 6 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-7215: Implement a templatized CountingBarrier
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10827 ) Change subject: IMPALA-7215: Implement a templatized CountingBarrier .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10827 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I05fc79228250408ae16481ae7ff3491a90d26b8e Gerrit-Change-Number: 10827 Gerrit-PatchSet: 5 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Thu, 28 Jun 2018 21:18:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6086: Use of permanent function should require SELECT privilege on DB
Zoram Thanga has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10842 Change subject: IMPALA-6086: Use of permanent function should require SELECT privilege on DB .. IMPALA-6086: Use of permanent function should require SELECT privilege on DB To use a permanent UDF should require at leat SELECT privilege on the database. Functions that have constant arguments get constant-folded into string literals, losing their privilege requests in the process. This patch saves the privilege requests found during the first phase of query analysis, where all the objects and the privileges required to access them are identified. The requests are added back to the new analyzer created for re-analysis post expression rewrite. New FE test cases have been added to AuthorizationTest. Manual tests were also done to identify the bug, as well as to test the fix. Ran private parameterized Jenkins job, exhaustive exploration and covering all tests. Change-Id: I45f5b51363afbe46653b131fad9bf68e3e3ce71f --- M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java 2 files changed, 44 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/10842/1 -- To view, visit http://gerrit.cloudera.org:8080/10842 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I45f5b51363afbe46653b131fad9bf68e3e3ce71f Gerrit-Change-Number: 10842 Gerrit-PatchSet: 1 Gerrit-Owner: Zoram Thanga
[Impala-ASF-CR] IMPALA-6802 (part 6): Clean up authorization tests
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10841 Change subject: IMPALA-6802 (part 6): Clean up authorization tests .. IMPALA-6802 (part 6): Clean up authorization tests This is the last part of the authorization test clean up. This patch rewrites the following tests: - alter database - explain - comment on - function - alter table/view This patch also adds the following authorization tests: - update - upsert - delete The tests in AuthorizationTest.java that have been rewritten into AuthorizationStmtTest.java are removed. Cherry-picks: not for 2.x Change-Id: Id594ce09a821aef4a1debfdd61569a11defd1c55 --- M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java 2 files changed, 517 insertions(+), 2,057 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/10841/3 -- To view, visit http://gerrit.cloudera.org:8080/10841 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id594ce09a821aef4a1debfdd61569a11defd1c55 Gerrit-Change-Number: 10841 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya
[Impala-ASF-CR] IMPALA-7140 (part 8): support views in LocalCatalog
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10805 ) Change subject: IMPALA-7140 (part 8): support views in LocalCatalog .. Patch Set 1: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/10805/1/fe/src/main/java/org/apache/impala/catalog/local/LocalView.java File fe/src/main/java/org/apache/impala/catalog/local/LocalView.java: http://gerrit.cloudera.org:8080/#/c/10805/1/fe/src/main/java/org/apache/impala/catalog/local/LocalView.java@61 PS1, Line 61: previous 'View' nit: slightly confusing since its still there. how about just replacing this with for View http://gerrit.cloudera.org:8080/#/c/10805/1/fe/src/main/java/org/apache/impala/catalog/local/LocalView.java@73 PS1, Line 73: ull sp: null -- To view, visit http://gerrit.cloudera.org:8080/10805 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib3516b9ceff6dce12ded68d93afde09728627e08 Gerrit-Change-Number: 10805 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 28 Jun 2018 18:26:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6976: Parser to parse Impala query profiles
Nithya Janarthanan has abandoned this change. ( http://gerrit.cloudera.org:8080/10309 ) Change subject: IMPALA-6976: Parser to parse Impala query profiles .. Abandoned A decision has been made to not have this script here. Hence abandoning the review -- To view, visit http://gerrit.cloudera.org:8080/10309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I30e9443e87a500c081892d670ed49c7393826c59 Gerrit-Change-Number: 10309 Gerrit-PatchSet: 2 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Mostafa Mokhtar
[Impala-ASF-CR] IMPALA-6976: Parser to parse Impala query profiles
Nithya Janarthanan has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10309 Change subject: IMPALA-6976: Parser to parse Impala query profiles .. IMPALA-6976: Parser to parse Impala query profiles Script to parse Query profiles to generate a one line summary for each of the profiles, which can then be used for generating reports Testing: Tested by running the script on a few profiles to generate the summary Change-Id: I30e9443e87a500c081892d670ed49c7393826c59 --- A tests/benchmark/ParseQueryProfile.py 1 file changed, 668 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/10309/2 -- To view, visit http://gerrit.cloudera.org:8080/10309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I30e9443e87a500c081892d670ed49c7393826c59 Gerrit-Change-Number: 10309 Gerrit-PatchSet: 2 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Mostafa Mokhtar
[Impala-ASF-CR] IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
Vuk Ercegovac has posted comments on this change. (
http://gerrit.cloudera.org:8080/10798 )
Change subject: IMPALA-7140 (part 7): small fixes to enable most queries on
HDFS tables
..
Patch Set 2: Code-Review+2
(1 comment)
thx, good to separate builtins from catalog.
http://gerrit.cloudera.org:8080/#/c/10798/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java:
http://gerrit.cloudera.org:8080/#/c/10798/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@250
PS2, Line 250: if (ids.isEmpty()) {
nit: one line
--
To view, visit http://gerrit.cloudera.org:8080/10798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f603e62b7a013c148c0905ebdec2f4303f9c4e5
Gerrit-Change-Number: 10798
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Tianyi Wang
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Thu, 28 Jun 2018 18:09:05 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7140 (part 6): fetch column stats for LocalTable
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10797 ) Change subject: IMPALA-7140 (part 6): fetch column stats for LocalTable .. Patch Set 1: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/10797/1/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java: http://gerrit.cloudera.org:8080/#/c/10797/1/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@395 PS1, Line 395: // TODO(todd): this calculation ends up setting the num_nulls stat hmm, that seems odd. I'll look into it more and file a jira. thx for pointing it out. http://gerrit.cloudera.org:8080/#/c/10797/1/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java File fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java: http://gerrit.cloudera.org:8080/#/c/10797/1/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java@57 PS1, Line 57: private static final Logger LOG = Logger.getLogger(Table.class); LocalTable.class -- To view, visit http://gerrit.cloudera.org:8080/10797 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6403c2bedf4ee29c5e6f90e947382cb44f46e0c Gerrit-Change-Number: 10797 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 28 Jun 2018 18:03:06 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7215: Implement a templatized CountingBarrier
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10827 ) Change subject: IMPALA-7215: Implement a templatized CountingBarrier .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2753/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10827 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I05fc79228250408ae16481ae7ff3491a90d26b8e Gerrit-Change-Number: 10827 Gerrit-PatchSet: 5 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Thu, 28 Jun 2018 17:49:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7215: Implement a templatized CountingBarrier
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10827 ) Change subject: IMPALA-7215: Implement a templatized CountingBarrier .. Patch Set 5: Code-Review+2 (4 comments) Thanks for the review! Rebase, carry +2. http://gerrit.cloudera.org:8080/#/c/10827/4/be/src/util/counting-barrier.h File be/src/util/counting-barrier.h: http://gerrit.cloudera.org:8080/#/c/10827/4/be/src/util/counting-barrier.h@36 PS4, Line 36: ks Wait() with > Not clear which "returned value" this is talking about - sounds like Notify Done http://gerrit.cloudera.org:8080/#/c/10827/4/be/src/util/counting-barrier.h@43 PS4, Line 43: : /// Sets the number of pending notificatio > that's clearer. you could say it this way for Notify() comment. Done http://gerrit.cloudera.org:8080/#/c/10827/4/be/src/util/counting-barrier.h@56 PS4, Line 56: > public comments shouldn't talk about private fields (the client of this cla Done http://gerrit.cloudera.org:8080/#/c/10827/4/be/src/util/counting-barrier.h@61 PS4, Line 61: ms' passes, in which : /// case '*timed_out' will be true. If > same Done -- To view, visit http://gerrit.cloudera.org:8080/10827 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I05fc79228250408ae16481ae7ff3491a90d26b8e Gerrit-Change-Number: 10827 Gerrit-PatchSet: 5 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Thu, 28 Jun 2018 17:49:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7215: Implement a templatized CountingBarrier
Hello Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10827 to look at the new patch set (#5). Change subject: IMPALA-7215: Implement a templatized CountingBarrier .. IMPALA-7215: Implement a templatized CountingBarrier Currently, our CountingBarrier util only notifies a 'bool' value and uses an underlying Promise. We're seeing cases in code where we might want to be notified of a different kind of Promise (other than bool). We add a templatized class TypedCountingBarrier and convert CountingBarrier to use the TypedCountingBarrier internally. This was identified while working on IMPALA-7163. Testing: Ran 'core' tests. Change-Id: I05fc79228250408ae16481ae7ff3491a90d26b8e --- M be/src/exec/hdfs-scan-node.cc M be/src/rpc/rpc-mgr-test.cc M be/src/runtime/coordinator.cc M be/src/util/counting-barrier.h M be/src/util/hdfs-bulk-ops.cc 5 files changed, 62 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/10827/5 -- To view, visit http://gerrit.cloudera.org:8080/10827 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I05fc79228250408ae16481ae7ff3491a90d26b8e Gerrit-Change-Number: 10827 Gerrit-PatchSet: 5 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-5981: [DOCS] Documented SET=""
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/10816 ) Change subject: IMPALA-5981: [DOCS] Documented SET="" .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/10816/1/docs/topics/impala_set.xml File docs/topics/impala_set.xml: http://gerrit.cloudera.org:8080/#/c/10816/1/docs/topics/impala_set.xml@57 PS1, Line 57: SET ALL > After checking the impala-shell command and start option doc, I removed imp Done http://gerrit.cloudera.org:8080/#/c/10816/2/docs/topics/impala_set.xml File docs/topics/impala_set.xml: http://gerrit.cloudera.org:8080/#/c/10816/2/docs/topics/impala_set.xml@142 PS2, Line 142: When the SET command is run through the JDBC or ODBC interfaces, the > This is true for any interface that isn't impala-shell, e.g. Hue. Remove th The paragraph was removed. The outputs are pretty-self explanatory. -- To view, visit http://gerrit.cloudera.org:8080/10816 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7211405d5cc0a548c05ea5218798591873c14417 Gerrit-Change-Number: 10816 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 28 Jun 2018 17:39:07 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5981: [DOCS] Documented SET=""
Hello Bharath Vissapragada, Philip Zeyliger, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10816 to look at the new patch set (#3). Change subject: IMPALA-5981: [DOCS] Documented SET="" .. IMPALA-5981: [DOCS] Documented SET="" Also, refactored the Impala SET doc and moved the command SET to the Impala Shell Commands doc. Change-Id: I7211405d5cc0a548c05ea5218798591873c14417 --- M docs/topics/impala_set.xml M docs/topics/impala_shell_commands.xml 2 files changed, 105 insertions(+), 225 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/10816/3 -- To view, visit http://gerrit.cloudera.org:8080/10816 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7211405d5cc0a548c05ea5218798591873c14417 Gerrit-Change-Number: 10816 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7215: Implement a templatized CountingBarrier
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10827 ) Change subject: IMPALA-7215: Implement a templatized CountingBarrier .. Patch Set 4: Code-Review+2 (4 comments) Just a few more comment cleanups for the public interface. http://gerrit.cloudera.org:8080/#/c/10827/4/be/src/util/counting-barrier.h File be/src/util/counting-barrier.h: http://gerrit.cloudera.org:8080/#/c/10827/4/be/src/util/counting-barrier.h@36 PS4, Line 36: returned value Not clear which "returned value" this is talking about - sounds like Notify()'s returned value. The comment for NotifyRemaining is clearer http://gerrit.cloudera.org:8080/#/c/10827/4/be/src/util/counting-barrier.h@43 PS4, Line 43: unblocks Wait() with : /// the returned value as 'promise_value'. that's clearer. you could say it this way for Notify() comment. http://gerrit.cloudera.org:8080/#/c/10827/4/be/src/util/counting-barrier.h@56 PS4, Line 56: 'promise_' public comments shouldn't talk about private fields (the client of this class shouldn't care that there is a promise in the implementation). Something like: Returns the value passed to the final Notify() or NotifyRemaining() call. http://gerrit.cloudera.org:8080/#/c/10827/4/be/src/util/counting-barrier.h@61 PS4, Line 61: returns the value set : /// on 'promise_' in Notify() or Notif same -- To view, visit http://gerrit.cloudera.org:8080/10827 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I05fc79228250408ae16481ae7ff3491a90d26b8e Gerrit-Change-Number: 10827 Gerrit-PatchSet: 4 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Thu, 28 Jun 2018 17:35:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7190: Remove unsupported format writer support
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10823 ) Change subject: IMPALA-7190: Remove unsupported format writer support .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/10823/1/be/src/exec/hdfs-table-sink.cc File be/src/exec/hdfs-table-sink.cc: http://gerrit.cloudera.org:8080/#/c/10823/1/be/src/exec/hdfs-table-sink.cc@484 PS1, Line 484: } presumably we do this error checking at runtime rather than analysis so that we can insert into new partitions of a table that has old partitions in a format that's supported for reading but not writing. Do we have a test case to demonstrate that this works? i.e. the case that shows we don't hit these errors when inserting to a table that has these formats in other partitions. -- To view, visit http://gerrit.cloudera.org:8080/10823 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I821dc7495a901f1658daa500daf3791b386c7185 Gerrit-Change-Number: 10823 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 28 Jun 2018 17:28:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7215: Implement a templatized CountingBarrier
Sailesh Mukil has posted comments on this change. (
http://gerrit.cloudera.org:8080/10827 )
Change subject: IMPALA-7215: Implement a templatized CountingBarrier
..
Patch Set 4:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/10827/3/be/src/util/counting-barrier.h
File be/src/util/counting-barrier.h:
http://gerrit.cloudera.org:8080/#/c/10827/3/be/src/util/counting-barrier.h@34
PS3, Line 34: /// Sends one notification, decrementing the number of pending
notifications by one.
> Add something like:
Done
http://gerrit.cloudera.org:8080/#/c/10827/3/be/src/util/counting-barrier.h@36
PS3, Line 36: /// If
> looks like no caller actually uses this return value and so you've annotate
We just don't use it anywhere now, but I think it is useful to have as a Util
API, since in some cases we may want to know if we were the last one to
Notify() or not. Otherwise, it's also useful to return the number of pending
counts.
http://gerrit.cloudera.org:8080/#/c/10827/3/be/src/util/counting-barrier.h@42
PS3, Line 42:
> similarly, add comment about 'promise_value'
Done
http://gerrit.cloudera.org:8080/#/c/10827/3/be/src/util/counting-barrier.h@54
PS3, Line 54: }
> comment the return value
Done
http://gerrit.cloudera.org:8080/#/c/10827/3/be/src/util/counting-barrier.h@55
PS3, Line 55:
> would be good to return "const T&" here and below (to match Promise.Get())
Done
http://gerrit.cloudera.org:8080/#/c/10827/3/be/src/util/counting-barrier.h@58
PS3, Line 58: const T& Wait() { return promise_.Get(); }
> comment return value
Done
--
To view, visit http://gerrit.cloudera.org:8080/10827
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05fc79228250408ae16481ae7ff3491a90d26b8e
Gerrit-Change-Number: 10827
Gerrit-PatchSet: 4
Gerrit-Owner: Sailesh Mukil
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Sailesh Mukil
Gerrit-Comment-Date: Thu, 28 Jun 2018 17:24:35 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7215: Implement a templatized CountingBarrier
Hello Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10827 to look at the new patch set (#4). Change subject: IMPALA-7215: Implement a templatized CountingBarrier .. IMPALA-7215: Implement a templatized CountingBarrier Currently, our CountingBarrier util only notifies a 'bool' value and uses an underlying Promise. We're seeing cases in code where we might want to be notified of a different kind of Promise (other than bool). We add a templatized class TypedCountingBarrier and convert CountingBarrier to use the TypedCountingBarrier internally. This was identified while working on IMPALA-7163. Testing: Ran 'core' tests. Change-Id: I05fc79228250408ae16481ae7ff3491a90d26b8e --- M be/src/exec/hdfs-scan-node.cc M be/src/rpc/rpc-mgr-test.cc M be/src/runtime/coordinator.cc M be/src/util/counting-barrier.h M be/src/util/hdfs-bulk-ops.cc 5 files changed, 61 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/10827/4 -- To view, visit http://gerrit.cloudera.org:8080/10827 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I05fc79228250408ae16481ae7ff3491a90d26b8e Gerrit-Change-Number: 10827 Gerrit-PatchSet: 4 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] [DOCS] Corrected the supported values for parquet array resolution
Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10840 Change subject: [DOCS] Corrected the supported values for parquet_array_resolution .. [DOCS] Corrected the supported values for parquet_array_resolution Change-Id: Icfc1b7209d0f6b28c814be4149b0bacfebad2356 --- M docs/topics/impala_parquet_array_resolution.xml 1 file changed, 3 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/10840/1 -- To view, visit http://gerrit.cloudera.org:8080/10840 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Icfc1b7209d0f6b28c814be4149b0bacfebad2356 Gerrit-Change-Number: 10840 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni
[Impala-ASF-CR] IMPALA-7190: Remove unsupported format writer support
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10823 ) Change subject: IMPALA-7190: Remove unsupported format writer support .. Patch Set 1: Code-Review+1 (1 comment) I agree with Csaba's comments too. http://gerrit.cloudera.org:8080/#/c/10823/1/tests/common/test_dimensions.py File tests/common/test_dimensions.py: http://gerrit.cloudera.org:8080/#/c/10823/1/tests/common/test_dimensions.py@111 PS1, Line 111: # Available Exec Options: Let's just delete this whole comment, it's insanely stale. It's a terrible idea to keep a redundant list of query options in a comment. -- To view, visit http://gerrit.cloudera.org:8080/10823 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I821dc7495a901f1658daa500daf3791b386c7185 Gerrit-Change-Number: 10823 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 28 Jun 2018 16:00:57 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7095: clean up scan node profiles
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10810 ) Change subject: IMPALA-7095: clean up scan node profiles .. Patch Set 6: Code-Review+1 (5 comments) Looks good, only had some minor comments. http://gerrit.cloudera.org:8080/#/c/10810/5/be/src/exec/hdfs-scan-node-base.h File be/src/exec/hdfs-scan-node-base.h: http://gerrit.cloudera.org:8080/#/c/10810/5/be/src/exec/hdfs-scan-node-base.h@489 PS5, Line 489: per_read_thread > The name of the counter is "PerReadThreadRawHdfsThroughput", which isn't a Oh, I see. The new comment is much better I think! http://gerrit.cloudera.org:8080/#/c/10810/6/be/src/exec/scan-node.h File be/src/exec/scan-node.h: http://gerrit.cloudera.org:8080/#/c/10810/6/be/src/exec/scan-node.h@52 PS6, Line 52: TotalRawHdfsReadTime This counter is in HdfsScanNodeBase. PerReadThreadRawHdfsThroughput and NumDiskAccessed counter as well. It it is meant to be a complete list of all the counters then hdfs_open_file_timer_ and counters related to row batches are missing. http://gerrit.cloudera.org:8080/#/c/10810/5/be/src/exec/scan-node.h File be/src/exec/scan-node.h: http://gerrit.cloudera.org:8080/#/c/10810/5/be/src/exec/scan-node.h@256 PS5, Line 256: * > Yeah we had an extended discussion about this pattern at one point: https:/ Thanks, it was an interesting thread. Yeah, about BlockingQueue, I think the issue exists for all functions that take r-value references and don't always take ownership. And since BlockingQueue::BlockingPutWithTimeout() takes universal/forwarding references it can also take r-values. http://gerrit.cloudera.org:8080/#/c/10810/6/be/src/exec/scan-node.cc File be/src/exec/scan-node.cc: http://gerrit.cloudera.org:8080/#/c/10810/6/be/src/exec/scan-node.cc@222 PS6, Line 222: // faster producer. Also used for Kudu under the assumption that the scan runs co-located Nit: too long line http://gerrit.cloudera.org:8080/#/c/10810/6/be/src/exec/scan-node.cc@247 PS6, Line 247: num_active Nit: Since C++14 the lambda capture can be [_active = num_active_] and no need to create local variable at L245. -- To view, visit http://gerrit.cloudera.org:8080/10810 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I77286282d42e7764bfdf94c7ec47cec9d743f787 Gerrit-Change-Number: 10810 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 28 Jun 2018 14:07:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7205: Respond to ReportExecStatus() RPC with CANCELLED if query execution has terminated
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10815 ) Change subject: IMPALA-7205: Respond to ReportExecStatus() RPC with CANCELLED if query execution has terminated .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7bb2c26edace89853f14a329f891d1f9a065a991 Gerrit-Change-Number: 10815 Gerrit-PatchSet: 7 Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Thu, 28 Jun 2018 07:38:41 + Gerrit-HasComments: No
