From 841a656513d1b3e0490260d89afc8dc6d0d8a62c Mon Sep 17 00:00:00 2001 From: Mryange Date: Tue, 25 Feb 2025 15:45:41 +0800 Subject: [PATCH 1/2] [fix](column) fix ColumnWithTypeAndName::get_nested use-after-free when input Const(Nullable) column (#48288) ### What problem does this PR solve? error code ``` ColumnPtr nested_column = column; if (column) { nested_column = nested_column->convert_to_full_column_if_const(); const auto* source_column = assert_cast(nested_column.get()); nested_column = source_column->get_nested_column_ptr(); ``` If column is a const(nullable) column, execute: nested_column = nested_column->convert_to_full_column_if_const(); nested_column points to a new nullable column. Execute: const auto* source_column = assert_cast(nested_column.get()); source_column, this raw pointer, points to the new nullable column (but does not have ownership). Execute: nested_column = source_column->get_nested_column_ptr(); nested_column points to the nested column of the new nullable column, and the original nullable column is released. ### Release note None ### Check List (For Author) - Test - [ ] 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 - Behavior changed: - [x] No. - [ ] Yes. - Does this need documentation? - [x] No. - [ ] Yes. ### Check List (For Reviewer who merge this PR) - [x] Confirm the release note - [x] Confirm test cases - [x] Confirm document - [x] Add branch pick label --- be/src/vec/core/column_with_type_and_name.cpp | 5 ++- .../core/column_with_type_and_name_test.cpp | 40 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 be/test/vec/core/column_with_type_and_name_test.cpp diff --git a/be/src/vec/core/column_with_type_and_name.cpp b/be/src/vec/core/column_with_type_and_name.cpp index cd0f7194004073..efd887b194b16f 100644 --- a/be/src/vec/core/column_with_type_and_name.cpp +++ b/be/src/vec/core/column_with_type_and_name.cpp @@ -93,8 +93,9 @@ ColumnWithTypeAndName ColumnWithTypeAndName::get_nested(bool replace_null_data_t auto nested_type = assert_cast(type.get())->get_nested_type(); ColumnPtr nested_column = column; if (column) { - nested_column = nested_column->convert_to_full_column_if_const(); - const auto* source_column = assert_cast(nested_column.get()); + // A column_ptr is needed here to ensure that the column in convert_to_full_column_if_const is not released. + auto column_ptr = nested_column->convert_to_full_column_if_const(); + const auto* source_column = assert_cast(column_ptr.get()); nested_column = source_column->get_nested_column_ptr(); if (replace_null_data_to_default) { diff --git a/be/test/vec/core/column_with_type_and_name_test.cpp b/be/test/vec/core/column_with_type_and_name_test.cpp new file mode 100644 index 00000000000000..1cdda959a82fb9 --- /dev/null +++ b/be/test/vec/core/column_with_type_and_name_test.cpp @@ -0,0 +1,40 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "vec/core/column_with_type_and_name.h" + +#include + +#include "testutil/column_helper.h" +#include "vec/core/types.h" +#include "vec/data_types/data_type_nullable.h" +#include "vec/data_types/data_type_number.h" + +namespace doris::vectorized { + +TEST(ColumnWithTypeAndNameTest, get_nested_test) { + ColumnWithTypeAndName column_with_type_and_name; + auto null_column = ColumnNullable::create(ColumnHelper::create_column({1}), + ColumnHelper::create_column({true})); + column_with_type_and_name.column = ColumnConst::create(null_column, 3); + column_with_type_and_name.type = + std::make_shared(std::make_shared()); + column_with_type_and_name.name = "column_with_type_and_name"; + column_with_type_and_name.get_nested(true); +} + +} // namespace doris::vectorized From 8c40cbef3411fc4a0c6944688e47c6a196c550cf Mon Sep 17 00:00:00 2001 From: Mryange Date: Wed, 19 Mar 2025 17:37:18 +0800 Subject: [PATCH 2/2] column helper --- be/test/testutil/column_helper.cpp | 39 ++++++++++ be/test/testutil/column_helper.h | 116 +++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 be/test/testutil/column_helper.cpp create mode 100644 be/test/testutil/column_helper.h diff --git a/be/test/testutil/column_helper.cpp b/be/test/testutil/column_helper.cpp new file mode 100644 index 00000000000000..b00a681ec72cdf --- /dev/null +++ b/be/test/testutil/column_helper.cpp @@ -0,0 +1,39 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +#include "column_helper.h" + +#include + +#include "vec/data_types/data_type_number.h" + +namespace doris::vectorized { + +TEST(ColumnHelperTest, test) { + EXPECT_TRUE(ColumnHelper::column_equal( + ColumnHelper::create_column({1, 2, 3, 4, 5}), + ColumnHelper::create_column({1, 2, 3, 4, 5}))); + + EXPECT_FALSE(ColumnHelper::column_equal( + ColumnHelper::create_column({1, 2, 3, 4, 5}), + ColumnHelper::create_column({1, 2, 3, 4, 5, 6}))); + + EXPECT_FALSE(ColumnHelper::column_equal( + ColumnHelper::create_column({1, 1, 3, 4, 5}), + ColumnHelper::create_column({1, 2, 3, 4, 5}))); +} + +} // namespace doris::vectorized diff --git a/be/test/testutil/column_helper.h b/be/test/testutil/column_helper.h new file mode 100644 index 00000000000000..a9cf58bb880599 --- /dev/null +++ b/be/test/testutil/column_helper.h @@ -0,0 +1,116 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +#include +#include +#include + +#include "vec/columns/column_nullable.h" +#include "vec/core/block.h" +#include "vec/data_types/data_type_string.h" + +namespace doris::vectorized { +struct ColumnHelper { +public: + template + static ColumnPtr create_column(const std::vector& data) { + auto column = DataType::ColumnType::create(); + if constexpr (std::is_same_v) { + for (const auto& datum : data) { + column->insert_data(datum.data(), datum.size()); + } + } else { + for (const auto& datum : data) { + column->insert_value(datum); + } + } + return std::move(column); + } + + template + static ColumnPtr create_nullable_column( + const std::vector& data, + const std::vector& null_map) { + auto null_col = ColumnUInt8::create(); + for (const auto& datum : null_map) { + null_col->insert_value(datum); + } + auto ptr = create_column(data); + return ColumnNullable::create(std::move(ptr), std::move(null_col)); + } + + static bool column_equal(const ColumnPtr& column1, const ColumnPtr& column2) { + if (column1->size() != column2->size()) { + return false; + } + for (size_t i = 0; i < column1->size(); i++) { + if (column1->compare_at(i, i, *column2, 1) != 0) { + return false; + } + } + return true; + } + + static bool block_equal(const Block& block1, const Block& block2) { + if (block1.columns() != block2.columns()) { + return false; + } + for (size_t i = 0; i < block1.columns(); i++) { + if (!column_equal(block1.get_by_position(i).column, block2.get_by_position(i).column)) { + return false; + } + } + return true; + } + + template + static Block create_block(const std::vector& data) { + auto column = create_column(data); + auto data_type = std::make_shared(); + Block block({ColumnWithTypeAndName(column, data_type, "column")}); + return block; + } + + template + static Block create_nullable_block(const std::vector& data, + const std::vector& null_map) { + auto column = create_nullable_column(data, null_map); + auto data_type = std::make_shared(std::make_shared()); + Block block({ColumnWithTypeAndName(column, data_type, "column")}); + return block; + } + + template + static ColumnWithTypeAndName create_column_with_name( + const std::vector& datas) { + auto column = create_column(datas); + auto data_type = std::make_shared(); + return ColumnWithTypeAndName(column, data_type, "column"); + } + + template + static ColumnWithTypeAndName create_nullable_column_with_name( + const std::vector& datas, + const std::vector& null_map) { + auto column = create_nullable_column(datas, null_map); + auto data_type = std::make_shared(std::make_shared()); + return ColumnWithTypeAndName(column, data_type, "column"); + } +}; +} // namespace doris::vectorized