You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Alice Fan (Code Review)" <ge...@cloudera.org> on 2019/04/25 23:59:58 UTC

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

Alice Fan has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13050


Change subject: IMPALA-966: Type errors are attributed to wrong expression with insert
......................................................................

IMPALA-966: Type errors are attributed to wrong expression with insert

When multiple incompatible values insert into a table, error should blame associated expression.

Testing:
- Added a test to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
6 files changed, 55 insertions(+), 17 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 2
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Alice Fan (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Paul Rogers, xiaomeng@cloudera.com, Bikramjeet Vig, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................

IMPALA-966: Attribute type error to the right expression in an insert
statement

Currently, if an insert statement contains multiple expressions
that are incompatible with the column type, the error message
returned attributes the error to the wrong expression.
This patch makes sure the right expression is blamed. If there are
multiple incompatible type values for the target column, then the
error is attributed to the first widest (highest precision)
incompatible type expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
7 files changed, 86 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/13050/11
-- 
To view, visit http://gerrit.cloudera.org:8080/13050
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 11
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Type errors are attributed to wrong expression with insert
......................................................................


Patch Set 3:

(12 comments)

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

http://gerrit.cloudera.org:8080/#/c/13050/1//COMMIT_MSG@7
PS1, Line 7: Type errors are attributed to wrong expression with insert
nit: Fix type errors being attributed to wrong expressions


http://gerrit.cloudera.org:8080/#/c/13050/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13050/3//COMMIT_MSG@9
PS3, Line 9: insert a union statement with incompatible types
can you clarify what this mean "insert a union"?


http://gerrit.cloudera.org:8080/#/c/13050/3//COMMIT_MSG@10
PS3, Line 10: make error message blame on correct expression. If there are multiple
multiple expressions with an incompatible type


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

http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2326
PS3, Line 2326: return a list of widestExprs
return a list of the first widest compatible expression for each column


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2326
PS3, Line 2326: log 
where are we logging this?


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2340
PS3, Line 2340: // Remember widest compatible Expr for error reporting.
nit: dont think this context is necessary here


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2347
PS3, Line 2347: //The compatibleType will be the wider type of previous and next Exprs
nit: spavce between // and The
nit: maybe write something like: compatibleType only changes if a new wider type is encountered


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
File fe/src/main/java/org/apache/impala/analysis/QueryStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@53
PS3, Line 53:   // When a union statement runs castToUnionCompatibleTypes(),
            :   // a list of widest type expression for each column will be returned.
            :   // widestExpr is the expression with highest precision data type.
            :   // The widestExprs_ is stored as a member of QueryStmt
            :   // is because it is used when an insert statement prepares expression
can you rephrase this to be more concise. Please look at other comments on variable to get an idea on what kind of info should be mentioned.


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@58
PS3, Line 58: widestExprs_
Like bharath mentioned, you can add this to UnionStmt instead and in InsertStmt, just check if its on UnionStmt, cast it and use an assessor method or something to get the required widest expression


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/StatementBase.java
File fe/src/main/java/org/apache/impala/analysis/StatementBase.java:

http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@196
PS3, Line 196:    * QueryStmt's widestExprs_ holds list of source expression with widest (highest
             :    * precision) type. Error message will blame the source expression with widest
             :    * incompatible type.
looks like an outdated comment, can up update it based on the changes in your latest patchset


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@232
PS3, Line 232:   protected Expr checkTypeCompatibility(String dstTableName, Column dstCol,
             :       Expr srcExpr, boolean strictDecimal) throws AnalysisException {
             :     return checkTypeCompatibility(dstTableName, dstCol, srcExpr, strictDecimal, srcExpr);
             :   }
since checkTypeCompatibility is only used in 2 places, there is no need to have another method just to have a default param. Also instead of passing srcExprm, you can just pass null since you are already handling that case in L209


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

