Marcel Kornacker has posted comments on this change. Change subject: IMPALA-2548: Codegen Tuple::MaterializeExprs() and use in TopN node ......................................................................
Patch Set 10: (3 comments) http://gerrit.cloudera.org:8080/#/c/1901/10/be/src/runtime/descriptors.h File be/src/runtime/descriptors.h: Line 395: static const char* LLVM_CLASS_NAME; > Can you explain the underlying principle here? This is used by codegen func if it's used widely it's best to leave as is. http://gerrit.cloudera.org:8080/#/c/1901/10/be/src/runtime/tuple.cc File be/src/runtime/tuple.cc: Line 201: template <bool collect_string_vals, bool no_pool> format as constants Line 208: SlotDescriptor* slot_desc = desc.slots()[i]; > I implemented a quick prototype of the interpreted functionality so you can that's not what i had in mind. I was thinking in -ir.cc: void MaterializeExpr(const SlotDesc& desc, TupleRow* row, const ExprContext& expr_ctx, ...) { void* src = materialize_expr_ctxs[i]->GetValue(row); if (src = NULL) { SetNull(desc->null_indicator_offset()); return; } void* dst = GetSlot(desc->tuple_offset()); RawValue::Write(src, dst, desc->type(), pool); ... } void MaterializeExprsLoop(row, desc, ctxs, pool, ...) { for (int i = 0; i < desc.slots().size(); ++i) { MaterializeExpr(desc.slots()[i], row, ctxs[i], ...); } } void MaterializeExprs(...) { memset(this, 0, dessc.num_null_bytes()); MaterializeExprsLoop(...); } in someone's ::Prepare(): materialize_loop = GenLoop(tuple_desc.slots(), materialize_expr_fn); } -- To view, visit http://gerrit.cloudera.org:8080/1901 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib422a8d50303c21c6a228675157bf867e8619444 Gerrit-PatchSet: 10 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Skye Wanderman-Milne <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Skye Wanderman-Milne <[email protected]> Gerrit-HasComments: Yes
