morningman commented on code in PR #69711:
URL: https://github.com/apache/doris/pull/69711#discussion_r4407457020


##########
be/src/format_v2/table/iceberg_reader.cpp:
##########
@@ -974,6 +1075,209 @@ 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();
+    case TYPE_VARCHAR:
+        return to_primitive == TYPE_STRING || to_primitive == TYPE_VARBINARY;
+    case TYPE_STRING:
+        return to_primitive == TYPE_VARBINARY;
+    default:
+        return false;
+    }
+}
+
+// Iceberg equality is by value, so a key is compared in the wider of the 
delete file's and the
+// data file's key types, and neither side is narrowed. One scan-wide table 
type cannot serve: after
+// expireSnapshots().cleanExpiredMetadata(true) drops the schema of a 
schema-only INT -> LONG
+// promotion, the table type of a dropped key is INT while its tombstones are 
LONG, and casting
+// 4294967296 to INT yields NULL, which deletes the rows whose key is NULL. 
`table_type` is the
+// key's type in the scan's schema metadata; it decides only how a timestamptz 
field read as
+// DATETIMEV2 and a TIME field behind a STRING placeholder compare.
+static Status equality_delete_comparison_type(int field_id, const DataTypePtr& 
delete_type,
+                                              const DataTypePtr& data_type,
+                                              const DataTypePtr& table_type,
+                                              const std::string& 
data_file_path,
+                                              DataTypePtr* comparison_type) {
+    DORIS_CHECK(delete_type != nullptr);
+    DORIS_CHECK(data_type != nullptr);
+    DORIS_CHECK(table_type != nullptr);
+    DORIS_CHECK(comparison_type != nullptr);
+    if (is_equality_delete_widening(delete_type, data_type)) {
+        *comparison_type = remove_nullable(data_type);
+        return Status::OK();
+    }
+    if (is_equality_delete_widening(data_type, delete_type)) {
+        *comparison_type = remove_nullable(delete_type);
+        return Status::OK();
+    }
+    const auto delete_value_type = remove_nullable(delete_type);
+    const auto data_value_type = remove_nullable(data_type);
+    const auto delete_primitive = delete_value_type->get_primitive_type();
+    const auto data_primitive = data_value_type->get_primitive_type();
+    // Parquet INT96 carries no instant annotation, so a timestamptz key in a 
Spark or Hive file
+    // that Iceberg imported reads as DATETIMEV2(6), while an adjusted-UTC 
tombstone of the same
+    // field, as a Flink upsert writes it, reads as TIMESTAMPTZ(6). Neither 
widens to the other.
+    // The projection converts that DATETIMEV2 to the table's TIMESTAMPTZ in 
the session time zone
+    // and the comparison cast does the same, so a tombstone matches exactly 
the value a query
+    // returns: in Asia/Shanghai the key 2024-01-01 08:00:00 compares as 
2024-01-01 00:00:00 UTC.
+    if (remove_nullable(table_type)->get_primitive_type() == TYPE_TIMESTAMPTZ 
&&
+        ((delete_primitive == TYPE_DATETIMEV2 && data_primitive == 
TYPE_TIMESTAMPTZ) ||
+         (delete_primitive == TYPE_TIMESTAMPTZ && data_primitive == 
TYPE_DATETIMEV2))) {
+        *comparison_type = std::make_shared<DataTypeTimeStampTz>(
+                std::max(delete_value_type->get_scale(), 
data_value_type->get_scale()));
+        return Status::OK();
+    }
+    // The FE sends an Iceberg TIME, which Doris cannot represent, as a STRING 
placeholder, so only
+    // the files type its values: microseconds from midnight in both formats, 
but ORC stores them as
+    // a LONG annotated iceberg.long-type=TIME, which reads as BIGINT, while 
Parquet's TIME(MICROS)
+    // reads as TIMEV2(6). Such a pair compares as TIMEV2(6), which holds 
microseconds too, so the
+    // ORC key 3600000001 matches the Parquet tombstone 01:00:00.000001; 
cast_equality_delete_key
+    // converts the BIGINT side without a SQL cast. Under any other table type 
the pair fails below.
+    if (remove_nullable(table_type)->get_primitive_type() == TYPE_STRING &&
+        ((delete_primitive == TYPE_BIGINT && data_primitive == TYPE_TIMEV2) ||
+         (delete_primitive == TYPE_TIMEV2 && data_primitive == TYPE_BIGINT))) {
+        *comparison_type = 
std::make_shared<DataTypeTimeV2>(TimeValue::MICROS_SCALE);
+        return Status::OK();
+    }
+    // No promotion leads from one type to the other, so no third type 
compares them faithfully:
+    // STRING tombstones cast to an INT key would delete the row with key 1 
for the string "1".
+    return Status::NotSupported(
+            "Iceberg equality delete field id {} is {} in the delete file and 
{} for data file "
+            "{}, and neither type converts exactly to the other",
+            field_id, remove_nullable(delete_type)->get_name(),
+            remove_nullable(data_type)->get_name(), data_file_path);
+}
+
+// The comparison type of a missing key whose non-NULL initial default sits 
behind a STRING
+// placeholder, an Iceberg TIME or timestamptz_ns (see 
equality_delete_comparison_type), or nullptr
+// for any other table or tombstone type. The default is Iceberg's text of the 
value, `00:00` for a
+// TIME midnight or `1970-01-01 00:00:00+00:00` for a timestamptz_ns epoch, so 
it is parsed into the
+// tombstones' type by the strict comparison cast, and text that does not 
parse fails the scan; an
+// ORC TIME tombstone, BIGINT, compares in TIMEV2(6) as a present key does. 
Compared as text, the
+// default `00:00` never matched a tombstone printing 00:00:00.000000.
+static DataTypePtr equality_delete_placeholder_default_type(const DataTypePtr& 
table_type,
+                                                            const DataTypePtr& 
delete_type) {
+    if (remove_nullable(table_type)->get_primitive_type() != TYPE_STRING) {
+        return nullptr;
+    }
+    switch (remove_nullable(delete_type)->get_primitive_type()) {
+    case TYPE_TIMEV2:
+    case TYPE_TIMESTAMPTZ:
+        return remove_nullable(delete_type);

Review Comment:
   Confirmed, and fixed in `0d70d8c3718`, by rejecting this comparison rather 
than parsing the default more precisely, because the precision is already gone 
on the tombstone side. The V2 readers carry every `timestamptz_ns` value as 
`TIMESTAMPTZ(6)`: Parquet floors the nanoseconds (`parquet_timestamp_micros`) 
and ORC rounds them half-up (`round_orc_timestamp_to_microseconds`). The 500 ns 
example is that loss, and it also hits a microsecond-exact default: with the 
default `1970-01-01 00:00:00+00:00`, a tombstone 100 ns after the epoch reads 
as the epoch and deletes every row of the files written before `k` was added. 
So no parsing of the default can tell a tombstone equal to it from one in the 
same microsecond.
   
   On the previous behaviour: the STRING comparison kept these rows only 
because it almost never matched a non-NULL tombstone. It compared Iceberg's 
text of the default with the tombstone cast to text, which has six fractional 
digits and the session's offset, so a tombstone equal to the default was 
silently not applied either. The previous head of this PR failed such a split 
with `NotSupported`, and the parsing added in the third round turned that error 
into these silent results.
   
   A non-NULL initial default of a `timestamptz_ns` key (a `TIMESTAMPTZ` 
tombstone behind the STRING placeholder) now fails the split with 
`NotSupported` again, with a message naming the field and the data file. This 
narrows the change made for 4398396750: TIME defaults are still parsed, which 
is exact, since an Iceberg TIME is microseconds both in its text (at most six 
fractional digits) and in the files. NULL defaults are unaffected. A key the 
data file has still compares as `TIMESTAMPTZ(6)`, as on master, which compares 
the same microsecond values through the STRING placeholder; making that exact 
needs a nanosecond carrier in both readers and is outside this PR. 
`IcebergMissingPlaceholderTimestampTzNsDefaultFailsTheScan` covers a nested 
`timestamptz_ns` key with the defaults `...0000001`, the epoch and `...0000005` 
against an epoch, a 100 ns and a 500 ns tombstone, and a key without a default 
(a NULL tombstone still deletes, an epoch one does not).
   



-- 
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]

Reply via email to