[Impala-ASF-CR] IMPALA-4014: Introduce query-wide execution state.

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4014: Introduce query-wide execution state.
..


Patch Set 7:

(18 comments)

http://gerrit.cloudera.org:8080/#/c/4418/7/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

PS7, Line 938: !
nit: missing space


http://gerrit.cloudera.org:8080/#/c/4418/7/be/src/runtime/coordinator.h
File be/src/runtime/coordinator.h:

PS7, Line 507: fragment_instance_state
instance_state to match below and the definition.


PS7, Line 511: fragment_instance_state
same


http://gerrit.cloudera.org:8080/#/c/4418/7/be/src/runtime/fragment-instance-state.cc
File be/src/runtime/fragment-instance-state.cc:

Line 75: // including the final status when execution finishes.
I had deleted this comment because it wasn't accurate (and had added the 
accurate info the the header comment).  Let's not add it back.


http://gerrit.cloudera.org:8080/#/c/4418/7/be/src/runtime/fragment-instance-state.h
File be/src/runtime/fragment-instance-state.h:

Line 53: /// - move ReportStatusCb() logic into PFE::SendReport() and get rid 
of the callback
thanks, these four are going to make things a lot better.


http://gerrit.cloudera.org:8080/#/c/4418/7/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:

Line 115:   // always decrement refcount
not sure what this comment is trying to tell me.  would be more informative as: 
decrement the refcount taken in StartFInstance()


Line 148: qs = it->second;
it's probably worth commenting here explaining why we need to get a new qs 
pointer, i.e. someone else may have gc'd the old one and then someone else 
created a new one.  it's as subtle as the other cases that are commented.


http://gerrit.cloudera.org:8080/#/c/4418/7/be/src/runtime/query-exec-mgr.h
File be/src/runtime/query-exec-mgr.h:

Line 38: /// entry point for gaining refcounted access to a QueryState.
this doesn't really explain that it's also the thing that executes instances.


PS7, Line 47: decrement
this really happens after execution finishes, so would be good to reword.


PS7, Line 53: CancelFInstance
what's that? (not a method of this class)


http://gerrit.cloudera.org:8080/#/c/4418/7/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:

PS7, Line 79: desc_tbl
do we plan to pull this out then? (maybe it's mentioned somewhere i haven't 
read yet).


Line 83:   bool released_resources() const { return released_resources_; }
who outside this class will care about this? besides, in order to call these 
methods, you have to have a refcnt, which means the resources couldn't have 
been released, right?


Line 93:   /// Illegal to call any other function of this class after this call 
returns.
Isn't it more that a reference must be held when calling the other methods, and 
that this method can't be called until all references are returned? why not 
specify that?


http://gerrit.cloudera.org:8080/#/c/4418/7/be/src/runtime/runtime-state.cc
File be/src/runtime/runtime-state.cc:

Line 90:   DCHECK(status.ok()) << status.GetDetail();
not for this change but we should clean this up.


Line 345: }
are any of these on critical paths (i.e. does the call instruction you're 
adding matter)?


http://gerrit.cloudera.org:8080/#/c/4418/7/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:

Line 354:   /// TODO: get rid of this and use ExecEnv::GetInstance() instead
thanks


http://gerrit.cloudera.org:8080/#/c/4418/7/be/src/service/impala-internal-service.cc
File be/src/service/impala-internal-service.cc:

Line 68:   fis->Cancel();
is it intentional that we ignore the status returned by Cancel()?  We used to 
propagate back as the return_val status (inside CancelPlanFragment).


http://gerrit.cloudera.org:8080/#/c/4418/7/be/src/service/query-exec-state.h
File be/src/service/query-exec-state.h:

Line 55: /// TODO: Consider renaming to RequestExecState for consistency.
don't do it as part of this change, but this renaming is more important now 
given the other similarly named class.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I962ae6b7cb7dc0d07fbb8f70317aeb01d88d400b
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Marcel Kornacker 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3167: Fix assignment of WHERE conjunct through grouping agg + OJ.

[Internal Jenkins (Code Review)]

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-3167: Fix assignment of WHERE conjunct through grouping 
agg + OJ.
..


Patch Set 4: Verified+1

-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Anonymous Coward #27
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3167: Fix assignment of WHERE conjunct through grouping agg + OJ.

[Internal Jenkins (Code Review)]

Internal Jenkins has submitted this change and it was merged.

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
the slots referenced by the predicate have equivalent slots that
belong to an outer-joined tuple. 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
Reviewed-on: http://gerrit.cloudera.org:8080/4960
Reviewed-by: Alex Behm 
Tested-by: Internal Jenkins
---
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, 52 insertions(+), 31 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I774d13a13ad1e8fe82512df98dc29983bdd232eb
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Anonymous Coward #27
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Marcel Kornacker 

[Impala-ASF-CR] IMPALA-4585: Replace file paths with HDFS FILENAME in expected exceptions

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change.

Change subject: IMPALA-4585: Replace file paths with __HDFS_FILENAME__ in 
expected exceptions
..


Patch Set 1:

(1 comment)

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

Line 9: Recently we introduced a change where we should replace file paths in
> Can you point me to the change so I can understand why we made it?
It was introduced by me here:
https://github.com/cloudera/Impala/commit/0e67a56282ebd9c931ed7317311124dde0ca5201#diff-bb8f84b351f4f7e3c5adb6787ab37038R7

