[Impala-ASF-CR] IMPALA-6444: CTAS STORED AS KUDU not supporting reordering of columns

2018-01-29 Thread Pranay Singh (Code Review) 
Pranay Singh has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9147 )

Change subject: IMPALA-6444: CTAS STORED AS KUDU not supporting reordering of 
columns
..


Patch Set 1:

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/9147/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-6444: CTAS STORED AS KUDU not supporting reordering of 
columns
> The summary should contain what the change does, not what was broken.
Changed the summary to reflect the support of re-ordering of columns


http://gerrit.cloudera.org:8080/#/c/9147/1//COMMIT_MSG@16
PS1, Line 16: Change-Id: Ica1c8ec1544339e9e80733a7a0c78594e0a727d2
> I think the Change-Id line needs to be the last one of the commit message.
My bad restored the comment id to it's location.


http://gerrit.cloudera.org:8080/#/c/9147/1//COMMIT_MSG@17
PS1, Line 17: Testing: The fix is verified against the test case in JIRA that 
fails.
> Can you add an automated test for this?
Added a test case.


http://gerrit.cloudera.org:8080/#/c/9147/1/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
File fe/src/main/java/org/apache/impala/catalog/KuduTable.java:

http://gerrit.cloudera.org:8080/#/c/9147/1/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@393
PS1, Line 393: if (result == false) {
> "if (!result)" ?
Done


http://gerrit.cloudera.org:8080/#/c/9147/1/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@396
PS1, Line 396:   result = true;
> Why not "return true?"
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ica1c8ec1544339e9e80733a7a0c78594e0a727d2
Gerrit-Change-Number: 9147
Gerrit-PatchSet: 1
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Pranay Singh
Gerrit-Comment-Date: Mon, 29 Jan 2018 22:58:05 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6444: CTAS STORED AS KUDU not supporting reordering of columns

2018-01-29 Thread Lars Volker (Code Review) 
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9147 )

Change subject: IMPALA-6444: CTAS STORED AS KUDU not supporting reordering of 
columns
..


Patch Set 1:

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/9147/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-6444: CTAS STORED AS KUDU not supporting reordering of 
columns
The summary should contain what the change does, not what was broken.


http://gerrit.cloudera.org:8080/#/c/9147/1//COMMIT_MSG@16
PS1, Line 16: Change-Id: Ica1c8ec1544339e9e80733a7a0c78594e0a727d2
I think the Change-Id line needs to be the last one of the commit message. This 
might be a convention rather than a strict requirement by Gerrit.


http://gerrit.cloudera.org:8080/#/c/9147/1//COMMIT_MSG@17
PS1, Line 17: Testing: The fix is verified against the test case in JIRA that 
fails.
Can you add an automated test for this?


http://gerrit.cloudera.org:8080/#/c/9147/1/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
File fe/src/main/java/org/apache/impala/catalog/KuduTable.java:

http://gerrit.cloudera.org:8080/#/c/9147/1/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@393
PS1, Line 393: if (result == false) {
"if (!result)" ?


http://gerrit.cloudera.org:8080/#/c/9147/1/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@396
PS1, Line 396:   result = true;
Why not "return true?"



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ica1c8ec1544339e9e80733a7a0c78594e0a727d2
Gerrit-Change-Number: 9147
Gerrit-PatchSet: 1
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Mon, 29 Jan 2018 17:56:19 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6444: CTAS STORED AS KUDU not supporting reordering of columns

2018-01-26 Thread Pranay Singh (Code Review) 
Pranay Singh has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9147


Change subject: IMPALA-6444: CTAS STORED AS KUDU not supporting reordering of 
columns
..

IMPALA-6444: CTAS STORED AS KUDU not supporting reordering of columns

In the function KuduTable.isPrimaryKeyColumn() primaryKeyColumnNames_ does not
check for a matching case which causes primaryKeyExprs_ to be empty. This 
causes to
hit an assertion in InsertStmt.prepareExpressions() that generates the exception
reported in the jira.

The problem is fixed by having an ignoreCase match of the column names.

Change-Id: Ica1c8ec1544339e9e80733a7a0c78594e0a727d2
Testing: The fix is verified against the test case in JIRA that fails.
---
M fe/src/main/java/org/apache/impala/catalog/KuduTable.java
1 file changed, 10 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ica1c8ec1544339e9e80733a7a0c78594e0a727d2
Gerrit-Change-Number: 9147
Gerrit-PatchSet: 1
Gerrit-Owner: Pranay Singh