date:20161201

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

2016-12-01 Thread Impala Public Jenkins (Code Review)

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

2016-12-01 Thread Impala Public Jenkins (Code Review)

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

2016-12-01 Thread Dimitris Tsirogiannis (Code Review)

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-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

2016-12-01 Thread Impala Public Jenkins (Code Review)

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

2016-12-01 Thread Impala Public Jenkins (Code Review)

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

2016-12-01 Thread Thomas Tauber-Marshall (Code Review)

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

2016-12-01 Thread Thomas Tauber-Marshall (Code Review)

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

2016-12-01 Thread Impala Public Jenkins (Code Review)

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 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 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

2016-12-01 Thread Michael Ho (Code Review)

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 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-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

2016-12-01 Thread Taras Bobrovytsky (Code Review)

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-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] 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

2016-12-01 Thread Thomas Tauber-Marshall (Code Review)

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

2016-12-01 Thread Thomas Tauber-Marshall (Code Review)

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 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-3398: Rework Impala documentation to be non-Cloudera-specific

2016-12-01 Thread Greg Rahn (Code Review)

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

2016-12-01 Thread Greg Rahn (Code Review)

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-4504: fix races in PlanFragmentExecutor regarding status reporting

2016-12-01 Thread Dan Hecht (Code Review)

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

2016-12-01 Thread Impala Public Jenkins (Code Review)

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"

2016-12-01 Thread Dimitris Tsirogiannis (Code Review)

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-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

2016-12-01 Thread Sailesh Mukil (Code Review)

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

2016-12-01 Thread Dimitris Tsirogiannis (Code Review)

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

2016-12-01 Thread Thomas Tauber-Marshall (Code Review)

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

[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

2016-12-01 Thread David Knupp (Code Review)

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

2016-12-01 Thread Thomas Tauber-Marshall (Code Review)

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

2016-12-01 Thread Sailesh Mukil (Code Review)

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

2016-12-01 Thread Impala Public Jenkins (Code Review)

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

2016-12-01 Thread Henry Robinson (Code Review)

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.

2016-12-01 Thread Greg Rahn (Code Review)

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

2016-12-01 Thread Michael Ho (Code Review)

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

2016-12-01 Thread Michael Ho (Code Review)

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

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

[Impala-ASF-CR] Fix E2E test infrastructure to handle missing exceptions correctly

Jim Apple has posted comments on this change.

Change subject: Fix E2E test infrastructure to handle missing exceptions 
correctly
..


Patch Set 2:

I've seen this breaking Impala Public Jenkins test runs. Is this ready for 
merge?

-- 
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] Bump Kudu python version to 1.1

Matthew Jacobs has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/5307

Change subject: Bump Kudu python version to 1.1
..

Bump Kudu python version to 1.1

Change-Id: I5834b3aa4eeae363eae938f61e473c52a0fe5596
---
M infra/python/deps/download_requirements
M infra/python/deps/requirements.txt
2 files changed, 2 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/5307/1
-- 
To view, visit http://gerrit.cloudera.org:8080/5307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5834b3aa4eeae363eae938f61e473c52a0fe5596
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs

[Impala-ASF-CR] IMPALA-4564,IMPALA-4565: mt dop fixes for old aggs and joins

2016-12-01 Thread Impala Public Jenkins (Code Review)

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 1:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/63/

-- 
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: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4172/IMPALA-3653: Improvements to block metadata loading

2016-12-01 Thread Bharath Vissapragada (Code Review)

Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-4172/IMPALA-3653: Improvements to block metadata loading
..


Patch Set 17: Code-Review+2

- Hdfs core exhaustive failed on a single test "test_cache_reload_validation". 
PS17 fixes that. The bug there was that we don't call 
HdfsCachingUtil.validateCacheParams() on msPart objects and hence it contains 
stale caching metadata, even if the cache directives were deleted from hdfs 
(outside of Impala).

- Couldn't get S3 tests working. Looking into it.

Trivial change, carrying +2.

-- 
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-HasComments: No

[Impala-ASF-CR] IMPALA-4172/IMPALA-3653: Improvements to block metadata loading

2016-12-01 Thread Bharath Vissapragada (Code Review)

Hello Dimitris Tsirogiannis, Alex Behm,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/5148