For my tests we don't know what path of the HDFS file will be upfront, that's 
why I added the template replacement in the CATCH section of the test file. 
(It's already being done in other sections)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If0f6ae8dea7ac4cdaf0c61ebd8f0c589c353a96e
Gerrit-PatchSet: 1
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-4578: Pick up bound predicates for Kudu scan nodes.

[Internal Jenkins (Code Review)]

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4578: Pick up bound predicates for Kudu scan nodes.
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I19a38d6ea8cc0d2b0ddc3808d1f9ffef5ce306a8
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-HasComments: No

[Impala-ASF-CR] IMPALA-4566: Kudu client glog contention can cause timeouts

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4566: Kudu client glog contention can cause timeouts
..


Patch Set 2: Code-Review+2

I think this addressed the issue, thanks!

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie535d89ec2525232d4f6a29dd44f51cd6e18a0d2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4585: Replace file paths with HDFS FILENAME in expected exceptions

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-4585: Replace file paths with __HDFS_FILENAME__ in 
expected exceptions
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If0f6ae8dea7ac4cdaf0c61ebd8f0c589c353a96e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4585: Replace file paths with HDFS FILENAME in expected exceptions

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-4585: Replace file paths with __HDFS_FILENAME__ in 
expected exceptions
..


Patch Set 1:

(1 comment)

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

Line 9: Recently we introduced a change where we should replace file paths in
Can you point me to the change so I can understand why we made it?


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

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

[Impala-ASF-CR] IMPALA-4574: Do not treat UUID() like a constant expr.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-4574: Do not treat UUID() like a constant expr.
..


Patch Set 5: Code-Review+2

rebase and resolve conflict

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2499f5f6ecdcb098623202c8e6dc2d02727194a
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4574: Do not treat UUID() like a constant expr.

[Alex Behm (Code Review)]

Hello Impala Public Jenkins, Internal Jenkins, Tim Armstrong,

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

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

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

Change subject: IMPALA-4574: Do not treat UUID() like a constant expr.
..

IMPALA-4574: Do not treat UUID() like a constant expr.

A recent change (IMPALA-1788) lead UUID() to be constant folded,
and therefore, produce the same value for every invocation across
rows. Similar issues might also occur due to the BE optimizing
UUID() during codegen of scalar-fn-call.h/cc.

The fix is to not treat UUID() like a constant expr in both
the FE and BE.

Discussion:
The fix in this patch is rather blunt, but minimally invasive to
reduce the risk of adding new bugs. Ideally, the constness of an
Expr should be determined in one place and the FE and BE should agree
on which Exprs are constant. I considered the following alternatives
but concluded they were too risky:
1. Pass a flag from FE to BE for ever Expr indicating its constness.
   This simple solution would populate a thrift field with the
   result of Expr.isConstant() for every Expr in an Expr tree.
   There are several issues. Calling isConstant() for every Expr
   in an Expr tree is rather expensive due to repeated traversals
   of the tree. That could be mitigated by populating an isConstant
   flag during Expr.analyze() to avoid re-computing the constness
   repeatedly. This requires changes to analyze(), clone(), reset(),
   and possibly other places for many Exprs. There is potential
   for missing a place and adding a new bug.
2. The above solution could be limited to only FunctionCallExpr.
   However, the BE expr type FUNCTION_CALL which maps to
   scalar-fn-call.h/cc is created from various FE Exprs, not just
   FunctionCallExpr. So adding a flag only to scalar-fn-call.h/cc
   would be confusing because it would only sometimes be set
   in a meaningful way. This seems more confusing than the current
   straightforward solution.

Testing: Added FE and EE tests.

Change-Id: If2499f5f6ecdcb098623202c8e6dc2d02727194a
---
M be/src/exprs/scalar-fn-call.cc
M be/src/exprs/scalar-fn-call.h
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
6 files changed, 31 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/5324/5
-- 
To view, visit http://gerrit.cloudera.org:8080/5324
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If2499f5f6ecdcb098623202c8e6dc2d02727194a
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-4585: Replace file paths with HDFS FILENAME in expected exceptions

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new change for review.

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

Change subject: IMPALA-4585: Replace file paths with __HDFS_FILENAME__ in 
expected exceptions
..

IMPALA-4585: Replace file paths with __HDFS_FILENAME__ in expected exceptions

Recently we introduced a change where we should replace file paths in
the CATCH section of a test file. Several test cases having to do with
non-hdfs filesystems slipped through the cracks. This patch fixes the
problem

Change-Id: If0f6ae8dea7ac4cdaf0c61ebd8f0c589c353a96e
---
M testdata/workloads/functional-query/queries/QueryTest/udf-errors.test
1 file changed, 2 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If0f6ae8dea7ac4cdaf0c61ebd8f0c589c353a96e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE TABLE

[Dimitris Tsirogiannis (Code Review)]

Hello Internal Jenkins, Alex Behm,

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

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

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

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/KuduPartitionParam.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/AuthorizationTest.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/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, 416 insertions(+), 415 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0e07c41eabb4c8cb95754cf04293cbd9e03d6ab2
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Internal Jenkins

[Impala-ASF-CR] IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE TABLE

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE 
TABLE
..


Patch Set 5: Code-Review+2

Fix test. Carry +2

-- 
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: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Internal Jenkins
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4467: Add support for DML statements in stress test

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#2).

Change subject: IMPALA-4467: Add support for DML statements in stress test
..

IMPALA-4467: Add support for DML statements in stress test

- Add support for insert, upsert, update and and delete statements.
- Add support for compute stats with mt_dop query options.
- Update impyla version in order to be able to have access to query
  error text for DML queries.
- Made flake8 fixes. flake8 on this file is clean.

Change-Id: Ia2aafdc6851cc0e1677a3c668d3350e47c4bfe40
---
M infra/python/deps/requirements.txt
M tests/stress/concurrent_select.py
M tests/util/parse_util.py
3 files changed, 564 insertions(+), 206 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia2aafdc6851cc0e1677a3c668d3350e47c4bfe40
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-4467: Add support for DML statements in stress test

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#2).

Change subject: IMPALA-4467: Add support for DML statements in stress test
..

IMPALA-4467: Add support for DML statements in stress test

- Add support for insert, upsert, update and and delete statements.
- Add support for compute stats with mt_dop query options.
- Update impyla version in order to be able to have access to query
  error text for DML queries.
- Made flake8 fixes. flake8 on this file is clean.

Change-Id: Ia2aafdc6851cc0e1677a3c668d3350e47c4bfe40
---
M infra/python/deps/requirements.txt
M tests/stress/concurrent_select.py
M tests/util/parse_util.py
3 files changed, 564 insertions(+), 206 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia2aafdc6851cc0e1677a3c668d3350e47c4bfe40
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-4467: Add support for DML statements in stress test

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#2).

Change subject: IMPALA-4467: Add support for DML statements in stress test
..

IMPALA-4467: Add support for DML statements in stress test

- Add support for insert, upsert, update and and delete statements.
- Add support for compute stats with mt_dop query options.
- Update impyla version in order to be able to have access to query
  error text for DML queries.
- Made flake8 fixes. flake8 on this file is clean.

Change-Id: Ia2aafdc6851cc0e1677a3c668d3350e47c4bfe40
---
M infra/python/deps/requirements.txt
M tests/stress/concurrent_select.py
M tests/util/parse_util.py
3 files changed, 564 insertions(+), 206 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia2aafdc6851cc0e1677a3c668d3350e47c4bfe40
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 

[Impala-ASF-CR] Preview: IMPALA-4467: Add support for CRUD operations in stress test

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change.

Change subject: Preview: IMPALA-4467: Add support for CRUD operations in stress 
test
..


Patch Set 1:

(23 comments)

http://gerrit.cloudera.org:8080/#/c/5093/1/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

Line 680: # Query to run before worst_case_sql to get the worst case 
performance
> What's worst_case_sql? Can you give a little more flavor to the comment? I'
I updated the patch significantly (based on our discussion), this is no longer 
here.


Line 688: self.options = {}
> needs a comment
Done


Line 691: self.modifies_table = False
> let's introduce a stmt_type enum that can have values like SELECT/INSERT/UP
Done


Line 694: self.MIN_INT = 10
> var names are not very telling, what are these numbers?
This not relevant any more. Removed.


PS1, Line 695: self.MIN_INT = 20
> You've defined self.MIN_INT twice.
Yeah, like I said it was a rough patch. These numbers are not needed any more.


PS1, Line 698:   def sql(self):
> This whole set of methods could use docstrings to explain to someone roughl
This method was removed.


PS1, Line 701:   return str(randint(10, 20))
> I would like for us to make our randomized tests more repeatable. To that e
After talking to Alex, I removed any randomness from the Query class. Now 1 
query object always represents a single sql statement.


PS1, Line 713: result = re.sub(pattern, "10", self._sql)
> Was the hard coding of 10 here meant to actually be self.MIN_INT?
Yes exactly, that was the intention. However, this is no longer relevant.


Line 832:   raise
> seems like we intentionally did not raise before?
this was a mistake, removed.


Line 942:   LOG.debug("Result set empty for query with id %s",
> Another possible use of stmt_type
No longer relevant.


Line 1239: try:
> we can use the stmt_type here to only run explain for those stmts that supp
Done


Line 1356: def clean_up_database(cursor):
> reset_database?
Done


Line 1357:   LOG.info("Cleaning up {0} database".format(cursor.db_name))
> needs comment
Added method docstring.


Line 1360: if not table.name.endswith("_original"):
> seems simpler to move the modified tables into a new database, then you don
I am not sure if it's simpler. It seems to me it's simpler to have everything 
in a single database. We can maybe discuss this offline.


Line 1371:   if len(table.primary_keys) < 1:
> add comment: only run for Kudu tables which must have a primary key
Done


Line 1373:   # For now we will not handle the case in a special way where 
several columns are a
> Why? Seems easy enough to do.
I am not really sure if it's necessary to handle this in some special way. Do 
you have any suggestions?


Line 1376:   if primary_key.exact_type not in (TinyInt, SmallInt, BigInt):
> Why this restriction?
We want to be able to apply the "modulo operation". Added comment. Added 
comment.


PS1, Line 1379:   query.modifies_table = False
> Why is this False?
It's relative to the original state. For example, if we run a query  If we have 
an unmodified lineitem item table, then we run an upsert query on it, it should 
remain unchanged. Added comment in class.


Line 1383:   if table.name + "_original" in set(table.name for table in 
tables):
> What's the motivation for this? We might be able to achieve the same thing 
As discussed in person, I agree that 1 Query object should represent 1 query. 
Fixed.


PS1, Line 1392: def generate_delete_queries(cursor):
> Lots of duplicated logic here and in generate_upsert_queries(). I think add
Rewrote to eliminate code duplication.


Line 1433:   drop_query.sql = "DROP STATS {0}".format(table.name)
> why drop? we're not checking the results anyway
Done


Line 1438:   compute_query.name = "compute_stats_{0}".format(table.name)
> can you add the mt_dop option as part of the name?
Done


PS1, Line 1664: if query._sql in 
queries_with_runtime_info_by_db_and_sql[query.db_name]:
> It doesn't seem right to be examining a private attribute from the outside.
Yeah, I agree, this was a rough patch. Fixed now.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2aafdc6851cc0e1677a3c668d3350e47c4bfe40
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

[Internal Jenkins (Code Review)]

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
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-HasComments: No

[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

[Internal Jenkins (Code Review)]

Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


IMPALA-4584: Make alter table operations on Kudu tables synchronous

This commit changes the behavior of alter table operations on Kudu
tables from asynchronous to synchronous. With this change, alter table
operations return when either the operations complete successfully or
a timeout is reached.

Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Reviewed-on: http://gerrit.cloudera.org:8080/5364
Reviewed-by: Dimitris Tsirogiannis 
Tested-by: Internal Jenkins
---
M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
2 files changed, 36 insertions(+), 28 deletions(-)

Approvals:
  Internal Jenkins: Verified
  Dimitris Tsirogiannis: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
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

[Impala-ASF-CR] IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE TABLE

[Internal Jenkins (Code Review)]

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE 
TABLE
..


Patch Set 4: Verified-1

Build failed: 
http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/600/

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

[Impala-ASF-CR] IMPALA-4574: Do not treat UUID() like a constant expr.

[Internal Jenkins (Code Review)]

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4574: Do not treat UUID() like a constant expr.
..


Patch Set 4: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2499f5f6ecdcb098623202c8e6dc2d02727194a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4571: Push IN predicates to Kudu

[Internal Jenkins (Code Review)]

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4571: Push IN predicates to Kudu
..


Patch Set 5: Verified+1

-- 
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: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4571: Push IN predicates to Kudu

[Internal Jenkins (Code Review)]

Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4571: Push IN predicates to Kudu
..


IMPALA-4571: Push IN predicates to Kudu

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 expression is of
the exact form:
IN (, , ...)

That means the InPredicate has the following properties:
1) It has a list of literal values (i.e. not a subquery);
   All values are LiteralExprs (not SlotRefs).
2) Not negative, i.e. only 'IN' supported, 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
Reviewed-on: http://gerrit.cloudera.org:8080/5316
Reviewed-by: Matthew Jacobs 
Tested-by: Internal Jenkins
---
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
3 files changed, 99 insertions(+), 7 deletions(-)

Approvals:
  Matthew Jacobs: Looks good to me, approved
  Internal Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I8988d4819d20d467b48e286917e347ca00f60cf0
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs 

[Impala-ASF-CR] IMPALA-4477: Bump Kudu version to latest master (60aa54e)

[Matthew Jacobs (Code Review)]

Matthew Jacobs has uploaded a new change for review.

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

Change subject: IMPALA-4477: Bump Kudu version to latest master (60aa54e)
..

IMPALA-4477: Bump Kudu version to latest master (60aa54e)

Bumps the toolchain version to get a newer Kudu build.

Also fixes test failures resulting from changes in Kudu.
Notably error strings have changed (IMPALA-4590) and the
number of replicas must be odd (IMPALA-4589).

Note: The toolchain binaries starting with this build are
now using the toolchain binutils rather than the system
binutils.

Testing: private exhaustive build.

Change-Id: If1912f058c240fbe82b06f77e31add7755289be1
---
M bin/impala-config.sh
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_partition_ddl.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_stats.test
5 files changed, 12 insertions(+), 12 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-3167: Fix assignment of WHERE conjunct through grouping agg + OJ.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-3167: Fix assignment of WHERE conjunct through grouping 
agg + OJ.
..


Patch Set 4: Code-Review+2

rebase

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

[Impala-ASF-CR] IMPALA-3167: Fix assignment of WHERE conjunct through grouping agg + OJ.

[Alex Behm (Code Review)]

Hello Marcel Kornacker,

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

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

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

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
the slots referenced by the predicate have equivalent slots that
belong to an outer-joined tuple. 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, 52 insertions(+), 31 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/4960/4
-- 
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: 4
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] IMPALA-4574: Do not treat UUID() like a constant expr.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4574: Do not treat UUID() like a constant expr.
..


