[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency

2016-10-06 Thread Alex Behm (Code Review)
Alex Behm has posted comments on this change.

Change subject: IMPALA-1702: Enforce table level consistency
..


Patch Set 11:

(2 comments)

Will have a final pass over the commit msg in the next patch set, then I'm 
ready to +1

http://gerrit.cloudera.org:8080/#/c/4349/11/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:

Line 426: HashSet seenTableId = Sets.newHashSet();
seenTableIds


Line 435:   }
Sorry to keep adding to this, but let's also loop through all tuple descriptors 
and check that the table id (if any) is in seenTableIds.

Also do that for the table sink if any is set.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Huaisi Xu 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-3725 Support Kudu UPSERT in Impala

2016-10-06 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-3725 Support Kudu UPSERT in Impala
..


Patch Set 5:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/4047/5/be/src/exec/kudu-table-sink.cc
File be/src/exec/kudu-table-sink.cc:

PS5, Line 140: DCHECK(sink_action_ == TSinkAction::DELETE) << "Sink type not 
supported. " << sink_action_;
nit: can you fix this long line?


http://gerrit.cloudera.org:8080/#/c/4047/5/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:

PS5, Line 729: LPAREN opt_ident_list:col_perm RPAREN opt_query_stmt:query
nit: can you indent this line?


PS5, Line 730: RESULT = new InsertStmt(w, table, false, null, null, query, 
col_perm, false, true); :}
nit: long line


http://gerrit.cloudera.org:8080/#/c/4047/5/fe/src/main/java/com/cloudera/impala/analysis/InsertStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/InsertStmt.java:

PS5, Line 83: Other columns, if not
:   // explicitly mentioned, will be assigned NULL values
Is this true for UPSERT as well?


