Matthew Jacobs has posted comments on this change. Change subject: IMPALA-5787: Dropped status in KuduTableSink::Send() ......................................................................
Patch Set 1: Code-Review+2 (1 comment) I'm OK with making this change now, but there are a bunch of open questions we should still address for 2.10. Fine to do that in IMPALA-5799. http://gerrit.cloudera.org:8080/#/c/7667/1/be/src/exec/kudu-table-sink.cc File be/src/exec/kudu-table-sink.cc: PS1, Line 241: // This can only fail if we set a col to an incorrect type, which would be a bug in : // planning, so we can DCHECK. > I checked with the Kudu team, and as long as the schema is what we want whe Hm I don't see how the writes fail at Apply() given the writes happen asynchronously. I'd think that we would see errors in CheckForErrors(). While Kudu doesn't allow col type changes, isn't that accomplished by: a col drop + col add w/ the same name but a different type? wrt the comment about adding cols: even w/o non-nullable columns, it seems unsafe to drop a string col and add another col with the same name but a different type. Seems like there's a potential for some code to dereference an invalid pointer. I'm OK with continuing this line of thinking in IMPALA-5799, but we should make sure to address it for 2.10. -- To view, visit http://gerrit.cloudera.org:8080/7667 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia228d98ca90de8638b0afcc242c788e225fb450f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
