Skip to content

[fix](be) Reject lossy file predicate literal conversion #65490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Gabriel39 wants to merge 2 commits into from
+235 −0
Closed

[fix](be) Reject lossy file predicate literal conversion #65490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2bffb2d
[fix](be) Reject lossy file predicate literal conversion
Gabriel39 Jul 12, 2026
3586633
[fix](be) Restrict file literal localization to lossless casts
Gabriel39 Jul 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions be/src/format_v2/column_mapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <algorithm>
#include <cstddef>
#include <memory>
#include <optional>
#include <sstream>
#include <string_view>
#include <utility>
Expand Down Expand Up @@ -530,6 +531,71 @@ static void collect_top_level_slot_columns(const VExprSPtr& expr,
}
}

static std::optional<uint8_t> 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<uint8_t> floating_width(PrimitiveType type) {
switch (type) {
case TYPE_FLOAT:
return 32;
case TYPE_DOUBLE:
return 64;
default:
return std::nullopt;
}
}

static std::optional<uint8_t> 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) {
Expand All @@ -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,
Expand All @@ -553,6 +629,18 @@ 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;
}
// 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) {
Comment thread
Gabriel39 marked this conversation as resolved.
return nullptr;
}
auto literal = std::make_shared<SplitLocalFileLiteral>(
rewrite_info.file_type, file_field, original_literal->data_type(), original_field);
rewrite_context->add_created_expr(literal);
Expand Down
147 changes: 147 additions & 0 deletions be/test/format_v2/column_mapper_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3486,6 +3486,111 @@ 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<ColumnDefinition> projected_columns {table_column};

auto file_field = name_col("value", i32(), 0);
std::vector<ColumnDefinition> 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<TYPE_DOUBLE>(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<const 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<const VSlotRef*>(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: 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<ColumnDefinition> projected_columns {table_column};

auto file_field = name_col("value", f64(), 0);
std::vector<ColumnDefinition> 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<TYPE_BIGINT>(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<const 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<const VSlotRef*>(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<ColumnDefinition> projected_columns {table_column};

auto file_field = array_col("value", -1, name_col("element", i32()), 0);
set_name_identifiers(&file_field, 0);
std::vector<ColumnDefinition> file_schema {file_field};

auto status = mapper.create_mapping(projected_columns, {}, file_schema);
ASSERT_TRUE(status.ok()) << status;

Array literal_values {Field::create_field<TYPE_DOUBLE>(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<TYPE_ARRAY>(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});
Expand Down Expand Up @@ -3524,6 +3629,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<ColumnDefinition> projected_columns {table_column};

auto file_field = name_col("value", i32(), 0);
std::vector<ColumnDefinition> 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<TYPE_DOUBLE>(1.0)));
predicate->add_child(
VLiteral::create_shared(table_column.type, Field::create_field<TYPE_DOUBLE>(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<const 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<const VSlotRef*>(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});
Expand Down
Loading