Tim Armstrong has posted comments on this change. Change subject: IMPALA-5479: Propagate the argument type in RawValue::Compare() ......................................................................
Patch Set 1: Code-Review+2 (4 comments) Nice! http://gerrit.cloudera.org:8080/#/c/7140/1/be/src/codegen/codegen-anyval.cc File be/src/codegen/codegen-anyval.cc: Line 702 Its a little weird that it can't infer that this is already constant. The new code seems better anyway. http://gerrit.cloudera.org:8080/#/c/7140/1/be/src/runtime/raw-value-ir.cc File be/src/runtime/raw-value-ir.cc: PS1, Line 29: IR_ALWAYS_INLINE We don't need the attribute in both declaration and definition do we? http://gerrit.cloudera.org:8080/#/c/7140/1/be/src/runtime/runtime-filter-ir.cc File be/src/runtime/runtime-filter-ir.cc: PS1, Line 24: IR_ALWAYS_INLINE We don't need the attribute in both declaration and definition do we? http://gerrit.cloudera.org:8080/#/c/7140/1/be/src/runtime/runtime-filter.h File be/src/runtime/runtime-filter.h: Line 63: bool IR_ALWAYS_INLINE Eval(void* val, const ColumnType& col_type) const noexcept; Maybe something like "Inlined in IR so that the constnt 'col_type' can be propagated" -- To view, visit http://gerrit.cloudera.org:8080/7140 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I10b5b284e3da03024476a9620a12d6e7fbf08b3c Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
