Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4828: Alter Kudu schema outside Impala may crash on read ......................................................................
Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/5840/1//COMMIT_MSG 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 Done http://gerrit.cloudera.org:8080/#/c/5840/1/be/src/exec/kudu-scanner.cc File be/src/exec/kudu-scanner.cc: 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. Done http://gerrit.cloudera.org:8080/#/c/5840/1/be/src/exec/kudu-util.cc File be/src/exec/kudu-util.cc: 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. http://gerrit.cloudera.org:8080/#/c/5840/1/common/thrift/generate_error_codes.py File common/thrift/generate_error_codes.py: PS1, Line 314: is type > nit: is of type Done -- 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: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-HasComments: Yes