http://gerrit.cloudera.org:8080/#/c/13050/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3889
PS1, Line 3889:           "Column permutation mentions fewer columns (9) than the SELECT /" +
              :           " VALUES clause returns (11)");
              :       // Omitting the row-key column is an error
              :       AnalysisError("insert " + qualifier + " table functional_hbase.alltypesagg" +
              :           "(bool_col, tinyint_col, smallint_col, int_col, bigin
did you change the clone method before incrementing this?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 3
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 01 May 2019 23:15:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 7:

(4 comments)

Looks good just a few more nits

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

http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2332
PS6, Line 2332: exprLists.size() == 
> Thanks for catching this!
yes it will be but in that case you will have to change the description of this function to not return anything if exprLists.size() == 1. I think its more consistent if we return something for a non empty exprLists


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

http://gerrit.cloudera.org:8080/#/c/13050/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2326
PS7, Line 2326: Return a list of the first widest compatible expression for each column in an
              :    * order consistent with the order of target columns.
sorry for changing the suggestions for the comments again and again, the terminology is kinda tricky.
I think the following should be sufficient and generic enough to make sense in the context of this method's input params:

Returns a list of exprs such that for every i-th expr in that list, it is the first widest compatible expression encountered among all i-th exprs in the expr lists.


http://gerrit.cloudera.org:8080/#/c/13050/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2332
PS7, Line 2332:     if (exprLists == null || exprLists.size() == 0) return null;
I think having an if statement for exprLists.size() == 1 wont hurt. Just return exprLists.get(0) in that case.


http://gerrit.cloudera.org:8080/#/c/13050/7/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
File fe/src/main/java/org/apache/impala/analysis/UnionStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/7/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java@161
PS7, Line 161:   // this contains a list of the first widest (highest precision) compatible
             :   // expression for each column.
Contains a list of exprs such that for every i-th expr in that list, it is the first widest compatible expression encountered among all i-th exprs in every result expr list of the union operands.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 7
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 May 2019 01:19:51 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 9:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3165/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 9
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 10 May 2019 01:44:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Type errors are attributed to wrong expression with insert
......................................................................


Patch Set 5:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/13050/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13050/5//COMMIT_MSG@7
PS5, Line 7: IMPALA-966: Type errors are attributed to wrong expression with insert
           : 
           : When insert multiple incompatible type values into a table,
           : error message should blame on the correct expression. If there
           : are multiple incompatible type values for a single target
           : column, error should blame on the first widest incompatible type
           : expression.
how about :
IMPALA-966: Attribute type errors to the right expression in an insert
statement

Currently if an insert statement contains multiple expressions that are incompatible with the column type, the error message returned attributes the error to the wrong expression. This patch makes sure the right expression is blamed. If there are multiple incompatible type values for the target column, then the error is attributed to the first widest (highest precision) incompatible type expression.


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

http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@692
PS5, Line 692:       // If the queryStmt_ is a unionStmt, it will return a WidestExprs list
             :       // when do castToUnionCompatibleTypes().
             :       // widestTypeExpr will be null if the queryStmt_ is a SelectStmt
nit: superfluous comment


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@695
PS5, Line 695:       UnionStmt unionStmt =
             :           (queryStmt_ instanceof UnionStmt) ? (UnionStmt) queryStmt_ : null;
             :       if (unionStmt != null && unionStmt.getWidestExprs() != null
             :           && unionStmt.getWidestExprs().size() > 0) {
             :         widestTypeExpr = unionStmt.getWidestExprs().get(i);
             :       }
nit: instead of doing this in every loop maybe just get the widestExprList before the loop and use it if not null


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
File fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@292
PS5, Line 292: null
nit: remove the comment above and add an inline comment here like 
.., analyzer.isDecimalV2(), null /*widestTypeSrcExpr*/);


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/StatementBase.java
File fe/src/main/java/org/apache/impala/analysis/StatementBase.java:

http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@196
PS5, Line 196: widestTypeSrcExpr
nit: add quotes since this refers to an input param


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@196
PS5, Line 196: for
nit: among


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@197
PS5, Line 197: Error message should blame on the widestTypeSrcExpr instead of the first
             :    * compatible source expression.
nit: is only used when constructing an AnalysisException message to make sure the right expression is blamed in the error message


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
File fe/src/main/java/org/apache/impala/analysis/UnionStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java@56
PS5, Line 56:   // widestExprs_ is a list of the first widest compatible expression for each column
nit: you can remove the first line  and write "widest (highest precision)" here.
Also, can you mention what order they are stored in and add a full stop at the end


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

http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3404
PS5, Line 3404: on
nit: the


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3404
PS5, Line 3404:  // Error should blame on correct expression.
              :     // The widest (highest precision) expression and type should appear in error.
nit: these two are a bit repetitive. can you consolidate both into one?


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3422
PS5, Line 3422:   }
can you add a case where there are multiple expressions with the same incompatible type and the first one gets blamed.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 5
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 May 2019 00:24:10 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 8: Code-Review+2

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/13050/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2327
PS8, Line 2327:    * widest compatible expression encountered among all i-th exprs in the expr list.
nit: lists


