Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201610121753.u9CHrrv9020022@ip-10-146-233-104.ec2.internal>
Date: Wed, 12 Oct 2016 17:53:53 +0000
From: "Matthew Jacobs (Code Review)" <gerrit@cloudera.org>
To: Alex Behm <alex.behm@cloudera.com>, impala-cr@cloudera.com,
        reviews@impala.incubator.apache.org
CC: Tim Armstrong <tarmstrong@cloudera.com>
Reply-To: mj@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-2789=3A_More_compact_mem_layout_with_null_bits_at_the_end=2E=0A?=
In-Reply-To: <gerrit.1475905149000.Ib6510c75d841bddafa6638f1bd2ac6731a7053f6@gerrit.cloudera.org>
References: <gerrit.1475905149000.Ib6510c75d841bddafa6638f1bd2ac6731a7053f6@gerrit.cloudera.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.12.2
archived-at: Wed, 12 Oct 2016 17:53:59 -0000

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-2789: More compact mem layout with null bits at the end.
......................................................................


Patch Set 6: Code-Review+1

(2 comments)

Thanks! LGTM, just a few comments for the commit msg. I didn't look at the codegen code changes in detail.

http://gerrit.cloudera.org:8080/#/c/4673/6//COMMIT_MSG
Commit Message:

Line 9: The main motivation of this change is to pave the way for full
There are 2 main motivations of this change, no?
1) More compact memory layout.
2) Zero copy (or at least avoiding conversions) with Kudu scans... (as you already have here)


Line 12: that goal.
Can you mention what is left after this? 
My understanding is that it is (correct me if I'm wrong):
1) planner needs to set null flags for nullable columns
2) kudu scan node can now _copy_ mem, i.e. rip out conversion code
2) kudu client needs to support mem xfer
3) kudu scan node updated to use mem xfer instead of copying
(KUDU-1695)


-- 
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: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes