Alex Behm has posted comments on this change. Change subject: IMPALA-2789: More compact mem layout with null bits at the end. ......................................................................
Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/4673/2/be/src/exec/hdfs-scanner.cc File be/src/exec/hdfs-scanner.cc: Line 303: // TODO: Before committing this patch, modify the other example IR functions. > IMO we shouldn't feel obliged to keep the sample IR consistent everywhere, Works for me. I agree that updating all example IR is a pretty time consuming task. http://gerrit.cloudera.org:8080/#/c/4673/4/be/src/exec/row-batch-list-test.cc File be/src/exec/row-batch-list-test.cc: Line 44: scoped_ptr<Frontend> fe; > Let's make this a member of RowBatchListTest and lazy-initialise in SetUp() Done http://gerrit.cloudera.org:8080/#/c/4673/4/be/src/runtime/collection-value-builder-test.cc File be/src/runtime/collection-value-builder-test.cc: Line 30: scoped_ptr<Frontend> fe; > See my comment in the other test. Made this one static as discussed. http://gerrit.cloudera.org:8080/#/c/4673/4/be/src/runtime/row-batch-serialize-test.cc File be/src/runtime/row-batch-serialize-test.cc: Line 44: scoped_ptr<Frontend> fe; > Same here Moved into RowBatchSerializeTest http://gerrit.cloudera.org:8080/#/c/4673/4/be/src/testutil/desc-tbl-builder.cc File be/src/testutil/desc-tbl-builder.cc: Line 47: DescriptorTbl* DescriptorTblBuilder::Build() { > Thanks, I think this helps with test coverage too. Seems generally saner to do it this way, thanks for the suggestion. http://gerrit.cloudera.org:8080/#/c/4673/4/common/thrift/Frontend.thrift File common/thrift/Frontend.thrift: PS4, Line 755: TTestDescriptorTable > TTestDescriptorTableParams? Made this TBuildTestTableDescriptorParams to be consistent with out other naming. -- To view, visit http://gerrit.cloudera.org:8080/4673 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6510c75d841bddafa6638f1bd2ac6731a7053f6 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