http://gerrit.cloudera.org:8080/#/c/13050/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2328
PS8, Line 2328:    * Throw an AnalysisException if the types are incompatible.
Returns null if an empty expression list or null is passed to it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 8
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 May 2019 23:42:21 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Alice Fan (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Paul Rogers, xiaomeng@cloudera.com, Bikramjeet Vig, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................

IMPALA-966: Attribute type error to the right expression in an insert
statement

Currently, if an insert statement contains multiple expressions
that are incompatible with the column type, the error message
returned attributes the error to the wrong expression.
This patch makes sure the right expression is blamed. If there are
multiple incompatible type values for the target column, then the
error is attributed to the first widest (highest precision)
incompatible type expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
6 files changed, 82 insertions(+), 22 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 7
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Type errors are attributed to wrong expression with insert
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3081/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 5
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Sun, 05 May 2019 03:39:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Alice Fan (Code Review)" <ge...@cloudera.org>.
Alice Fan has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 11:

Checked failed tests in verified build.
Fixed the relevant test 'AnalyzeExprsTest.TestDecimalCasts' by converting expression of error message to all upper case.

'metadata.test_compute_stats.TestComputeStats.test_compute_stats_compression_codec' and 'metadata.test_refresh_partition.TestRefreshPartition.test_refresh_partition_num_rows' are irrelevant to the change. Tested the patch 11 at private-parameterized build and these two tests are both passed.

Link to patch 11's private-parameterized build: https://master-02.jenkins.cloudera.com/job/impala-private-parameterized/5035/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 11
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 14 May 2019 17:54:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3137/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 7
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 May 2019 23:30:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Alice Fan (Code Review)" <ge...@cloudera.org>.
Alice Fan has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 9:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/13050/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2327
PS8, Line 2327:    * widest compatible expression encountered among all i-th exprs in the expr lists.
> nit: lists
among all i-th exprs in the expr list(S).


http://gerrit.cloudera.org:8080/#/c/13050/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2328
PS8, Line 2328:    * Returns null if an empty expression list or null is passed to it.
> Returns null if an empty expression list or null is passed to it.
Thanks. Added it



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 9
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 10 May 2019 00:05:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 10: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4242/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 10
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 14 May 2019 02:13:09 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 10:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4242/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 10
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Mon, 13 May 2019 20:57:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 8:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/3147/ : Initial code review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 8
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 May 2019 07:55:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 12: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 12
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 14 May 2019 20:45:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Alice Fan (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Paul Rogers, xiaomeng@cloudera.com, Bikramjeet Vig, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................

IMPALA-966: Attribute type error to the right expression in an insert
statement

Currently, if an insert statement contains multiple expressions
that are incompatible with the column type, the error message
returned attributes the error to the wrong expression.
This patch makes sure the right expression is blamed. If there are
multiple incompatible type values for the target column, then the
error is attributed to the first widest (highest precision)
incompatible type expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
6 files changed, 84 insertions(+), 22 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 8
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Type errors are attributed to wrong expression with insert
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2925/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 2
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2019 00:47:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 10: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 10
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Mon, 13 May 2019 20:57:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

Posted by "Alice Fan (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Paul Rogers, xiaomeng@cloudera.com, Bikramjeet Vig, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-966: Type errors are attributed to wrong expression with insert
......................................................................

IMPALA-966: Type errors are attributed to wrong expression with insert

When insert multiple incompatible type values into a table,
error message should blame on the correct expression. If there
are multiple incompatible type values for a single target
column, error should blame on the first widest incompatible type
expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
6 files changed, 73 insertions(+), 23 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 4
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

Posted by "Alice Fan (Code Review)" <ge...@cloudera.org>.
Alice Fan has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Type errors are attributed to wrong expression with insert
......................................................................


Patch Set 4:

(14 comments)

http://gerrit.cloudera.org:8080/#/c/13050/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13050/2//COMMIT_MSG@9
PS2, Line 9: 
> line wraps. Please don't exceed the col limit
Done


http://gerrit.cloudera.org:8080/#/c/13050/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13050/3//COMMIT_MSG@9
PS3, Line 9: insert multiple incompatible type values into a 
> can you clarify what this mean "insert a union"?
Done


http://gerrit.cloudera.org:8080/#/c/13050/3//COMMIT_MSG@10
PS3, Line 10: error message should blame on the correct expression. If there
> multiple expressions with an incompatible type
Done


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

http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2326
PS3, Line 2326: expression for each column i
> return a list of the first widest compatible expression for each column
Done


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2326
PS3, Line 2326: Retu
> where are we logging this?
updated comments. thanks!


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2340
PS3, Line 2340: Type compatibleType = firstList.get(i).getType();
> nit: dont think this context is necessary here
Done


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2347
PS3, Line 2347: // compatibleType will be updated if a new wider type is encountered
> nit: spavce between // and The
Done


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

http://gerrit.cloudera.org:8080/#/c/13050/2/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@690
PS2, Line 690:       // widestTypeExpr is widest type expression for column i
> +1. Can you simplify this?
Done


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

http://gerrit.cloudera.org:8080/#/c/13050/1/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@53
PS1, Line 53: 
> Any reason to put this here instead of in UnionStmt?
Done


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
File fe/src/main/java/org/apache/impala/analysis/QueryStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@53
PS3, Line 53: 
            :   protected List<OrderByElement> orderByElements_;
            :   protected LimitElement limitElement_;
            : 
            :   // For a select statment:
> can you rephrase this to be more concise. Please look at other comments on 
Thanks. updated comments


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@58
PS3, Line 58: xprs in sele
> Like bharath mentioned, you can add this to UnionStmt instead and in Insert
Done


http://gerrit.cloudera.org:8080/#/c/13050/2/fe/src/main/java/org/apache/impala/analysis/StatementBase.java
File fe/src/main/java/org/apache/impala/analysis/StatementBase.java:

http://gerrit.cloudera.org:8080/#/c/13050/2/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@195
PS2, Line 195:    *
> Best to remove trailing spaces. Eclipse can do this for you. I think it is 
Thanks Paul. it is handy!


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/StatementBase.java
File fe/src/main/java/org/apache/impala/analysis/StatementBase.java:

http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@196
PS3, Line 196:    * widestTypeSrcExpr is the widest type expression for all the source values.
             :    * Error message should blame on the widestTypeSrcExpr instead of the first
             :    * compatible source 
> looks like an outdated comment, can up update it based on the changes in yo
Done


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@232
PS3, Line 232: 
             : 
             : 
             : 
> since checkTypeCompatibility is only used in 2 places, there is no need to 
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 4
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 May 2019 05:33:17 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3096/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 6
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 May 2019 07:48:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 12:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4255/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 12
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 14 May 2019 20:45:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................

IMPALA-966: Attribute type error to the right expression in an insert
statement

Currently, if an insert statement contains multiple expressions
that are incompatible with the column type, the error message
returned attributes the error to the wrong expression.
This patch makes sure the right expression is blamed. If there are
multiple incompatible type values for the target column, then the
error is attributed to the first widest (highest precision)
incompatible type expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Reviewed-on: http://gerrit.cloudera.org:8080/13050
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
7 files changed, 86 insertions(+), 23 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 13
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Type errors are attributed to wrong expression with insert
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3052/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 4
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 May 2019 03:39:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 11:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3217/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 11
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 14 May 2019 16:58:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Alice Fan (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Paul Rogers, xiaomeng@cloudera.com, Bikramjeet Vig, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................

IMPALA-966: Attribute type error to the right expression in an insert
statement

Currently, if an insert statement contains multiple expressions
that are incompatible with the column type, the error message
returned attributes the error to the wrong expression.
This patch makes sure the right expression is blamed. If there are
multiple incompatible type values for the target column, then the
error is attributed to the first widest (highest precision)
incompatible type expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
6 files changed, 78 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/13050/6
-- 
To view, visit http://gerrit.cloudera.org:8080/13050
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 6
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

Posted by "Alice Fan (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Paul Rogers, xiaomeng@cloudera.com, Bikramjeet Vig, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-966: Type errors are attributed to wrong expression with insert
......................................................................

IMPALA-966: Type errors are attributed to wrong expression with insert

When insert a union statement with incompatible types, we should
make error message blame on correct expression. If there are multiple
incompatible types for a single column, error message should print the
widest data type expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
6 files changed, 64 insertions(+), 14 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 3
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 11: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 11
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 14 May 2019 20:45:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 9: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 9
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Mon, 13 May 2019 20:57:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 12: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 12
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 May 2019 02:03:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Alice Fan (Code Review)" <ge...@cloudera.org>.
Alice Fan has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 5:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/13050/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13050/5//COMMIT_MSG@7
PS5, Line 7: IMPALA-966: Type errors are attributed to wrong expression with insert
           : 
           : When insert multiple incompatible type values into a table,
           : error message should blame on the correct expression. If there
           : are multiple incompatible type values for a single target
           : column, error should blame on the first widest incompatible type
           : expression.
> how about :
Nice example. Much appreciate for the help!


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

http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@692
PS5, Line 692:       // If the queryStmt_ is a unionStmt, it will return a WidestExprs list
             :       // when do castToUnionCompatibleTypes().
             :       // widestTypeExpr will be null if the queryStmt_ is a SelectStmt
> nit: superfluous comment
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@695
PS5, Line 695:       UnionStmt unionStmt =
             :           (queryStmt_ instanceof UnionStmt) ? (UnionStmt) queryStmt_ : null;
             :       if (unionStmt != null && unionStmt.getWidestExprs() != null
             :           && unionStmt.getWidestExprs().size() > 0) {
             :         widestTypeExpr = unionStmt.getWidestExprs().get(i);
             :       }
> nit: instead of doing this in every loop maybe just get the widestExprList 
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
File fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@292
PS5, Line 292: null
> nit: remove the comment above and add an inline comment here like 
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/StatementBase.java
File fe/src/main/java/org/apache/impala/analysis/StatementBase.java:

http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@196
PS5, Line 196: for
> nit: among
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@196
PS5, Line 196: widestTypeSrcExpr
> nit: add quotes since this refers to an input param
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@197
PS5, Line 197: Error message should blame on the widestTypeSrcExpr instead of the first
             :    * compatible source expression.
> nit: is only used when constructing an AnalysisException message to make su
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
File fe/src/main/java/org/apache/impala/analysis/UnionStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java@56
PS5, Line 56:   // widestExprs_ is a list of the first widest compatible expression for each column
> nit: you can remove the first line  and write "widest (highest precision)" 
Done


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

http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3404
PS5, Line 3404: on
> nit: the
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3404
PS5, Line 3404:  // Error should blame on correct expression.
              :     // The widest (highest precision) expression and type should appear in error.
> nit: these two are a bit repetitive. can you consolidate both into one?
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3422
PS5, Line 3422:   }
> can you add a case where there are multiple expressions with the same incom
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 5
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 May 2019 06:52:32 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Alice Fan (Code Review)" <ge...@cloudera.org>.
Alice Fan has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 6:

(6 comments)

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

http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2332
PS6, Line 2332: exprLists.size() < 2
> looks like if exprLists.size() =1 then it should return exprLists.get(0)
Thanks for catching this!
Originally, I thought the exprLists.size() == 1 case will be handle by statementBase.checkTypeCompatibility(). widestExpr == null, basically will be replaced by srcExpr, which is the exprLists.get(0).


http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2348
PS6, Line 2348:         if (preType != compatibleType) {
              :           widestExprs.set(i, exprLists.get(j).get(i));
              :         }
> nit: might fit in one line
Done


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

http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@699
PS6, Line 699:       Expr widestTypeExpr = null;
             :       if (widestTypeExprList != null) {
             :         widestTypeExpr = widestTypeExprList.get(i);
             :       }
> nit: can be one line
Done


http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
File fe/src/main/java/org/apache/impala/analysis/UnionStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java@55
PS6, Line 55:   // Widest (highest precision) expression is a list of the first widest compatible
            :   // expression for each column. The list is stored in the order of target columns.
            :   protected List<Expr> widestExprs_ = new ArrayList<>();
> this needs to go after L133 ( in the list of members that need to be reset(
Moved into the list of "members that need to be reset()". 

Remove the part for stored in order, because there are other list members doesn't not explain the order. Also, the order is explained at comment area of analyzer.castToUnionCompatibleTypes() when the widestExprs is returned.

reset() method has been updated at previous patch. which is at line648. also update clone at line 199


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

http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3404
PS6, Line 3404: on
> nit: typo: blame the widest
Updated comments.


http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3407
PS6, Line 3407:  
> can you add a one line comment for each test case as to what it is testing.
added short comment for each of test.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 6
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 May 2019 22:18:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Alice Fan (Code Review)" <ge...@cloudera.org>.
Alice Fan has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 8:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/13050/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2326
PS7, Line 2326: Returns a list of exprs such that for every i-th expr in that list, it is the first
              :    * widest compatible expression encountered among all
> sorry for changing the suggestions for the comments again and again, the te
Sure. Thank you!


http://gerrit.cloudera.org:8080/#/c/13050/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2332
PS7, Line 2332:     if (exprLists == null || exprLists.size() == 0) return null;
> I think having an if statement for exprLists.size() == 1 wont hurt. Just re
Done


http://gerrit.cloudera.org:8080/#/c/13050/7/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
File fe/src/main/java/org/apache/impala/analysis/UnionStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/7/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java@161
PS7, Line 161:   // Contains a list of exprs such that for every i-th expr in that list, it is the first
             :   // widest compatible expressio
> Contains a list of exprs such that for every i-th expr in that list, it is 
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 8
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 May 2019 05:51:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Alice Fan (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Paul Rogers, xiaomeng@cloudera.com, Bikramjeet Vig, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................

IMPALA-966: Attribute type error to the right expression in an insert
statement

Currently, if an insert statement contains multiple expressions
that are incompatible with the column type, the error message
returned attributes the error to the wrong expression.
This patch makes sure the right expression is blamed. If there are
multiple incompatible type values for the target column, then the
error is attributed to the first widest (highest precision)
incompatible type expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
6 files changed, 85 insertions(+), 22 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 9
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 8:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3156/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 8
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 May 2019 22:42:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an insert statement
......................................................................


Patch Set 6:

(6 comments)

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

http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2332
PS6, Line 2332: exprLists.size() < 2
looks like if exprLists.size() =1 then it should return exprLists.get(0)


http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2348
PS6, Line 2348:         if (preType != compatibleType) {
              :           widestExprs.set(i, exprLists.get(j).get(i));
              :         }
nit: might fit in one line
if (preType != compatibleType) widestExprs.set(i, exprLists.get(j).get(i));


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

http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@699
PS6, Line 699:       Expr widestTypeExpr = null;
             :       if (widestTypeExprList != null) {
             :         widestTypeExpr = widestTypeExprList.get(i);
             :       }
nit: can be one line


http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
File fe/src/main/java/org/apache/impala/analysis/UnionStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java@55
PS6, Line 55:   // Widest (highest precision) expression is a list of the first widest compatible
            :   // expression for each column. The list is stored in the order of target columns.
            :   protected List<Expr> widestExprs_ = new ArrayList<>();
this needs to go after L133 ( in the list of members that need to be reset()). Also, can you update the comment with similar information as given in the members in that list. And update the reset() method too with the new addition


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

http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3404
PS6, Line 3404: on
nit: typo: blame the widest


http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3407
PS6, Line 3407:  
can you add a one line comment for each test case as to what it is testing. See the test cases in testInsertDynamic



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 6
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 May 2019 20:22:23 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Type errors are attributed to wrong expression with insert
......................................................................


Patch Set 2:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/13050/2/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@690
PS2, Line 690:       if (queryStmt_.widestExprs_ == null || queryStmt_.widestExprs_.size() != selectListExprs.size()) {
line too long (104 > 90)


http://gerrit.cloudera.org:8080/#/c/13050/2/fe/src/main/java/org/apache/impala/analysis/StatementBase.java
File fe/src/main/java/org/apache/impala/analysis/StatementBase.java:

http://gerrit.cloudera.org:8080/#/c/13050/2/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@195
PS2, Line 195:    * 
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/13050/2/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@204
PS2, Line 204:       Expr srcExpr, boolean strictDecimal, Expr widestTypeSrcExpr) throws AnalysisException {
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/13050/2/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@224
PS2, Line 224:           dstTableName, widestTypeSrcExpr.toSql(), srcExprType.toSql(), dstColType.toSql(),
line too long (91 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 2
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2019 00:00:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

Posted by "Alice Fan (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Paul Rogers, xiaomeng@cloudera.com, Bikramjeet Vig, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-966: Type errors are attributed to wrong expression with insert
......................................................................

IMPALA-966: Type errors are attributed to wrong expression with insert

When insert multiple incompatible type values into a table,
error message should blame on the correct expression. If there
are multiple incompatible type values for a single target
column, error should blame on the first widest incompatible type
expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
6 files changed, 74 insertions(+), 23 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 5
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Type errors are attributed to wrong expression with insert
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2931/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 3
Gerrit-Owner: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <xi...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2019 09:00:42 +0000
Gerrit-HasComments: No