[Impala-ASF-CR] IMPALA-4477: Upgrade Kudu version to latest master

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

Change subject: IMPALA-4477: Upgrade Kudu version to latest master
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I21c5bc0d28df83cd2e57cd30b6ab416e0d430775
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4477: Upgrade Kudu version to latest master

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

Change subject: IMPALA-4477: Upgrade Kudu version to latest master
..


Patch Set 1:

Private build passed:
http://sandbox.jenkins.cloudera.com/job/impala-umbrella-build-and-test/5697/

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

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


[Impala-ASF-CR] IMPALA-4444: Transfer row group resources to row batch on scan failure

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

Change subject: IMPALA-: Transfer row group resources to row batch on scan 
failure
..


Patch Set 1:

I don't think we need to trigger the bug with exactly the same query. If we 
find a test that repros the issue frequently enough on ASAN I think we're ok. 
Sounds like it could be sufficient to just have a scan plus an exchange, or 
some other simpler variant.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id70df470e98dd96284fd176bfbb946e9637ad126
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Ho 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4433: Always generate testdata using the same time zone setting

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

Change subject: IMPALA-4433: Always generate testdata using the same time zone 
setting
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5058/1/testdata/src/main/java/org/apache/impala/datagenerator/TestDataGenerator.java
File 
testdata/src/main/java/org/apache/impala/datagenerator/TestDataGenerator.java:

Line 156: TimeZone.setDefault(TimeZone.getTimeZone("America/Los_Angeles"));
add comment why


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf7cc796e44e9ff64880f9ae852f40961592f279
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Alex Behm 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4433: Always generate testdata using the same time zone setting

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

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

Change subject: IMPALA-4433: Always generate testdata using the same time zone 
setting
..

IMPALA-4433: Always generate testdata using the same time zone setting

Before this change, testdata was generated using the
java.util.TimeZone.getDefault() TimeZone of the machine it was running
on.  This patch standardizes on "America/Los_Angeles", which matches
the existing expected results in the end-to-end tests.

Change-Id: Iaf7cc796e44e9ff64880f9ae852f40961592f279
---
M testdata/src/main/java/org/apache/impala/datagenerator/TestDataGenerator.java
1 file changed, 3 insertions(+), 0 deletions(-)


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

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


[Impala-ASF-CR] IMPALA-4470: Avoid creating a NumericLiteral from NaN/infinity.

2016-11-11 Thread Marcel Kornacker (Code Review)
Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-4470: Avoid creating a NumericLiteral from NaN/infinity.
..


Patch Set 1: Code-Review+2

(1 comment)

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

Line 38:  * TODO: BigDecimal cannot represent special float values like NaN and 
infinity.
do we really want to do this at any point in the future?


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

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


[Impala-ASF-CR] IMPALA-4452: Always call AggFnEvaluator::Open() before AggFnEvaluator::Init()

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

Change subject: IMPALA-4452: Always call AggFnEvaluator::Open() before 
AggFnEvaluator::Init()
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5049/1/be/src/exec/aggregation-node.cc
File be/src/exec/aggregation-node.cc:

Line 204: hash_tbl_->Insert(singleton_intermediate_tuple_);
> no need to set output_iterator_ anymore?
Yes as it was set unconditionally at line 236 below unless there is any error, 
in which case we won't need output_iterator_.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2f261dee47821c517d8dbe1babf4112462d85807
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Ho 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4477: Upgrade Kudu version to latest master

2016-11-11 Thread Matthew Jacobs (Code Review)
Matthew Jacobs has uploaded a new change for review.

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

Change subject: IMPALA-4477: Upgrade Kudu version to latest master
..

IMPALA-4477: Upgrade Kudu version to latest master

Change the toolchain build and Kudu version to use the
latest master, using Kudu commit 88b023.

Change-Id: I21c5bc0d28df83cd2e57cd30b6ab416e0d430775
---
M bin/impala-config.sh
1 file changed, 2 insertions(+), 2 deletions(-)


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

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


