Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/18824 )
Change subject: IMPALA-11477: Adding Codegen to sorted-run-merger ...................................................................... Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/18824/8/be/src/codegen/gen_ir_descriptions.py File be/src/codegen/gen_ir_descriptions.py: http://gerrit.cloudera.org:8080/#/c/18824/8/be/src/codegen/gen_ir_descriptions.py@261 PS8, Line 261: ["TUPLE_SORTER_SORT_HELPER", nit: can you fix the indentation of the last one? http://gerrit.cloudera.org:8080/#/c/18824/8/be/src/exec/partial-sort-node.cc File be/src/exec/partial-sort-node.cc: http://gerrit.cloudera.org:8080/#/c/18824/8/be/src/exec/partial-sort-node.cc@101 PS8, Line 101: AddCodegenStatus( : SortedRunMerger::Codegen(state, compare_fn, &codegend_heapify_helper_fn_)); Without the minirun patch, partial sort will not use a merger. I would add this only in the minirun patch. http://gerrit.cloudera.org:8080/#/c/18824/8/be/src/runtime/sorted-run-merger.cc File be/src/runtime/sorted-run-merger.cc: http://gerrit.cloudera.org:8080/#/c/18824/8/be/src/runtime/sorted-run-merger.cc@30 PS8, Line 30: SortedRunMerger::SortedRunWrapper::SortedRunWrapper nit: the indentation of this functions should be removed as they are not inside a class block anymore -- To view, visit http://gerrit.cloudera.org:8080/18824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic35c7460bdbd54b8ec5872a83680e2f41ceae9fd Gerrit-Change-Number: 18824 Gerrit-PatchSet: 8 Gerrit-Owner: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Comment-Date: Tue, 18 Oct 2022 12:38:55 +0000 Gerrit-HasComments: Yes