Patch Set 4: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2499f5f6ecdcb098623202c8e6dc2d02727194a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4467: Add support for CRUD operations in stress test

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#2).

Change subject: IMPALA-4467: Add support for CRUD operations in stress test
..

IMPALA-4467: Add support for CRUD operations in stress test

- Added support for upsert and delete statements. Update and insert
  still need to be done.
- Added support for compute stats.
- Added support for query options to queries.

Change-Id: Ia2aafdc6851cc0e1677a3c668d3350e47c4bfe40
---
M infra/python/deps/requirements.txt
M tests/stress/concurrent_select.py
M tests/util/parse_util.py
3 files changed, 548 insertions(+), 204 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia2aafdc6851cc0e1677a3c668d3350e47c4bfe40
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 

[Impala-ASF-CR] IMPALA-4578: Pick up bound predicates for Kudu scan nodes.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-4578: Pick up bound predicates for Kudu scan nodes.
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I19a38d6ea8cc0d2b0ddc3808d1f9ffef5ce306a8
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3167: Fix assignment of WHERE conjunct through grouping agg + OJ.

[Marcel Kornacker (Code Review)]

Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-3167: Fix assignment of WHERE conjunct through grouping 
agg + OJ.
..


Patch Set 3: Code-Review+2

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

[Impala-ASF-CR] IMPALA-4578: Pick up bound predicates for Kudu scan nodes.

[Marcel Kornacker (Code Review)]

Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-4578: Pick up bound predicates for Kudu scan nodes.
..


Patch Set 1: Code-Review+2

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

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

[Impala-ASF-CR] Additional functional testing for default values on Kudu tables

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged.

Change subject: Additional functional testing for default values on Kudu tables
..


Additional functional testing for default values on Kudu tables

This commit also fixes an issue where an error is thrown if a default
value is set for a boolean column on a Kudu table.

Change-Id: I25b66275d29d1cf21df14e78ab58f625a83b0725
Reviewed-on: http://gerrit.cloudera.org:8080/5337
Reviewed-by: Matthew Jacobs 
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test
2 files changed, 74 insertions(+), 0 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Matthew Jacobs: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I25b66275d29d1cf21df14e78ab58f625a83b0725
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 

[Impala-ASF-CR] IMPALA-4014: Introduce query-wide execution state.

[Marcel Kornacker (Code Review)]

Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-4014: Introduce query-wide execution state.
..


Patch Set 7:

rebase past the recent pfe fixes

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I962ae6b7cb7dc0d07fbb8f70317aeb01d88d400b
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Marcel Kornacker 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] Additional functional testing for default values on Kudu tables

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: Additional functional testing for default values on Kudu tables
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I25b66275d29d1cf21df14e78ab58f625a83b0725
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2925: Mark test alloc update as xfail.

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-2925: Mark test_alloc_update as xfail.
..


Patch Set 1: Code-Review+2

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

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

[Impala-ASF-CR] IMPALA-4014: Introduce query-wide execution state.

