[Impala-ASF-CR] IMPALA-4525: follow-on: cleanup error handling
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4525: follow-on: cleanup error handling .. Patch Set 5: Code-Review+2 Rebase -- To view, visit http://gerrit.cloudera.org:8080/5212 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb2e532381c3e6c7b2e3ef37c2d91fbebb2db2d0 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4525: follow-on: cleanup error handling
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-4525: follow-on: cleanup error handling .. Patch Set 5: Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/67/ -- To view, visit http://gerrit.cloudera.org:8080/5212 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb2e532381c3e6c7b2e3ef37c2d91fbebb2db2d0 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4525: follow-on: cleanup error handling
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4525: follow-on: cleanup error handling .. Patch Set 4: Failed because of IMPALA-4567 -- To view, visit http://gerrit.cloudera.org:8080/5212 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb2e532381c3e6c7b2e3ef37c2d91fbebb2db2d0 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE TABLE
Alex Behm has posted comments on this change.
Change subject: IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE
TABLE
..
Patch Set 1:
(7 comments)
To avoid more rounds, maybe we should sync on the exact class/var names before
you make changes.
http://gerrit.cloudera.org:8080/#/c/5317/1/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:
Line 380: struct TPartitionParam {
TKuduPartitionParam?
http://gerrit.cloudera.org:8080/#/c/5317/1/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:
Line 404: nonterminal TableDataLayout opt_tbl_data_layout,
distributed_data_layout;
also change distributed_data_layout
Line 411: nonterminal PartitionParam partition_hash_param;
KuduPartitionParam?
Seems better to be specific and not confuse it with HDFS partitioning, since
"Param" is very generic.
Line 1084:
tbl_def.getPartitionColumnDefs().addAll(data_layout.getPartitionColumnDefs());
regarding my rename suggestion: I'd imagine these two lines here are rather
confusing for any one but the two of us
I understand that doing another rename is extremely annoying, but some parts of
the code are pretty confusing now because to other readers the difference
between partition "column defs" and "params" seems quite unclear.
http://gerrit.cloudera.org:8080/#/c/5317/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
Line 88: public List getPartitionColumnDefs() {
these two getters are also confusing
http://gerrit.cloudera.org:8080/#/c/5317/1/fe/src/main/java/org/apache/impala/analysis/RangePartition.java
File fe/src/main/java/org/apache/impala/analysis/RangePartition.java:
Line 113: public void analyze(Analyzer analyzer, List
partitioningColDefs)
partColDefs?
http://gerrit.cloudera.org:8080/#/c/5317/1/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
File fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java:
Line 25: * Represents the PARTITION BY and PARTITIONED BY clauses of a DDL
statement.
Fingers crossed.
--
To view, visit http://gerrit.cloudera.org:8080/5317
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e07c41eabb4c8cb95754cf04293cbd9e03d6ab2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis
Gerrit-Reviewer: Alex Behm
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4571: InList predicates not being pushed to Kudu scans
Alex Behm has posted comments on this change.
Change subject: IMPALA-4571: InList predicates not being pushed to Kudu scans
..
Patch Set 2:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/5316/2//COMMIT_MSG
Commit Message:
Line 7: IMPALA-4571: InList predicates not being pushed to Kudu scans
Push IN predicates to Kudu.
(or something along those lines which summarizes what this change does)
Line 12: An InPredicate can be pushed to the scan if:
might be more concise to mention that only these predicates are supported:
IN (, , ...)
Line 13: 1) It has a list of values (i.e. not a subquery)
list of literal values
Line 17:exactly.
one requirement missing from this list is is that all values in the IN list
must be literals
http://gerrit.cloudera.org:8080/#/c/5316/2/fe/src/main/java/org/apache/impala/analysis/InPredicate.java
File fe/src/main/java/org/apache/impala/analysis/InPredicate.java:
Line 54: Preconditions.checkArgument(isAnalyzed_);
Not really a requirement, why not just inline this one-line function at the
caller?
http://gerrit.cloudera.org:8080/#/c/5316/2/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:
Line 366: if (predicate.isNotIn() || !predicate.hasValuesList()) return
false;
the hasValuesList() check seems unecessary, it is already covered by checking
the children below
Line 371: PrimitiveType type = ref.getType().getPrimitiveType();
unused?
Line 397: private static Object getKuduInListValue(PrimitiveType type, Expr
e) {
Move to LiteralExpr.valueAsObject() or something like that?
Line 399: if (type != e.getType().getPrimitiveType()) return null;
This is a post-condition of InPredicate.analyze(), you don't need to check it
here again.
Line 409: default: return null;
It sounds like a bug if we hit this case because it implies we have a SlotRef
on a type that is not supported by Kudu.
Is there an easy way to check if a type is supported by Kudu? If so, you can
add a Preconditions check for that instead on the type of the uncast SlotRef.
http://gerrit.cloudera.org:8080/#/c/5316/2/testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
File
testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test:
Line 108: double_col in (0.0) and
string_col?
Line 117: double_col in (cast('inf' as double))
* add a NOT IN predicate
* add an IN predicate with a SlotRef in the IN list
http://gerrit.cloudera.org:8080/#/c/5316/2/testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
File testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test:
Line 1084:kudu predicates: l_shipmode IN ('AIR', 'AIR REG'), l_shipinstruct
= 'DELIVER IN PERSON'
nice improvements!
--
To view, visit http://gerrit.cloudera.org:8080/5316
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8988d4819d20d467b48e286917e347ca00f60cf0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs
Gerrit-Reviewer: Alex Behm
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4525: follow-on: cleanup error handling
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-4525: follow-on: cleanup error handling .. Patch Set 4: Verified-1 Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/66/ -- To view, visit http://gerrit.cloudera.org:8080/5212 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb2e532381c3e6c7b2e3ef37c2d91fbebb2db2d0 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE TABLE
Dimitris Tsirogiannis has uploaded a new change for review. http://gerrit.cloudera.org:8080/5317 Change subject: IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE TABLE .. IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE TABLE Change-Id: I0e07c41eabb4c8cb95754cf04293cbd9e03d6ab2 --- M common/thrift/CatalogObjects.thrift M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddDropRangePartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java R fe/src/main/java/org/apache/impala/analysis/PartitionParam.java M fe/src/main/java/org/apache/impala/analysis/RangePartition.java M fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java M fe/src/main/java/org/apache/impala/analysis/TableDef.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M testdata/bin/generate-schema-statements.py M testdata/datasets/functional/functional_schema_template.sql M testdata/datasets/tpcds/tpcds_kudu_template.sql M testdata/datasets/tpcds/tpcds_schema_template.sql M testdata/datasets/tpch/tpch_kudu_template.sql M testdata/datasets/tpch/tpch_schema_template.sql M testdata/workloads/functional-planner/queries/PlannerTest/lineage.test M testdata/workloads/functional-query/queries/QueryTest/kudu-scan-node.test M testdata/workloads/functional-query/queries/QueryTest/kudu-timeouts-catalogd.test M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test M testdata/workloads/functional-query/queries/QueryTest/kudu_delete.test M testdata/workloads/functional-query/queries/QueryTest/kudu_describe.test M testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test M testdata/workloads/functional-query/queries/QueryTest/kudu_partition_ddl.test M testdata/workloads/functional-query/queries/QueryTest/kudu_stats.test M testdata/workloads/functional-query/queries/QueryTest/kudu_update.test M testdata/workloads/functional-query/queries/QueryTest/kudu_upsert.test M tests/query_test/test_cancellation.py M tests/query_test/test_kudu.py M tests/shell/test_shell_commandline.py 36 files changed, 409 insertions(+), 411 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/17/5317/1 -- To view, visit http://gerrit.cloudera.org:8080/5317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0e07c41eabb4c8cb95754cf04293cbd9e03d6ab2 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis
[Impala-ASF-CR] IMPALA-4567: Fix test kudu alter table exhaustive failures
Michael Brown has posted comments on this change. Change subject: IMPALA-4567: Fix test_kudu_alter_table exhaustive failures .. Patch Set 2: I see this is already submitted, but: > Do you know how I can only run 1 of the 2 (e.g. just codegen enabled) for > this particular test fn? To be safe, I think the mark for serial is still a good idea, since the test truly needs to be serial. A quick method would simply be to inspect something in the vector and pytest.skip(), or separating out the Kudu DDL stuff separately into a separate class that only uses 1 dimension. There's no great, perfect solution available to us at this time though. -- To view, visit http://gerrit.cloudera.org:8080/5312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibca64d5567c24240606e454b052d130fcd0c3968 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4343,IMPALA-4354: qgen: model INSERTs; write INSERTs from query model
Michael Brown has posted comments on this change. Change subject: IMPALA-4343,IMPALA-4354: qgen: model INSERTs; write INSERTs from query model .. Patch Set 8: Code-Review+1 (1 comment) Hi Taras, please see patch set 8. http://gerrit.cloudera.org:8080/#/c/5162/7/tests/comparison/query.py File tests/comparison/query.py: PS7, Line 56: xprs = TableExprList([]) > I just tried this: Done. Thanks for catching that. -- To view, visit http://gerrit.cloudera.org:8080/5162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I38e24da78c49e908449b35f0a6276ebe4236ddba Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Brown Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4343,IMPALA-4354: qgen: model INSERTs; write INSERTs from query model
Hello Matthew Jacobs, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5162 to look at the new patch set (#8). Change subject: IMPALA-4343,IMPALA-4354: qgen: model INSERTs; write INSERTs from query model .. IMPALA-4343,IMPALA-4354: qgen: model INSERTs; write INSERTs from query model This patch adds support to the random query generator infrastructure to model and write SQL INSERTs. It does not actually randomly generate INSERTs at this time (tracked in IMPALA-4353 and umbrella task IMPALA-3740) but does provide necessary building blocks to do so. First, it's necessary to model the INSERTs as part of our data model. This was done by taking the current notion of a Query and making it a SelectQuery. We also then create an abstract Query containing some of the more common methods and attributes. We then model an INSERT query, INSERT clause, and VALUES clause (IMPALA-4343). Second, it's necessary to test the basics of this data model. It made sense to go ahead and implement the necessary SqlWriter methods to write the SQL for these clauses (IMPALA-4354). I could then use this writer with some existing and new tests that take a query written into our data model and write the SQL, verifying they're correct. For INSERT into Kudu tables, the equivalent PostgreSQL queries need to use "ON CONFLICT DO NOTHING", so all existing and new query tests verify they can be written as PostgreSQL as well. Testing: - all the query generator tests pass - I can run Leopard front_end.py and load older query generator reports, browse them, and re-run failed queries - I can run Leopard controller.py to actually do a query generator run - discrepancy_searcher.py --explain-only ran for hundreds of queries. There were no problems writing the SELECT queries Change-Id: I38e24da78c49e908449b35f0a6276ebe4236ddba --- M tests/comparison/model_translator.py M tests/comparison/query.py M tests/comparison/tests/query_object_testdata.py M tests/comparison/tests/test_query_objects.py 4 files changed, 642 insertions(+), 68 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/5162/8 -- To view, visit http://gerrit.cloudera.org:8080/5162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I38e24da78c49e908449b35f0a6276ebe4236ddba Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Brown Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky
[Impala-ASF-CR] IMPALA-4567: Fix test kudu alter table exhaustive failures
Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-4567: Fix test_kudu_alter_table exhaustive failures .. IMPALA-4567: Fix test_kudu_alter_table exhaustive failures The issue is that we set the Kudu table name explicitly via tblproperty so it doesn't have the unique db name in the underlying Kudu name. Meanwhile, the tests are run concurrently in exhaustive so this test may end up running the multiple times (w/ different parameters, e.g. disable_codegen) concurrently. This test needs to be run serially. Change-Id: Ibca64d5567c24240606e454b052d130fcd0c3968 Reviewed-on: http://gerrit.cloudera.org:8080/5312 Reviewed-by: David Knupp Reviewed-by: Dimitris Tsirogiannis Tested-by: Internal Jenkins --- M tests/query_test/test_kudu.py 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: David Knupp: Looks good to me, but someone else must approve Internal Jenkins: Verified Dimitris Tsirogiannis: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/5312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ibca64d5567c24240606e454b052d130fcd0c3968 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown
[Impala-ASF-CR] IMPALA-4567: Fix test kudu alter table exhaustive failures
Internal Jenkins has posted comments on this change. Change subject: IMPALA-4567: Fix test_kudu_alter_table exhaustive failures .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/5312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibca64d5567c24240606e454b052d130fcd0c3968 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4571: InList predicates not being pushed to Kudu scans
Matthew Jacobs has uploaded a new patch set (#2). Change subject: IMPALA-4571: InList predicates not being pushed to Kudu scans .. IMPALA-4571: InList predicates not being pushed to Kudu scans Fixes the KuduScanNode to convert InPredicates to KuduPredicates and push them to the Kudu scan if possible. An InPredicate can be pushed to the scan if: 1) It has a list of values (i.e. not a subquery) 2) It is not negative, i.e. only 'IN' not 'NOT IN' 3) The SlotRef is not wrapped in any casts 4) The types of all values match the type of the SlotRef exactly. A planner test was added exercising all supported types as well as exprs where the values would not be supported. TODO: perf testing TODO: consider a limit on the number of list values before keeping the predicate on the Impala scan node (determine from testing) Change-Id: I8988d4819d20d467b48e286917e347ca00f60cf0 --- M fe/src/main/java/org/apache/impala/analysis/InPredicate.java M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test 4 files changed, 119 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/5316/2 -- To view, visit http://gerrit.cloudera.org:8080/5316 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8988d4819d20d467b48e286917e347ca00f60cf0 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs
[Impala-ASF-CR] IMPALA-4571: InList predicates not being pushed to Kudu scans
Matthew Jacobs has uploaded a new change for review. http://gerrit.cloudera.org:8080/5316 Change subject: IMPALA-4571: InList predicates not being pushed to Kudu scans .. IMPALA-4571: InList predicates not being pushed to Kudu scans Fixes the KuduScanNode to convert InPredicates to KuduPredicates and push them to the Kudu scan if possible. An InPredicate can be pushed to the scan if: 1) It has a list of values (i.e. not a subquery) 2) It is not negative, i.e. only 'IN' not 'NOT IN' 3) The SlotRef is not wrapped in any casts 4) The types of all values match the type of the SlotRef exactly. A planner test was added exercising all supported types as well as exprs where the values would not be supported. Change-Id: I8988d4819d20d467b48e286917e347ca00f60cf0 --- M fe/src/main/java/org/apache/impala/analysis/InPredicate.java M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test 4 files changed, 119 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/5316/1 -- To view, visit http://gerrit.cloudera.org:8080/5316 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I8988d4819d20d467b48e286917e347ca00f60cf0 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs
[Impala-ASF-CR] IMPALA-4527: Columns in Kudu tables created from Impala default to "NULL"
Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-4527: Columns in Kudu tables created from Impala default to "NULL" .. IMPALA-4527: Columns in Kudu tables created from Impala default to "NULL" This commit reverts the behavior introduced by IMPALA-3719 which used the Kudu default behavior for column nullability if none was specified in the CREATE TABLE statement. With this commit, non-key columns of Kudu tables that are created from Impala are by default nullable unless specified otherwise. Change-Id: I950d9a9c64e3851e11a641573617790b340ece94 Reviewed-on: http://gerrit.cloudera.org:8080/5259 Reviewed-by: Alex Behm Tested-by: Internal Jenkins --- M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M testdata/workloads/functional-query/queries/QueryTest/kudu_describe.test M testdata/workloads/functional-query/queries/QueryTest/kudu_stats.test 3 files changed, 27 insertions(+), 20 deletions(-) Approvals: Internal Jenkins: Verified Alex Behm: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/5259 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I950d9a9c64e3851e11a641573617790b340ece94 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-4527: Columns in Kudu tables created from Impala default to "NULL"
Internal Jenkins has posted comments on this change. Change subject: IMPALA-4527: Columns in Kudu tables created from Impala default to "NULL" .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/5259 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I950d9a9c64e3851e11a641573617790b340ece94 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4572: Run COMPUTE STATS on Parquet tables with MT DOP=4.
Alex Behm has uploaded a new change for review. http://gerrit.cloudera.org:8080/5315 Change subject: IMPALA-4572: Run COMPUTE STATS on Parquet tables with MT_DOP=4. .. IMPALA-4572: Run COMPUTE STATS on Parquet tables with MT_DOP=4. COMPUTE STATS on Parquet tables is run with MT_DOP=4 by default. COMPUTE STATS on non-Parquet tables will run without MT_DOP. Users can always override the behavior by setting MT_DOP manually. Setting MT_DOP to 0 means a statement will be run in the conventional execution mode (without intra-node paralellism based on multiple fragment instances). Users can set a higher MT_DOP even for Parquet tables. Testing: Added a new test that checks the effective MT_DOP. Locally ran test_mt_dop.py, test_scanners.py, test_nested_types.py, test_compute_stats.py, and test_cancellation.py. Change-Id: I2be3c7c9f3004e9a759224a2e5756eb6e4efa359 --- M be/src/exec/exec-node.cc M be/src/runtime/coordinator.cc M be/src/scheduling/simple-scheduler.cc M common/thrift/ImpalaInternalService.thrift M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java M fe/src/test/java/org/apache/impala/service/FrontendTest.java M tests/query_test/test_mt_dop.py 12 files changed, 116 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/5315/1 -- To view, visit http://gerrit.cloudera.org:8080/5315 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2be3c7c9f3004e9a759224a2e5756eb6e4efa359 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm
[Impala-ASF-CR] IMPALA-4564,IMPALA-4565: mt dop fixes for old aggs and joins
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-4564,IMPALA-4565: mt_dop fixes for old aggs and joins .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/5302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I48c50c8aa0c23710eb099fba252bc3c0cb74b313 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4564,IMPALA-4565: mt dop fixes for old aggs and joins
Impala Public Jenkins has submitted this change and it was merged. Change subject: IMPALA-4564,IMPALA-4565: mt_dop fixes for old aggs and joins .. IMPALA-4564,IMPALA-4565: mt_dop fixes for old aggs and joins Fix a test bug where we need to skip nested types tests for the old aggs and joins. Fix a product bug where *eos is not initialised by the MT scan node. This causes incorrect results when the calling ExecNode does not initialise the eos variable, e.g. the sort node and the old agg and join nodes. Testing: Added a test that reproduces the incorrect results with the sort node when run under ASAN Tested the mt_dop tests locally with old aggs and joins to ensure they pass. Change-Id: I48c50c8aa0c23710eb099fba252bc3c0cb74b313 Reviewed-on: http://gerrit.cloudera.org:8080/5302 Reviewed-by: Alex Behm Tested-by: Impala Public Jenkins --- M be/src/exec/hdfs-scan-node-mt.cc A testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-nested.test M testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet.test M testdata/workloads/functional-query/queries/QueryTest/mt-dop.test M tests/query_test/test_mt_dop.py 5 files changed, 60 insertions(+), 33 deletions(-) Approvals: Impala Public Jenkins: Verified Alex Behm: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/5302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I48c50c8aa0c23710eb099fba252bc3c0cb74b313 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load
Alex Behm has posted comments on this change. Change subject: IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6b41fc3c0e95508ab67f1d420b033b02ec75a5da Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load
Thomas Tauber-Marshall has posted comments on this change.
Change subject: IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails
to load
..
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5144/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
Line 1277: } catch (NoSuchObjectException e) {
> I don't see the purpose of this patch that way. In my mind the purpose is t
As discussed, I'll go back to the previous behavior - table deleted from HMS
fails DROP with error, but update the error message to point out that this can
be solved with 'invalidate metadata'.
--
To view, visit http://gerrit.cloudera.org:8080/5144
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b41fc3c0e95508ab67f1d420b033b02ec75a5da
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load
Thomas Tauber-Marshall has uploaded a new patch set (#5). Change subject: IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load .. IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load If a table fails to load, eg. because it was deleted externally from Kudu, we should still allow 'DROP TABLE' to pass analysis. Otherwise, you may be unable to drop tables that are in a bad state. Testing: - Updates existing Kudu tests to reflect the new behavior, and fixes a couple of problems with those tests that were causing them to pass spuriously (as well as fixing the same problem with another test in the file while I'm here). Change-Id: I6b41fc3c0e95508ab67f1d420b033b02ec75a5da --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M tests/query_test/test_kudu.py 7 files changed, 49 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/5144/5 -- To view, visit http://gerrit.cloudera.org:8080/5144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6b41fc3c0e95508ab67f1d420b033b02ec75a5da Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-4525: follow-on: cleanup error handling
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-4525: follow-on: cleanup error handling .. Patch Set 4: Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/66/ -- To view, visit http://gerrit.cloudera.org:8080/5212 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb2e532381c3e6c7b2e3ef37c2d91fbebb2db2d0 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4525: follow-on: cleanup error handling
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4525: follow-on: cleanup error handling
..
Patch Set 4: Code-Review+2
(3 comments)
Carry +2
http://gerrit.cloudera.org:8080/#/c/5212/3/be/src/service/fe-support.cc
File be/src/service/fe-support.cc:
PS3, Line 82: Status status;
> Why not just Status status; ?
Done
Line 120:
There was a bug here - turns out we need to call Close() after Prepare() even
if Prepare() fails.
PS3, Line 138:
> for (ExprContext* expr_ctx : expr_ctxs) {
Done
--
To view, visit http://gerrit.cloudera.org:8080/5212
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb2e532381c3e6c7b2e3ef37c2d91fbebb2db2d0
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3200: Implement suballocator for splitting buffers
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3200: Implement suballocator for splitting buffers .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/4715/4/be/src/bufferpool/suballocator.h File be/src/bufferpool/suballocator.h: Line 50: ExpandReservations expand_reservations); > Missed the 31st comment :) will fix in a new PS Done -- To view, visit http://gerrit.cloudera.org:8080/4715 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8bfe0e429f67ad273f7c7d0816703a9e6c3da788 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3200: Implement suballocator for splitting buffers
Tim Armstrong has uploaded a new patch set (#8). Change subject: IMPALA-3200: Implement suballocator for splitting buffers .. IMPALA-3200: Implement suballocator for splitting buffers This is useful for situations like hash tables, where we want to make multiple non-spillable allocations of variable size from buffer pool memory and not incur the overhead of interacting with the global buffer pool. The allocator subdivides buffers to service allocations and uses a buddy allocation algorithm to merge freed allocations into larger chunks. This helps avoid external fragmentation and is quite effective at reusing memory given the typical doubling allocation patterns of hash tables in partitioned aggs and joins. Testing: The allocator has fairly robust internal consistency checks via assertions and unique_ptrs. Includes a unit test that exercises various allocation patterns. I tested porting hash tables over to allocate memory using an earlier version of the suballocator, which worked well (was able to run a wide range of queries successfully with good performance). Change-Id: I8bfe0e429f67ad273f7c7d0816703a9e6c3da788 --- M be/src/bufferpool/CMakeLists.txt A be/src/bufferpool/suballocator-test.cc A be/src/bufferpool/suballocator.cc A be/src/bufferpool/suballocator.h M be/src/common/names.h 5 files changed, 924 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/4715/8 -- To view, visit http://gerrit.cloudera.org:8080/4715 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8bfe0e429f67ad273f7c7d0816703a9e6c3da788 Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-3200: Implement suballocator for splitting buffers
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3200: Implement suballocator for splitting buffers .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/4715/4/be/src/bufferpool/suballocator.h File be/src/bufferpool/suballocator.h: Line 50: ExpandReservations expand_reservations); > Looks missing Missed the 31st comment :) will fix in a new PS -- To view, visit http://gerrit.cloudera.org:8080/4715 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8bfe0e429f67ad273f7c7d0816703a9e6c3da788 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3200: Implement suballocator for splitting buffers
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-3200: Implement suballocator for splitting buffers
..
Patch Set 4:
(30 comments)
http://gerrit.cloudera.org:8080/#/c/4715/4//COMMIT_MSG
Commit Message:
PS4, Line 13: buddy allocation
> Though the number of elements in HTs is a power of two, the size of the all
That's true.
IMO we can cross that bridge if we come to it. I don't think we'd want to
increase the size of hash table buckets. I think shrinking them further is an
outside possibility.
Right now this is one of multiple tasks required to switch over to the new
buffer pool so I'd really like to get this code clean and functional for the
current hash table case then move onto other things.
Choosing a # of buckets based on the memory being a power of two size might
make more sense than trying to implement a general-purpose memory allocator.
The buffer pool is also oriented around power-of-two buffers so fragmentation
would be problem for large non-power-of-two hash tables regardless of the
suballocator's behaviour.
http://gerrit.cloudera.org:8080/#/c/4715/5/be/src/bufferpool/suballocator-test.cc
File be/src/bufferpool/suballocator-test.cc:
PS5, Line 72: allocations
> "Suballocations", here and below
Done
Line 72: /// Verify that the memory for all of the allocations is writable
and disjoint by
> Maybe replace "Verify" with "ASSERT", for clarity?
Done
PS5, Line 77: list
> vector
Done
PS5, Line 77: clear
> call std::vector::clear() after the for loop to prevent UB if a caller uses
Done
Line 85: ObjectPool obj_pool_;
> Each of these member variables could use an explanation about why it is her
Done
Line 101: ASSERT_OK(pool.RegisterClient("client", &global_reservation_,
profile(), &client));
> The ASSERTs in this file could generally benefit from more info in the "<<"
I added debug strings and parameters in some places. It's kind of hard to
anticipate what will actually be useful but I added some of the obvious things.
Line 111: allocs.emplace_back();
> I generally think of ASSERT as being for failures in which the test should
My perspective is that EXPECT is only worth using when it's independent of the
remainder of the test. What I've seen a lot of the time with EXPECT in other
tests is that if there's a bug, then later parts of the tests go off into the
weeds and crash.
In this case there's no way to recover without adding extra untested code that
would pop the last allocation off the back.
Line 116:
> Can we allocate 0 bytes here?
I added a couple of separate tests to test edge cases (-1,0,MAX+1).
Line 127: EXPECT_EQ(global_reservation_.GetUsedReservation(), allocated_mem);
> call FreeAllocations?
Done
PS5, Line 147: EST
> It might also be good to test edge cases: (1 << i) - 1
I added some more sizes in.
Line 159: memset(alloc->data(), 0, alloc->len()); // Check memory is
writable.
> Add note that ASAN performs this check.
ASAN helps here, but we detect a lot of problems without ASAN, e.g. if the
memory is not mapped.
Line 180:
> either "NUM_ALLOCS" or "total_mem", above
Done
PS5, Line 184: // Start with allocations smaller than the page.
: int64_t curr_alloc_size = TEST_BUFFER_LEN / 8;
> Could be more readable as a for loop
Done
Line 188: shuffle(allocs.begin(), allocs.end(), rng_);
> Can this be range-based, like the loop on 205?
Done
Line 197: // Test that the memory isn't fragmented more than expected. In
the worst case, the
> Can you add a comment explaining why should they never require more than an
I added a fairly lengthy explanation. Hopefully it's sufficiently clear - it's
kind of hard to explain intuitively.
Line 206: for (unique_ptr& alloc : allocs) {
> call FreeAllocations()
Done
PS5, Line 215: TEST_F(SuballocatorTest, NeverExpandReservation) {
: const int64_t TOTAL_MEM = TEST_BUFFER_LEN * 100;
: global_reservation_.InitRootTracker(NULL, TOTAL_MEM);
: BufferPool pool(TEST_BUFFER_LEN, TOTAL_MEM);
:
: ReservationTracker reservation;
: reservation.InitChildTracker(NULL, &global_reservation_, NULL,
TOTAL_MEM);
: BufferPool::Client client;
: ASSERT_OK(pool.RegisterClient("client", &reservation,
profile(), &client));
:
> This section seems like a good candidate for a member function, as it appea
I tried to reduce the amount of boilerplate a bit by factoring out some common
functions. I like having the setup logic explicit at the top of the test, so
tried to avoid overabstracting to the point of obscuring what was actually
happening.
Line 263:
> This tests only correctness, not anything about fragmentation, right?
Correct.
Line 289:
> const
Done
PS5, Line 290:
> The first parameter below is 2 - that's the mean, right, so the average is
The documentation for
[Impala-ASF-CR] IMPALA-3200: Implement suballocator for splitting buffers
Tim Armstrong has uploaded a new patch set (#7). Change subject: IMPALA-3200: Implement suballocator for splitting buffers .. IMPALA-3200: Implement suballocator for splitting buffers This is useful for situations like hash tables, where we want to make multiple non-spillable allocations of variable size from buffer pool memory and not incur the overhead of interacting with the global buffer pool. The allocator subdivides buffers to service allocations and uses a buddy allocation algorithm to merge freed allocations into larger chunks. This helps avoid external fragmentation and is quite effective at reusing memory given the typical doubling allocation patterns of hash tables in partitioned aggs and joins. Testing: The allocator has fairly robust internal consistency checks via assertions and unique_ptrs. Includes a unit test that exercises various allocation patterns. I tested porting hash tables over to allocate memory using an earlier version of the suballocator, which worked well (was able to run a wide range of queries successfully with good performance). Change-Id: I8bfe0e429f67ad273f7c7d0816703a9e6c3da788 --- M be/src/bufferpool/CMakeLists.txt A be/src/bufferpool/suballocator-test.cc A be/src/bufferpool/suballocator.cc A be/src/bufferpool/suballocator.h M be/src/common/names.h 5 files changed, 920 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/4715/7 -- To view, visit http://gerrit.cloudera.org:8080/4715 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8bfe0e429f67ad273f7c7d0816703a9e6c3da788 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4570: shell tarball breaks with certain setuptools versions
Michael Ho has posted comments on this change. Change subject: IMPALA-4570: shell tarball breaks with certain setuptools versions .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/5314 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0565c0e6c1be7d82c3f35d2545ba044a684bb075 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3200: Implement suballocator for splitting buffers
Tim Armstrong has uploaded a new patch set (#6). Change subject: IMPALA-3200: Implement suballocator for splitting buffers .. IMPALA-3200: Implement suballocator for splitting buffers This is useful for situations like hash tables, where we want to make multiple non-spillable allocations of variable size from buffer pool memory and not incur the overhead of interacting with the global buffer pool. The allocator subdivides buffers to service allocations and uses a buddy allocation algorithm to merge freed allocations into larger chunks. This helps avoid external fragmentation and is quite effective at reusing memory given the typical doubling allocation patterns of hash tables in partitioned aggs and joins. Testing: The allocator has fairly robust internal consistency checks via assertions and unique_ptrs. Includes a unit test that exercises various allocation patterns. I tested porting hash tables over to allocate memory using an earlier version of the suballocator, which worked well (was able to run a wide range of queries successfully with good performance). Change-Id: I8bfe0e429f67ad273f7c7d0816703a9e6c3da788 --- M be/src/bufferpool/CMakeLists.txt A be/src/bufferpool/suballocator-test.cc A be/src/bufferpool/suballocator.cc A be/src/bufferpool/suballocator.h M be/src/common/names.h 5 files changed, 919 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/4715/6 -- To view, visit http://gerrit.cloudera.org:8080/4715 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8bfe0e429f67ad273f7c7d0816703a9e6c3da788 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4570: shell tarball breaks with certain setuptools versions
Tim Armstrong has uploaded a new change for review. http://gerrit.cloudera.org:8080/5314 Change subject: IMPALA-4570: shell tarball breaks with certain setuptools versions .. IMPALA-4570: shell tarball breaks with certain setuptools versions The bug was in the third-party pkg_resources.py script. The version check was broken because it matches any version with a "0.7" substring instead of just versions starting with 0.7. This is a known bug. setuptools even re-released 20.7.0 as version 20.8.0 to avoid it: https://github.com/pypa/setuptools/commit/e5822f0d5be6386bf86cde03988bfdf1bfc2e935 Change-Id: I0565c0e6c1be7d82c3f35d2545ba044a684bb075 --- M shell/pkg_resources.py 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/5314/1 -- To view, visit http://gerrit.cloudera.org:8080/5314 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0565c0e6c1be7d82c3f35d2545ba044a684bb075 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-4570: shell tarball breaks with certain setuptools versions
Tim Armstrong has uploaded a new patch set (#2). Change subject: IMPALA-4570: shell tarball breaks with certain setuptools versions .. IMPALA-4570: shell tarball breaks with certain setuptools versions The bug was in the third-party pkg_resources.py script. The version check was broken because it matches any version with a "0.7" substring instead of just versions starting with 0.7. This is a known bug. setuptools even re-released 20.7.0 as version 20.8.0 to avoid it: https://github.com/pypa/setuptools/commit/e5822f0d5be6386bf86cde03988bfdf1bfc2e935 Testing: I was unable to reproduce this locally, but I think the fix is clear-cut enough that this is ok. Change-Id: I0565c0e6c1be7d82c3f35d2545ba044a684bb075 --- M shell/pkg_resources.py 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/5314/2 -- To view, visit http://gerrit.cloudera.org:8080/5314 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0565c0e6c1be7d82c3f35d2545ba044a684bb075 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-3875: Thrift threaded server hang in some cases
Internal Jenkins has posted comments on this change. Change subject: IMPALA-3875: Thrift threaded server hang in some cases .. Patch Set 2: Verified-1 Build failed: http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/580/ -- To view, visit http://gerrit.cloudera.org:8080/5263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c8f91a88f723e0e58e81bb385c5a8f190021868 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4504: fix races in PlanFragmentExecutor regarding status reporting
Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-4504: fix races in PlanFragmentExecutor regarding status reporting .. IMPALA-4504: fix races in PlanFragmentExecutor regarding status reporting The PlanFragmentExecutor has some state shared between the main execution thread and the periodic reporting thread that isn't synchronized properly. IMPALA-4504 describes one such problem, and that bug was introduced in an attempt to fix another similar race. Let's just simplify all of this and remove this shared state. Instead, the profile thread will always be responsible for sending periodic '!done' messages, and the main execution thread will always be responsible for sending the final 'done' message (after joining the periodic thread). This will allow for even more simplification, in particular the interaction between FragementExecState and PlanFragementExecutor, but I'm not doing that now as to avoid more conflicts with the MT work. Change-Id: I052b7b4fabb341ad27ad294cd5b0a53728d87d0e Reviewed-on: http://gerrit.cloudera.org:8080/5250 Reviewed-by: Dan Hecht Tested-by: Internal Jenkins --- M be/src/runtime/plan-fragment-executor.cc M be/src/runtime/plan-fragment-executor.h M be/src/service/fragment-exec-state.cc M be/src/service/fragment-exec-state.h 4 files changed, 67 insertions(+), 121 deletions(-) Approvals: Internal Jenkins: Verified Dan Hecht: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/5250 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I052b7b4fabb341ad27ad294cd5b0a53728d87d0e Gerrit-PatchSet: 13 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load
Alex Behm has posted comments on this change.
Change subject: IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails
to load
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5144/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
Line 1277: } catch (NoSuchObjectException e) {
> I don't quite understand how this is different from what the patch was orig
I don't see the purpose of this patch that way. In my mind the purpose is to
allow dropping tables even if they failed to load (that was not possible
before). The fact that you get a TableLoadingException in the "externally
deleted from Kudu" case is specific to Kudu, and an artifact of the
inconsistency between HMS and Kudu with respect to that one table. The Kudu
specific fix here is the ability to clean up the inconsistent metadata in the
HMS, but that is not necessary for the Hive-table only case.
The Hive table case is different. There is only one source of truth and that's
the HMS. The semantics of DROP without IF EXISTS is that errors are shown to
the user.
I didn't follow the part about the usability regression, maybe you can help me
understand in person.
--
To view, visit http://gerrit.cloudera.org:8080/5144
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b41fc3c0e95508ab67f1d420b033b02ec75a5da
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4504: fix races in PlanFragmentExecutor regarding status reporting
Internal Jenkins has posted comments on this change. Change subject: IMPALA-4504: fix races in PlanFragmentExecutor regarding status reporting .. Patch Set 12: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/5250 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I052b7b4fabb341ad27ad294cd5b0a53728d87d0e Gerrit-PatchSet: 12 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4525: follow-on: cleanup error handling
Hello Impala Public Jenkins, Michael Ho, Alex Behm, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5212 to look at the new patch set (#4). Change subject: IMPALA-4525: follow-on: cleanup error handling .. IMPALA-4525: follow-on: cleanup error handling Testing: Ran exhaustive build. There is already some test coverage in test_exprs.py for errors returned during constant expr evaluation by the frontend. Change-Id: Ifb2e532381c3e6c7b2e3ef37c2d91fbebb2db2d0 --- M be/src/exprs/expr-context.cc M be/src/exprs/expr-context.h M be/src/service/fe-support.cc 3 files changed, 34 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/5212/4 -- To view, visit http://gerrit.cloudera.org:8080/5212 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifb2e532381c3e6c7b2e3ef37c2d91fbebb2db2d0 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-4498: crash in to utc timestamp/from utc timestamp
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4498: crash in to_utc_timestamp/from_utc_timestamp .. Patch Set 6: It doesn't seem to cause any actual problems, so maybe I'll hold off on it. That will make it easier for people to cherry-pick this fix if they want to. -- To view, visit http://gerrit.cloudera.org:8080/5251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idc427b06ac33ec874a05cb98d01c00e970d3dde6 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4343,IMPALA-4354: qgen: model INSERTs; write INSERTs from query model
Taras Bobrovytsky has posted comments on this change. Change subject: IMPALA-4343,IMPALA-4354: qgen: model INSERTs; write INSERTs from query model .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/5162/7/tests/comparison/query.py File tests/comparison/query.py: PS7, Line 56: self.with_clause.table_exprs > I talked to Taras separately and the concern here is, does the += modify th I just tried this: class A(object): def __init__(self): self._x = [1, 2, 3] @property def x(self): return self._x a = A() lst = a.x lst.extend([5, 6]) print a.x # prints [1, 2, 3, 5, 6] So looks like @property does not return a copy. -- To view, visit http://gerrit.cloudera.org:8080/5162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I38e24da78c49e908449b35f0a6276ebe4236ddba Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Brown Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4567: Fix test kudu alter table exhaustive failures
Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4567: Fix test_kudu_alter_table exhaustive failures .. Patch Set 1: > I used this command to see the test simply runs the .test file with > codegen enabled and disabled. > > impala-py.test --collect-only --workload_exploration_strategy > functional-query:exhaustive -k test_kudu_alter query_test/test_kudu.py > > Are both query options needed for these tests? > I used this command to see the test simply runs the .test file with > codegen enabled and disabled. > > impala-py.test --collect-only --workload_exploration_strategy > functional-query:exhaustive -k test_kudu_alter query_test/test_kudu.py > > Are both query options needed for these tests? Yeah that's a good question. I was thinking about only running w/ codegen enabled since it probably doesn't really matter for this test. However, I thought if the matrix grows in the future it could be an issue again. I'd be OK with limiting it & adding a comment, though. Do you know how I can only run 1 of the 2 (e.g. just codegen enabled) for this particular test fn? I could always split it into a separate class and change the filters. -- To view, visit http://gerrit.cloudera.org:8080/5312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibca64d5567c24240606e454b052d130fcd0c3968 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4525: follow-on: cleanup error handling
Michael Ho has posted comments on this change.
Change subject: IMPALA-4525: follow-on: cleanup error handling
..
Patch Set 3: Code-Review+1
(2 comments)
Sorry, didn't see this one until now.
http://gerrit.cloudera.org:8080/#/c/5212/3/be/src/service/fe-support.cc
File be/src/service/fe-support.cc:
PS3, Line 82: Status status = Status::OK();
Why not just Status status; ?
PS3, Line 138: for (int i = 0; i < expr_ctxs.size(); ++i) {
for (ExprContext* expr_ctx : expr_ctxs) {
--
To view, visit http://gerrit.cloudera.org:8080/5212
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb2e532381c3e6c7b2e3ef37c2d91fbebb2db2d0
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4343,IMPALA-4354: qgen: model INSERTs; write INSERTs from query model
Michael Brown has posted comments on this change. Change subject: IMPALA-4343,IMPALA-4354: qgen: model INSERTs; write INSERTs from query model .. Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/5162/7/tests/comparison/model_translator.py File tests/comparison/model_translator.py: Line 727: def _write_concat(self, func): > Wasn't this in a separate review? Yes, and it was submitted after your +2 (thanks!), and then this was rebased on top of it in patch set 6. http://gerrit.cloudera.org:8080/#/c/5162/7/tests/comparison/query.py File tests/comparison/query.py: PS7, Line 56: self.with_clause.table_exprs > After thinking about this some more, we might need to make a copy here with I talked to Taras separately and the concern here is, does the += modify the list we get back from the property reader? It seems like we shouldn't, since that defeats the purpose of a property. I'll look into it. Also, I'll change the += to extend() Last, this needs to be a TableExprList, not []. -- To view, visit http://gerrit.cloudera.org:8080/5162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I38e24da78c49e908449b35f0a6276ebe4236ddba Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Brown Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4498: crash in to utc timestamp/from utc timestamp
Dan Hecht has posted comments on this change. Change subject: IMPALA-4498: crash in to_utc_timestamp/from_utc_timestamp .. Patch Set 6: > (2 comments) > > Dan: so there's actually a special case in exprs/scalar-fn-call.cc > that disables codegen for functions with "AddSub" in the name. This > is a bit of a hack and could be cleaned up in various way. > > I also experimented with disabling that hack and it looks like > exception handling in IR actually works to some extent, so maybe > it's worth reevaluating later. I wonder if we should just move some > of those functions to timestamp-functions.cc, since I don't think > we're using the IR. Yes, I was thinking we'd just move those functions to timestamp-function.cc. But if it's not a real bug, no need to do it right now. -- To view, visit http://gerrit.cloudera.org:8080/5251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idc427b06ac33ec874a05cb98d01c00e970d3dde6 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3167: Fix assignment of WHERE-clause predicate through grouping agg + outer join.
Alex Behm has posted comments on this change. Change subject: IMPALA-3167: Fix assignment of WHERE-clause predicate through grouping agg + outer join. .. Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/4960/2//COMMIT_MSG Commit Message: Line 7: IMPALA-3167: Fix assignment of WHERE-clause predicate through grouping agg + outer join. > long lines I though we allowed up to 90 chars. If that's the case then our gerrit is misconfigured. Fixed anyway. Line 28: The fix is to conservatively not mark bound predicates as assigned if there exists > if there are Done Line 30: duplicate assignments of predicates. Those are simply deduped now. > i don't understand that last part. Expanded commit msg with an example that shows how duplicate assignment can happen. http://gerrit.cloudera.org:8080/#/c/4960/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: Line 1548: (evalAfterJoin(srcConjunct) > move to preceding line and fix indentation, like l1551 Done Line 1550: != globalState_.outerJoinedTupleIds.get(srcTid))) > fix indentation Done Line 1553:!= globalState_.outerJoinedTupleIds.get(destTid))); > same here Done http://gerrit.cloudera.org:8080/#/c/4960/2/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java: Line 1194: Expr.removeDuplicates(conjuncts); > how do you end up with duplicate predicates in a *scan* node due to the cha Explained duplicates in commit msg with an example. Your point about HDFS scans only is a good one. Looks like we don't pick up bound predicates in other scan nodes! That's a bug. I think we should address that very soon, at least for Kudu. But I'd prefer to separate those changes from this bugfix. -- To view, visit http://gerrit.cloudera.org:8080/4960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I774d13a13ad1e8fe82512df98dc29983bdd232eb Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Anonymous Coward #27 Gerrit-Reviewer: Marcel Kornacker Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3167: Fix assignment of WHERE conjunct through grouping agg + OJ.
Alex Behm has uploaded a new patch set (#3). Change subject: IMPALA-3167: Fix assignment of WHERE conjunct through grouping agg + OJ. .. IMPALA-3167: Fix assignment of WHERE conjunct through grouping agg + OJ. Background: We generally allow the assignment of predicates below the nullable side of a left/right outer join, explained as follows using an example: SELECT * FROM t1 LEFT OUTER JOIN t2 ON t1.id = t2.id WHERE t2.int_col < 10 The scan of 't2' picks up 't2.int_col < 10' via Analyzer.getBoundPredicates() and recognizes that the predicate must also be evaluated by a join later, so the predicate is not marked as assigned. The join then picks up the unassigned predicate via Analyzer.getUnassignedConjuncts(). The bug was that our logic for detecting whether a bound predicate must also be evaluated at a join node was flawed because it only considered whether the tuples of the source or destination predicate were outer joined (plus other conditions). The underlying assumption is that either the source or destination tuple are bound by a tuple produced by a TableRef, but in the buggy query the source predicate is bound by an aggregation tuple, so we incorrectly marked the bound predicate as assigned in Analyzer.getBoundPredicates(). The fix is to conservatively not mark bound predicates as assigned if there are equivalent outer-joined tuples. As a result, a plan node may pick up the same predicate multiple times, once via Analyzer.getBoundPredicates() and another time via Analyzer.getUnassignedConjuncts(). Those are deduped now. The following example explains the duplicate predicate assignment: SELECT * FROM (SELECT * FROM t t1) a LEFT OUTER JOIN t b ON a.id = b.id WHERE a.id < 10 1. The predicate 'a.id < 10' gets migrated into the inline view. 'a.id < 10' is marked as assigned but is still registered as a single-tid conjunct in the Analyzer for potential propagation 2. The scan node of 't1' calls Analyzer.getBoundPredicates() and generates 't1.id < 10' based on the source predicate 'a.id < 10'. 3. The scan node of 't1' picks up the migrated conjunct 't1.id < 10' via Analyzer.getUnassignedConjuncts(). Change-Id: I774d13a13ad1e8fe82512df98dc29983bdd232eb --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/Expr.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java M testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test 6 files changed, 50 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/4960/3 -- To view, visit http://gerrit.cloudera.org:8080/4960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I774d13a13ad1e8fe82512df98dc29983bdd232eb Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Anonymous Coward #27 Gerrit-Reviewer: Marcel Kornacker
[Impala-ASF-CR] Add Apache license header to files in doc directory
Jim Apple has posted comments on this change. Change subject: Add Apache license header to files in doc directory .. Patch Set 2: Code-Review+1 rebase, carry Greg' +1 -- To view, visit http://gerrit.cloudera.org:8080/5232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2ad06435f84a65ba126759e42a18fdaf52cd7036 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Anonymous Coward #250 Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: John Russell Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4525: follow-on: cleanup error handling
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-4525: follow-on: cleanup error handling .. Patch Set 3: Verified-1 Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/64/ -- To view, visit http://gerrit.cloudera.org:8080/5212 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb2e532381c3e6c7b2e3ef37c2d91fbebb2db2d0 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] Fix E2E test infrastructure to handle missing exceptions correctly
Internal Jenkins has submitted this change and it was merged. Change subject: Fix E2E test infrastructure to handle missing exceptions correctly .. Fix E2E test infrastructure to handle missing exceptions correctly This change fixes a bug in the E2E infrastructure that handles the case when an expected exception wasn't thrown. The code was expecting that test_section['CATCH'] to be a string but in reality it's a list of strings. It also clarifies the error message about the missing exception. This change also enforces that the CATCH subsection in tests cannot be empty. Change-Id: I7d83c5db59e8a239e4e70694a1e625af6f21419c Reviewed-on: http://gerrit.cloudera.org:8080/5260 Reviewed-by: Michael Ho Tested-by: Internal Jenkins --- M tests/common/impala_test_suite.py M tests/util/test_file_parser.py 2 files changed, 7 insertions(+), 1 deletion(-) Approvals: Michael Ho: Looks good to me, approved Internal Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/5260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I7d83c5db59e8a239e4e70694a1e625af6f21419c Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Fix E2E test infrastructure to handle missing exceptions correctly
Internal Jenkins has posted comments on this change. Change subject: Fix E2E test infrastructure to handle missing exceptions correctly .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/5260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7d83c5db59e8a239e4e70694a1e625af6f21419c Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR](asf-site) Add "Effective Coding Practices" doc to site
Tim Armstrong has abandoned this change. Change subject: Add "Effective Coding Practices" doc to site .. Abandoned will add to wiki -- To view, visit http://gerrit.cloudera.org:8080/4961 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I21fdce898a71be836b658e0c914e05a6868d6263 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4498: crash in to utc timestamp/from utc timestamp
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4498: crash in to_utc_timestamp/from_utc_timestamp .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/5251/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test File testdata/workloads/functional-query/queries/QueryTest/exprs.test: Line 2596: > I'm going to add some e2e tests for date_add/date_sub too Done -- To view, visit http://gerrit.cloudera.org:8080/5251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idc427b06ac33ec874a05cb98d01c00e970d3dde6 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4498: crash in to utc timestamp/from utc timestamp
Hello Marcel Kornacker, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5251 to look at the new patch set (#6). Change subject: IMPALA-4498: crash in to_utc_timestamp/from_utc_timestamp .. IMPALA-4498: crash in to_utc_timestamp/from_utc_timestamp The bugs was that the functions did not check whether the conversion pushed the value out of range. The fix is to use boost's validation immediately to check the validity of the timestamp and catch any exceptions thrown. It would be preferable to avoid the exceptions, but Boost does not provide a straightforward way to disable the exceptions or extract potentially-invalid values from a date object. Testing: Added expression tests that exercise out-of-range cases. Also added additional tests to confirm that date addition and subtraction weren't affected by similar bugs. Change-Id: Idc427b06ac33ec874a05cb98d01c00e970d3dde6 --- M be/src/exprs/expr-test.cc M be/src/exprs/timestamp-functions.cc M testdata/workloads/functional-query/queries/QueryTest/exprs.test 3 files changed, 147 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/51/5251/6 -- To view, visit http://gerrit.cloudera.org:8080/5251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idc427b06ac33ec874a05cb98d01c00e970d3dde6 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load
Thomas Tauber-Marshall has posted comments on this change.
Change subject: IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails
to load
..
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/5144/3/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
File fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java:
Line 105: // we set it as TABLE as VIEW loading is unlikely to fail and
even if it does
> What do you think about changing this to TCatalogObjectType.TABLE? We can a
Done
http://gerrit.cloudera.org:8080/#/c/5144/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
Line 1277: } catch (NoSuchObjectException e) {
> This doesn't seem quite right. If we don't have IF EXISTS, then we should t
I don't quite understand how this is different from what the patch was
originally trying to solve - making it possible to drop tables that were
deleted externally in Kudu, except that its for tables deleted externally from
Hive.
IF EXISTS would still apply, but only for whether or not a table exists in
Impala's catalog cache, such that any table that is displayed in "SHOW TABLES"
can be deleted without "IF EXISTS".
This also brings up the point that the patch only works for the original Kudu
situation because we've hard-coded IF EXISTS to true for Kudu operations on
line 1259 here.
I guess the answer to those concerns is that we're intentionally treating Hive
and Kudu differently, since HMS is the ground truth of our catalog, though it
seems inconsistent to me.
Either way, if I don't make this change another change is needed somewhere, as
the AnalysisException that you used to get in the "externally deleted from
Hive" situation hinted that "invalidate metadata" was the solution to the
situation, but with this patch the ImpalaRuntimeException you get doesn't say
that, which would seem to be a usability regression.
--
To view, visit http://gerrit.cloudera.org:8080/5144
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b41fc3c0e95508ab67f1d420b033b02ec75a5da
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load
Thomas Tauber-Marshall has uploaded a new patch set (#4). Change subject: IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load .. IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load If a table fails to load, eg. because it was deleted externally from Kudu, we should still allow 'DROP TABLE' to pass analysis. Otherwise, you may be unable to drop tables that are in a bad state. Testing: - Updates existing Kudu tests to reflect the new behavior, and fixes a couple of problems with those tests that were causing them to pass spuriously (as well as fixing the same problem with another test in the file while I'm here). Change-Id: I6b41fc3c0e95508ab67f1d420b033b02ec75a5da --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M tests/query_test/test_kudu.py 7 files changed, 47 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/5144/4 -- To view, visit http://gerrit.cloudera.org:8080/5144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6b41fc3c0e95508ab67f1d420b033b02ec75a5da Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] Bump Kudu python version to 1.1
Internal Jenkins has submitted this change and it was merged. Change subject: Bump Kudu python version to 1.1 .. Bump Kudu python version to 1.1 Change-Id: I5834b3aa4eeae363eae938f61e473c52a0fe5596 Reviewed-on: http://gerrit.cloudera.org:8080/5307 Reviewed-by: Tim Armstrong Tested-by: Internal Jenkins --- M infra/python/deps/download_requirements M infra/python/deps/requirements.txt 2 files changed, 2 insertions(+), 2 deletions(-) Approvals: Internal Jenkins: Verified Tim Armstrong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/5307 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5834b3aa4eeae363eae938f61e473c52a0fe5596 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Bump Kudu python version to 1.1
Internal Jenkins has posted comments on this change. Change subject: Bump Kudu python version to 1.1 .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/5307 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5834b3aa4eeae363eae938f61e473c52a0fe5596 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4498: crash in to utc timestamp/from utc timestamp
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4498: crash in to_utc_timestamp/from_utc_timestamp
..
Patch Set 4:
(2 comments)
Dan: so there's actually a special case in exprs/scalar-fn-call.cc that
disables codegen for functions with "AddSub" in the name. This is a bit of a
hack and could be cleaned up in various way.
I also experimented with disabling that hack and it looks like exception
handling in IR actually works to some extent, so maybe it's worth reevaluating
later. I wonder if we should just move some of those functions to
timestamp-functions.cc, since I don't think we're using the IR.
http://gerrit.cloudera.org:8080/#/c/5251/4/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
Line 3616: "as timestamp), interval 13 months) as string)",
> odd indentation.
I don't think it's configurable - it wants to line up the multi-line strings
and there's no option I could find to disable that.
Manually reverted it.
http://gerrit.cloudera.org:8080/#/c/5251/4/be/src/exprs/timestamp-functions.cc
File be/src/exprs/timestamp-functions.cc:
Line 54: void ThrowExceptionIfTimestampOutOfRange(const boost::gregorian::date&
date) {
> remove Exception, it's implied in Throw.
Done
--
To view, visit http://gerrit.cloudera.org:8080/5251
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc427b06ac33ec874a05cb98d01c00e970d3dde6
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Marcel Kornacker
Gerrit-Reviewer: Taras Bobrovytsky
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4498: crash in to utc timestamp/from utc timestamp
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4498: crash in to_utc_timestamp/from_utc_timestamp .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/5251/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test File testdata/workloads/functional-query/queries/QueryTest/exprs.test: Line 2596: I'm going to add some e2e tests for date_add/date_sub too -- To view, visit http://gerrit.cloudera.org:8080/5251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idc427b06ac33ec874a05cb98d01c00e970d3dde6 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3398: Rework Impala documentation to be non-Cloudera-specific
Greg Rahn has posted comments on this change. Change subject: IMPALA-3398: Rework Impala documentation to be non-Cloudera-specific .. Patch Set 2: For the goal mentioned, it LGTM. I'll defer to others for tag specific comments. -- To view, visit http://gerrit.cloudera.org:8080/5239 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib3f63fb309e0617d7fe014231bb0ab0ad67c8474 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Anonymous Coward #250 Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: John Russell Gerrit-HasComments: No
[Impala-ASF-CR] Add Apache license header to files in doc directory
Greg Rahn has posted comments on this change. Change subject: Add Apache license header to files in doc directory .. Patch Set 1: Code-Review+1 LGTM -- To view, visit http://gerrit.cloudera.org:8080/5232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2ad06435f84a65ba126759e42a18fdaf52cd7036 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Anonymous Coward #250 Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: John Russell Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4498: crash in to utc timestamp/from utc timestamp
Hello Marcel Kornacker, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5251 to look at the new patch set (#5). Change subject: IMPALA-4498: crash in to_utc_timestamp/from_utc_timestamp .. IMPALA-4498: crash in to_utc_timestamp/from_utc_timestamp The bugs was that the functions did not check whether the conversion pushed the value out of range. The fix is to use boost's validation immediately to check the validity of the timestamp and catch any exceptions thrown. It would be preferable to avoid the exceptions, but Boost does not provide a straightforward way to disable the exceptions or extract potentially-invalid values from a date object. Testing: Added expression tests that exercise out-of-range cases. Also added additional tests to confirm that date addition and subtraction weren't affected by similar bugs. Change-Id: Idc427b06ac33ec874a05cb98d01c00e970d3dde6 --- M be/src/exprs/expr-test.cc M be/src/exprs/timestamp-functions.cc M testdata/workloads/functional-query/queries/QueryTest/exprs.test 3 files changed, 119 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/51/5251/5 -- To view, visit http://gerrit.cloudera.org:8080/5251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idc427b06ac33ec874a05cb98d01c00e970d3dde6 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4343,IMPALA-4354: qgen: model INSERTs; write INSERTs from query model
Taras Bobrovytsky has posted comments on this change. Change subject: IMPALA-4343,IMPALA-4354: qgen: model INSERTs; write INSERTs from query model .. Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/5162/7/tests/comparison/model_translator.py File tests/comparison/model_translator.py: Line 727: def _write_concat(self, func): Wasn't this in a separate review? http://gerrit.cloudera.org:8080/#/c/5162/7/tests/comparison/query.py File tests/comparison/query.py: PS7, Line 56: self.with_clause.table_exprs After thinking about this some more, we might need to make a copy here with list(self.with_clause.table_exprs). In InsertStatement.table_exprs, we do this: table_exprs += self.select_query.table_exprs Does the += operator call extend? -- To view, visit http://gerrit.cloudera.org:8080/5162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I38e24da78c49e908449b35f0a6276ebe4236ddba Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Brown Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4504: fix races in PlanFragmentExecutor regarding status reporting
Dan Hecht has posted comments on this change. Change subject: IMPALA-4504: fix races in PlanFragmentExecutor regarding status reporting .. Patch Set 12: Code-Review+2 Rebase. Promote +1 to +2. -- To view, visit http://gerrit.cloudera.org:8080/5250 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I052b7b4fabb341ad27ad294cd5b0a53728d87d0e Gerrit-PatchSet: 12 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4564,IMPALA-4565: mt dop fixes for old aggs and joins
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-4564,IMPALA-4565: mt_dop fixes for old aggs and joins .. Patch Set 4: Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/65/ -- To view, visit http://gerrit.cloudera.org:8080/5302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I48c50c8aa0c23710eb099fba252bc3c0cb74b313 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4527: Columns in Kudu tables created from Impala default to "NULL"
Internal Jenkins has posted comments on this change. Change subject: IMPALA-4527: Columns in Kudu tables created from Impala default to "NULL" .. Patch Set 3: Verified-1 Build failed: http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/581/ -- To view, visit http://gerrit.cloudera.org:8080/5259 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I950d9a9c64e3851e11a641573617790b340ece94 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4564,IMPALA-4565: mt dop fixes for old aggs and joins
Alex Behm has posted comments on this change. Change subject: IMPALA-4564,IMPALA-4565: mt_dop fixes for old aggs and joins .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I48c50c8aa0c23710eb099fba252bc3c0cb74b313 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4527: Columns in Kudu tables created from Impala default to "NULL"
Alex Behm has posted comments on this change. Change subject: IMPALA-4527: Columns in Kudu tables created from Impala default to "NULL" .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5259 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I950d9a9c64e3851e11a641573617790b340ece94 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load
Alex Behm has posted comments on this change. Change subject: IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load .. Patch Set 3: Also, thanks for digging up IMPALA-594, that's indeed the one I meant. How about changing the title of description of IMPALA-4357 to reflect your more generic fix. -- To view, visit http://gerrit.cloudera.org:8080/5144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6b41fc3c0e95508ab67f1d420b033b02ec75a5da Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load
Alex Behm has posted comments on this change.
Change subject: IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails
to load
..
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/5144/3/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
File fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java:
Line 105: tableName_.toString(), TCatalogObjectType.UNKNOWN,
Privilege.DROP.toString()));
What do you think about changing this to TCatalogObjectType.TABLE? We can add a
comment that we're not sure whether it's a table or view, but view loading
should basically never fail, and even then it's small leap from table to view.
However, UNKNOWN could really be anything.
http://gerrit.cloudera.org:8080/#/c/5144/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
Line 1277: } catch (NoSuchObjectException e) {
This doesn't seem quite right. If we don't have IF EXISTS, then we should throw
an error if dropping the table failed for whatever reason.
--
To view, visit http://gerrit.cloudera.org:8080/5144
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b41fc3c0e95508ab67f1d420b033b02ec75a5da
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4527: Columns in Kudu tables created from Impala default to "NULL"
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-4527: Columns in Kudu tables created from Impala default
to "NULL"
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5259/2/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:
Line 552: CREATE TABLE {db_name}{db_suffix}.{table_name}_idx (
> Can we defer changes to this file until later? I'm worried having a stale d
Done
--
To view, visit http://gerrit.cloudera.org:8080/5259
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I950d9a9c64e3851e11a641573617790b340ece94
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4527: Columns in Kudu tables created from Impala default to "NULL"
Hello Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5259 to look at the new patch set (#3). Change subject: IMPALA-4527: Columns in Kudu tables created from Impala default to "NULL" .. IMPALA-4527: Columns in Kudu tables created from Impala default to "NULL" This commit reverts the behavior introduced by IMPALA-3719 which used the Kudu default behavior for column nullability if none was specified in the CREATE TABLE statement. With this commit, non-key columns of Kudu tables that are created from Impala are by default nullable unless specified otherwise. Change-Id: I950d9a9c64e3851e11a641573617790b340ece94 --- M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M testdata/workloads/functional-query/queries/QueryTest/kudu_describe.test M testdata/workloads/functional-query/queries/QueryTest/kudu_stats.test 3 files changed, 27 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/5259/3 -- To view, visit http://gerrit.cloudera.org:8080/5259 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I950d9a9c64e3851e11a641573617790b340ece94 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-4567: Fix test kudu alter table exhaustive failures
Michael Brown has posted comments on this change. Change subject: IMPALA-4567: Fix test_kudu_alter_table exhaustive failures .. Patch Set 1: I used this command to see the test simply runs the .test file with codegen enabled and disabled. impala-py.test --collect-only --workload_exploration_strategy functional-query:exhaustive -k test_kudu_alter query_test/test_kudu.py Are both query options needed for these tests? -- To view, visit http://gerrit.cloudera.org:8080/5312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibca64d5567c24240606e454b052d130fcd0c3968 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4172/IMPALA-3653: Improvements to block metadata loading
Sailesh Mukil has posted comments on this change. Change subject: IMPALA-4172/IMPALA-3653: Improvements to block metadata loading .. Patch Set 17: Was just going through this patch. I think it's necessary to run some tests on a table with partitions on multiple filesystems as well. Since our test infra doesn't allow for that currently, it has to be done manually. -- To view, visit http://gerrit.cloudera.org:8080/5148 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie127658172e6e70dae441374530674a4ac9d5d26 Gerrit-PatchSet: 17 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4567: Fix test kudu alter table exhaustive failures
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-4567: Fix test_kudu_alter_table exhaustive failures .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibca64d5567c24240606e454b052d130fcd0c3968 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load
Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load .. Patch Set 3: (1 comment) The closest JIRA I could find was IMPALA-594, but that's more general than what we're really doing here. Other situations (besides externally dropped from Kudu) that I tested: - Dropping a table that was externally removed from Hive: the previous version of the patch didn't actually work for this situation. I fixed that with the change in CatalogOpExecutor. - Dropping a table when Hive (or Kudu as appropriate) is unreachable, eg. because it was killed. Even with the patch, this still doesn't work. Instead of getting the AnalysisException you would get previously, you now get an ImpalaRuntimeException. I think this is reasonable for now. - Running a DROP that actually should fail analysis, eg. DROP TABLE on a view or vice versa, that now passes analysis due to a transient TableLoadingException, eg. HMS is under load and the loading times out, then actually does go through to execution - we'll get a CatalogException, which is fine. - Dropping a table that fails to load due to unsupported column types, works with this patch. http://gerrit.cloudera.org:8080/#/c/5144/2/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java File fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java: Line 100: } > We need to add an access event for auditing in this case, because we are in The one problem here is that if table loading failed, we don't know if its a table or a view. I put TCatalogObjectType.UNKNOWN, but that seems less than ideal. -- To view, visit http://gerrit.cloudera.org:8080/5144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6b41fc3c0e95508ab67f1d420b033b02ec75a5da Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4564,IMPALA-4565: mt dop fixes for old aggs and joins
Hello Michael Ho, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5302 to look at the new patch set (#3). Change subject: IMPALA-4564,IMPALA-4565: mt_dop fixes for old aggs and joins .. IMPALA-4564,IMPALA-4565: mt_dop fixes for old aggs and joins Fix a test bug where we need to skip nested types tests for the old aggs and joins. Fix a product bug where *eos is not initialised by the MT scan node. This causes incorrect results when the calling ExecNode does not initialise the eos variable, e.g. the sort node and the old agg and join nodes. Testing: Added a test that reproduces the incorrect results with the sort node when run under ASAN Tested the mt_dop tests locally with old aggs and joins to ensure they pass. Change-Id: I48c50c8aa0c23710eb099fba252bc3c0cb74b313 --- M be/src/exec/hdfs-scan-node-mt.cc A testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-nested.test M testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet.test M testdata/workloads/functional-query/queries/QueryTest/mt-dop.test M tests/query_test/test_mt_dop.py 5 files changed, 60 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/5302/3 -- To view, visit http://gerrit.cloudera.org:8080/5302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I48c50c8aa0c23710eb099fba252bc3c0cb74b313 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-4564,IMPALA-4565: mt dop fixes for old aggs and joins
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4564,IMPALA-4565: mt_dop fixes for old aggs and joins .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/5302/1/testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet.test File testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet.test: Line 9: # IMPALA-4565: incorrect results because mt scan node does not set eos > move this into mt-dop.test since it's not specific to Parquet and getting c Done Line 11: set disable_outermost_topn=true; > I'm a little wary of having this test coverage depend on this 'exotic' quer Done -- To view, visit http://gerrit.cloudera.org:8080/5302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I48c50c8aa0c23710eb099fba252bc3c0cb74b313 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4567: Fix test kudu alter table exhaustive failures
David Knupp has posted comments on this change. Change subject: IMPALA-4567: Fix test_kudu_alter_table exhaustive failures .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/5312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibca64d5567c24240606e454b052d130fcd0c3968 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: David Knupp Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4564,IMPALA-4565: mt dop fixes for old aggs and joins
Hello Michael Ho, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5302 to look at the new patch set (#2). Change subject: IMPALA-4564,IMPALA-4565: mt_dop fixes for old aggs and joins .. IMPALA-4564,IMPALA-4565: mt_dop fixes for old aggs and joins Fix a test bug where we need to skip nested types tests for the old aggs and joins. Fix a product bug where *eos is not initialised by the MT scan node. This causes incorrect results when the calling ExecNode does not initialise the eos variable, e.g. the sort node and the old agg and join nodes. Testing: Added a test that reproduces the incorrect results with the sort node when run under ASAN Tested the mt_dop tests locally with old aggs and joins to ensure they pass. Change-Id: I48c50c8aa0c23710eb099fba252bc3c0cb74b313 --- M be/src/exec/hdfs-scan-node-mt.cc A testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-nested.test M testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet.test M testdata/workloads/functional-query/queries/QueryTest/mt-dop.test M tests/query_test/test_mt_dop.py 5 files changed, 54 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/5302/2 -- To view, visit http://gerrit.cloudera.org:8080/5302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I48c50c8aa0c23710eb099fba252bc3c0cb74b313 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load
Thomas Tauber-Marshall has uploaded a new patch set (#3). Change subject: IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load .. IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load If a table fails to load, eg. because it was deleted externally from Kudu, we should still allow 'DROP TABLE' to pass analysis. Otherwise, you may be unable to drop tables that are in a bad state. Testing: - Updates existing Kudu tests to reflect the new behavior, and fixes a couple of problems with those tests that were causing them to pass spuriously (as well as fixing the same problem with another test in the file while I'm here). Change-Id: I6b41fc3c0e95508ab67f1d420b033b02ec75a5da --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M tests/query_test/test_kudu.py 7 files changed, 46 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/5144/3 -- To view, visit http://gerrit.cloudera.org:8080/5144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6b41fc3c0e95508ab67f1d420b033b02ec75a5da Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-4562: Fix for crash on kerberized clusters w/o Kudu support
Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-4562: Fix for crash on kerberized clusters w/o Kudu support .. IMPALA-4562: Fix for crash on kerberized clusters w/o Kudu support commit de88f0c4af3a07ae6bd6b8c94edcb8748468f522 for "IMPALA-4497: Fix Kudu client crash w/ SASL initialization" causes a crash on secure clusters where kudu is not supported. kudu::client::DisableSaslInitialization() from libkudu_client.so.0 impala::InitAuth(std::string const&) () impala::InitCommonRuntime() () ImpaladMain(int, char**) () main () This ensures Impala does not call the Kudu client to handle SASL init on systems where libkudu_client.so is a stub. Change-Id: Ib517d17ab12e215fe87f35bc5d03cdda736ff672 Reviewed-on: http://gerrit.cloudera.org:8080/5295 Reviewed-by: Henry Robinson Tested-by: Internal Jenkins --- M be/src/rpc/authentication.cc 1 file changed, 4 insertions(+), 2 deletions(-) Approvals: Henry Robinson: Looks good to me, approved Internal Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/5295 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ib517d17ab12e215fe87f35bc5d03cdda736ff672 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Internal Jenkins
[Impala-ASF-CR] IMPALA-4562: Fix for crash on kerberized clusters w/o Kudu support
Internal Jenkins has posted comments on this change. Change subject: IMPALA-4562: Fix for crash on kerberized clusters w/o Kudu support .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/5295 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib517d17ab12e215fe87f35bc5d03cdda736ff672 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Internal Jenkins Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4567: Fix test kudu alter table exhaustive failures
Matthew Jacobs has uploaded a new change for review. http://gerrit.cloudera.org:8080/5312 Change subject: IMPALA-4567: Fix test_kudu_alter_table exhaustive failures .. IMPALA-4567: Fix test_kudu_alter_table exhaustive failures The issue is that we set the Kudu table name explicitly via tblproperty so it doesn't have the unique db name in the underlying Kudu name. Meanwhile, the tests are run concurrently in exhaustive so this test may end up running the multiple times (w/ different parameters, e.g. disable_codegen) concurrently. This test needs to be run serially. Change-Id: Ibca64d5567c24240606e454b052d130fcd0c3968 --- M tests/query_test/test_kudu.py 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/5312/1 -- To view, visit http://gerrit.cloudera.org:8080/5312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibca64d5567c24240606e454b052d130fcd0c3968 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs
[Impala-ASF-CR] IMPALA-3875: Thrift threaded server hang in some cases
Sailesh Mukil has posted comments on this change. Change subject: IMPALA-3875: Thrift threaded server hang in some cases .. Patch Set 2: Code-Review+2 Rebase, carry +2. -- To view, visit http://gerrit.cloudera.org:8080/5263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c8f91a88f723e0e58e81bb385c5a8f190021868 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4525: follow-on: cleanup error handling
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-4525: follow-on: cleanup error handling .. Patch Set 3: Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/64/ -- To view, visit http://gerrit.cloudera.org:8080/5212 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb2e532381c3e6c7b2e3ef37c2d91fbebb2db2d0 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3875: Thrift threaded server hang in some cases
Henry Robinson has posted comments on this change. Change subject: IMPALA-3875: Thrift threaded server hang in some cases .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c8f91a88f723e0e58e81bb385c5a8f190021868 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Henry Robinson Gerrit-HasComments: No
[Impala-ASF-CR] Start a docs build system.
Jim Apple has submitted this change and it was merged. Change subject: Start a docs build system. .. Start a docs build system. The docs can be built by running "make" from the docs directory. This does not hook into buildall.sh for now, as users who run buildall.sh do not usually edit docs/. Change-Id: I9551b75268cb7cb29a58367a3ef03b127dccbfca Reviewed-on: http://gerrit.cloudera.org:8080/5238 Reviewed-by: Tim Armstrong Reviewed-by: Greg Rahn Tested-by: Jim Apple --- A docs/.gitignore A docs/Makefile 2 files changed, 30 insertions(+), 0 deletions(-) Approvals: Jim Apple: Verified Greg Rahn: Looks good to me, but someone else must approve Tim Armstrong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/5238 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I9551b75268cb7cb29a58367a3ef03b127dccbfca Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Anonymous Coward #250 Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: John Russell Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Start a docs build system.
Jim Apple has posted comments on this change. Change subject: Start a docs build system. .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/5238 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9551b75268cb7cb29a58367a3ef03b127dccbfca Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Anonymous Coward #250 Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: John Russell Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] Start a docs build system.
Greg Rahn has posted comments on this change. Change subject: Start a docs build system. .. Patch Set 2: Code-Review+1 LGTM. Thanks! -- To view, visit http://gerrit.cloudera.org:8080/5238 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9551b75268cb7cb29a58367a3ef03b127dccbfca Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Anonymous Coward #250 Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: John Russell Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] Fix E2E test infrastructure to handle missing exceptions correctly
Michael Ho has posted comments on this change. Change subject: Fix E2E test infrastructure to handle missing exceptions correctly .. Patch Set 3: Code-Review+2 Carry Tim's +2. -- To view, visit http://gerrit.cloudera.org:8080/5260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7d83c5db59e8a239e4e70694a1e625af6f21419c Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] Fix E2E test infrastructure to handle missing exceptions correctly
Michael Ho has posted comments on this change. Change subject: Fix E2E test infrastructure to handle missing exceptions correctly .. Patch Set 2: Yes, merging now. The private exhaustive build just completed. -- To view, visit http://gerrit.cloudera.org:8080/5260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7d83c5db59e8a239e4e70694a1e625af6f21419c Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4564,IMPALA-4565: mt dop fixes for old aggs and joins
Alex Behm has posted comments on this change. Change subject: IMPALA-4564,IMPALA-4565: mt_dop fixes for old aggs and joins .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/5302/1/testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet.test File testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet.test: Line 9: # IMPALA-4565: incorrect results because mt scan node does not set eos move this into mt-dop.test since it's not specific to Parquet and getting coverage over all file formats seems not bad Line 11: set disable_outermost_topn=true; I'm a little wary of having this test coverage depend on this 'exotic' query option. Maybe the same can be achieved by using an order by without limit on alltypestiny, but setting the batch size to 1? -- To view, visit http://gerrit.cloudera.org:8080/5302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I48c50c8aa0c23710eb099fba252bc3c0cb74b313 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4525: follow-on: cleanup error handling
Alex Behm has posted comments on this change. Change subject: IMPALA-4525: follow-on: cleanup error handling .. Patch Set 3: Code-Review+2 (1 comment) Thank you! http://gerrit.cloudera.org:8080/#/c/5212/3/be/src/service/fe-support.cc File be/src/service/fe-support.cc: Line 82: Status status = Status::OK(); goto is a real life saver here, much simpler -- To view, visit http://gerrit.cloudera.org:8080/5212 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb2e532381c3e6c7b2e3ef37c2d91fbebb2db2d0 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4525: follow-on: cleanup error handling
Tim Armstrong has uploaded a new patch set (#3). Change subject: IMPALA-4525: follow-on: cleanup error handling .. IMPALA-4525: follow-on: cleanup error handling Testing: Ran exhaustive build. There is already some test coverage in test_exprs.py for errors returned during constant expr evaluation by the frontend. Change-Id: Ifb2e532381c3e6c7b2e3ef37c2d91fbebb2db2d0 --- M be/src/exprs/expr-context.cc M be/src/exprs/expr-context.h M be/src/service/fe-support.cc 3 files changed, 26 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/5212/3 -- To view, visit http://gerrit.cloudera.org:8080/5212 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifb2e532381c3e6c7b2e3ef37c2d91fbebb2db2d0 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] Bump Kudu python version to 1.1
Tim Armstrong has posted comments on this change. Change subject: Bump Kudu python version to 1.1 .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5307 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5834b3aa4eeae363eae938f61e473c52a0fe5596 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] Start a docs build system.
Tim Armstrong has posted comments on this change. Change subject: Start a docs build system. .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5238 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9551b75268cb7cb29a58367a3ef03b127dccbfca Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Anonymous Coward #250 Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: John Russell Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] Start a docs build system.
Jim Apple has posted comments on this change. Change subject: Start a docs build system. .. Patch Set 2: Any additional concerns, here? -- To view, visit http://gerrit.cloudera.org:8080/5238 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9551b75268cb7cb29a58367a3ef03b127dccbfca Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Anonymous Coward #250 Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: John Russell Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] Add Apache license header to files in doc directory
Jim Apple has posted comments on this change. Change subject: Add Apache license header to files in doc directory .. Patch Set 1: Once this is merged, we can add RAT checks to our pre-merge testing. -- To view, visit http://gerrit.cloudera.org:8080/5232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2ad06435f84a65ba126759e42a18fdaf52cd7036 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Anonymous Coward #250 Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: John Russell Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3398: Rework Impala documentation to be non-Cloudera-specific
Jim Apple has posted comments on this change. Change subject: IMPALA-3398: Rework Impala documentation to be non-Cloudera-specific .. Patch Set 3: Any additional concerns, here? -- To view, visit http://gerrit.cloudera.org:8080/5239 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib3f63fb309e0617d7fe014231bb0ab0ad67c8474 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Anonymous Coward #250 Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: John Russell Gerrit-HasComments: No
