Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4379: Throw if Kudu table created with var/char ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4857/1//COMMIT_MSG Commit Message: PS1, Line 10: analysis > Actually, this is not correct. We throw an ImpalaRuntimeException in the ca Good point. I thought about this and I think that ideally we would do this check during analysis, but we can't easily do it for EXTERNAL tables right now since the kudu table loading happens in the catalog. While we could do it for internal tables, I don't think it's worth having 2 different failure paths. What do you think? If we keep it this way I'll fix this comment. http://gerrit.cloudera.org:8080/#/c/4857/1/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java: PS1, Line 231: String storageHandler = properties.get(KuduTable.KEY_STORAGE_HANDLER); : Preconditions.checkState(!Strings.isNullOrEmpty(storageHandler)); : if (!storageHandler.equals(KuduTable.KUDU_STORAGE_HANDLER)) > Maybe we can replace all this with if (!KuduTable.isKuduTable(msTbl)) which Done -- To view, visit http://gerrit.cloudera.org:8080/4857 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I475273cbbf4110db8d0f78ddf9a56abfc6221e3e Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: Yes
