Skye Wanderman-Milne 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 function, maybe "Fix CHAR codegen logic" or something? 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'? Line 72: char_arg_or_result = char_arg_or_result || (children_[i]->type_.type == TYPE_CHAR); nit: char_arg_or_result |= ... 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? -- 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-HasComments: Yes
