github-actions[bot] commented on code in PR #69711:
URL: https://github.com/apache/doris/pull/69711#discussion_r4395446127
##########
be/src/format_v2/table/iceberg_reader.cpp:
##########
@@ -1569,11 +1682,18 @@ Status
IcebergTableReader::_build_missing_equality_delete_key_expr(
const auto* table_leaf = missing_path.back();
DORIS_CHECK(table_leaf != nullptr);
DORIS_CHECK(table_leaf->type != nullptr);
- if (!table_leaf->type->equals(*filter.key_types[key_idx])) {
- auto cast_expr =
EvolutionCast::create_shared(filter.key_types[key_idx]);
- cast_expr->add_child(default_expr);
- default_expr = std::move(cast_expr);
+ if (missing_root.initial_default_value.has_value()) {
Review Comment:
[P2] Recognize NULL leaf defaults inside a struct default
A non-null default on `missing_root` does not mean the extracted key is
non-null: [Iceberg permits a struct default `{}` whose child defaults to
NULL](https://iceberg.apache.org/spec/#default-values). Consider a missing
`payload` struct containing only optional `k UNKNOWN`, with parent default `{}`
and child default NULL. Promote `k` to DATE without a snapshot, prepare a NULL
DATE equality delete, drop `payload`, commit the delete, then clean expired
metadata while retaining the UNKNOWN snapshot. Only the historical UNKNOWN
definition remains, so FE sends `k` as STRING. The recursive default builder
correctly produces a NULL STRING leaf, but this branch sees the parent's `{}`
and rejects DATE/STRING before NULL matching. Previously the NULL tombstone and
default matched in STRING. Determine whether the extracted leaf is provably
NULL, including ancestor defaults, and use the tombstone domain for that case;
extend the missing-whole-struct test with this history.
##########
be/src/format_v2/table/iceberg_reader.cpp:
##########
@@ -974,6 +974,114 @@ static Status build_equality_delete_key_expr(
return build_nested_equality_delete_key_expr(path, std::move(root_expr),
key_expr);
}
+static int equality_delete_integer_width(PrimitiveType type) {
+ switch (type) {
+ case TYPE_TINYINT:
+ return 1;
+ case TYPE_SMALLINT:
+ return 2;
+ case TYPE_INT:
+ return 4;
+ case TYPE_BIGINT:
+ return 8;
+ case TYPE_LARGEINT:
+ return 16;
+ default:
+ return 0;
+ }
+}
+
+// Whether every `from` value converts exactly to a `to` value that compares
equal to the same
+// Iceberg value. These are the Iceberg promotions (INT -> LONG, FLOAT ->
DOUBLE, decimal
+// precision, DATE -> TIMESTAMP and DATE -> TIMESTAMP_NS), a wider fractional
scale of the same
+// date-time kind, and STRING -> VARBINARY: an Iceberg BINARY key decodes as
either one, depending
+// on enable_mapping_varbinary, and both hold the same bytes. TIMESTAMP ->
TIMESTAMP_NS is not an
+// Iceberg promotion, but a delete can be prepared under a nanosecond schema
that the table later
+// abandons. Scaling microseconds by 1000 is exact, whereas rounding the
tombstone
+// 12:00:00.000000500 to microseconds would delete the data key
12:00:00.000000.
+static bool is_equality_delete_widening(const DataTypePtr& from, const
DataTypePtr& to) {
+ const auto from_type = remove_nullable(from);
+ const auto to_type = remove_nullable(to);
+ if (from_type->equals(*to_type)) {
+ return true;
+ }
+ const auto from_primitive = from_type->get_primitive_type();
+ const auto to_primitive = to_type->get_primitive_type();
+ if (const int from_width = equality_delete_integer_width(from_primitive);
from_width > 0) {
+ return equality_delete_integer_width(to_primitive) >= from_width;
+ }
+ switch (from_primitive) {
+ case TYPE_FLOAT:
+ return to_primitive == TYPE_DOUBLE;
+ case TYPE_DECIMAL32:
+ case TYPE_DECIMAL64:
+ case TYPE_DECIMAL128I:
+ case TYPE_DECIMAL256:
+ return is_decimalv3(to_primitive) && to_type->get_scale() ==
from_type->get_scale() &&
+ to_type->get_precision() >= from_type->get_precision();
+ case TYPE_DATEV2:
+ return to_primitive == TYPE_DATETIMEV2 || to_primitive ==
TYPE_TIMESTAMP_NS;
+ case TYPE_DATETIMEV2:
Review Comment:
[P2] Resolve INT96 timestamp semantics before selecting the key type
With `enable_mapping_timestamp_tz=true`, an imported Spark/Hive Parquet file
can expose an Iceberg `timestamptz` key as `DATETIMEV2(6)` (INT96), while a
normal adjusted-UTC INT64 equality-delete file exposes the same logical key as
`TIMESTAMPTZ(6)`. [Iceberg's importer explicitly treats INT96 as a zoned
timestamp](https://github.com/apache/iceberg/blob/apache-iceberg-1.12.0/parquet/src/main/java/org/apache/iceberg/parquet/MessageTypeToType.java).
For an id-only query the hidden key reaches this selector from the raw file
schema, before `ParquetReader::open` applies projection timestamp semantics.
Neither widening direction accepts this pair, so the scan now fails with
`NotSupported`, even in UTC; previously both keys were converted to the table's
`TIMESTAMPTZ` type and the delete applied. Resolve the carrier's timestamp
semantics from the Iceberg field metadata before choosing the comparison
domain, and cover imported INT96 data against ordinary timestamp tombstones.
##########
be/src/format_v2/table/iceberg_reader.cpp:
##########
@@ -1597,37 +1716,54 @@ Status
IcebergTableReader::_build_missing_equality_delete_key_expr(
Status
IcebergTableReader::_append_equality_delete_predicates(format::FileScanRequest*
request) {
DORIS_CHECK(request != nullptr);
for (const auto& filter : _equality_delete_filters) {
- auto delete_predicate = std::make_shared<EqualityDeletePredicate>(
- filter.delete_block, filter.field_ids, filter.hash_index);
DCHECK_EQ(filter.field_ids.size(), filter.key_types.size());
+ DCHECK_EQ(filter.field_ids.size(), filter.delete_key_types.size());
+ VExprSPtrs key_exprs;
+ DataTypes comparison_types;
bool has_missing_key = false;
for (size_t idx = 0; idx < filter.field_ids.size(); ++idx) {
EqualityDeleteColumnPath data_path;
bool complete_path = false;
RETURN_IF_ERROR(
_find_equality_delete_data_field(filter, idx, &data_path,
&complete_path));
+ VExprSPtr key_expr;
+ DataTypePtr comparison_type;
if (!complete_path) {
- VExprSPtr key_expr;
-
RETURN_IF_ERROR(_build_missing_equality_delete_key_expr(filter, idx, data_path,
-
request, &key_expr));
- delete_predicate->add_child(key_expr);
+ RETURN_IF_ERROR(_build_missing_equality_delete_key_expr(
+ filter, idx, data_path, request, &key_expr,
&comparison_type));
has_missing_key = true;
- continue;
- }
- const auto* root = data_path.front();
- const auto* field = data_path.back();
- const auto field_column_id =
format::LocalColumnId(root->file_local_id());
- _append_file_scan_column(request, field_column_id,
&request->predicate_columns);
- const auto block_position =
request->local_positions.at(field_column_id).value();
- VExprSPtr key_expr;
- RETURN_IF_ERROR(build_equality_delete_key_expr(data_path,
block_position, &key_expr));
- if (field->type->equals(*filter.key_types[idx])) {
- delete_predicate->add_child(key_expr);
} else {
- auto cast_expr =
EvolutionCast::create_shared(filter.key_types[idx]);
- cast_expr->add_child(key_expr);
- delete_predicate->add_child(cast_expr);
+ const auto* root = data_path.front();
+ const auto field_column_id =
format::LocalColumnId(root->file_local_id());
+ _append_file_scan_column(request, field_column_id,
&request->predicate_columns);
+ const auto block_position =
request->local_positions.at(field_column_id).value();
+ RETURN_IF_ERROR(
+ build_equality_delete_key_expr(data_path,
block_position, &key_expr));
+ RETURN_IF_ERROR(equality_delete_comparison_type(
+ filter.field_ids[idx], filter.delete_key_types[idx],
key_expr->data_type(),
+ _data_file_path(), &comparison_type));
+ key_expr = cast_equality_delete_key(std::move(key_expr),
comparison_type);
Review Comment:
[P2] Skip empty delete sets before constructing strict key casts
This adds a strict comparison cast even when the loaded delete block has
zero rows. [Iceberg accepts zero-record equality-delete
descriptors](https://github.com/apache/iceberg/blob/apache-iceberg-1.12.0/core/src/main/java/org/apache/iceberg/FileMetadata.java),
and absent optional statistics let them reach this reader. For example, retain
DATE key `1000-01-01`, promote it to TIMESTAMP_NS without a snapshot, prepare
an empty NS delete, then drop the key, commit the delete and clean the
unreferenced promotion schema. The recovered table key is DATE. With an
unrelated nonempty, nonmatching delete keeping the scan's equality-delete
summary positive, the empty NS filter still reaches this line and casts the
DATE key to NS. `EqualityDeletePredicate` evaluates its children before
checking the empty hash index, so the scan fails with timestamp overflow
despite no tombstone to compare. Previously the data key stayed DATE and the
row survived. Skip empty loaded filters before key resolution/
comparison casts, and cover this case separately from the intentional nonempty
NULL-tombstone overflow error.
--
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]
