Skip to content

Commit

Permalink
Replace 'null_bitmap' variable names with 'non_null_bitmap'
Browse files Browse the repository at this point in the history
These bitmaps have always had the semantics that a set bit indicated a
non-null entry. Thus the name 'null_bitmap' was misleading. This is a
simple find/replace to 'non_null_bitmap'.

Notably the code in CFileWriter::AppendNullableEntries now makes a lot
more sense.

Change-Id: Ie67dea54ce41ea42a7de5640845e402ac98ee3ce
Reviewed-on: http://gerrit.cloudera.org:8080/15548
Reviewed-by: Andrew Wong <[email protected]>
Tested-by: Todd Lipcon <[email protected]>
  • Loading branch information
toddlipcon committed Mar 24, 2020
1 parent 6dd3fc1 commit ffef96b
Show file tree
Hide file tree
Showing 20 changed files with 92 additions and 87 deletions.
14 changes: 7 additions & 7 deletions src/kudu/cfile/cfile-test-base.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ namespace cfile {
// StringDataGenerator<true> datagen;
// datagen.Build(10);
// for (int i = 0; i < datagen.block_entries(); ++i) {
// bool is_null = BitmpTest(datagen.null_bitmap(), i);
// bool is_null = !BitmapTest(datagen.non_null_bitmap(), i);
// Slice& v = datagen[i];
// }
template<DataType DATA_TYPE, bool HAS_NULLS>
Expand Down Expand Up @@ -82,14 +82,14 @@ class DataGenerator {
}

// Build "num_entries" using (offset + i) as value
// You can get the data values and the null bitmap using values() and null_bitmap()
// You can get the data values and the null bitmap using values() and non_null_bitmap()
// both are valid until the class is destructed or until Build() is called again.
void Build(size_t offset, size_t num_entries) {
Resize(num_entries);

for (size_t i = 0; i < num_entries; ++i) {
if (HAS_NULLS) {
BitmapChange(null_bitmap_.get(), i, !TestValueShouldBeNull(offset + i));
BitmapChange(non_null_bitmap_.get(), i, !TestValueShouldBeNull(offset + i));
}
values_[i] = BuildTestValue(i, offset + i);
}
Expand Down Expand Up @@ -129,15 +129,15 @@ class DataGenerator {
}

values_.reset(new cpp_type[num_entries]);
null_bitmap_.reset(new uint8_t[BitmapSize(num_entries)]);
non_null_bitmap_.reset(new uint8_t[BitmapSize(num_entries)]);
block_entries_ = num_entries;
}

size_t block_entries() const { return block_entries_; }
size_t total_entries() const { return total_entries_; }

const cpp_type *values() const { return values_.get(); }
const uint8_t *null_bitmap() const { return null_bitmap_.get(); }
const uint8_t *non_null_bitmap() const { return non_null_bitmap_.get(); }

const cpp_type& operator[](size_t index) const {
return values_[index];
Expand All @@ -147,7 +147,7 @@ class DataGenerator {

private:
std::unique_ptr<cpp_type[]> values_;
std::unique_ptr<uint8_t[]> null_bitmap_;
std::unique_ptr<uint8_t[]> non_null_bitmap_;
size_t block_entries_;
size_t total_entries_;
};
Expand Down Expand Up @@ -396,7 +396,7 @@ class CFileTestBase : public KuduTest {
DCHECK_EQ(towrite, data_generator->block_entries());

if (DataGeneratorType::has_nulls()) {
ASSERT_OK_FAST(w.AppendNullableEntries(data_generator->null_bitmap(),
ASSERT_OK_FAST(w.AppendNullableEntries(data_generator->non_null_bitmap(),
data_generator->values(),
towrite));
} else {
Expand Down
6 changes: 3 additions & 3 deletions src/kudu/cfile/cfile-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,7 @@ TEST_P(TestCFileBothCacheMemoryTypes, TestDefaultColumnIter) {
RETURN_IF_NO_NVM_CACHE(GetParam());

const int kNumItems = 64;
uint8_t null_bitmap[BitmapSize(kNumItems)];
uint8_t non_null_bitmap[BitmapSize(kNumItems)];
uint32_t data[kNumItems];

// Test Int Default Value
Expand All @@ -828,7 +828,7 @@ TEST_P(TestCFileBothCacheMemoryTypes, TestDefaultColumnIter) {
// Test Int Nullable Default Value
int_value = 321;
DefaultColumnValueIterator nullable_iter(GetTypeInfo(UINT32), &int_value);
ColumnBlock nullable_col(GetTypeInfo(UINT32), null_bitmap, data, kNumItems, nullptr);
ColumnBlock nullable_col(GetTypeInfo(UINT32), non_null_bitmap, data, kNumItems, nullptr);
ColumnMaterializationContext nullable_ctx = CreateNonDecoderEvalContext(&nullable_col, &sel);
ASSERT_OK(nullable_iter.Scan(&nullable_ctx));
for (size_t i = 0; i < nullable_col.nrows(); ++i) {
Expand All @@ -838,7 +838,7 @@ TEST_P(TestCFileBothCacheMemoryTypes, TestDefaultColumnIter) {

// Test NULL Default Value
DefaultColumnValueIterator null_iter(GetTypeInfo(UINT32), nullptr);
ColumnBlock null_col(GetTypeInfo(UINT32), null_bitmap, data, kNumItems, nullptr);
ColumnBlock null_col(GetTypeInfo(UINT32), non_null_bitmap, data, kNumItems, nullptr);
ColumnMaterializationContext null_ctx = CreateNonDecoderEvalContext(&null_col, &sel);
ASSERT_OK(null_iter.Scan(&null_ctx));
for (size_t i = 0; i < null_col.nrows(); ++i) {
Expand Down
10 changes: 5 additions & 5 deletions src/kudu/cfile/cfile_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -919,18 +919,18 @@ string CFileIterator::PreparedBlock::ToString() const {
}

// Decode the null header in the beginning of the data block
Status DecodeNullInfo(Slice *data_block, uint32_t *num_rows_in_block, Slice *null_bitmap) {
Status DecodeNullInfo(Slice *data_block, uint32_t *num_rows_in_block, Slice *non_null_bitmap) {
if (!GetVarint32(data_block, num_rows_in_block)) {
return Status::Corruption("bad null header, num elements in block");
}

uint32_t null_bitmap_size;
if (!GetVarint32(data_block, &null_bitmap_size)) {
uint32_t non_null_bitmap_size;
if (!GetVarint32(data_block, &non_null_bitmap_size)) {
return Status::Corruption("bad null header, bitmap size");
}

*null_bitmap = Slice(data_block->data(), null_bitmap_size);
data_block->remove_prefix(null_bitmap_size);
*non_null_bitmap = Slice(data_block->data(), non_null_bitmap_size);
data_block->remove_prefix(non_null_bitmap_size);
return Status::OK();
}

Expand Down
22 changes: 11 additions & 11 deletions src/kudu/cfile/cfile_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ Status CFileWriter::Start() {
if (is_nullable_) {
size_t nrows = ((options_.storage_attributes.cfile_block_size + typeinfo_->size() - 1) /
typeinfo_->size());
null_bitmap_builder_.reset(new NullBitmapBuilder(nrows * 8));
non_null_bitmap_builder_.reset(new NullBitmapBuilder(nrows * 8));
}

state_ = kWriterWriting;
Expand Down Expand Up @@ -328,16 +328,16 @@ Status CFileWriter::AppendNullableEntries(const uint8_t *bitmap,
const uint8_t *ptr = reinterpret_cast<const uint8_t *>(entries);

size_t nitems;
bool is_null = false;
bool is_non_null = false;
BitmapIterator bmap_iter(bitmap, count);
while ((nitems = bmap_iter.Next(&is_null)) > 0) {
if (is_null) {
while ((nitems = bmap_iter.Next(&is_non_null)) > 0) {
if (is_non_null) {
size_t rem = nitems;
do {
int n = data_block_->Add(ptr, rem);
DCHECK_GE(n, 0);

null_bitmap_builder_->AddRun(true, n);
non_null_bitmap_builder_->AddRun(true, n);
ptr += n * typeinfo_->size();
value_count_ += n;
rem -= n;
Expand All @@ -348,7 +348,7 @@ Status CFileWriter::AppendNullableEntries(const uint8_t *bitmap,

} while (rem > 0);
} else {
null_bitmap_builder_->AddRun(false, nitems);
non_null_bitmap_builder_->AddRun(false, nitems);
ptr += nitems * typeinfo_->size();
value_count_ += nitems;
}
Expand All @@ -360,7 +360,7 @@ Status CFileWriter::AppendNullableEntries(const uint8_t *bitmap,
Status CFileWriter::FinishCurDataBlock() {
uint32_t num_elems_in_block = data_block_->Count();
if (is_nullable_) {
num_elems_in_block = null_bitmap_builder_->nitems();
num_elems_in_block = non_null_bitmap_builder_->nitems();
}

if (PREDICT_FALSE(num_elems_in_block == 0)) {
Expand Down Expand Up @@ -388,11 +388,11 @@ Status CFileWriter::FinishCurDataBlock() {
vector<Slice> v;
faststring null_headers;
if (is_nullable_) {
Slice null_bitmap = null_bitmap_builder_->Finish();
Slice non_null_bitmap = non_null_bitmap_builder_->Finish();
PutVarint32(&null_headers, num_elems_in_block);
PutVarint32(&null_headers, null_bitmap.size());
PutVarint32(&null_headers, non_null_bitmap.size());
v.emplace_back(null_headers.data(), null_headers.size());
v.push_back(null_bitmap);
v.push_back(non_null_bitmap);
}
v.push_back(data);
Status s = AppendRawBlock(v, first_elem_ord,
Expand All @@ -401,7 +401,7 @@ Status CFileWriter::FinishCurDataBlock() {
"data block");

if (is_nullable_) {
null_bitmap_builder_->Reset();
non_null_bitmap_builder_->Reset();
}

if (validx_builder_ != nullptr) {
Expand Down
2 changes: 1 addition & 1 deletion src/kudu/cfile/cfile_writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ class CFileWriter {
std::unique_ptr<BlockBuilder> data_block_;
std::unique_ptr<IndexTreeBuilder> posidx_builder_;
std::unique_ptr<IndexTreeBuilder> validx_builder_;
std::unique_ptr<NullBitmapBuilder> null_bitmap_builder_;
std::unique_ptr<NullBitmapBuilder> non_null_bitmap_builder_;
std::unique_ptr<CompressedBlockBuilder> block_compressor_;

enum State {
Expand Down
2 changes: 1 addition & 1 deletion src/kudu/client/write_op.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ int64_t KuduWriteOperation::SizeInBuffer() const {
// Add size of isset bitmap (always present).
size += BitmapSize(schema->num_columns());
// Add size of null bitmap (present if the schema has nullables)
size += ContiguousRowHelper::null_bitmap_size(*schema);
size += ContiguousRowHelper::non_null_bitmap_size(*schema);
// The column data itself:
for (int i = 0; i < schema->num_columns(); i++) {
if (row_.IsColumnSet(i) && !row_.IsNull(i)) {
Expand Down
8 changes: 4 additions & 4 deletions src/kudu/common/column_predicate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ int ApplyPredicatePrimitive(const ColumnBlock& block, uint8_t* __restrict__ sel_
}
if (block.is_nullable()) {
for (int i = 0; i < n_chunks; i++) {
sel_bitmap[i] &= block.null_bitmap()[i];
sel_bitmap[i] &= block.non_null_bitmap()[i];
}
}
return n_chunks * 8;
Expand Down Expand Up @@ -722,9 +722,9 @@ template<bool IS_NOT_NULL>
void ApplyNullPredicate(const ColumnBlock& block, uint8_t* __restrict__ sel_vec) {
int n_bytes = KUDU_ALIGN_UP(block.nrows(), 8) / 8;
for (int i = 0; i < n_bytes; i++) {
uint8_t null_byte = block.null_bitmap()[i];
if (!IS_NOT_NULL) null_byte = ~null_byte;
sel_vec[i] &= null_byte;
uint8_t non_null_byte = block.non_null_bitmap()[i];
if (!IS_NOT_NULL) non_null_byte = ~non_null_byte;
sel_vec[i] &= non_null_byte;
}
}
} // anonymous namespace
Expand Down
6 changes: 3 additions & 3 deletions src/kudu/common/columnblock.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ Status ColumnBlock::CopyTo(const SelectionVector& sel_vec,
memcpy(dst->data_ + (dst_cell_off * type_->size()),
data_ + (src_cell_off * type_->size()),
num_cells * type_->size());
if (null_bitmap_) {
BitmapCopy(dst->null_bitmap_, dst_cell_off,
null_bitmap_, src_cell_off,
if (non_null_bitmap_) {
BitmapCopy(dst->non_null_bitmap_, dst_cell_off,
non_null_bitmap_, src_cell_off,
num_cells);
}
}
Expand Down
30 changes: 17 additions & 13 deletions src/kudu/common/columnblock.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@ class ColumnBlock {
typedef ColumnBlockCell Cell;

ColumnBlock(const TypeInfo* type,
uint8_t *null_bitmap,
uint8_t *non_null_bitmap,
void *data,
size_t nrows,
Arena *arena)
: type_(type),
null_bitmap_(null_bitmap),
non_null_bitmap_(non_null_bitmap),
data_(reinterpret_cast<uint8_t *>(data)),
nrows_(nrows),
arena_(arena) {
Expand All @@ -60,7 +60,7 @@ class ColumnBlock {

void SetCellIsNull(size_t idx, bool is_null) {
DCHECK(is_nullable());
BitmapChange(null_bitmap_, idx, !is_null);
BitmapChange(non_null_bitmap_, idx, !is_null);
}

void SetCellValue(size_t idx, const void *new_val) {
Expand All @@ -87,18 +87,22 @@ class ColumnBlock {

Cell cell(size_t idx) const;

uint8_t *null_bitmap() const {
return null_bitmap_;
// Return the bitmap indicating whether each cell is non-NULL.
// A set bit indicates non-NULL.
//
// This returns nullptr for a non-nullable column.
uint8_t *non_null_bitmap() const {
return non_null_bitmap_;
}

bool is_nullable() const {
return null_bitmap_ != NULL;
return non_null_bitmap_ != nullptr;
}

bool is_null(size_t idx) const {
DCHECK(is_nullable());
DCHECK_LT(idx, nrows_);
return !BitmapTest(null_bitmap_, idx);
return !BitmapTest(non_null_bitmap_, idx);
}

const size_t stride() const { return type_->size(); }
Expand Down Expand Up @@ -154,7 +158,7 @@ class ColumnBlock {
}

const TypeInfo *type_;
uint8_t *null_bitmap_;
uint8_t *non_null_bitmap_;

uint8_t *data_;
size_t nrows_;
Expand All @@ -175,7 +179,7 @@ inline bool operator==(const ColumnBlock& a, const ColumnBlock& b) {

// 3. If nullable, same null bitmap contents.
if (a.is_nullable() &&
!BitmapEquals(a.null_bitmap(), b.null_bitmap(), a.nrows())) {
!BitmapEquals(a.non_null_bitmap(), b.non_null_bitmap(), a.nrows())) {
return false;
}

Expand Down Expand Up @@ -245,7 +249,7 @@ class ColumnDataView {
// Set 'nrows' bits of the the null-bitmap to "value"
// true if not null, false if null.
void SetNullBits(size_t nrows, bool value) {
BitmapChangeBits(column_block_->null_bitmap(), row_offset_, nrows, value);
BitmapChangeBits(column_block_->non_null_bitmap(), row_offset_, nrows, value);
}

uint8_t *data() {
Expand Down Expand Up @@ -292,12 +296,12 @@ class ScopedColumnBlock : public ColumnBlock {
new cpp_type[n_rows],
n_rows,
new Arena(1024)),
null_bitmap_(null_bitmap()),
non_null_bitmap_(non_null_bitmap()),
data_(reinterpret_cast<cpp_type *>(data())),
arena_(arena()) {
if (allow_nulls) {
// All rows begin null.
BitmapChangeBits(null_bitmap(), /*offset=*/ 0, n_rows, /*value=*/ false);
BitmapChangeBits(non_null_bitmap(), /*offset=*/ 0, n_rows, /*value=*/ false);
}
}

Expand All @@ -310,7 +314,7 @@ class ScopedColumnBlock : public ColumnBlock {
}

private:
std::unique_ptr<uint8_t[]> null_bitmap_;
std::unique_ptr<uint8_t[]> non_null_bitmap_;
std::unique_ptr<cpp_type[]> data_;
std::unique_ptr<Arena> arena_;

Expand Down
2 changes: 1 addition & 1 deletion src/kudu/common/partial_row.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ KuduPartialRow::KuduPartialRow(const Schema* schema)
row_size, "NEWNEWNEWNEWNEW");
#endif
ContiguousRowHelper::InitNullsBitmap(
*schema_, row_data_, ContiguousRowHelper::null_bitmap_size(*schema_));
*schema_, row_data_, ContiguousRowHelper::non_null_bitmap_size(*schema_));
}

KuduPartialRow::~KuduPartialRow() {
Expand Down
Loading

0 comments on commit ffef96b

Please sign in to comment.