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

2016-12-04 Thread 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 1:

(1 comment)

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: if (fnName.equalsIgnoreCase("rand") || 
fnName.equalsIgnoreCase("random")
> There is a regression in the 2.8.0 release though because I added more cach
Ahh, thanks for clarifying, I did not realize that part was also a regression.

Would be great if you could create a separate JIRA. Let's sync on the possible 
fixes.


-- 
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: 1
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-3126: Conservative assignment of inner-join On-clause predicates.

2016-12-04 Thread Alex Behm (Code Review) 
Alex Behm has uploaded a new patch set (#2).

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

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

Implements the following conservative but correct policy for assigning
predicates from the On-clause of an inner join:
If the predicate references an outer-joined tuple from a left or right
outer join, then evaluate it at the inner join that the On-clause
belongs to. Note that tuples from full outer joins are treated specially
because the rules for assining predicates referencing them are very
different from those for left-/right-outer joins.

Cleans up Analyzer.canEvalPredicate().

Change-Id: Idf45323ed9102ffb45c9d94a130ea3692286f215
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test
2 files changed, 143 insertions(+), 61 deletions(-)


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

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

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

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

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


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4982/1/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
> i'm not following: this predicate is from the inner join on clause and refe
Good point, clarified in commit msg. Full outer joins are treated specially 
because their assignment rules are different from left/right outer joins. The 
fix here only applies to left/right outer joins.

The reason why this changed is that I reworked the logic of 
Analyzer.canEvalPredicate() to check for full outer joins first.


-- 
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: 1
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] IMPALA-3125: Fix assignment of equality predicates from an outer-join On-clause.

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

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


Patch Set 3: Code-Review+2

rebase, fix expected plan which was a copy+paste mistake

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I719e0eeacccad070b1f9509d80aaf761b572add0
Gerrit-PatchSet: 3
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-3125: Fix assignment of equality predicates from an outer-join On-clause.

2016-12-04 Thread Alex Behm (Code Review) 
Hello Marcel Kornacker, Internal Jenkins,

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

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

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

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


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I719e0eeacccad070b1f9509d80aaf761b572add0
Gerrit-PatchSet: 3
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-2057: Better error message for incorrect avro decimal column declaration

2016-12-04 Thread anujphadke (Code Review) 
anujphadke has posted comments on this change.

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


Patch Set 2:

(3 comments)

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

Line 168:* Attempts to parse decimal type information from the Avro schema, 
returning
> This comment is now out-of-sync with the method.
Done


Line 178: if (logicalType.equalsIgnoreCase("decimal")) {
> I think this should be checked before calling getDecimalType(). It makes mo
Done


Line 193:   "Unsupported logicalType");
> Should include the type name in the exception method so that users can unde
Previous comment -
  3. getDecimalType() should now either return a valid type or throw an 
exception.

Done.
Had to move the exception in an else statement 
since 
if (logicalType.equalsIgnoreCase("decimal")) 
this check was moved outside.


-- 
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: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

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

2016-12-04 Thread anujphadke (Code Review) 
anujphadke has uploaded a new patch set (#3).

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

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

Adding a better error message when logical type is specified at a wrong
level or is not not specified in an avro decimal column declaration.

Change-Id: Iad23706128223b6537d565471ef5d8faa91b0b5a
---
M fe/src/main/java/org/apache/impala/util/AvroSchemaParser.java
1 file changed, 24 insertions(+), 19 deletions(-)


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

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

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

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

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


Patch Set 2: Verified-1

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I719e0eeacccad070b1f9509d80aaf761b572add0
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-4574: Do not treat UUID() like a constant expr.

2016-12-04 Thread 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 1:

(1 comment)

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: if (fnName.equalsIgnoreCase("rand") || 
fnName.equalsIgnoreCase("random")
> Yes, I see your point, but that seems to be an existing bug, so does not ha
There is a regression in the 2.8.0 release though because I added more caching 
of constant values in the backend in this commit:

  commit 10fa472fa6aa036be02748ae54daed1722449c68
  Author: Tim Armstrong 
  Date:   Wed Oct 26 10:55:23 2016 -0700

  IMPALA-4302,IMPALA-2379: constant expr arg fixes

I'll create a JIRA for that since we're not going to fix it as part of this 
patch, right?


-- 
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: 1
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-4303: Do not reset() qualifier of union operands.

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

Change subject: IMPALA-4303: Do not reset() qualifier of union operands.
..


IMPALA-4303: Do not reset() qualifier of union operands.

The bug: We used to reset() the qualifier of union operands
to their original value obtained during parsing. This leads to
problems when union operands are unnested and we need to rewrite
Subqueries. In particular, the first union operand of a nested union
was reset() to a null qualifier, but that operand could be somewhere
in the middle of the list of unnested operands in the parent. At that
point, we've lost information about the qualifier of the unnested
operand.

The fix: The simplest solution is to not reset() the qualifier.
The other alternative is be to reset() the qualifier, but also
undo any unnesting. That seems unnecessary and wasteful.

Change-Id: I157bb0f08c4a94fd779487d7c23edd64a537a1f6
Reviewed-on: http://gerrit.cloudera.org:8080/4963
Reviewed-by: Alex Behm 
Tested-by: Internal Jenkins
---
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
2 files changed, 74 insertions(+), 12 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I157bb0f08c4a94fd779487d7c23edd64a537a1f6
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-3125: Fix assignment of equality predicates from an outer-join On-clause.

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

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


Patch Set 2: Code-Review+2

rebase, carry +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I719e0eeacccad070b1f9509d80aaf761b572add0
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-3125: Fix assignment of equality predicates from an outer-join On-clause.

2016-12-04 Thread Alex Behm (Code Review) 
Hello Marcel Kornacker,

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

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

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

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
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test
2 files changed, 58 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I719e0eeacccad070b1f9509d80aaf761b572add0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Marcel Kornacker

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

2016-12-04 Thread 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 1:

(1 comment)

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: if (fnName.equalsIgnoreCase("rand") || 
fnName.equalsIgnoreCase("random")
> That doesn't avoid caching of constant expression values in the backend tho
Yes, I see your point, but that seems to be an existing bug, so does not have 
the same urgency as fixing the uuid() regression specifically.

It's definitely good to fix as follow-on work.


-- 
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: 1
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-4579: SHOW CREATE VIEW fails for view containing a subquery

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

Change subject: IMPALA-4579: SHOW CREATE VIEW fails for view containing a 
subquery
..


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4a89e46a022f0ccec198b6e3e2b30230103831ce
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-4579: SHOW CREATE VIEW fails for view containing a subquery

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

Change subject: IMPALA-4579: SHOW CREATE VIEW fails for view containing a 
subquery
..


IMPALA-4579: SHOW CREATE VIEW fails for view containing a subquery

This commit fixes an issue where a SHOW CREATE VIEW statement throws an
analysis error if the view contains a subquery.

Change-Id: I4a89e46a022f0ccec198b6e3e2b30230103831ce
Reviewed-on: http://gerrit.cloudera.org:8080/5333
Reviewed-by: Dimitris Tsirogiannis 
Tested-by: Internal Jenkins
---
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
2 files changed, 10 insertions(+), 1 deletion(-)

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



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

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