[Marcel Kornacker (Code Review)]

Marcel Kornacker has uploaded a new patch set (#7).

Change subject: IMPALA-4014: Introduce query-wide execution state.
..

IMPALA-4014: Introduce query-wide execution state.

This introduces a global structure to coordinate execution
of fragment instances on a backend for a single query.

New classes:
- QueryExecMgr: subsumes FragmentMgr
- QueryState
- FragmentInstanceState: replaces FragmentExecState

It also adds a static function Thread::Run() to run a function in a
newly created thread. This is now used to execute a fragment
instance, instead of managing and destroying Thread objects via
shared_ptrs, etc.

Change-Id: I962ae6b7cb7dc0d07fbb8f70317aeb01d88d400b
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/exec/catalog-op-executor.cc
M be/src/exec/data-source-scan-node.cc
M be/src/exec/exchange-node.cc
M be/src/exec/hash-table-test.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/union-node.cc
M be/src/exprs/expr-test.cc
M be/src/runtime/CMakeLists.txt
M be/src/runtime/buffered-block-mgr-test.cc
M be/src/runtime/buffered-tuple-stream-test.cc
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
R be/src/runtime/fragment-instance-state.cc
A be/src/runtime/fragment-instance-state.h
M be/src/runtime/plan-fragment-executor.cc
M be/src/runtime/plan-fragment-executor.h
A be/src/runtime/query-exec-mgr.cc
A be/src/runtime/query-exec-mgr.h
A be/src/runtime/query-state.cc
A be/src/runtime/query-state.h
M be/src/runtime/runtime-filter-bank.cc
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/test-env.cc
M be/src/runtime/test-env.h
M be/src/runtime/thread-resource-mgr.h
M be/src/scheduling/query-schedule.cc
M be/src/scheduling/request-pool-service.cc
M be/src/scheduling/simple-scheduler.cc
M be/src/service/CMakeLists.txt
M be/src/service/fe-support.cc
D be/src/service/fragment-exec-state.h
D be/src/service/fragment-mgr.cc
D be/src/service/fragment-mgr.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
A be/src/service/impala-internal-service.cc
M be/src/service/impala-internal-service.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/query-exec-state.cc
M be/src/service/query-exec-state.h
M be/src/testutil/desc-tbl-builder.h
M be/src/udf/udf.cc
M be/src/util/container-util.h
M be/src/util/thread.cc
M be/src/util/thread.h
M be/src/util/uid-util.h
M common/thrift/ImpalaInternalService.thrift
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.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 fe/src/main/java/org/apache/impala/planner/UnionNode.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
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M fe/src/test/java/org/apache/impala/testutil/TestUtils.java
67 files changed, 1,216 insertions(+), 772 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I962ae6b7cb7dc0d07fbb8f70317aeb01d88d400b
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Marcel Kornacker 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-2925: Mark test alloc update as xfail.

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change.

Change subject: IMPALA-2925: Mark test_alloc_update as xfail.
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5366/1/tests/custom_cluster/test_alloc_fail.py
File tests/custom_cluster/test_alloc_fail.py:

Line 40: pytest.xfail("IMPALA-2925: the execution is not deterministic so 
some "
> The test still continues to execute after this, right?
Yes.  This may still catch problem if Impala crashes due to improper handling 
of failed allocations.


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

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

[Impala-ASF-CR] IMPALA-2925: Mark test alloc update as xfail.

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-2925: Mark test_alloc_update as xfail.
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5366/1/tests/custom_cluster/test_alloc_fail.py
File tests/custom_cluster/test_alloc_fail.py:

Line 40: pytest.xfail("IMPALA-2925: the execution is not deterministic so 
some "
The test still continues to execute after this, right?


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

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

[Impala-ASF-CR] Avoid std::function when possible.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: Avoid std::function when possible.
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If9dc034c2e094ea7f87f78d8d9101a71d8d2e295
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-HasComments: No

[Impala-ASF-CR] Avoid std::function when possible.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged.

Change subject: Avoid std::function when possible.
..


Avoid std::function when possible.

std::function does some tricky stuff under the hood, and can in fact
throw in its constructor when it allocates heap memory. This patch
changes ScopeExitTrigger to use templates instead, avoiding the
complexity of std::function. It uses a std::make_pair-like constructor
helper so clients don't have to try to name the types of lambdas.

Change-Id: If9dc034c2e094ea7f87f78d8d9101a71d8d2e295
Reviewed-on: http://gerrit.cloudera.org:8080/5230
Reviewed-by: Jim Apple 
Tested-by: Impala Public Jenkins
---
M be/src/service/fe-support.cc
M be/src/util/scope-exit-trigger.h
2 files changed, 22 insertions(+), 6 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Jim Apple: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: If9dc034c2e094ea7f87f78d8d9101a71d8d2e295
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 

[Impala-ASF-CR] IMPALA-4574: Do not treat UUID() like a constant expr.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-4574: Do not treat UUID() like a constant expr.
..


Patch Set 4: Code-Review+2

fix test section header

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2499f5f6ecdcb098623202c8e6dc2d02727194a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4574: Do not treat UUID() like a constant expr.

[Alex Behm (Code Review)]

Hello Internal Jenkins, Tim Armstrong,

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

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

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

Change subject: IMPALA-4574: Do not treat UUID() like a constant expr.
..

IMPALA-4574: Do not treat UUID() like a constant expr.

A recent change (IMPALA-1788) lead UUID() to be constant folded,
and therefore, produce the same value for every invocation across
rows. Similar issues might also occur due to the BE optimizing
UUID() during codegen of scalar-fn-call.h/cc.

The fix is to not treat UUID() like a constant expr in both
the FE and BE.

Discussion:
The fix in this patch is rather blunt, but minimally invasive to
reduce the risk of adding new bugs. Ideally, the constness of an
Expr should be determined in one place and the FE and BE should agree
on which Exprs are constant. I considered the following alternatives
but concluded they were too risky:
1. Pass a flag from FE to BE for ever Expr indicating its constness.
   This simple solution would populate a thrift field with the
   result of Expr.isConstant() for every Expr in an Expr tree.
   There are several issues. Calling isConstant() for every Expr
   in an Expr tree is rather expensive due to repeated traversals
   of the tree. That could be mitigated by populating an isConstant
   flag during Expr.analyze() to avoid re-computing the constness
   repeatedly. This requires changes to analyze(), clone(), reset(),
   and possibly other places for many Exprs. There is potential
   for missing a place and adding a new bug.
2. The above solution could be limited to only FunctionCallExpr.
   However, the BE expr type FUNCTION_CALL which maps to
   scalar-fn-call.h/cc is created from various FE Exprs, not just
   FunctionCallExpr. So adding a flag only to scalar-fn-call.h/cc
   would be confusing because it would only sometimes be set
   in a meaningful way. This seems more confusing than the current
   straightforward solution.

Testing: Added FE and EE tests.

Change-Id: If2499f5f6ecdcb098623202c8e6dc2d02727194a
---
M be/src/exprs/scalar-fn-call.cc
M be/src/exprs/scalar-fn-call.h
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
6 files changed, 32 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/5324/4
-- 
To view, visit http://gerrit.cloudera.org:8080/5324
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If2499f5f6ecdcb098623202c8e6dc2d02727194a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


Patch Set 3: Code-Review+2

Carry Alex's +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

[Dimitris Tsirogiannis (Code Review)]

Hello Alex Behm,

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

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

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

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..

IMPALA-4584: Make alter table operations on Kudu tables synchronous

This commit changes the behavior of alter table operations on Kudu
tables from asynchronous to synchronous. With this change, alter table
operations return when either the operations complete successfully or
a timeout is reached.

Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
---
M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
2 files changed, 36 insertions(+), 28 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 

[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5364/2/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java:

Line 296: throw new ImpalaRuntimeException(errMsg);
> add "timed out blurb"
ha forgot that. Thanks :)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5364/2/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java:

Line 296: throw new ImpalaRuntimeException(errMsg);
add "timed out blurb"


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has uploaded a new patch set (#2).

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..

IMPALA-4584: Make alter table operations on Kudu tables synchronous

This commit changes the behavior of alter table operations on Kudu
tables from asynchronous to synchronous. With this change, alter table
operations return when either the operations complete successfully or
a timeout is reached.

Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
---
M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
2 files changed, 36 insertions(+), 28 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 

[Impala-ASF-CR] IMPALA-2925: Mark test alloc update as xfail.

[Michael Ho (Code Review)]

Michael Ho has uploaded a new change for review.

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

Change subject: IMPALA-2925: Mark test_alloc_update as xfail.
..

IMPALA-2925: Mark test_alloc_update as xfail.

test_alloc_update.py is flaky and the expected failure sometimes
doesn't occur. Mark this test as xfail for now to unblock the build.

Change-Id: If4e86e7b9c064bc78b672814cd3569453ecc268d
---
M tests/custom_cluster/test_alloc_fail.py
1 file changed, 3 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If4e86e7b9c064bc78b672814cd3569453ecc268d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 

[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5364/1/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java:

Line 432:* the alter table operation is finished or until the operation 
timeout is reached.
> I think we should distinguish those cases for supportability reasons.
Good point. Modified the error message for the case of a timeout.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4574: Do not treat UUID() like a constant expr.

[Internal Jenkins (Code Review)]

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4574: Do not treat UUID() like a constant expr.
..


Patch Set 3: Verified-1

Build failed: 
http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/599/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2499f5f6ecdcb098623202c8e6dc2d02727194a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5364/1/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java:

Line 432:* the alter table operation is finished or until the operation 
timeout is reached.
> is there a way for us to distinguish failure from timeout?
If the function returns false it means it timed out.


Line 435:   public static void alterKuduTable(KuduTable tbl, AlterTableOptions 
ato, String errMsg)
> pass name and master hosts list? then you can use this for the renameTable(
Hm, not sure how this would work. For the rename you need to call the operation 
on the old name but check (call isAlterTableDone) on the new name.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4571: Push IN predicates to Kudu

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4571: Push IN predicates to Kudu
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5316/4/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:

Line 403: "Unsupported Kudu type considered for predicate: %s", 
type);
> e.getType().toSql()
Done


-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4571: Push IN predicates to Kudu

[Matthew Jacobs (Code Review)]

Hello Alex Behm,

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

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

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

Change subject: IMPALA-4571: Push IN predicates to Kudu
..

IMPALA-4571: Push IN predicates to Kudu

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 expression is of
the exact form:
IN (, , ...)

That means the InPredicate has the following properties:
1) It has a list of literal values (i.e. not a subquery);
   All values are LiteralExprs (not SlotRefs).
2) Not negative, i.e. only 'IN' supported, 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/planner/KuduScanNode.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
3 files changed, 99 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/5316/5
-- 
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: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Matthew Jacobs 

[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5364/1/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java:

Line 432:* the alter table operation is finished or until the operation 
timeout is reached.
is there a way for us to distinguish failure from timeout?


Line 435:   public static void alterKuduTable(KuduTable tbl, AlterTableOptions 
ato, String errMsg)
pass name and master hosts list? then you can use this for the renameTable() 
case as well


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
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-4578: Pick up bound predicates for Kudu scan nodes.

[Alex Behm (Code Review)]

Alex Behm has uploaded a new change for review.

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

Change subject: IMPALA-4578: Pick up bound predicates for Kudu scan nodes.
..

IMPALA-4578: Pick up bound predicates for Kudu scan nodes.

The bug was a simple oversight. In KuduScanNiode.init()
we forgot to call Analyzer.getBoundPredicates().

Change-Id: I19a38d6ea8cc0d2b0ddc3808d1f9ffef5ce306a8
---
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
2 files changed, 42 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I19a38d6ea8cc0d2b0ddc3808d1f9ffef5ce306a8
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 

[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has uploaded a new change for review.

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

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..

IMPALA-4584: Make alter table operations on Kudu tables synchronous

This commit changes the behavior of alter table operations on Kudu
tables from asynchronous to synchronous. With this change, alter table
operations return when either the operations complete successfully or
a timeout is reached.

Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
---
M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
2 files changed, 36 insertions(+), 28 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 

[Impala-ASF-CR] IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE TABLE

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE 
TABLE
..


Patch Set 4: Code-Review+2

Fix error in data loading caused by accidentally using Kudu syntax on a 
non-kudu table. Rebase and carry +2

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

[Impala-ASF-CR] IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE TABLE

[Dimitris Tsirogiannis (Code Review)]

Hello Internal Jenkins, Alex Behm,

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

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

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

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/KuduPartitionParam.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/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
35 files changed, 414 insertions(+), 413 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-4571: Push IN predicates to Kudu

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-4571: Push IN predicates to Kudu
..


Patch Set 4: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5316/4/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:

Line 403: "Unsupported Kudu type considered for predicate: %s", 
type);
e.getType().toSql()


-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4571: Push IN predicates to Kudu

[Matthew Jacobs (Code Review)]

Matthew Jacobs has uploaded a new patch set (#4).

Change subject: IMPALA-4571: Push IN predicates to Kudu
..

IMPALA-4571: Push IN predicates to Kudu

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 expression is of
the exact form:
IN (, , ...)

That means the InPredicate has the following properties:
1) It has a list of literal values (i.e. not a subquery);
   All values are LiteralExprs (not SlotRefs).
2) Not negative, i.e. only 'IN' supported, 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/planner/KuduScanNode.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
3 files changed, 99 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/5316/4
-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Matthew Jacobs 

[Impala-ASF-CR] IMPALA-4571: Push IN predicates to Kudu

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4571: Push IN predicates to Kudu
..


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/5316/3/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:

Line 370: PrimitiveType type = ref.getType().getPrimitiveType();
> not needed (use e.getType() in getKuduInListValue())
Done


Line 375:   if (!(predicate.getChild(i) instanceof LiteralExpr)) return 
false;
> predicate,getChild(i).isLiteral()
Done


Line 377:   // Get the value as an Object.
> remove this comment and blank line above, code seems self-explanatory
Done


Line 395:   private static Object getKuduInListValue(PrimitiveType type, Expr 
e) {
> pass a LiteralExpr, and make the caller case
Done


Line 396: Preconditions.checkArgument(e instanceof LiteralExpr);
> not really needed if you address above comments
Done


Line 397: switch (type) {
> use e.getType()
Done


-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4498: crash in to utc timestamp/from utc timestamp

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4498: crash in to_utc_timestamp/from_utc_timestamp
..


Patch Set 7: Code-Review+2

Carry +2

-- 
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: 7
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-4498: crash in to utc timestamp/from utc timestamp

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4498: crash in to_utc_timestamp/from_utc_timestamp
..


Patch Set 7:

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

-- 
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: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE TABLE

[Internal Jenkins (Code Review)]

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE 
TABLE
..


Patch Set 3: Verified-1

Build failed: 
http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/598/

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

[Impala-ASF-CR] IMPALA-4574: Do not treat UUID() like a constant expr.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-4574: Do not treat UUID() like a constant expr.
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2499f5f6ecdcb098623202c8e6dc2d02727194a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4574: Do not treat UUID() like a constant expr.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4574: Do not treat UUID() like a constant expr.
..


Patch Set 3:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2499f5f6ecdcb098623202c8e6dc2d02727194a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4498: crash in to utc timestamp/from utc timestamp

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4498: crash in to_utc_timestamp/from_utc_timestamp
..


Patch Set 6: Code-Review+2

No, not necessary as mentioned early. Sorry, thought Marcel had +2 this one.

-- 
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-4498: crash in to utc timestamp/from utc timestamp

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4498: crash in to_utc_timestamp/from_utc_timestamp
..


Patch Set 6:

Any more comments? I don't think there were any more to address here, unless we 
want to move code into timestamp-functions-ir as part of this patch.

-- 
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] Additional functional testing for default values on Kudu tables

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: Additional functional testing for default values on Kudu tables
..


Patch Set 2:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I25b66275d29d1cf21df14e78ab58f625a83b0725
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] Fix typo in DDL statement for loading Kudu table in stress test

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has submitted this change and it was merged.

Change subject: Fix typo in DDL statement for loading Kudu table in stress test
..


Fix typo in DDL statement for loading Kudu table in stress test

Change-Id: I198babd2b52bd64c7f0d27c1be945d2fe7f9f55c
Reviewed-on: http://gerrit.cloudera.org:8080/5158
Tested-by: Impala Public Jenkins
Reviewed-by: Dimitris Tsirogiannis 
---
M testdata/datasets/tpcds/tpcds_kudu_template.sql
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Impala Public Jenkins: Verified
  Dimitris Tsirogiannis: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I198babd2b52bd64c7f0d27c1be945d2fe7f9f55c
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] Fix typo in DDL statement for loading Kudu table in stress test

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change.

Change subject: Fix typo in DDL statement for loading Kudu table in stress test
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I198babd2b52bd64c7f0d27c1be945d2fe7f9f55c
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No

[Impala-ASF-CR] Additional functional testing for default values on Kudu tables

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: Additional functional testing for default values on Kudu tables
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I25b66275d29d1cf21df14e78ab58f625a83b0725
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3200: Implement suballocator for splitting buffers

[Jim Apple (Code Review)]

Jim Apple has posted comments on this change.

Change subject: IMPALA-3200: Implement suballocator for splitting buffers
..


Patch Set 11:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4715/4//COMMIT_MSG
Commit Message:

Line 14: larger chunks. This helps avoid external fragmentation and is quite
> I really really really really don't want "implement a general-purpose memor
I think there are a few ways to get both what I am concerned about (buddy 
allocator internal fragmentation reduction) and what you want (a simple 
allocator).

One way to is static_assert that sizeof(HashTable::Bucket) is a power of 2. I 
think this will be true in the status quo.

Another is to add a parameter to the constructor of the Suballocator that sets 
the size of an Atom. All blocks would be of size 2^i * sizeof(Atom), and the 
logic would remain the same. Setting Atom = HashTable::Bucket would reduce 
internal fragmentation.


http://gerrit.cloudera.org:8080/#/c/4715/11/be/src/bufferpool/suballocator-test.cc
File be/src/bufferpool/suballocator-test.cc:

Line 107:   static void ExpectReservationUnused(ReservationTracker& 
reservation) {
What happened to the const?


http://gerrit.cloudera.org:8080/#/c/4715/4/be/src/bufferpool/suballocator.h
File be/src/bufferpool/suballocator.h:

Line 171: /// An allocation made by a Suballocator. Each allocation returned by 
Suballocator must
> But why shouldn't we use it for a different case where it's the right tool?
Just so I'm clear, in the current system, a Suballocation may have several 
pointers to it, but the only unique_ptr is:

1. in the free_lists_ if it is free and the head of its free list

2. In the Suballocation before it if it is free but not the head of its free 
list

3. In its left child if it is not free and already split.

4. In some client code if it is not free and not split.

Additionally, at the function boundaries of the methods of Suballocator and 
Suballocation, there is always exactly one unique_ptr to each Suballocation.

Did I get those right?


-- 
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: 11
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] Additional functional testing for default values on Kudu tables

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has uploaded a new patch set (#2).

Change subject: Additional functional testing for default values on Kudu tables
..

Additional functional testing for default values on Kudu tables

This commit also fixes an issue where an error is thrown if a default
value is set for a boolean column on a Kudu table.

Change-Id: I25b66275d29d1cf21df14e78ab58f625a83b0725
---
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test
2 files changed, 74 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I25b66275d29d1cf21df14e78ab58f625a83b0725
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Matthew Jacobs 

[Impala-ASF-CR] Additional functional testing for default values on Kudu tables

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change.

Change subject: Additional functional testing for default values on Kudu tables
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5337/1/testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test
File testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test:

PS1, Line 336: a int primary key, b int null default 10,
 :   c int not null default 100, d int default 1000, e int null, f 
string default 'test',
 :   g boolean default true) distribute by hash (a) into 3 buckets 
stored as kudu
> Hm, though primary keys are frequently handled differently, it would probab
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I25b66275d29d1cf21df14e78ab58f625a83b0725
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2057: Better error message for incorrect avro decimal column declaration

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-2057: Better error message for incorrect avro decimal 
column declaration
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5255/3/fe/src/main/java/org/apache/impala/util/AvroSchemaParser.java
File fe/src/main/java/org/apache/impala/util/AvroSchemaParser.java:

Line 139: "Unsupported logicalType: " + logicalType));
Should also include something like "for column '%s' with type BYTES" so users 
can understand better what happened.


