This is an automated email from the ASF dual-hosted git repository.
Gabriel39 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 7191837ece2 [fix](parquet) Restore column ownership after interrupted
reads (#66780)
7191837ece2 is described below
commit 7191837ece228a32fcb4913054df94f55fc9657d
Author: daidai <[email protected]>
AuthorDate: Mon Aug 24 16:05:22 2026 +0800
[fix](parquet) Restore column ownership after interrupted reads (#66780)
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: The direct Parquet conversion path temporarily moves
the caller-owned column into the physical read column. If an interrupted
read returns before conversion, the caller is left with a null column
pointer. Complex-column cleanup can then dereference that null child and
crash the BE. Restore the transferred column on every early return, and
disarm the restoration only after conversion moves ownership back
successfully. Add a focused test that triggers the IO stop path and
verifies the caller retains a valid column.
coredump:
```
04:09:40
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/core/column/column_struct.h:98:23:
runtime error: member call on null pointer of type 'doris::IColumn'
04:09:40 *** Query id: 346501e80b2a4276-b032f92f6e9b6c3c ***
04:09:40 *** is nereids: 1 ***
04:09:40 *** tablet id: 0 ***
04:09:40 *** Aborted at 1786478874 (unix time) try "date -d @1786478874"
if you are using GNU date ***
04:09:40 *** Current BE git commitID: 206d71cd3c ***
04:09:40 *** SIGSEGV address not mapped to object (@0x0) received by PID
12549 (TID 37198 OR 0x7af300fc4640) from PID 0; stack trace: ***
04:09:40 0# doris::signal::(anonymous
namespace)::FailureSignalHandler(int, siginfo_t*, void*) at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/common/signal_handler.h:420
04:09:40 1# PosixSignals::chained_handler(int, siginfo*, void*) [clone
.part.0] in /usr/lib/jvm/java-17-openjdk-amd64/lib/server/libjvm.so
04:09:40 2# JVM_handle_linux_signal in
/usr/lib/jvm/java-17-openjdk-amd64/lib/server/libjvm.so
04:09:40 3# 0x00007F007F83D520 in /lib/x86_64-linux-gnu/libc.so.6
04:09:40 4# doris::ColumnArray::is_exclusive() const at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/core/column/column_array.h:141
04:09:40 5# doris::ColumnNullable::is_exclusive() const at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/core/column/column_nullable.h:268
04:09:40 6# doris::ColumnStruct::is_exclusive() const at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/core/column/column_struct.h:98
04:09:40 7# doris::ColumnNullable::is_exclusive() const at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/core/column/column_nullable.h:268
04:09:40 8# doris::ColumnArray::is_exclusive() const at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/core/column/column_array.h:141
04:09:40 9# doris::ColumnNullable::is_exclusive() const at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/core/column/column_nullable.h:268
04:09:40 10# doris::ColumnStruct::is_exclusive() const at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/core/column/column_struct.h:98
04:09:40 11# doris::ColumnNullable::is_exclusive() const at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/core/column/column_nullable.h:268
04:09:40 12# doris::ColumnArray::is_exclusive() const at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/core/column/column_array.h:141
04:09:40 13# doris::ColumnNullable::is_exclusive() const at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/core/column/column_nullable.h:268
04:09:40 14# doris::ColumnStruct::is_exclusive() const at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/core/column/column_struct.h:98
04:09:40 15# doris::ColumnNullable::is_exclusive() const at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/core/column/column_nullable.h:268
04:09:40 16# doris::ColumnArray::is_exclusive() const at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/core/column/column_array.h:141
04:09:40 17# doris::ColumnNullable::is_exclusive() const at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/core/column/column_nullable.h:268
04:09:40 18# doris::Block::clear_column_data(long) at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/core/block/block.cpp:816
04:09:40 19# doris::ParquetReader::get_next_block(doris::Block*, unsigned
long*, bool*) at
/mnt/disk3/pipeline/repo/selectdb-core_branch-selectdb-doris-4.1/selectdb-core/be/src/format/parquet/vparquet_reader.cpp:734
```
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
---
be/src/format/parquet/vparquet_column_reader.cpp | 22 ++++++++++---
.../parquet/parquet_column_chunk_reader_test.cpp | 37 ++++++++++++++++++++++
2 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/be/src/format/parquet/vparquet_column_reader.cpp
b/be/src/format/parquet/vparquet_column_reader.cpp
index 5fd9adcff06..8797b54c0a6 100644
--- a/be/src/format/parquet/vparquet_column_reader.cpp
+++ b/be/src/format/parquet/vparquet_column_reader.cpp
@@ -41,6 +41,7 @@
#include "format/table/iceberg_default_value.h"
#include "io/fs/tracing_file_reader.h"
#include "runtime/runtime_profile.h"
+#include "util/defer_op.h"
namespace doris {
static void fill_struct_null_map(FieldSchema* field, NullMap& null_map,
@@ -539,11 +540,26 @@ Status ScalarColumnReader<IN_COLLECTION,
OFFSET_INDEX>::read_column_data(
ColumnPtr resolved_column =
_converter->get_physical_column(_field_schema->physical_type,
_field_schema->data_type,
doris_column, type,
is_dict_filter);
+ // Direct reads transfer the caller's only ColumnPtr so mutate() can avoid
cloning it. Restore
+ // that ownership if any read step returns before convert() moves the
column back.
+ bool restore_doris_column = false;
+ Defer restore_column([&]() {
+ if (restore_doris_column) {
+ doris_column = std::move(resolved_column);
+ }
+ });
if (_converter->read_directly_into_dst_logical_column()) {
DCHECK_EQ(resolved_column.get(), doris_column.get());
resolved_column = std::move(doris_column);
+ restore_doris_column = true;
}
DataTypePtr& resolved_type = _converter->get_physical_type();
+ auto convert_column = [&]() -> Status {
+ RETURN_IF_ERROR(_converter->convert(resolved_column,
_field_schema->data_type, type,
+ doris_column, is_dict_filter));
+ restore_doris_column = false;
+ return Status::OK();
+ };
_def_levels.clear();
_rep_levels.clear();
@@ -552,8 +568,7 @@ Status ScalarColumnReader<IN_COLLECTION,
OFFSET_INDEX>::read_column_data(
if (_in_nested) {
RETURN_IF_ERROR(_read_nested_column(resolved_column, resolved_type,
filter_map, batch_size,
read_rows, eof, is_dict_filter));
- return _converter->convert(resolved_column, _field_schema->data_type,
type, doris_column,
- is_dict_filter);
+ return convert_column();
}
int64_t right_row = 0;
@@ -631,8 +646,7 @@ Status ScalarColumnReader<IN_COLLECTION,
OFFSET_INDEX>::read_column_data(
{
SCOPED_RAW_TIMER(&_convert_time);
- RETURN_IF_ERROR(_converter->convert(resolved_column,
_field_schema->data_type, type,
- doris_column, is_dict_filter));
+ RETURN_IF_ERROR(convert_column());
}
return Status::OK();
}
diff --git a/be/test/format/parquet/parquet_column_chunk_reader_test.cpp
b/be/test/format/parquet/parquet_column_chunk_reader_test.cpp
index be9616c523f..d4327ee05d5 100644
--- a/be/test/format/parquet/parquet_column_chunk_reader_test.cpp
+++ b/be/test/format/parquet/parquet_column_chunk_reader_test.cpp
@@ -27,11 +27,13 @@
#include "core/assert_cast.h"
#include "core/column/column_string.h"
+#include "core/data_type/data_type_string.h"
#include "format/parquet/schema_desc.h"
#include "format/parquet/vparquet_column_chunk_reader.h"
#include "format/parquet/vparquet_column_reader.h"
#include "io/fs/buffered_reader.h"
#include "io/fs/file_reader.h"
+#include "io/io_common.h"
#include "runtime/runtime_state.h"
#include "util/coding.h"
#include "util/thrift_util.h"
@@ -418,6 +420,41 @@ TEST(ParquetColumnChunkReaderTest,
ScalarDictionaryReadUsesExplicitProbe) {
EXPECT_EQ(std::string(strings.get_data_at(2)), "carol");
}
+TEST(ParquetColumnChunkReaderTest, ScalarNestedReadRestoresColumnWhenStopped) {
+ ColumnChunkFixture fixture;
+ ASSERT_TRUE(make_plain_fixture(&fixture).ok());
+ auto file_reader =
std::make_shared<CountingFileReader>(std::move(fixture.data));
+ auto string_type = std::make_shared<DataTypeString>();
+ fixture.field_schema.data_type = string_type;
+ fixture.field_schema.parquet_schema.__set_type(tparquet::Type::BYTE_ARRAY);
+
+ RowRanges row_ranges;
+ row_ranges.add({0, 1});
+ io::IOContext io_ctx;
+ ScalarColumnReader<true, false> reader(row_ranges, 1, fixture.chunk,
nullptr, nullptr, &io_ctx);
+ reader.set_column_in_nested();
+
+ TQueryOptions query_options;
+ query_options.__set_enable_parquet_file_page_cache(false);
+ RuntimeState runtime_state(query_options, TQueryGlobals());
+ ASSERT_TRUE(reader.init(file_reader, &fixture.field_schema,
+ /*max_buf_size=*/1024 * 1024, &runtime_state)
+ .ok());
+
+ ColumnPtr column = ColumnString::create();
+ FilterMap filter_map;
+ ASSERT_TRUE(filter_map.init(nullptr, 0, false).ok());
+ size_t read_rows = 0;
+ bool eof = false;
+ io_ctx.should_stop = true;
+
+ Status status = reader.read_column_data(column, string_type, nullptr,
filter_map, 1, &read_rows,
+ &eof, false);
+ EXPECT_TRUE(status.is<ErrorCode::END_OF_FILE>()) << status;
+ ASSERT_TRUE(column);
+ EXPECT_TRUE(column->empty());
+}
+
void expect_offset_index_skip(ColumnChunkFixture fixture) {
CountingBufferedReader buffered_reader(std::move(fixture.data));
ParquetPageReadContext page_read_ctx(false);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
