Tim Armstrong has posted comments on this change. Change subject: IMPALA-3412: codegen DCHECK with tuple comparator ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/2876/1//COMMIT_MSG Commit Message: Line 7: IMPALA-3412: codegen DCHECK with tuple comparator > This commit message made me think you were inserting DCHECKs into codegen'd Haha yeah I see that now. http://gerrit.cloudera.org:8080/#/c/2876/1/be/src/exprs/scalar-fn-call.cc File be/src/exprs/scalar-fn-call.cc: Line 69: bool char_arg_or_result = type_.type == TYPE_CHAR; > nit: 'char_arg_or_result' is confusing, how about 'has_char_arg'? Done Line 72: char_arg_or_result = char_arg_or_result || (children_[i]->type_.type == TYPE_CHAR); > nit: char_arg_or_result |= ... Done http://gerrit.cloudera.org:8080/#/c/2876/1/testdata/workloads/functional-query/queries/QueryTest/top-n.test File testdata/workloads/functional-query/queries/QueryTest/top-n.test: Line 1252: select cast(string_col as char(20)) from alltypes order by 1 limit 5 > Does this test the sorter or just top n? Both actually, test_top_n() runs these normally, then test_sort() sets disable_outermost_topn to run all of these tests with the sorter. -- To view, visit http://gerrit.cloudera.org:8080/2876 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I189073d46a10988803d572928a38f4a718690fa3 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Skye Wanderman-Milne <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
