Michael Ho has posted comments on this change. Change subject: IMPALA-4883: Union Codegen ......................................................................
Patch Set 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/6459/8/be/src/codegen/gen_ir_descriptions.py File be/src/codegen/gen_ir_descriptions.py: PS8, Line 183: ["UNION_MATERIALIZE_BATCH", : "_ZN6impala9UnionNode16MaterializeBatchEPNS_8RowBatchEPPh"], nit: please consider putting it in a different entry so as not to break up READ_AVRO* sequence. http://gerrit.cloudera.org:8080/#/c/6459/8/be/src/exec/union-node.cc File be/src/exec/union-node.cc: PS8, Line 223: codegend_union_materialize_batch_fns_.data() Why not codegend_union_materialize_batch_fns_[child_idx_] ? Same below. Line 290: int num_rows_before = row_batch->num_rows(); So, in that case, is it true that the rows in the non-empty batch are already counted towards num_rows_returned_ ? Line 303 seems to imply so but I could be wrong. PS8, Line 304: num_rows_added I probably misunderstood something here. Can you please explain why we need to subtract num_rows_added too ? -- To view, visit http://gerrit.cloudera.org:8080/6459 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib4107d27582ff5416172810364a6e76d3d93c439 Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
