Alex Behm has posted comments on this change. Change subject: IMPALA-4098: Open()/Close() partition exprs once per fragment instance. ......................................................................
Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/4340/1/be/src/runtime/descriptors.cc File be/src/runtime/descriptors.cc: Line 216: for (const auto& entry: tdesc.hdfsTable.partitions) { > clang-format puts a space before :. Done Line 512: for (const auto& part_entry: hdfs_tbl->partition_descriptors()) { > space before : to avoid later reformatting by clang-format (the way you wro Done http://gerrit.cloudera.org:8080/#/c/4340/1/be/src/runtime/descriptors.h File be/src/runtime/descriptors.h: PS1, Line 239: threads > ... after the descriptor table is opened ... Done Line 260: std::vector<ExprContext*> partition_key_value_ctxs_; > This was already like this but it feels a little wonky to have non-immutabl Agree. It feels to me like this belongs in the new query-wide state, indexed by partition id or something like that. Added a TODO. PS1, Line 436: Open > Maybe PrepareAndOpenExprs()? I think we want to limit the scope of what we Agree. Done. PS1, Line 439: Close > CloseExprs()? Done -- To view, visit http://gerrit.cloudera.org:8080/4340 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id179df645e500530f4418988f6ce64a03d669892 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
