github-actions[bot] commented on code in PR #63092: URL: https://github.com/apache/doris/pull/63092#discussion_r3279782081
########## be/test/exprs/vcondition_expr_test.cpp: ########## @@ -0,0 +1,362 @@ +// 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 "exprs/vcondition_expr.h" + +#include <gen_cpp/Exprs_types.h> +#include <gen_cpp/Types_types.h> +#include <gmock/gmock.h> +#include <gtest/gtest.h> + +#include <cmath> +#include <limits> +#include <memory> +#include <vector> + +#include "core/column/column_nullable.h" +#include "core/column/column_vector.h" +#include "core/data_type/data_type_nullable.h" +#include "core/data_type/data_type_number.h" +#include "exprs/vexpr.h" +#include "exprs/vexpr_context.h" + +namespace doris { + +// Build a minimal TExprNode as the input of VectorizedCoalesceExpr. +// Only fields required by the VExpr base ctor (so that create_data_type works) are set. +static TExprNode make_coalesce_node(TPrimitiveType::type ptype, bool is_nullable) { + TExprNode node; + node.node_type = TExprNodeType::FUNCTION_CALL; + node.num_children = 0; + node.__set_is_nullable(is_nullable); + + TTypeDesc type_desc; + TTypeNode type_node; + type_node.type = TTypeNodeType::SCALAR; + TScalarType scalar_type; + scalar_type.__set_type(ptype); + type_node.__set_scalar_type(scalar_type); + type_desc.types.push_back(type_node); + node.__set_type(type_desc); + + TFunction fn; + TFunctionName fn_name; + fn_name.function_name = "coalesce"; + fn.name = fn_name; + node.__set_fn(fn); + + return node; +} + +// Mock child expression: returns the pre-injected ColumnPtr / DataTypePtr to the parent expr. +// Behavior is modeled after MockVExprForTryCast in try_cast_expr_test.cpp. +class MockChildVExpr : public VExpr { +public: + MockChildVExpr(ColumnPtr column, DataTypePtr type) + : _column(std::move(column)), _type(std::move(type)) {} + + MOCK_CONST_METHOD0(clone, VExprSPtr()); + + const std::string& expr_name() const override { return _name; } + + Status execute(VExprContext* context, Block* block, int* result_column_id) const override { + block->insert({_column, _type, "mock_child"}); + *result_column_id = static_cast<int>(block->columns()) - 1; + return Status::OK(); + } + + Status execute_column_impl(VExprContext* /*context*/, const Block* /*block*/, + const Selector* /*selector*/, size_t /*count*/, + ColumnPtr& result_column) const override { + result_column = _column; + return Status::OK(); + } + + DataTypePtr execute_type(const Block* /*block*/) const override { return _type; } + +private: + ColumnPtr _column; + DataTypePtr _type; + std::string _name = "mock_child"; +}; + +// Helper: build a nullable Float64 column from a list of (value, is_null) pairs. +static ColumnPtr make_nullable_float64_column(const std::vector<std::pair<double, bool>>& values) { + auto nested = ColumnFloat64::create(); + auto null_map = ColumnUInt8::create(); + for (auto& [v, is_null] : values) { + nested->insert_value(v); + null_map->insert_value(is_null ? 1 : 0); + } + return ColumnNullable::create(std::move(nested), std::move(null_map)); +} + +// Helper: build a non-nullable Float64 column from a list of doubles. +static ColumnPtr make_float64_column(const std::vector<double>& values) { + auto col = ColumnFloat64::create(); + for (auto v : values) { + col->insert_value(v); + } + return col; +} + +// Helper: extract the Float64 value at `row` from the result column, +// handling both nullable and non-nullable cases. +static double get_float64_value(const ColumnPtr& column, size_t row, bool* is_null = nullptr) { + if (const auto* nullable = check_and_get_column<ColumnNullable>(column.get())) { + if (is_null) { + *is_null = nullable->is_null_at(row); + } + const auto& nested = + assert_cast<const ColumnFloat64&>(nullable->get_nested_column()).get_data(); + return nested[row]; + } + if (is_null) { + *is_null = false; + } + return assert_cast<const ColumnFloat64*>(column.get())->get_data()[row]; +} + +class VConditionExprCoalesceTest : public ::testing::Test {}; + +// Targets the fix: after some rows of col0 are filled, NaN/Inf values in the +// same rows of later columns must not pollute the already-filled result via +// "value * 0" arithmetic. +// Input: +// col0 (nullable, double): [1.0, NULL] +// col1 (non-nullable, double): [NaN, 100.0] +// Expected coalesce result: [1.0, 100.0] +TEST_F(VConditionExprCoalesceTest, Float64_NaN_NotPolluteResult) { + auto coalesce_node = make_coalesce_node(TPrimitiveType::DOUBLE, /*is_nullable=*/true); + auto coalesce_expr = VectorizedCoalesceExpr::create_shared(coalesce_node); + + // _data_type is already set to Nullable(Float64) by the base ctor; reassert it explicitly. + coalesce_expr->_data_type = Review Comment: This test file will not compile because it writes `coalesce_expr->_data_type` here and `coalesce_expr->_open_finished` below, but both members are `protected` in `VExpr` (`be/src/exprs/vexpr.h`). The test is neither a derived class nor a friend of `VExpr`/`VectorizedCoalesceExpr`, so the BE UT target fails at compile time before exercising the regression. Please avoid direct protected-member access, for example use the public `data_type()` accessor if the type really needs overriding and remove the `_open_finished` assignments since these tests call `execute_column_impl()` directly. The same pattern appears in the other test cases in this file as well. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