PS3, Line 180: , String logicalType
Unused argument


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad23706128223b6537d565471ef5d8faa91b0b5a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE TABLE

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE 
TABLE
..


Patch Set 3: Code-Review+2

Rebase. Carry +2

-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4574: Do not treat UUID() like a constant expr.

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4574: Do not treat UUID() like a constant expr.
..


Patch Set 2: Code-Review+2

(1 comment)

The fix seems safe and targeted for the specific problem. We can continue to 
discuss the more general UDF problem on IMPALA-4586

http://gerrit.cloudera.org:8080/#/c/5324/1/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java:

Line 246:   List path = fnName_.getFnNamePath();
> Ahh, thanks for clarifying, I did not realize that part was also a regressi
Filed IMPALA-4586


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2499f5f6ecdcb098623202c8e6dc2d02727194a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3125: Fix assignment of equality predicates from an outer-join On-clause.

[Internal Jenkins (Code Review)]

Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-3125: Fix assignment of equality predicates from an 
outer-join On-clause.
..


IMPALA-3125: Fix assignment of equality predicates from an outer-join On-clause.

Impala used to incorrectly assign On-clause equality predicates from an
outer join if those predicates referenced multiple tables, but only one
side of the outer join.

The fix is to add an additional check in Analyzer.getEqJoinConjuncts()
to prevent that incorrect assignment.

Change-Id: I719e0eeacccad070b1f9509d80aaf761b572add0
Reviewed-on: http://gerrit.cloudera.org:8080/4986
Reviewed-by: Alex Behm 
Tested-by: Internal Jenkins
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test
2 files changed, 44 insertions(+), 3 deletions(-)

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



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

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

[Impala-ASF-CR] IMPALA-3126: Conservative assignment of inner-join On-clause predicates.

[Marcel Kornacker (Code Review)]

Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-3126: Conservative assignment of inner-join On-clause 
predicates.
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4982/2/testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test
File testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test:

Line 459: |  |  other predicates: a.tinyint_col < 10, b.tinyint_col > 20
why is this correct? this only applies to matches, so a non-match wouldn't need 
to satisfy this predicate.

is there a good reason not to deal with all 'foreign' predicates (that only 
reference columns from other tables) by always evaluating them in the join 
where they originated?


Line 892: # may not be assigned below the join materializing 'd'.
are you saying it can be assigned to the join materializing d?


Line 900: inner join functional.alltypes e
are two joins/3 tables sufficient for this test case?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf45323ed9102ffb45c9d94a130ea3692286f215
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-HasComments: Yes

[Impala-ASF-CR] Avoid std::function when possible.

[Marcel Kornacker (Code Review)]

Marcel Kornacker has posted comments on this change.

Change subject: Avoid std::function when possible.
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If9dc034c2e094ea7f87f78d8d9101a71d8d2e295
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-HasComments: No