github-actions[bot] commented on code in PR #69711:
URL: https://github.com/apache/doris/pull/69711#discussion_r4398396743
##########
be/src/format_v2/table/iceberg_reader.cpp:
##########
@@ -974,6 +980,166 @@ static Status build_equality_delete_key_expr(
return build_nested_equality_delete_key_expr(path, std::move(root_expr),
key_expr);
}
+// Whether a missing key's default is NULL. `missing_path` runs from the
missing root to the key,
+// and the root's initial default is walked down it with the key expression's
child matching. The
+// root having a default is not enough: a missing `payload` defaulting to the
struct `{}` takes
+// each child's own default, so payload.k is NULL when `k` has none.
+static Status equality_delete_default_leaf_is_null(
+ const std::vector<const format::ColumnDefinition*>& missing_path,
bool* is_null) {
+ DORIS_CHECK(!missing_path.empty());
+ DORIS_CHECK(is_null != nullptr);
+ const auto* root = missing_path.front();
+ DORIS_CHECK(root != nullptr);
+ if (!root->initial_default_value.has_value()) {
+ *is_null = true;
+ return Status::OK();
+ }
+ std::deque<std::string> binary_storage;
+ Field root_default;
+ RETURN_IF_ERROR(
+ build_v2_initial_default_field(*root, root->type, &binary_storage,
&root_default));
+ const Field* value = &root_default;
+ for (size_t index = 1; index < missing_path.size() && !value->is_null();
++index) {
+ DORIS_CHECK(value->get_type() == TYPE_STRUCT);
+ const auto& children = value->get<TYPE_STRUCT>();
+ const size_t child_index =
+ equality_delete_child_index(*missing_path[index - 1],
*missing_path[index]);
+ DORIS_CHECK(child_index < children.size());
+ value = &children[child_index];
+ }
+ *is_null = value->is_null();
+ return Status::OK();
+}
+
+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:
+ return (to_primitive == TYPE_DATETIMEV2 &&
+ to_type->get_scale() >= from_type->get_scale()) ||
+ to_primitive == TYPE_TIMESTAMP_NS;
+ case TYPE_TIMESTAMPTZ:
+ case TYPE_TIMEV2:
+ return to_primitive == from_primitive && to_type->get_scale() >=
from_type->get_scale();
Review Comment:
[P2] Normalize ORC TIME keys before choosing the comparison domain
An id-only scan now fails for valid mixed-format equality deletes on Iceberg
TIME. The [Iceberg
mappings](https://iceberg.apache.org/spec/#appendix-a-format-specific-requirements)
store ORC TIME as LONG with `iceberg.long-type=TIME`, which `OrcReader`
exposes as BIGINT, while a Parquet TIME(MICROS, false) tombstone exposes
TIMEV2(6). Neither widening direction accepts this pair. For a data row `(id=1,
k=NULL)` and a nonempty NULL tombstone on `k`, FE permits `SELECT id` and
carries the hidden key as STRING; previously both nullable casts preserved NULL
and the row was deleted, but this now returns `NotSupported` before matching.
TIME being unprojectable does not prevent this hidden dependency. Normalize its
physical carriers with the correct microsecond units and add mixed ORC/Parquet
hidden-key coverage; an ordinary BIGINT-to-TIME SQL cast has different
semantics.
##########
be/src/format_v2/table/iceberg_reader.cpp:
##########
@@ -1569,11 +1740,20 @@ 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);
+ bool default_is_null = false;
+ RETURN_IF_ERROR(equality_delete_default_leaf_is_null(missing_path,
&default_is_null));
+ if (default_is_null) {
+ // A NULL default makes the key NULL in this file whatever its table
type, so it is
+ // compared in the delete file's type, to which the table type need
not convert: a dropped
+ // key whose retained schemas only typed it Iceberg `unknown` arrives
as STRING, while its
+ // tombstones are, for example, DATE.
+ *comparison_type = remove_nullable(filter.delete_key_types[key_idx]);
+ } else {
+ RETURN_IF_ERROR(equality_delete_comparison_type(
+ filter.field_ids[key_idx], filter.delete_key_types[key_idx],
table_leaf->type,
Review Comment:
[P2] Preserve NULL parents when resolving a missing key's default domain
This rejects valid missing-child keys before their runtime ancestor NULL
mask can apply. Write Parquet data with nullable `payload` and a sibling field,
then add optional `payload.k TIME` with initial default midnight (or
`timestamptz_ns` with initial default epoch), and apply a nonempty Parquet NULL
tombstone on `k` to a row whose `payload` is NULL. FE carries these
unprojectable leaf types/defaults as STRING; the tombstone reads as TIMEV2 or
TIMESTAMPTZ. Because the default literal is non-NULL, this call rejects STRING
versus the physical type before building `AncestorNullDefaultExpr`, so `SELECT
id` fails. Previously the STRING default was masked to NULL by the parent and
matched the NULL tombstone. These are valid primitive defaults, and missing
old-child statistics retain the delete. Resolve the typed default without
rejecting this NULL-parent path, and add an all-Parquet regression; normalizing
ORC TIME alone does not fix it.
--
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]
