github-actions[bot] commented on code in PR #69845:
URL: https://github.com/apache/doris/pull/69845#discussion_r4414291784
##########
be/src/format_v2/table/iceberg_reader.cpp:
##########
@@ -165,6 +165,67 @@ const char* file_format_name(FileFormat format) {
return "UNKNOWN";
}
+const format::ColumnDefinition* find_matched_file_column(
+ const format::ColumnDefinition& table_column,
+ const std::vector<format::ColumnDefinition>& file_columns,
+ format::TableColumnMappingMode mode, bool
allow_idless_complex_wrapper_projection) {
+ return mode == format::TableColumnMappingMode::BY_FIELD_ID
+ ? format::find_column_by_field_id(table_column,
file_columns,
+
allow_idless_complex_wrapper_projection)
+ : format::find_column_by_name(table_column, file_columns);
+}
+
+// Copy the table field's timestamp marker onto the file column
TableColumnMapper selects for it,
+// descending the way the mapper does: ARRAY and MAP children by position,
STRUCT children by the
+// mapping mode. Only a DATETIMEV2 or TIMESTAMPTZ leaf takes the marker. A
TIMESTAMP_NS leaf (an
+// INT64 NANOS column without the UTC annotation) keeps its nanoseconds: the
Parquet reader rewrites
+// any marked timestamp leaf, so a marker there would make it decode
TIMESTAMPTZ while the file
+// schema still says TIMESTAMP_NS. Returns whether any leaf took a marker.
+bool annotate_matched_timestamp_semantics(const format::ColumnDefinition&
table_column,
+ format::ColumnDefinition*
file_column,
+ format::TableColumnMappingMode mode)
{
+ DORIS_CHECK(file_column != nullptr);
+ DORIS_CHECK(file_column->type != nullptr);
+ switch (remove_nullable(file_column->type)->get_primitive_type()) {
+ case TYPE_DATETIMEV2:
+ case TYPE_TIMESTAMPTZ:
+ file_column->timestamp_is_adjusted_to_utc =
table_column.timestamp_is_adjusted_to_utc;
+ return file_column->timestamp_is_adjusted_to_utc.has_value();
+ case TYPE_ARRAY:
+ DORIS_CHECK(file_column->children.size() == 1);
+ // FE omits nested metadata for a field it describes only by its root.
+ return table_column.children.size() == 1 &&
+ annotate_matched_timestamp_semantics(table_column.children[0],
+ &file_column->children[0],
mode);
+ case TYPE_MAP: {
+ DORIS_CHECK(file_column->children.size() == 2);
+ if (table_column.children.size() != 2) {
+ return false;
+ }
+ const bool key =
annotate_matched_timestamp_semantics(table_column.children[0],
+
&file_column->children[0], mode);
+ const bool value =
annotate_matched_timestamp_semantics(table_column.children[1],
+
&file_column->children[1], mode);
+ return key || value;
+ }
+ case TYPE_STRUCT: {
+ bool annotated = false;
Review Comment:
[P1] Propagate Iceberg timestamp markers through nested ID-less wrappers
`TableColumnMapper` explicitly enables
`allow_idless_complex_wrapper_projection` recursively for Iceberg Parquet, so
it can resolve a selected leaf through an ID-less intermediate STRUCT. The new
timestamp walk uses `find_matched_file_column(..., false)` for every STRUCT
child, stops at that wrapper, and leaves the leaf's
`timestamp_is_adjusted_to_utc` unset. With an outer-ID/inner-ID-less/leaf-ID
file (the shape covered by
`ParquetRecursivelyRetainsIdlessWrapperForSelectedLeafId`) and an INT96
timestamptz leaf, the scan still maps the leaf but decodes it as a
session-local DATETIMEV2, shifting the instant; a hidden equality-delete key
through the same path is affected too. Pass the same allow-wrapper flag through
the recursive annotation or reuse the mapper's recursive resolver, and add a
nested INT96 timestamptz regression test.
--
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]
