Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/14975 )
Change subject: IMPALA-9154: Make runtime filter propagation asynchronous ...................................................................... Patch Set 6: (2 comments) Patch looks good, thanks for your work on this! Just two more tiny things, and I think we can go ahead and submit this http://gerrit.cloudera.org:8080/#/c/14975/6/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/14975/6/be/src/runtime/coordinator.cc@1142 PS6, Line 1142: DCHECK(it != filter_routing_table_->id_to_filter.end()); This will need to be moved above the 'if' or it can never be hit. Or, another thing we do sometimes to make it clearer would be to just do a DCHECK(false) inside the 'if'. Either way, it'd be good to also have a brief comment saying that this shouldn't be possible. http://gerrit.cloudera.org:8080/#/c/14975/6/be/src/runtime/coordinator.cc@1192 PS6, Line 1192: ( nit: sort of weird to have a '(' by itself like this, maybe move it to the next line, or just remove the parenthesis entirely -- To view, visit http://gerrit.cloudera.org:8080/14975 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifb6726d349be701f3a0602b2ad5a934082f188a0 Gerrit-Change-Number: 14975 Gerrit-PatchSet: 6 Gerrit-Owner: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Thu, 16 Jan 2020 19:32:04 +0000 Gerrit-HasComments: Yes