PS5, Line 462: if (table_ instanceof KuduTable) {
 :   if (partitionKeyValues_ != null) {
 : throw new AnalysisException("PARTITION clause is not 
valid for Kudu tables.");
 :   }
 : } else {
 :   throw new AnalysisException("UPSERT is only supported for 
Kudu tables");
 : }
Hm I don't think this function is very useful. Why don't we inline this in L190 
and change the condition in L376?


PS5, Line 472: Checks that the column permutation + select list + static 
partition exprs + dynamic
 :* partition exprs collectively cover exactly all required 
columns in the target table,
 :* where the required columns are:
 :* - Any partition columns.
 :* - All key columns if the target is a Kudu table.
 :* - The row-key column if the target is an HBase table
This comment is a bit confusing. First of all static and dynamic partition 
exprs don't exist in the context of Kudu tables. Second, as the comment 
suggests, this function has too many responsibilities. Can you make an attempt 
to separate the logic that is specific to each table type while keeping the 
parts that are common (L527-552)?


PS5, Line 731: if (isUpsert_) {
 :   strBuilder.append("UPSERT ");
 : } else {
 :   strBuilder.append("INSERT ");
 : }
Can't you just do strBuilder.append(getOpName() + " ");


http://gerrit.cloudera.org:8080/#/c/4047/5/fe/src/main/java/com/cloudera/impala/planner/KuduTableSink.java
File fe/src/main/java/com/cloudera/impala/planner/KuduTableSink.java:

PS5, Line 61: output.append(detailPrefix);
Why is this not applied to UPSERT?


http://gerrit.cloudera.org:8080/#/c/4047/5/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/com/cloudera/impala/analysis/AnalyzeStmtsTest.java:

PS5, Line 2454: AnalyzesOk("upsert into table functional_kudu.testtbl values(1, 
'a', 1)");
Also with a permutation list?


PS5, Line 2593: AnalyzesOk("upsert into functional_kudu.testtbl with t1 as 
(select * from " +
  : "functional.alltypes) select bigint_col, string_col, 
int_col from t1");
Also one case with permutation list.


Line 3480: 
Do you have any positive test cases with permutation lists?
- only primary keys
- primary keys + some other columns
- columns in permutation list are not in the same order as columns in target 
table


http://gerrit.cloudera.org:8080/#/c/4047/5/testdata/workloads/functional-planner/queries/PlannerTest/insert.test
File testdata/workloads/functional-planner/queries/PlannerTest/insert.test:

PS5, Line 593: select * from functional.alltypes where year=2009 and month=05
Can you make this case a bit more interesting so that the plan is not just 
upsert followed by a scan? e.g. add a join/agg in the view


http://gerrit.cloudera.org:8080/#/c/4047/5/testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test
File testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test:

PS5, Line 289: upsert into table tdata (id, valf) values (2, 10), (120, 20)
Can you also add a case with a permutation list that results in a mix of 
inserts and updates? Also a case where the columns in the permutation list are 
not in the same order as the columns in the target table.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8df5cea36b642e267f85ff6b163f3dd96b8386e9
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Matthew Jacobs 

[Impala-ASF-CR] IMPALA-4237: Fix materialization of 4 byte decimals in data source scan node.

2016-10-06 Thread Internal Jenkins (Code Review)
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4237: Fix materialization of 4 byte decimals in data 
source scan node.
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5340c2eda813afc032ba72203bd59eb3f2c4f482
Gerrit-PatchSet: 2
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: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4123: Fast bit unpacking

2016-10-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#7).

Change subject: IMPALA-4123: Fast bit unpacking
..

IMPALA-4123: Fast bit unpacking

Adds utility functions for fast unpacking of batches of bit-packed
values. These support reading batches of any number of values provided
that the start of the batch is aligned to a byte boundary. Callers that
want to read smaller batches that don't align to byte boundaries will
need to implement their own buffering.

The unpacking code uses only portable C++ and no SIMD intrinsics, but is
fairly efficient because unpacking a full batch of 32 values compiles
down to 64-bit loads, shifts by constants, masks by constants, bitwise
ors at 64-bit boundaries, and stores. Further speedups should be
possible using SIMD intrinsics.

Testing:
Added unit tests for unpacking, exhaustively covering different
bitwidths with additional test dimensions (memory alignment, various
input sizes, etc).

Tested under ASAN to ensure the bit unpacking doesn't read past the end
of buffers.

Perf:
Added microbenchmark that shows on average an 8-9x speedup over the
existing BitReader code.

Change-Id: I12db69409483d208cd4c0f41c27a78aeb6cd3622
---
M be/src/benchmarks/CMakeLists.txt
A be/src/benchmarks/bit-packing-benchmark.cc
M be/src/util/CMakeLists.txt
A be/src/util/bit-packing-test.cc
A be/src/util/bit-packing.h
A be/src/util/bit-packing.inline.h
M be/src/util/bit-stream-utils.h
M be/src/util/bit-stream-utils.inline.h
8 files changed, 807 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/4494/7
-- 
To view, visit http://gerrit.cloudera.org:8080/4494
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I12db69409483d208cd4c0f41c27a78aeb6cd3622
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

2016-10-06 Thread Matthew Jacobs (Code Review)
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3920: TotalStorageWaitTime counter not populated for 
fragments with Kudu scan node
..


Patch Set 1:

Thanks, Dan.

In that case:
Anuj, it looks like we can remove the counter we have and use 
TotalStorageWaitTime wherever we were calling the previous one.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If0c793930799fdcaff53e705f94b52cadac2f53a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4086: Add benchmark for simple scheduler

2016-10-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4086: Add benchmark for simple scheduler
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4554/1/be/src/benchmarks/simple-scheduler-benchmark.cc
File be/src/benchmarks/simple-scheduler-benchmark.cc:

Line 130: /// according to the parameter 'replica_preference'.
Mention that it uses the default # blocks?


Line 147: /// Build and run a benchmark suite for various table sizes. 
Scheduling will be done
Mention that it's done with the default cluster size?


http://gerrit.cloudera.org:8080/#/c/4554/1/be/src/util/benchmark.cc
File be/src/util/benchmark.cc:

PS1, Line 51: by
I don't think this "by" is needed


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I89ec1c6c1828bb0b86d1e13ce4dfc5a8df865c2e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency

2016-10-06 Thread Huaisi Xu (Code Review)
Huaisi Xu has posted comments on this change.

Change subject: IMPALA-1702: Enforce table level consistency
..


Patch Set 11:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4349/11//COMMIT_MSG
Commit Message:

Line 11: analyzing: reference to the same table may refer to a
analyzed


Line 12: different version, and different table may share same
the same


Line 16: This commit removes table Id from Table object; instead
the Table


PS11, Line 17: it
execRequest


PS11, Line 21: references
reference


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Huaisi Xu 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4076: Fix runtime filter sort compare method

2016-10-06 Thread Alex Behm (Code Review)
Alex Behm has posted comments on this change.

Change subject: IMPALA-4076: Fix runtime filter sort compare method
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4652/2/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:

Line 341:   }
> The equation is trying to get the selectivity from cardinalities. Only doin
Sorry, I confused myself (and the join sides). You are right. Please leave as 
is :)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad433f2ece423ea29e79e81b68fa53cb0af18378
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4076: Fix runtime filter sort compare method

2016-10-06 Thread Taras Bobrovytsky (Code Review)
Taras Bobrovytsky has posted comments on this change.

Change subject: IMPALA-4076: Fix runtime filter sort compare method
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4652/2/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:

Line 341:   }
> This doesn't seem quite right. A cardinality of 0 translates to a selectivi
We are checking the cardinality of the child left child (the potential 
destination of the runtime filters). X/0 can never equal zero no matter what X 
is.


http://gerrit.cloudera.org:8080/#/c/4652/2/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test:

Line 1121: # IMPALA-4076: This query was constructed by hand to trigger the 
issue with sorting of
> Should also have brief description of the scenario being tested, something 
Done


Line 1123: # so the number of runtime filters has to be greater than 32 and 
they have to be in a
> We should set this 32 number a part of the planner test. Otherwise, if we c
I don't think it would silently fail, because the plan would change. But just 
in case, I set the query option to 10.


Line 1203:   inner join small_two on (True)
> you can remove the "on (True)" conditions
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad433f2ece423ea29e79e81b68fa53cb0af18378
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4076: Fix runtime filter sort compare method

2016-10-06 Thread Taras Bobrovytsky (Code Review)
Taras Bobrovytsky has uploaded a new patch set (#3).

Change subject: IMPALA-4076: Fix runtime filter sort compare method
..

IMPALA-4076: Fix runtime filter sort compare method

Fixed 2 isssues:
- The getSelectivity() method sometimes returned NaN double values which
could not be sorted properly.
- The compare method for sorting runtime filters was swtiched to use
the builtin Double comparison method.

Change-Id: Iad433f2ece423ea29e79e81b68fa53cb0af18378
---
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
3 files changed, 206 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/4652/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4652
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iad433f2ece423ea29e79e81b68fa53cb0af18378
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 


[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency

2016-10-06 Thread Huaisi Xu (Code Review)
Huaisi Xu has posted comments on this change.

Change subject: IMPALA-1702: Enforce table level consistency
..


Patch Set 10:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/4349/9//COMMIT_MSG
Commit Message:

PS9, Line 7: Enforce table level consistency
> IMO, this is a little misleading as its sounds similar to consistency guara
I think this is just a high level description. It hides implementation details. 
This is the goal we are trying to achieve. and it contains two things.


http://gerrit.cloudera.org:8080/#/c/4349/9/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

Line 295: private final HashMap referencedTables_ = 
Maps.newHashMap();
> Thanks for the explanation Alex. Makes sense to me.
Although this is not strictly necessary, this is more of completing the circle.


http://gerrit.cloudera.org:8080/#/c/4349/9/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java:

PS9, Line 187: 
 :   table.load(true, client.getHiveClient(), msTbl);
 :   insertStmt_.setTargetTable(table);
> update this as per new design.
Done


http://gerrit.cloudera.org:8080/#/c/4349/9/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java:

PS9, Line 154: /**
 :* Connect tupleDescriptors to tableDescriptors with unique 
table ids and get
 :* this DescriptorTable ready to be sent to backend.
 :*/
> How about we prefix this comment with a brief description of what this meth
Done


Line 164: 
> Sorry, I see what you mean now. Yes, that would work as well, but we'd need
This is only for table validation so same table references have the same 
instance. If this is for validation purposes then it makes more sense to 
isolate this from anything we used in the code.


http://gerrit.cloudera.org:8080/#/c/4349/9/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:

Line 37: import org.apache.kudu.client.KuduClient;
> Move it below to org.apache.impala group.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Huaisi Xu 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency

2016-10-06 Thread Huaisi Xu (Code Review)
Huaisi Xu has uploaded a new patch set (#10).

Change subject: IMPALA-1702: Enforce table level consistency
..

IMPALA-1702: Enforce table level consistency

Catalogd managed table id generation causes problems when
new update arrives at frontend while queries are being
analyzing: reference to the same table may refer to a
different version, and different table may share same
table id. This causes problems when one table override
another in the backend table map.

This commit removes table Id from Table object; instead
frontend assigns a unique Id to each table just before it
is sent to backend. It also implements a referecedTables_
cache in Analyzer::globalState_ so that calling
Analyzer::getTable() on the same table returns the same
references for the same query.

Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java
M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java
M fe/src/main/java/org/apache/impala/catalog/KuduTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
D fe/src/main/java/org/apache/impala/catalog/TableId.java
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
M fe/src/main/java/org/apache/impala/catalog/View.java
M fe/src/main/java/org/apache/impala/common/Id.java
M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/JoinTableId.java
M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
25 files changed, 176 insertions(+), 212 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/10
-- 
To view, visit http://gerrit.cloudera.org:8080/4349
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Huaisi Xu 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-1430,IMPALA-4108: codegen all builtin aggregate functions

2016-10-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#2).

Change subject: IMPALA-1430,IMPALA-4108: codegen all builtin aggregate functions
..

IMPALA-1430,IMPALA-4108: codegen all builtin aggregate functions

This change enables codegen for all builtin aggregate functions,
e.g. timestamp functions and group_concat.

There are several parts to the change:
* Adding support for generic UDAs. Previous the codegen code did not
  handle multiple input arguments or NULL return values.
* Defaulting to using the UDA interface when there is not a special
  codegen path (we have implementations of all builtin aggregate
  functions for the interpreted path).
* Remove all the logic to disable codegen for the special cases that now
  are supported.

Also fix the generation of code to get/set NULL bits since I needed
to add functionality there anyway.

Testing:
Add tests that check that codegen was enabled for builtin aggregate
functions. Also fix some gaps in the preexisting tests.

Also add tests for UDAs that check input/output nulls are handled
correctly, in anticipation of enabling codegen for arbitrary UDAs.

Perf:
Ran local TPC-H and targeted perf. Spent a lot of time on TPC-H Q1,
since my original approach regressed it ~5%. In the end the problem was
to do with the ordering of loads/stores to the slot and null bit in the
generated code: the previous version of the code exploited some
properties of the particular aggregate function. I ended up replicating
this behaviour to avoid regressing perf.

Change-Id: Id9dc21d1d676505d3617e1e4f37557397c4fb260
---
M be/src/benchmarks/hash-benchmark.cc
M be/src/codegen/codegen-anyval.cc
M be/src/codegen/codegen-anyval.h
M be/src/codegen/gen_ir_descriptions.py
M be/src/codegen/llvm-codegen-test.cc
M be/src/codegen/llvm-codegen.h
M be/src/exec/aggregation-node.cc
M be/src/exec/exec-node.cc
M be/src/exec/hash-join-node.cc
M be/src/exec/hash-table.cc
M be/src/exec/hdfs-avro-scanner.cc
M be/src/exec/hdfs-scanner.cc
M be/src/exec/old-hash-table.cc
M be/src/exec/partitioned-aggregation-node-ir.cc
M be/src/exec/partitioned-aggregation-node.cc
M be/src/exec/partitioned-aggregation-node.h
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/text-converter.cc
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/case-expr.cc
M be/src/exprs/compound-predicates.cc
M be/src/exprs/expr.cc
M be/src/exprs/literal.cc
M be/src/exprs/null-literal.cc
M be/src/exprs/scalar-fn-call.cc
M be/src/exprs/slot-ref.cc
M be/src/runtime/descriptors.cc
M be/src/runtime/descriptors.h
M be/src/runtime/tuple.cc
M be/src/testutil/test-udas.cc
M be/src/testutil/test-udfs.cc
M be/src/util/tuple-row-compare.cc
M fe/src/main/java/org/apache/impala/analysis/AggregateInfoBase.java
M testdata/workloads/functional-query/queries/QueryTest/uda.test
M tests/common/test_result_verifier.py
M tests/query_test/test_aggregation.py
M tests/query_test/test_udfs.py
37 files changed, 691 insertions(+), 450 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/4655/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4655
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id9dc21d1d676505d3617e1e4f37557397c4fb260
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 


[Impala-ASF-CR] IMPALA-4076: Fix runtime filter sort compare method

2016-10-06 Thread Alex Behm (Code Review)
Alex Behm has posted comments on this change.

Change subject: IMPALA-4076: Fix runtime filter sort compare method
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4652/2/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:

Line 341:   }
This doesn't seem quite right. A cardinality of 0 translates to a selectivity 
of 0 (not -1)


http://gerrit.cloudera.org:8080/#/c/4652/2/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test:

Line 1121: # IMPALA-4076: This query was constructed by hand to trigger the 
issue with sorting of
Should also have brief description of the scenario being tested, something like:

Test pruning the least selective runtime filters to obey QUERY_OPTION_NAME in 
the presence of zero-cardinality plan nodes.


Line 1123: # so the number of runtime filters has to be greater than 32 and 
they have to be in a
We should set this 32 number a part of the planner test. Otherwise, if we 
change default value of the query option, we may silently lose test coverage.


Line 1203:   inner join small_two on (True)
you can remove the "on (True)" conditions


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad433f2ece423ea29e79e81b68fa53cb0af18378
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

2016-10-06 Thread Matthew Jacobs (Code Review)
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3920: TotalStorageWaitTime counter not populated for 
fragments with Kudu scan node
..


Patch Set 1:

(1 comment)

Thanks Dan, 1 more below...

http://gerrit.cloudera.org:8080/#/c/4639/1/be/src/exec/kudu-scanner.cc
File be/src/exec/kudu-scanner.cc:

PS1, Line 152: scanner_->Open()
@Dan, I meant to call this one out as well.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If0c793930799fdcaff53e705f94b52cadac2f53a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-3920: TotalStorageWaitTime counter not populated for fragments with Kudu scan node

2016-10-06 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: IMPALA-3920: TotalStorageWaitTime counter not populated for 
fragments with Kudu scan node
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4639/1/be/src/exec/kudu-scanner.cc
File be/src/exec/kudu-scanner.cc:

PS1, Line 137: 
KUDU_RETURN_IF_ERROR(kudu::client::KuduScanToken::DeserializeIntoScanner(
 :   scan_node_->kudu_client(), scan_token, ),
> @Kudu-team: Is this worth accounting for?
This operation doesn't require any RPCs, so it should be fast.


PS1, Line 159: scanner_->Close();
> @Kudu-team: Is this worth accounting for?
If the scan node doesn't scan until completion, calling Close may issue an 
async RPC to the tserver to close the scanner.  However, I doubt y'all don't 
finish the scan, and the RPC is not waited on, so this should be fast.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If0c793930799fdcaff53e705f94b52cadac2f53a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4076: Fix runtime filter sort compare method

2016-10-06 Thread Taras Bobrovytsky (Code Review)
Taras Bobrovytsky has uploaded a new patch set (#2).

Change subject: IMPALA-4076: Fix runtime filter sort compare method
..

IMPALA-4076: Fix runtime filter sort compare method

Fixed 2 isssues:
- The getSelectivity() method sometimes returned NaN double values which
could not be sorted properly.
- The compare method for sorting runtime filters was swtiched to use
the builtin Double comparison method.

Change-Id: Iad433f2ece423ea29e79e81b68fa53cb0af18378
---
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
2 files changed, 201 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/4652/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4652
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iad433f2ece423ea29e79e81b68fa53cb0af18378
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 


[Impala-ASF-CR] IMPALA-4023: don't attach buffered tuple streams to batches

2016-10-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4023: don't attach buffered tuple streams to batches
..


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4448/7/be/src/runtime/buffered-tuple-stream-test.cc
File be/src/runtime/buffered-tuple-stream-test.cc:

Line 723:   DCHECK(batch->AtCapacity()); // Flush resources flash should have 
been set.
> typo: flash -> flag
Done


http://gerrit.cloudera.org:8080/#/c/4448/7/be/src/runtime/row-batch.h
File be/src/runtime/row-batch.h:

Line 219:   /// the original owner, even when the ownership of batches s 
transferred. If the
> typo: is
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6471422f86ce71e4c6ab277a27651bc2e8ff
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service

2016-10-06 Thread Huaisi Xu (Code Review)
Huaisi Xu has uploaded a new patch set (#9).

Change subject: IMPALA-1702: Enforce table level consistency accross service
..

IMPALA-1702: Enforce table level consistency accross service

Problems:
1. CatalogServiceCatalog::reset() resets table ID, so same
tables can have different ID and different tables can have
same table ID before/after this call. Backend could
overwrites each table in an 1:1 table ID to table descriptor
map.

2. 1 can happen because frontend gets table directly
from catalog cache, which is updated concurrently with
query analyzing. so Analyzer::getTable() could return
different references for the same table when analyzing
the same query.

This commit removes table Id from Table object; instead
frontend assigns a unique Id to each table just before it
is sent to backend. It also implements a referecedTables_
cache in Analyzer::globalState_ so that calling
Analyzer::getTable() on the same table returns the same
references for the same query.

Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java
M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java
M fe/src/main/java/org/apache/impala/catalog/KuduTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
D fe/src/main/java/org/apache/impala/catalog/TableId.java
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
M fe/src/main/java/org/apache/impala/catalog/View.java
M fe/src/main/java/org/apache/impala/common/Id.java
M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/JoinTableId.java
M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
25 files changed, 178 insertions(+), 209 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/9
-- 
To view, visit http://gerrit.cloudera.org:8080/4349
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Huaisi Xu 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-4023: don't attach buffered tuple streams to batches

2016-10-06 Thread Alex Behm (Code Review)
Alex Behm has posted comments on this change.

Change subject: IMPALA-4023: don't attach buffered tuple streams to batches
..


Patch Set 7: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4448/7/be/src/runtime/buffered-tuple-stream-test.cc
File be/src/runtime/buffered-tuple-stream-test.cc:

Line 723:   DCHECK(batch->AtCapacity()); // Flush resources flash should have 
been set.
typo: flash -> flag


http://gerrit.cloudera.org:8080/#/c/4448/7/be/src/runtime/row-batch.h
File be/src/runtime/row-batch.h:

Line 219:   /// the original owner, even when the ownership of batches s 
transferred. If the
typo: is


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6471422f86ce71e4c6ab277a27651bc2e8ff
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-2905: Handle coordinator fragment lifecycle like all others

2016-10-06 Thread Henry Robinson (Code Review)
Henry Robinson has uploaded a new patch set (#8).

Change subject: IMPALA-2905: Handle coordinator fragment lifecycle like all 
others
..

IMPALA-2905: Handle coordinator fragment lifecycle like all others

The plan-root fragment instance that runs on the coordinator should be
handled like all others: started via RPC and run asynchronously. Without
this, the fragment requires special-case code throughout the
coordinator, and does not show up in system metrics etc.

This patch adds a new sink type, PlanRootSink, to the root fragment
instance so that the coordinator can pull row batches that are pushed by
the root instance. The coordinator signals completion to the fragment
instance via closing the consumer side of the sink, whereupon the
instance is free to complete.

Since the root instance now runs asynchronously wrt to the coordinator,
we add several coordination methods to allow the coordinator to wait for
a point in the instance's execution to be hit - e.g. to wait until the
instance has been opened.

Done in this patch:

* Add PlanRootSink
* Add coordination to PFE to allow coordinator to observe lifecycle
* Make FragmentMgr a singleton
* Removed dead code from Coordinator::Wait() and elsewhere.
* Moved result output exprs out of QES and into PlanRootSink.
* Remove special-case limit-based teardown of coordinator fragment, and
  supporting functions in PlanFragmentExecutor.
* Simplified lifecycle of PlanFragmentExecutor by separating Open() into
  OpenPlan() and Exec(), the latter of which drives the sink by reading
  rows from the plan tree.
* Add child profile to PlanFragmentExecutor to measure time spent in
  each lifecycle phase.
* Removed dependency between InitExecProfiles() and starting root fragment.

Not yet done:

* Fix planner tests to reflect new sink added at root.

Change-Id: Ibb0064ec2f085fa3a5598ea80894fb489a01e4df
---
M be/src/exec/CMakeLists.txt
M be/src/exec/data-sink.cc
A be/src/exec/plan-root-sink.cc
A be/src/exec/plan-root-sink.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/plan-fragment-executor.cc
M be/src/runtime/plan-fragment-executor.h
M be/src/scheduling/query-schedule.cc
M be/src/scheduling/query-schedule.h
M be/src/scheduling/simple-scheduler.cc
M be/src/service/fragment-exec-state.cc
M be/src/service/fragment-exec-state.h
M be/src/service/fragment-mgr.cc
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-internal-service.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/query-exec-state.cc
M be/src/service/query-exec-state.h
M be/src/testutil/in-process-servers.cc
M common/thrift/DataSinks.thrift
M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
A fe/src/main/java/org/apache/impala/planner/PlanRootSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/PlannerContext.java
M tests/hs2/test_json_endpoints.py
30 files changed, 840 insertions(+), 709 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/4402/8
-- 
To view, visit http://gerrit.cloudera.org:8080/4402
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibb0064ec2f085fa3a5598ea80894fb489a01e4df
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh

2016-10-06 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change.

Change subject: IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh
..


Patch Set 5:

> Any updates on this?

I'm currently focusing on IMPALA-4223, after that I will get back to this one.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb37e2ef0cd9fa0e581d359c5dd3db7812b7b2c8
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4231: fix codegen time regression

2016-10-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#2).

Change subject: IMPALA-4231: fix codegen time regression
..

IMPALA-4231: fix codegen time regression

The commit "IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder"
slightly increased codegen time, which caused TPC-H Q2 to sometimes
regress significantly because of races in runtime filter arrival.

This patch attempts to fix the regression by improving codegen time in a
few places.

* Revert to using the old bool/Status return pattern. The regular Status
  return pattern results in significantly more complex IR because it has
  to emit code to copy and free statuses. I spent some time trying to
  convince it to optimise the extra code out, but didn't have much success.
* Remove some code that cannot be specialized from cross-compilation.
* Add noexcept to some functions that are used from the IR to ensure
  exception-handling IR is not emitted. This is less important after the
  first change but still should help produce cleaner IR.

Performance:
I was able to reproduce a regression locally, which is fixed by this
patch. I'm in the process of trying to verify the fix on a cluster.

Change-Id: Idf0fdedabd488550b6db90167a30c582949d608d
---
M be/src/common/status.h
M be/src/exec/hash-table.cc
M be/src/exec/hash-table.h
M be/src/exec/partitioned-aggregation-node-ir.cc
M be/src/exec/partitioned-aggregation-node.cc
M be/src/exec/partitioned-aggregation-node.h
M be/src/exec/partitioned-hash-join-builder-ir.cc
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/exec/partitioned-hash-join-builder.h
M be/src/exec/partitioned-hash-join-node-ir.cc
M be/src/exec/partitioned-hash-join-node.h
M be/src/runtime/buffered-tuple-stream.cc
M be/src/runtime/buffered-tuple-stream.h
M be/src/runtime/buffered-tuple-stream.inline.h
M be/src/runtime/raw-value.cc
M be/src/runtime/raw-value.h
M be/src/runtime/raw-value.inline.h
M be/src/util/bloom-filter.cc
M be/src/util/bloom-filter.h
19 files changed, 293 insertions(+), 268 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/4623/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4623
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idf0fdedabd488550b6db90167a30c582949d608d
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho 


[Impala-ASF-CR] Not for review: IR for IMPALA-4231: fix codegen time regression

2016-10-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#2).

Change subject: Not for review: IR for IMPALA-4231: fix codegen time 
regression
..

Not for review: IR for IMPALA-4231: fix codegen time regression

Change-Id: I1900e7605b6c7eeb6f2f14c7660887137f9e4d16
---
A llvm-opt-modules/TPC-H-Q2-fragment1.ll
1 file changed, 3,797 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4649/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4649
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1900e7605b6c7eeb6f2f14c7660887137f9e4d16
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 


[Impala-ASF-CR] IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh

2016-10-06 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change.

Change subject: IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh
..


Patch Set 5:

Any updates on this?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb37e2ef0cd9fa0e581d359c5dd3db7812b7b2c8
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No


[Impala-ASF-CR](asf-site) Add 'Downloads' link to bylaws page

2016-10-06 Thread Jim Apple (Code Review)
Jim Apple has uploaded a new change for review.

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

Change subject: Add 'Downloads' link to bylaws page
..

Add 'Downloads' link to bylaws page

Change-Id: I7e7373976f32b5f9904e22ba3f137ed94291dd56
---
M bylaws.html
1 file changed, 1 insertion(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7e7373976f32b5f9904e22ba3f137ed94291dd56
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-Owner: Jim Apple 


[Impala-ASF-CR] IMPALA-3853: squeasel is MIT (and dual copyright) not Apache

2016-10-06 Thread Jim Apple (Code Review)
Jim Apple has uploaded a new change for review.

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

Change subject: IMPALA-3853: squeasel is MIT (and dual copyright) not Apache
..

IMPALA-3853: squeasel is MIT (and dual copyright) not Apache

Squeasel was erroneously marked Apache 2.0 in the top-level
LICENSE.txt, with no copyright notice. It's actually MIT, with two
copyrights.

Change-Id: I9711ad60dbe00c3b8b1ce7b9ccc3ca1dd637b88c
---
M LICENSE.txt
M be/src/thirdparty/squeasel/LICENSE
2 files changed, 27 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I9711ad60dbe00c3b8b1ce7b9ccc3ca1dd637b88c
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 


[Impala-ASF-CR] IMPALA-4231: fix codegen time regression

2016-10-06 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change.

Change subject: IMPALA-4231: fix codegen time regression
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4623/1//COMMIT_MSG
Commit Message:

PS1, Line 26: y, which is fixed by this
: patch.
Does it recover all the performance ?


http://gerrit.cloudera.org:8080/#/c/4623/1/be/src/exec/partitioned-hash-join-builder-ir.cc
File be/src/exec/partitioned-hash-join-builder-ir.cc:

PS1, Line 32: inline bool PhjBuilder::AppendRow(
I thought the destructor is inlined so it only calls FreeMessage() which is not 
supposed to cross-compiled. So, I suppose the regression is not the codegen 
time but the runtime of this function, right ?


http://gerrit.cloudera.org:8080/#/c/4623/1/be/src/runtime/raw-value.inline.h
File be/src/runtime/raw-value.inline.h:

PS1, Line 215: 
this may need to be inlined again eventually for loop unrolling to work.


http://gerrit.cloudera.org:8080/#/c/4623/1/be/src/util/bloom-filter.h
File be/src/util/bloom-filter.h:

PS1, Line 176: inline void IR_ALWAYS_INLINE
void ALWAYS_INLINE


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf0fdedabd488550b6db90167a30c582949d608d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho 
Gerrit-HasComments: Yes