From 2bffb2d9116892055f348d0a61eaf6c12e2020e0 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 12:55:41 +0800 Subject: [PATCH 1/2] [fix](be) Reject lossy file predicate literal conversion ### What problem does this PR solve? Issue Number: N/A Related PR: N/A Problem Summary: FileScannerV2 localized predicate literals to the file column type whenever conversion succeeded, even when the conversion changed the literal value. For example, localizing a DOUBLE predicate value of 1.5 to an INT file column changed the boundary to 1 and could make file-level filtering silently drop matching rows. Require an exact round trip through the file type before localizing a literal; otherwise, preserve table semantics by casting the file slot. Exact conversions such as BIGINT 1 to INT 1 remain localized. ### Release note Fix FileScannerV2 predicate localization to avoid missing rows after lossy literal conversion. ### Check List (For Author) - Test: Unit Test - ColumnMapperCastTest targeted literal localization tests - Behavior changed: Yes, lossy literal conversions now fall back to casting the file slot. - Does this need documentation: No --- be/src/format_v2/column_mapper.cpp | 15 +++++ be/test/format_v2/column_mapper_test.cpp | 80 ++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/be/src/format_v2/column_mapper.cpp b/be/src/format_v2/column_mapper.cpp index e60e22b85e7fdf..fd922d333e238a 100644 --- a/be/src/format_v2/column_mapper.cpp +++ b/be/src/format_v2/column_mapper.cpp @@ -553,6 +553,21 @@ static VExprSPtr rewrite_literal_to_file_type(const VExprSPtr& literal_expr, if (file_field.get_type() != remove_nullable(rewrite_info.file_type)->get_primitive_type()) { return nullptr; } + Field round_trip_field; + try { + convert_field_to_type(file_field, *original_literal->data_type(), &round_trip_field, + rewrite_info.file_type.get()); + } catch (const Exception&) { + return nullptr; + } + // Only localize a literal when converting it to the file type is lossless. For example, + // BIGINT 1 -> INT 1 -> BIGINT 1 succeeds. However, for a table predicate `value < 1.5` + // with DOUBLE table type and INT file type, rewriting 1.5 to INT 1 would make a file value + // of 1 fail `value < 1` and silently drop a matching row. Its round trip + // DOUBLE 1.5 -> INT 1 -> DOUBLE 1.0 fails here and keeps the table-typed predicate instead. + if (round_trip_field != original_field) { + return nullptr; + } auto literal = std::make_shared( rewrite_info.file_type, file_field, original_literal->data_type(), original_field); rewrite_context->add_created_expr(literal); diff --git a/be/test/format_v2/column_mapper_test.cpp b/be/test/format_v2/column_mapper_test.cpp index 4e815d6cbf3c5e..0e5e51c62cf020 100644 --- a/be/test/format_v2/column_mapper_test.cpp +++ b/be/test/format_v2/column_mapper_test.cpp @@ -3486,6 +3486,44 @@ TEST_F(ColumnMapperCastTest, ColumnMapperCastsLiteralForLiteralSlotPredicateType file_request.conjuncts[0]->close(); } +// Scenario: a fractional table literal cannot be localized to an integral file type without +// changing the predicate boundary, so the mapper must cast the file slot instead. +TEST_F(ColumnMapperCastTest, ColumnMapperRejectsLossyBinaryLiteralConversion) { + TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_NAME}); + auto table_column = name_col("value", f64()); + std::vector projected_columns {table_column}; + + auto file_field = name_col("value", i32(), 0); + std::vector file_schema {file_field}; + + auto status = mapper.create_mapping(projected_columns, {}, file_schema); + ASSERT_TRUE(status.ok()) << status; + + auto predicate = binary_predicate( + TExprOpcode::LT, VSlotRef::create_shared(0, 0, -1, table_column.type, "value"), + VLiteral::create_shared(table_column.type, Field::create_field(1.5))); + TableFilter table_filter; + table_filter.conjunct = VExprContext::create_shared(predicate); + table_filter.global_indices = {GlobalIndex(0)}; + + FileScanRequest file_request; + ASSERT_TRUE(mapper.create_scan_request({table_filter}, projected_columns, &file_request, &state) + .ok()); + ASSERT_EQ(file_request.conjuncts.size(), 1); + const auto& localized_expr = file_request.conjuncts[0]->root(); + ASSERT_EQ(localized_expr->get_num_children(), 2); + const auto& localized_slot_cast = localized_expr->children()[0]; + ASSERT_NE(dynamic_cast(localized_slot_cast.get()), nullptr); + EXPECT_TRUE(localized_slot_cast->data_type()->equals(*table_column.type)); + ASSERT_EQ(localized_slot_cast->get_num_children(), 1); + const auto* localized_slot = + assert_cast(localized_slot_cast->children()[0].get()); + EXPECT_EQ(localized_slot->column_id(), 0); + EXPECT_TRUE(localized_slot->data_type()->equals(*file_field.type)); + EXPECT_TRUE(localized_expr->children()[1]->is_literal()); + EXPECT_TRUE(localized_expr->children()[1]->data_type()->equals(*table_column.type)); +} + // Scenario: IN predicate literals are all rewritten to file type when every literal conversion is safe. TEST_F(ColumnMapperCastTest, ColumnMapperCastsInPredicateLiteralsForTypeMismatch) { TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_NAME}); @@ -3524,6 +3562,48 @@ TEST_F(ColumnMapperCastTest, ColumnMapperCastsInPredicateLiteralsForTypeMismatch EXPECT_TRUE(localized_expr->children()[2]->data_type()->equals(*file_field.type)); } +// Scenario: one lossy IN literal prevents the entire predicate from being localized to file type. +TEST_F(ColumnMapperCastTest, ColumnMapperRejectsLossyInPredicateLiteralConversion) { + TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_NAME}); + auto table_column = name_col("value", f64()); + std::vector projected_columns {table_column}; + + auto file_field = name_col("value", i32(), 0); + std::vector file_schema {file_field}; + + auto status = mapper.create_mapping(projected_columns, {}, file_schema); + ASSERT_TRUE(status.ok()) << status; + + auto predicate = create_in_predicate(); + predicate->add_child(VSlotRef::create_shared(0, 0, -1, table_column.type, "value")); + predicate->add_child( + VLiteral::create_shared(table_column.type, Field::create_field(1.0))); + predicate->add_child( + VLiteral::create_shared(table_column.type, Field::create_field(1.5))); + TableFilter table_filter; + table_filter.conjunct = VExprContext::create_shared(predicate); + table_filter.global_indices = {GlobalIndex(0)}; + + FileScanRequest file_request; + ASSERT_TRUE(mapper.create_scan_request({table_filter}, projected_columns, &file_request, &state) + .ok()); + ASSERT_EQ(file_request.conjuncts.size(), 1); + const auto& localized_expr = file_request.conjuncts[0]->root(); + ASSERT_EQ(localized_expr->get_num_children(), 3); + const auto& localized_slot_cast = localized_expr->children()[0]; + ASSERT_NE(dynamic_cast(localized_slot_cast.get()), nullptr); + EXPECT_TRUE(localized_slot_cast->data_type()->equals(*table_column.type)); + ASSERT_EQ(localized_slot_cast->get_num_children(), 1); + const auto* localized_slot = + assert_cast(localized_slot_cast->children()[0].get()); + EXPECT_EQ(localized_slot->column_id(), 0); + EXPECT_TRUE(localized_slot->data_type()->equals(*file_field.type)); + EXPECT_TRUE(localized_expr->children()[1]->is_literal()); + EXPECT_TRUE(localized_expr->children()[1]->data_type()->equals(*table_column.type)); + EXPECT_TRUE(localized_expr->children()[2]->is_literal()); + EXPECT_TRUE(localized_expr->children()[2]->data_type()->equals(*table_column.type)); +} + // Scenario: IN predicate falls back to casting the file slot when any literal cannot be converted safely. TEST_F(ColumnMapperCastTest, ColumnMapperFallsBackToSlotCastWhenInPredicateLiteralRewriteFails) { TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_NAME}); From 35866333ef976d0e641d984e2047a14b63d469df Mon Sep 17 00:00:00 2001 From: Gabriel Date: Sun, 12 Jul 2026 13:51:46 +0800 Subject: [PATCH 2/2] [fix](be) Restrict file literal localization to lossless casts ### What problem does this PR solve? Issue Number: N/A Related PR: #65490 Problem Summary: Round-trip equality alone can accept unsafe predicate localization when file values lose information during table materialization or when complex Field equality ignores nested contents. Restrict localization to scalar numeric file-to-table casts that preserve every file value, retain the exact literal round-trip check, and fall back to the table predicate for complex or unsupported casts. ### Release note Prevent file predicate localization from dropping rows for lossy schema-evolution casts. ### Check List (For Author) - Test: Unit Test - ColumnMapperCastTest (19 tests passed) - Behavior changed: Yes. Unsafe literal localization now falls back to evaluating the table-typed predicate after materialization. - Does this need documentation: No --- be/src/format_v2/column_mapper.cpp | 83 ++++++++++++++++++++++-- be/test/format_v2/column_mapper_test.cpp | 67 +++++++++++++++++++ 2 files changed, 145 insertions(+), 5 deletions(-) diff --git a/be/src/format_v2/column_mapper.cpp b/be/src/format_v2/column_mapper.cpp index fd922d333e238a..895e2fe2c91270 100644 --- a/be/src/format_v2/column_mapper.cpp +++ b/be/src/format_v2/column_mapper.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -530,6 +531,71 @@ static void collect_top_level_slot_columns(const VExprSPtr& expr, } } +static std::optional signed_integer_width(PrimitiveType type) { + switch (type) { + case TYPE_TINYINT: + return 8; + case TYPE_SMALLINT: + return 16; + case TYPE_INT: + return 32; + case TYPE_BIGINT: + return 64; + case TYPE_LARGEINT: + return 128; + default: + return std::nullopt; + } +} + +static std::optional floating_width(PrimitiveType type) { + switch (type) { + case TYPE_FLOAT: + return 32; + case TYPE_DOUBLE: + return 64; + default: + return std::nullopt; + } +} + +static std::optional floating_exact_integer_width(PrimitiveType type) { + switch (type) { + case TYPE_FLOAT: + return 24; + case TYPE_DOUBLE: + return 53; + default: + return std::nullopt; + } +} + +static bool is_lossless_file_to_table_numeric_cast(const DataTypePtr& file_type, + const DataTypePtr& table_type) { + const auto file_nested_type = remove_nullable(file_type); + const auto table_nested_type = remove_nullable(table_type); + if (file_nested_type->equals(*table_nested_type)) { + return true; + } + + const auto file_primitive_type = file_nested_type->get_primitive_type(); + const auto table_primitive_type = table_nested_type->get_primitive_type(); + if (const auto file_width = signed_integer_width(file_primitive_type)) { + if (const auto table_width = signed_integer_width(table_primitive_type)) { + return *table_width >= *file_width; + } + if (const auto table_width = floating_exact_integer_width(table_primitive_type)) { + return *table_width >= *file_width; + } + return false; + } + if (const auto file_width = floating_width(file_primitive_type)) { + const auto table_width = floating_width(table_primitive_type); + return table_width.has_value() && *table_width >= *file_width; + } + return false; +} + static VExprSPtr rewrite_literal_to_file_type(const VExprSPtr& literal_expr, const FileSlotRewriteInfo& rewrite_info, RewriteContext* rewrite_context) { @@ -540,6 +606,16 @@ static VExprSPtr rewrite_literal_to_file_type(const VExprSPtr& literal_expr, if (rewrite_info.file_type->equals(*original_literal->data_type())) { return original_literal; } + // A literal round trip alone cannot prove that file-local evaluation is safe: the file slot + // itself may lose information when materialized as the table type. For example, DOUBLE 1.5 + // becomes BIGINT 1, so table predicate `value = 1` is true while file predicate + // `value = 1.0` is false. Complex Field equality also does not compare nested contents. + // Restrict localization to scalar numeric casts that preserve every file value; unsupported + // and complex casts keep the table predicate and evaluate after materialization. + if (!is_lossless_file_to_table_numeric_cast(rewrite_info.file_type, + original_literal->data_type())) { + return nullptr; + } Field file_field; try { convert_field_to_type(original_field, *rewrite_info.file_type, &file_field, @@ -560,11 +636,8 @@ static VExprSPtr rewrite_literal_to_file_type(const VExprSPtr& literal_expr, } catch (const Exception&) { return nullptr; } - // Only localize a literal when converting it to the file type is lossless. For example, - // BIGINT 1 -> INT 1 -> BIGINT 1 succeeds. However, for a table predicate `value < 1.5` - // with DOUBLE table type and INT file type, rewriting 1.5 to INT 1 would make a file value - // of 1 fail `value < 1` and silently drop a matching row. Its round trip - // DOUBLE 1.5 -> INT 1 -> DOUBLE 1.0 fails here and keeps the table-typed predicate instead. + // The file-to-table type check protects every possible file value. This round trip separately + // proves that the specific predicate boundary is exactly representable in the file type. if (round_trip_field != original_field) { return nullptr; } diff --git a/be/test/format_v2/column_mapper_test.cpp b/be/test/format_v2/column_mapper_test.cpp index 0e5e51c62cf020..90d4e7b8d552d8 100644 --- a/be/test/format_v2/column_mapper_test.cpp +++ b/be/test/format_v2/column_mapper_test.cpp @@ -3524,6 +3524,73 @@ TEST_F(ColumnMapperCastTest, ColumnMapperRejectsLossyBinaryLiteralConversion) { EXPECT_TRUE(localized_expr->children()[1]->data_type()->equals(*table_column.type)); } +// Scenario: an exactly representable literal is still unsafe to localize when arbitrary file +// values lose information during materialization to the table type. +TEST_F(ColumnMapperCastTest, ColumnMapperRejectsLossyFileToTableConversion) { + TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_NAME}); + auto table_column = name_col("value", i64()); + std::vector projected_columns {table_column}; + + auto file_field = name_col("value", f64(), 0); + std::vector file_schema {file_field}; + + auto status = mapper.create_mapping(projected_columns, {}, file_schema); + ASSERT_TRUE(status.ok()) << status; + + auto predicate = binary_predicate( + TExprOpcode::EQ, VSlotRef::create_shared(0, 0, -1, table_column.type, "value"), + VLiteral::create_shared(table_column.type, Field::create_field(1))); + TableFilter table_filter; + table_filter.conjunct = VExprContext::create_shared(predicate); + table_filter.global_indices = {GlobalIndex(0)}; + + FileScanRequest file_request; + ASSERT_TRUE(mapper.create_scan_request({table_filter}, projected_columns, &file_request, &state) + .ok()); + ASSERT_EQ(file_request.conjuncts.size(), 1); + const auto& localized_expr = file_request.conjuncts[0]->root(); + ASSERT_EQ(localized_expr->get_num_children(), 2); + const auto& localized_slot_cast = localized_expr->children()[0]; + ASSERT_NE(dynamic_cast(localized_slot_cast.get()), nullptr); + EXPECT_TRUE(localized_slot_cast->data_type()->equals(*table_column.type)); + ASSERT_EQ(localized_slot_cast->get_num_children(), 1); + const auto* localized_slot = + assert_cast(localized_slot_cast->children()[0].get()); + EXPECT_EQ(localized_slot->column_id(), 0); + EXPECT_TRUE(localized_slot->data_type()->equals(*file_field.type)); + EXPECT_TRUE(localized_expr->children()[1]->data_type()->equals(*table_column.type)); +} + +// Scenario: complex Field equality does not compare nested values, so complex literals must not +// use the scalar round-trip guard. +TEST_F(ColumnMapperCastTest, ColumnMapperRejectsComplexLiteralLocalization) { + TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_NAME}); + auto table_column = array_col("value", -1, name_col("element", f64())); + set_name_identifiers(&table_column, -1); + const auto& table_type = table_column.type; + std::vector projected_columns {table_column}; + + auto file_field = array_col("value", -1, name_col("element", i32()), 0); + set_name_identifiers(&file_field, 0); + std::vector file_schema {file_field}; + + auto status = mapper.create_mapping(projected_columns, {}, file_schema); + ASSERT_TRUE(status.ok()) << status; + + Array literal_values {Field::create_field(1.5)}; + auto predicate = binary_predicate( + TExprOpcode::EQ, VSlotRef::create_shared(0, 0, -1, table_type, "value"), + VLiteral::create_shared(table_type, Field::create_field(literal_values))); + TableFilter table_filter; + table_filter.conjunct = VExprContext::create_shared(predicate); + table_filter.global_indices = {GlobalIndex(0)}; + + FileScanRequest file_request; + ASSERT_TRUE(mapper.create_scan_request({table_filter}, projected_columns, &file_request, &state) + .ok()); + EXPECT_TRUE(file_request.conjuncts.empty()); +} + // Scenario: IN predicate literals are all rewritten to file type when every literal conversion is safe. TEST_F(ColumnMapperCastTest, ColumnMapperCastsInPredicateLiteralsForTypeMismatch) { TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_NAME});