impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Tauber-Marshall (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4828: Alter Kudu schema outside Impala may crash on read
Date Tue, 07 Feb 2017 16:57:22 GMT
Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-4828: Alter Kudu schema outside Impala may crash on read
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5840/2/be/src/exec/kudu-scanner.cc
File be/src/exec/kudu-scanner.cc:

Line 149:     if (slot->type().type != KuduColTypeToPrimitiveType(col.type())) {
This doesn't address the case where you removed a column and then added a new one with the
same type but with other attributes changed, eg. name and nullability.

Do we keep our column names consistent with the column names in Kudu? If so, maybe we should
log an error in that case, though probably not fail the query.


http://gerrit.cloudera.org:8080/#/c/5840/2/common/thrift/generate_error_codes.py
File common/thrift/generate_error_codes.py:

PS2, Line 317: KUDU_COL_MISMATCH
> nit: maybe call this 'KUDU_NUM_COLS_MISMATCH', else it sounds like it could
Maybe also explicitly mention refreshing here, as above.


http://gerrit.cloudera.org:8080/#/c/5840/2/tests/query_test/test_kudu.py
File tests/query_test/test_kudu.py:

Line 148: 
nit: extra line


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d43f5bb9811e728ad592933066d006c8fb4553a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message