morningman commented on code in PR #69711:
URL: https://github.com/apache/doris/pull/69711#discussion_r4400869674
##########
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:
Confirmed, and fixed in `3c2a8e9cb87`. ORC stores an Iceberg TIME as a LONG
annotated `iceberg.long-type=TIME`, which the V2 ORC reader exposes as
`BIGINT`, while Parquet's `TIME(MICROS, isAdjustedToUTC=false)` reads as
`TIMEV2(6)`; the FE carries the field, which Doris cannot represent, as a
STRING placeholder, so neither widening direction applied and the split failed
with `NotSupported`. Two corrections to the framing. It was not limited to
id-only scans: the column cannot be projected at all, so the key is always
hidden and every scan of such a data file failed. And the previous result only
held for NULL tombstones: the old code compared the pair as two differently
formatted strings (`3600000001` from the ORC key, `01:00:00.000001` from the
Parquet tombstone), so a non-NULL tombstone on a mixed-format TIME key was
silently not applied.
When the table field is that STRING placeholder and the delete and data
files hold the key as `BIGINT` and `TIMEV2`, in either order, the key now
compares as `TIMEV2(6)`. The `BIGINT` side is converted as microseconds from
midnight by a dedicated expression (`IcebergTimeMicrosExpr`; `TIMEV2` holds
microseconds, so the conversion is exact), not by `CAST(BIGINT AS TIME)`, which
reads the integer's digits as HHMMSS, and a value outside
00:00:00..23:59:59.999999 fails the scan like the other strict comparison
casts. The same conversion widens ORC tombstones against a Parquet data file.
Every other pair still fails as before.
`IcebergEqualityDeleteComparesTimeKeysAcrossOrcAndParquet` covers all four ORC
/ Parquet data and delete file pairs with a NULL and a non-NULL tombstone;
removing the rule fails the mixed pairs with `NotSupported`, and a SQL cast
instead of the exact conversion fails them as well.
##########
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:
Confirmed, and fixed in `3c2a8e9cb87`. For a primitive Doris cannot
represent (Iceberg TIME, `timestamptz_ns`) the FE sends a STRING placeholder
whose initial default is Iceberg's text of the value (`00:00` for a TIME
midnight, `1970-01-01 00:00:00+00:00` for a `timestamptz_ns` epoch), so a
non-NULL default compared STRING against the tombstones' physical type and the
split failed before `AncestorNullDefaultExpr` was built. One correction: this
was not limited to rows under a NULL parent, since the comparison type is
chosen per data file, so every data file written before the field was added
failed. And the previous result only held for NULL tombstones: the default
`00:00` never matched a tombstone cast to `00:00:00.000000`, so a delete of the
default value was silently not applied.
A non-NULL default behind such a placeholder is now parsed in the delete
file's type with the strict comparison cast: `TIMEV2` or `TIMESTAMPTZ`, and
`TIMEV2(6)` against an ORC TIME tombstone (`BIGINT`, see the other thread).
Rows under a NULL parent still get NULL from `AncestorNullDefaultExpr` and the
others the parsed value; text that does not parse fails the scan. Testing this
also exposed that `AncestorNullDefaultExpr::open()` never opened its children
(its `RETURN_IF_ERROR_OR_PREPARED` guard returned at once, since `prepare()`
had set the flag), so any default cast to the comparison type below it aborted
the BE with `Check failed: _open_finished` in `Cast`; it now opens them, as
`EqualityDeletePredicate` does. On reachability: the history needs a v3 table,
a nested field added with an initial default through the Iceberg Java API
(Spark and Doris cannot set defaults), and an equality delete keyed on that
field. `IcebergMissingNestedPlaceholderKeyParsesTextDefaultInTombstoneTyp
e` covers a TIME and a `timestamptz_ns` default under NULL and non-NULL
parents, against Parquet TIME, ORC TIME and Parquet `timestamptz_ns` tombstones.
--
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]
