impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4828: Alter Kudu schema outside Impala may crash on read
Date Sat, 04 Feb 2017 00:48:52 GMT
Matthew Jacobs has posted comments on this change.

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

Patch Set 1:

Commit Message:

Line 20: open, even if the schema changes concurrently.
> It would probably be worth adding that this is the earliest point and the c
File be/src/exec/

PS1, Line 133: for (int i = 0; i < tuple_desc->slots().size(); ++i) {
> As we spoke, if an ALTER TABLE is done and a column is removed, a crash mig
Really good point.

I handled the cases where there are more or fewer cols than expected. If there are fewer,
I fail the query. If there are more, I add a warning but then continue. I added new test cases.

Line 150:       "Unable to deserialize scan token");
> a quick comment explaining why this is needed would be helpful.
File be/src/exec/

PS1, Line 77: PrimitiveType KuduColTypeToPrimitiveType(
            :     const KuduColumnSchema::DataType& type) {
> This may be an over optimization, but what if we had an array mapping KuduC
Good idea but we only use this on a per-partition basis, so not really on the hot path. Lets
change it if it ever shows up in perf.
File common/thrift/

PS1, Line 314: is type
> nit: is of type

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d43f5bb9811e728ad592933066d006c8fb4553a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-Reviewer: Thomas Tauber-Marshall <>
Gerrit-HasComments: Yes

View raw message