You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Will Berkeley (Code Review)" <ge...@cloudera.org> on 2017/06/28 17:13:03 UTC
[kudu-CR](branch-1.3.x) KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock
Will Berkeley has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7318
Change subject: KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock
......................................................................
KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock
In a CFile block with n non-null values, it's permissible to
seek to the spot after all rows. However, a CHECK_LT in
RleIntBlockDecoder::SeekToPositionInBlock was enforcing seeks to
be < n. This patches switches the CHECK_LT to a DCHECK_LE,
making it consistent with the checks in other decoders. The
problem was not caught in testing due to an omission of RLE
encoding from cfile-test.cc's
TestCFileBothCacheTypes.TestNullInts. The test now has coverage
of RLE encoding and, with slow tests on, fails every time
without the change to the CHECK.
Additionally, TestCFileBothCacheTypes.TestNullFloats was missing
coverage for bit shuffle encoding. This was also fixed.
Finally, RleBitMapBlockDecoder::SeekToPositionInBlock was
missing any check on its 'pos' argument, so the checks from
RleIntBlockDecoder::SeekToPositionInBlock were added to that
method as well.
Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Reviewed-on: http://gerrit.cloudera.org:8080/7262
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Kudu Jenkins
(cherry picked from commit f9e51ca636fdcc29071be7f76868fa7b4509803d)
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/rle_block.h
2 files changed, 13 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/7318/1
--
To view, visit http://gerrit.cloudera.org:8080/7318
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
[kudu-CR](branch-1.3.x) KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has submitted this change and it was merged.
Change subject: KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock
......................................................................
KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock
In a CFile block with n non-null values, it's permissible to
seek to the spot after all rows. However, a CHECK_LT in
RleIntBlockDecoder::SeekToPositionInBlock was enforcing seeks to
be < n. This patches switches the CHECK_LT to a DCHECK_LE,
making it consistent with the checks in other decoders. The
problem was not caught in testing due to an omission of RLE
encoding from cfile-test.cc's
TestCFileBothCacheTypes.TestNullInts. The test now has coverage
of RLE encoding and, with slow tests on, fails every time
without the change to the CHECK.
Additionally, TestCFileBothCacheTypes.TestNullFloats was missing
coverage for bit shuffle encoding. This was also fixed.
Finally, RleBitMapBlockDecoder::SeekToPositionInBlock was
missing any check on its 'pos' argument, so the checks from
RleIntBlockDecoder::SeekToPositionInBlock were added to that
method as well.
Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Reviewed-on: http://gerrit.cloudera.org:8080/7262
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Kudu Jenkins
(cherry picked from commit f9e51ca636fdcc29071be7f76868fa7b4509803d)
Reviewed-on: http://gerrit.cloudera.org:8080/7318
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/rle_block.h
2 files changed, 13 insertions(+), 3 deletions(-)
Approvals:
Todd Lipcon: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/7318
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR](branch-1.3.x) KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-2049 Fix too-strict CHECK in RleIntBlockDecoder::SeekToPositionInBlock
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7318
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae72ca1d2af06de1e77cde233e98aa6af97d07e8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No