to look at the new patch set (#17).

Change subject: IMPALA-4172/IMPALA-3653: Improvements to block metadata loading
..

IMPALA-4172/IMPALA-3653: Improvements to block metadata loading

This patch improves the block metadata loading (locations and disk
storage IDs) for partitioned and un-partitioned tables in the Catalog
server.

Without this patch:
--
We loop through each and every file in the table/partition directories
and call getFileBlockLocations() on it to obtain the block metadata.
This results in large number of RPC calls to the Namenode, especially
for tables with large no. of files/partitions.

With this patch:
---
We move the block metadata querying to use listStatus() call which
accepts a directory as input and fetches the 'BlockLocation' objects
for every file recursively in that directory. This improves the
metadata loading in the following ways.

- For non-partitioned tables, we query all the BlockLocations in a
single RPC call in the base table directory and load the corresponding
disk IDs.

- For partitioned tables, we query the BlockLocations for all the
partitions residing under the base table directories in a single RPC
and then load every partition with non-default partition directory
separately.

- REFRESH on a table reloads the block metadata from scratch for
every data file every time. So it can be used as a replacement for
invalidate in situations like HDFS block rebalancing which needs
block metadata update.

Also, this patch does away with VolumeIds returned by the HDFS NN
and uses the new StorageIDs returned by the BlockLocation class.
These StorageIDs are UUID strings and hence are mapped to a
per-node 0-based index as expected by the backend. In the upcoming
versions of Hadoop APIs, getFileBlockStorageLocations() is deprecated
and instead the listStatus() returns BlockLocations with storage IDs
embedded. This patch makes use of this improvement to reduce an
additional RPC to the NN to fetch the storage locations.

Change-Id: Ie127658172e6e70dae441374530674a4ac9d5d26
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
A fe/src/main/java/org/apache/impala/catalog/DiskIdMapper.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
6 files changed, 371 insertions(+), 338 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/5148/17
-- 
To view, visit http://gerrit.cloudera.org:8080/5148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
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

[Impala-ASF-CR] IMPALA-4557: Fix flakiness with FLAGS stress free pool alloc

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc
..


Patch Set 3: Verified+1

-- 
To view, visit http://gerrit.cloudera.org:8080/5281
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4557: Fix flakiness with FLAGS stress free pool alloc

Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc
..


IMPALA-4557: Fix flakiness with FLAGS_stress_free_pool_alloc

FLAGS_stress_free_pool_alloc is a gflag for simulating malloc
failure in debug builds. If set, FreePool::Allocate() and
FreePool::Reallocate() will return NULL on every
FLAGS_stress_free_pool_alloc allocations. The problem is that
the counter for tracking number of allocations is updated
racily by multiple threads, leading to non-determinism and
flaky tests. This change fixes the problem by making the counter
an AtomicInt32.

Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0
Reviewed-on: http://gerrit.cloudera.org:8080/5281
Reviewed-by: Jim Apple 
Reviewed-by: Dan Hecht 
Tested-by: Internal Jenkins
---
M be/src/common/atomic.h
M be/src/runtime/CMakeLists.txt
A be/src/runtime/free-pool.cc
M be/src/runtime/free-pool.h
4 files changed, 41 insertions(+), 6 deletions(-)

Approvals:
  Jim Apple: Looks good to me, but someone else must approve
  Internal Jenkins: Verified
  Dan Hecht: Looks good to me, approved



-- 
To view, visit http://gerrit.cloudera.org:8080/5281
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I373035d209a0f3477650336ee6da458fa289dbe0
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] Bracketing Java logging output with log level checks part 2.

Internal Jenkins has posted comments on this change.

Change subject: Bracketing Java logging output with log level checks part 2.
..


Patch Set 1: Verified+1

-- 
To view, visit http://gerrit.cloudera.org:8080/5297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib94b3a20d439d854f579d9086755eb19699fcb68
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-HasComments: No

[Impala-ASF-CR] Bracketing Java logging output with log level checks part 2.

Internal Jenkins has submitted this change and it was merged.

Change subject: Bracketing Java logging output with log level checks part 2.
..


Bracketing Java logging output with log level checks part 2.

This reduces creation of intermediate objects and improves performance.

Change-Id: Ib94b3a20d439d854f579d9086755eb19699fcb68
Reviewed-on: http://gerrit.cloudera.org:8080/5297
Reviewed-by: Marcel Kornacker 
Tested-by: Internal Jenkins
---
M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/AggregateInfoBase.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDropRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateUdfStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/ExprSubstitutionMap.java
M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java
M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizeableDb.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizeableUri.java
M fe/src/main/java/org/apache/impala/authorization/ImpalaInternalAdminUser.java
M fe/src/main/java/org/apache/impala/authorization/Privilege.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequest.java
M fe/src/main/java/org/apache/impala/authorization/User.java
30 files changed, 148 insertions(+), 135 deletions(-)

Approvals:
  Marcel Kornacker: Looks good to me, approved
  Internal Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/5297
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib94b3a20d439d854f579d9086755eb19699fcb68
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Marcel Kornacker

[Impala-ASF-CR] IMPALA-4458: Fix resource cleanup of cancelled mt scan nodes.

Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4458: Fix resource cleanup of cancelled mt scan nodes.
..


IMPALA-4458: Fix resource cleanup of cancelled mt scan nodes.

The bug was that HdfsScanNodeMt::Close() did not properly
clean up all in-flight resources when called through the
query cancellation path.

The main change is to clean up all resources when passing
a NULL batch into HdfsparquetScanner::Close() which also
needed similar changes in the scanner context.

Testing: Ran test_cancellation.py, test_scanners.py and
test_nested_types.py with MT_DOP=3. Added a test query
with a limit that was failing before.
A regular private hdfs/core test run succeeded.

Change-Id: Ib32f87b3289ed9e8fc2db0885675845e11207438
Reviewed-on: http://gerrit.cloudera.org:8080/5274
Reviewed-by: Alex Behm 
Tested-by: Internal Jenkins
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-scan-node-mt.cc
M be/src/exec/parquet-column-readers.h
M be/src/exec/scanner-context.cc
M be/src/exec/scanner-context.h
M testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet.test
6 files changed, 53 insertions(+), 35 deletions(-)

Approvals:
  Internal Jenkins: Verified
  Alex Behm: Looks good to me, approved



-- 
To view, visit http://gerrit.cloudera.org:8080/5274
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib32f87b3289ed9e8fc2db0885675845e11207438
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4458: Fix resource cleanup of cancelled mt scan nodes.