Lchangliang opened a new pull request, #10684:
URL: https://github.com/apache/doris/pull/10684
# Proposed changes
Issue Number: close #xxx
## Problem Summary:
In
```
SegmentWriter::append_block(const vectorized::Block* block, size_t row_pos,
size_t num_rows)
```,
if a block is divided into several parts to append, we need to use row_pos
and num_rows to know the range to append. There is a problem when the column is
nullable type.
In
```
void OlapBlockDataConvertor::OlapColumnDataConvertorBase::set_source_column(
const ColumnWithTypeAndName& typed_column, size_t row_pos, size_t
num_rows) {
DCHECK(row_pos + num_rows <= typed_column.column->size())
<< "row_pos=" << row_pos << ", num_rows=" << num_rows
<< ", typed_column.column->size()=" <<
typed_column.column->size();
_typed_column = typed_column;
_row_pos = row_pos;
_num_rows = num_rows;
if (_typed_column.column->is_nullable()) {
auto nullable_column =
assert_cast<const
vectorized::ColumnNullable*>(_typed_column.column.get());
_nullmap = nullable_column->get_null_map_data().data();
}
}
```,
OlapColumnDataConvertorBase will get the nullmap which associated with
column.
When call convert_to_olap method
```
Status convert_to_olap() override {
const vectorized::ColumnVector<T>* column_data = nullptr;
if (_nullmap) {
auto nullable_column =
assert_cast<const
vectorized::ColumnNullable*>(_typed_column.column.get());
column_data = assert_cast<const
vectorized::ColumnVector<T>*>(
nullable_column->get_nested_column_ptr().get());
} else {
column_data =
assert_cast<const
vectorized::ColumnVector<T>*>(_typed_column.column.get());
}
assert(column_data);
_values = (const T*)(column_data->get_data().data()) + _row_pos;
return Status::OK();
}
```
it will move the ptr of values through the _row_pos. But nullmap doesn't do
it and it will return directly.
and then when call append_nullable method
```
Status ColumnWriter::append_nullable(const uint8_t* null_map, const
uint8_t** ptr,
size_t num_rows) {
size_t offset = 0;
auto next_run_step = [&]() {
size_t step = 1;
for (auto i = offset + 1; i < num_rows; ++i) {
if (null_map[offset] == null_map[i])
step++;
else
break;
}
return step;
};
do {
auto step = next_run_step();
if (null_map[offset]) {
RETURN_IF_ERROR(append_nulls(step));
*ptr += get_field()->size() * step;
} else {
// TODO:
// 1. `*ptr += get_field()->size() * step;` should do in this
function, not append_data;
// 2. support array vectorized load and ptr offset add
RETURN_IF_ERROR(append_data(ptr, step));
}
offset += step;
} while (offset < num_rows);
return Status::OK();
}
```
it will always read the nullmap starting from zero so that the datas in
block starting from row_pos doesn't match the nullmap. This can lead to that
the position of nullmap is one, but the data exists.
## Checklist(Required)
1. Does it affect the original behavior: (Yes/No/I Don't know)
2. Has unit tests been added: (Yes/No/No Need)
3. Has document been added or modified: (Yes/No/No Need)
4. Does it need to update dependencies: (Yes/No)
5. Are there any changes that cannot be rolled back: (Yes/No)
## Further comments
If this is a relatively large or complex change, kick off the discussion at
[d...@doris.apache.org](mailto:d...@doris.apache.org) by explaining why you
chose the solution you did and what alternatives you considered, etc...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