[Impala-ASF-CR] IMPALA-4466: Improve Kudu UPSERT test coverage

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

Change subject: IMPALA-4466: Improve Kudu UPSERT test coverage
..


Patch Set 9: Verified-1

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9e7afbef60186edb00a9d11fbe5a8c64931add6
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4466: Improve Kudu UPSERT test coverage

2016-11-11 Thread Thomas Tauber-Marshall (Code Review)
Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-4466: Improve Kudu UPSERT test coverage
..


Patch Set 9:

GVO manually aborted - forget trunk is supposed to be locked.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9e7afbef60186edb00a9d11fbe5a8c64931add6
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4372: 'Describe formatted' returns types in upper case

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

Change subject: IMPALA-4372: 'Describe formatted' returns types in upper case
..


Patch Set 3: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4861/2/tests/metadata/test_metadata_query_statements.py
File tests/metadata/test_metadata_query_statements.py:

PS2, Line 84: results,
> RIght, except that after calling lower() it never directly compares the two
Right, thanks for the explanation.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4163: Add sortby() query hint

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

Change subject: IMPALA-4163: Add sortby() query hint
..


Patch Set 1:

(15 comments)

Add planner and end-to-end tests

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

Line 2248:   {: RESULT = PlanHint.parsePlanHintsString(l); :}
Isn't it easier overall (testing, etc.) to allow the legacy hint style as well? 
I though that's what we settled on.


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

Line 128:   // that will be written into the columns referenced in that hint. 
The list is populated
The comment says it but maybe more explicit that the hint references columns of 
the target table (and not e.g., the column labels of the feeding select stmt)


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

Line 32:  * query statements. Plan consist of a name and an optional list of 
arguments.
A hint consists of ...


Line 34: // TODO: Should we make this class a proper ParseNode?
Don't think so, unless it's really directly produced by the parser.


Line 42:   /// TODO: This is code that parses parts of the query (the sortby 
hint). It would be
Agree. We'll need to change the lexer as well. Might be a tricky. Let's 
investigate this option together.

We could also generate a new plan hint parser, but let's first try the regex 
solution. Seems like that should be easy and readable enough.


Line 45:   throws AnalysisException {
weird to throw an AnalysisException from the parser, consider just throwing 
Exception


Line 46: ArrayList hints = Lists.newArrayList();
This code looks a little scary. Could it be simplified with a regex? I 
understand the error reporting may not be as nice as with this custom solution, 
but I think code simplicity may trump that in this case.


Line 112:   /// Check wether this hint equals to a given string, ignoring case.
typo: whether


Line 113:   public boolean is(String s) { return 
hintName_.equals(s.toLowerCase()); }
equalsIgnoreCase


http://gerrit.cloudera.org:8080/#/c/5051/1/fe/src/main/java/org/apache/impala/planner/Planner.java
File fe/src/main/java/org/apache/impala/planner/Planner.java:

Line 495:* Insert a sort node on top of the plan, depending on the 
clustered/noclustered plan
update comment


Line 510: orderingExprs.addAll(insertStmt.getSortByExprs());
nice!


http://gerrit.cloudera.org:8080/#/c/5051/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java:

Line 1723:   public void TestInsertHints() throws AnalysisException {
don't we test the clustered hint here?


Line 1777:   prefix, suffix), "Could not find SORTBY hint column foo in 
table.");
in target table

but 'foo' in single quotes


Line 1782:   "SORTBY hint column list must not contain Hdfs partition 
columns.");
mention the offending column name


Line 1786:   "SORTBY hint column list must not contain Kudu primary key 
columns.");
mention the offending column


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I37a3ffab99aaa5d5a4fd1ac674b3e8b394a3c4c0
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Alex Behm 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-3726: Add support for Kudu-specific column options

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

Change subject: IMPALA-3726: Add support for Kudu-specific column options
..


Patch Set 4:

(1 comment)

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

Line 920:   | KW_ALTER KW_TABLE table_name:table KW_DROP opt_kw_column 
IDENT:col_name
> Sorry, don't understand the problem in this case. col_name is an identifier
Pretty much anywhere, unfortunately. In this example, you cannot write:

ALTER TABLE mytable DROP default;

you have to say

ALTER TABLE mytable DROP `default`;

so "default" being a keyword leaks to all places that accept IDENT


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I727b9ae1b7b2387db752b58081398dd3f3449c02
Gerrit-PatchSet: 4
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-3552: Make incremental stats max serialized size configurable

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

Change subject: IMPALA-3552: Make incremental stats max serialized size 
configurable
..


Patch Set 11: Code-Review+2

+2 for FE. You can carry Dan's +2.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Huaisi Xu 
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-Reviewer: Yonghyun Hwang 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4372: 'Describe formatted' returns types in upper case

2016-11-11 Thread Thomas Tauber-Marshall (Code Review)
Thomas Tauber-Marshall has uploaded a new patch set (#3).

Change subject: IMPALA-4372: 'Describe formatted' returns types in upper case
..

IMPALA-4372: 'Describe formatted' returns types in upper case

A recent change caused 'describe formatted' to display the types
in all upper case, but we want 'describe formatted' to match Hive's
'describe' output, which displays the types in lower case.

This patch also fixes several problems with test_describe_formatted,
which was encountering an error but reporting success.

Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
---
M fe/src/main/java/org/apache/impala/catalog/Column.java
M testdata/workloads/functional-query/queries/QueryTest/avro-schema-changes.test
M tests/common/impala_test_suite.py
M tests/metadata/test_metadata_query_statements.py
M tests/performance/query_exec_functions.py
M tests/performance/query_executor.py
6 files changed, 49 insertions(+), 26 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Thomas Tauber-Marshall 


[Impala-ASF-CR] IMPALA-4452: Always call AggFnEvaluator::Open() before AggFnEvaluator::Init()

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

Change subject: IMPALA-4452: Always call AggFnEvaluator::Open() before 
AggFnEvaluator::Init()
..


Patch Set 1: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5049/1/be/src/exec/aggregation-node.cc
File be/src/exec/aggregation-node.cc:

Line 204: hash_tbl_->Insert(singleton_intermediate_tuple_);
no need to set output_iterator_ anymore?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2f261dee47821c517d8dbe1babf4112462d85807
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4454: test kudu.TestShowCreateTable flaky

2016-11-11 Thread Internal Jenkins (Code Review)
Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4454: test_kudu.TestShowCreateTable flaky
..


IMPALA-4454: test_kudu.TestShowCreateTable flaky

The cause of the flakiness is Kudu CREATE TABLE operations
that are sometimes taking a long time, leading to timeouts
in the hiveserver2 connection. This patch adds the ability
for tests using the 'conn' pytest fixture to specify a
timeout to connect(), and sets a timeout of 5 minutes for
this test.

Change-Id: I2727c27ff66140ac4043bcad332cd4e1d72b255f
Reviewed-on: http://gerrit.cloudera.org:8080/5040
Reviewed-by: Michael Brown 
Reviewed-by: Alex Behm 
Tested-by: Internal Jenkins
---
M tests/conftest.py
M tests/query_test/test_kudu.py
2 files changed, 14 insertions(+), 5 deletions(-)

Approvals:
  Michael Brown: Looks good to me, but someone else must approve
  Internal Jenkins: Verified
  Alex Behm: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I2727c27ff66140ac4043bcad332cd4e1d72b255f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Thomas Tauber-Marshall 


[Impala-ASF-CR] IMPALA-3726: Add support for Kudu-specific column options

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

Change subject: IMPALA-3726: Add support for Kudu-specific column options
..


Patch Set 4:

(6 comments)

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

Line 242: // List of keywords. Please keep them sorted alphabetically.
Can you add a note about the default keyword? Alternatively, if you can make 
ident_or_default work, you can add a comment there.


Line 920:   | KW_ALTER KW_TABLE table_name:table KW_DROP opt_kw_column 
IDENT:col_name
I think we may have to replace IDENT everywhere with ident_or_default, 
otherwise the uses here would need quoting for "default".


Line 1503: // TODO: Consider introducing a DEFAULT keyword
remove


Line 2027:   | KW_USE KW_DEFAULT
Have you tried simplifying these changes with a production ident_or_default?


Line 2189:   KW_SET IDENT:key EQUAL literal:l
Hmm IDENT really is everywhere. Maybe this is not such a good approach after 
all. Maybe we should give the %prec solution another try (I can help)? What do 
you think?


Line 2243:   | KW_DEFAULT DOT IDENT:tbl
Unfortunately, you also need other cases like:
IDENT DEFAULT
DEFAULT DEFAULT


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I727b9ae1b7b2387db752b58081398dd3f3449c02
Gerrit-PatchSet: 4
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-4466: Improve Kudu UPSERT test coverage

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

Change subject: IMPALA-4466: Improve Kudu UPSERT test coverage
..


Patch Set 9: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9e7afbef60186edb00a9d11fbe5a8c64931add6
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4466: Improve Kudu UPSERT test coverage

2016-11-11 Thread Thomas Tauber-Marshall (Code Review)
Hello Matthew Jacobs,

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

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

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

Change subject: IMPALA-4466: Improve Kudu UPSERT test coverage
..

IMPALA-4466: Improve Kudu UPSERT test coverage

Test results in kudu_upsert.test were verified by hand.

This patch also introduces a new test section 'DML_RESULTS', which
takes the name of a table as a comment and the contents of the
table as its body and then verifies that the body matches the
actual contents of the table. This makes it easy to check that a
DML operation has the desired effect on the contents of a table,
rather than always having to add another test case that runs a
select on the table. For now, this section cannot be used in a
test along with the RESULTS or ERRORS sections.

TODO: Refactor the DML test case handling (IMPALA-4471)

Change-Id: Ib9e7afbef60186edb00a9d11fbe5a8c64931add6
---
M testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test
A testdata/workloads/functional-query/queries/QueryTest/kudu_upsert.test
M tests/common/impala_test_suite.py
M tests/common/test_result_verifier.py
M tests/query_test/test_kudu.py
M tests/util/test_file_parser.py
6 files changed, 535 insertions(+), 67 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9e7afbef60186edb00a9d11fbe5a8c64931add6
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 


[Impala-ASF-CR] IMPALA-4431: Add audit event log control mechanism to prevent disk overflow

2016-11-11 Thread Donghui Xu (Code Review)
Donghui Xu has posted comments on this change.

Change subject: IMPALA-4431: Add audit event log control mechanism to prevent 
disk  overflow
..


Patch Set 8:

(3 comments)

Thank you for reviewing my low level format errors so many time.
I will learn to use clang-format tool.:)

http://gerrit.cloudera.org:8080/#/c/4971/7//COMMIT_MSG
Commit Message:

Line 11: event log is enabled the growing number of log files will fill up the
> nit: comma
Done


http://gerrit.cloudera.org:8080/#/c/4971/7/be/src/common/logging.cc
File be/src/common/logging.cc:

Line 17: 
> Please move this to the block of impala includes, starting in L34. Please k
Done


Line 57: mutex logging_mutex;
> Please remove this new line.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8c3229cbdb6275f969c15258c9ccab6efeb24368
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Donghui Xu 
Gerrit-Reviewer: Donghui Xu 
Gerrit-Reviewer: Lars Volker 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4431: Add audit event log control mechanism to prevent disk overflow

2016-11-11 Thread Donghui Xu (Code Review)
Donghui Xu has uploaded a new patch set (#8).

Change subject: IMPALA-4431: Add audit event log control mechanism to prevent 
disk  overflow
..

IMPALA-4431: Add audit event log control mechanism to prevent disk 
overflow

There is no limit to the number of audit event log files. When audit
event log is enabled the growing number of log files will fill up the
disk space.

This patch adds checking and rotation mechanism on audit event log
files to prevent file number out of control which causes disk overflow.

Change-Id: I8c3229cbdb6275f969c15258c9ccab6efeb24368
---
M be/src/common/init.cc
M be/src/common/logging.cc
M be/src/common/logging.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
5 files changed, 37 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8c3229cbdb6275f969c15258c9ccab6efeb24368
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Donghui Xu 
Gerrit-Reviewer: Donghui Xu 
Gerrit-Reviewer: Lars Volker 


[Impala-ASF-CR] IMPALA-4444: Transfer row group resources to row batch on scan failure

2016-11-11 Thread Michael Ho (Code Review)
Michael Ho has uploaded a new change for review.

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

Change subject: IMPALA-: Transfer row group resources to row batch on scan 
failure
..

IMPALA-: Transfer row group resources to row batch on scan failure

Previously, if any column reader fails in HdfsParqetScanner::AssembleRows(),
the memory pools associated with the ScratchTupleBatch will be freed. This
is problematic as ScratchTupleBatch may contain memory pools which are still
referenced by row batches shipped upstream. This is possible because memory
pools used by parquet column readers (e.g. decompressor_pool_) won't be
transferred to a ScratchTupleBatch until the data page is exhausted. So,
the memory pools of the previous data page is always attached to the
ScratchTupleBatch of the current data page. On a scan failure, it's not
necessarily safe to free the memory pool attached to the current 
ScratchTupleBatch.

This patch fixes the problem above by transferring the memory pool and other
resources associated with a row group to the current row batch in the parquet
scanner on scan failure so it can eventually be freed by upstream operators as
the row batch is consumed.

Change-Id: Id70df470e98dd96284fd176bfbb946e9637ad126
---
M be/src/exec/exec-node.cc
M be/src/exec/hdfs-parquet-scanner.cc
2 files changed, 3 insertions(+), 2 deletions(-)


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

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


[Impala-ASF-CR] IMPALA-4431: Add audit event log control mechanism to prevent disk overflow.

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

Change subject: IMPALA-4431: Add audit event log control mechanism to prevent 
disk  overflow.
..


Patch Set 7:

(3 comments)

Only minor formatting issues left. Please see here for a description on how to 
use clang-format: 
https://cwiki.apache.org/confluence/display/IMPALA/Contributing+to+Impala

It's a good tool to become familiar with. :)

http://gerrit.cloudera.org:8080/#/c/4971/7//COMMIT_MSG
Commit Message:

Line 11: event log is enabled,the growing number of log files will fill up the 
nit: comma


http://gerrit.cloudera.org:8080/#/c/4971/7/be/src/common/logging.cc
File be/src/common/logging.cc:

Line 17: #include "service/impala-server.h"
Please move this to the block of impala includes, starting in L34. Please keep 
that block lexicographically sorted.


Line 57: 
Please remove this new line.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8c3229cbdb6275f969c15258c9ccab6efeb24368
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Donghui Xu 
Gerrit-Reviewer: Donghui Xu 
Gerrit-Reviewer: Lars Volker 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4163: Add sortby() query hint

2016-11-11 Thread Lars Volker (Code Review)
Lars Volker has uploaded a new change for review.

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

Change subject: IMPALA-4163: Add sortby() query hint
..

IMPALA-4163: Add sortby() query hint

This change introduces the sortby() query plan hint for insert
statements. When specified, sortby(a, b) will add an additional sort
step to the plan to order data by columns a, b before inserting it into
the target table.

Change-Id: I37a3ffab99aaa5d5a4fd1ac674b3e8b394a3c4c0
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
A fe/src/main/java/org/apache/impala/analysis/PlanHint.java
M fe/src/main/java/org/apache/impala/analysis/SelectList.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
9 files changed, 332 insertions(+), 79 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I37a3ffab99aaa5d5a4fd1ac674b3e8b394a3c4c0
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker