Gabriel39 commented on code in PR #69163:
URL: https://github.com/apache/doris/pull/69163#discussion_r4290799065
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java:
##########
@@ -544,6 +565,17 @@ private static Expression checkConversion(Expression
expression, Schema schema)
}
public static Object extractDorisLiteral(org.apache.iceberg.types.Type
icebergType, Expr expr) {
+ if (icebergType.typeId() == TypeID.TIMESTAMP_NANO) {
+ if (expr instanceof NullLiteral || ((Types.TimestampNanoType)
icebergType).shouldAdjustToUTC()) {
+ return null;
+ }
+ try {
+ // Iceberg treats raw Long literals as micros; preserve the
unit explicitly.
+ return
Expressions.nanos(IcebergTimestampUtils.parseNanos(expr.getStringValue(),
ZoneOffset.UTC));
Review Comment:
Fixed in df32882c846. Iceberg predicate conversion now retains casts from
TIMESTAMP_NANO to a different Doris type as residual filters instead of
stripping them. The guard covers comparisons in both operand orders and IN/NOT
IN; direct nanos predicates and identity casts remain eligible for pushdown.
Added FE unit coverage and regression assertions for rounded DATETIMEV2
equality/IN across Iceberg partition transforms in both Parquet and ORC. The
focused FE tests and the expanded Iceberg regression suite pass locally.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergTimestampUtils.java:
##########
@@ -0,0 +1,101 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.datasource.iceberg;
+
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.analysis.SlotRef;
+
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.types.Type;
+import org.apache.iceberg.types.TypeUtil;
+import org.apache.iceberg.types.Types;
+
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.time.temporal.TemporalAccessor;
+import java.time.temporal.TemporalQueries;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/** Nanosecond conversion and retained DATE-metric safeguards shared by the
4.2 planners. */
+public final class IcebergTimestampUtils {
+ private IcebergTimestampUtils() {
+ }
+
+ public static long parseNanos(String value, ZoneId zone) {
+ LocalDateTime timestamp;
+ if (value.length() == 10) {
+ timestamp = LocalDate.parse(value).atStartOfDay();
+ } else {
+ TemporalAccessor parsed =
DateTimeFormatter.ISO_DATE_TIME.parse(value.replace(' ', 'T'));
Review Comment:
Fixed in df32882c846. Static partition parsing now shares TimeStampNsLiteral
parsing with the sink, using an explicit destination timezone and retaining
checked signed-nanosecond arithmetic. This accepts compact date/datetime forms
and applies the same fractional rounding as the sink. Added unit tests for both
compact forms and the tenth fractional digit, plus actual INSERT OVERWRITE
static-partition regression tests for Parquet and ORC. The focused FE tests and
the expanded Iceberg regression suite pass locally.
##########
be/src/format_v2/schema_reader.cpp:
##########
@@ -0,0 +1,67 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "format_v2/schema_reader.h"
+
+#include "format_v2/orc/orc_reader.h"
+#include "format_v2/parquet/parquet_reader.h"
+#include "io/file_factory.h"
+#include "runtime/runtime_state.h"
+
+namespace doris::format {
+
+Status fetch_file_schema(const TFileScanRangeParams& params, const
TFileRangeDesc& range,
+ const std::shared_ptr<io::IOContext>& io_ctx,
RuntimeProfile* profile,
+ std::vector<ColumnDefinition>* schema) {
+ auto properties = std::make_shared<io::FileSystemProperties>();
+ // Schema RPCs carry the filesystem kind on the range, not necessarily in
scan params.
+ properties->system_type = range.__isset.file_type ? range.file_type :
params.file_type;
+ properties->properties = params.properties;
+ properties->hdfs_params = params.hdfs_params;
+ properties->broker_addresses = params.broker_addresses;
+ auto file = std::make_unique<io::FileDescription>();
+ file->path = range.path;
+ file->file_size = range.__isset.file_size ? range.file_size : -1;
+ file->mtime = range.__isset.modification_time ? range.modification_time :
0;
+ file->fs_name = range.fs_name;
+ if (range.__isset.file_cache_admission) {
+ file->file_cache_admission = range.file_cache_admission;
+ }
+ const bool timestamp_tz =
+ params.__isset.enable_mapping_timestamp_tz &&
params.enable_mapping_timestamp_tz;
+ const bool varbinary =
+ params.__isset.enable_mapping_varbinary &&
params.enable_mapping_varbinary;
+ RuntimeState state {TQueryOptions(), TQueryGlobals()};
+ std::unique_ptr<FileReader> reader;
+ // TVF targets must retain file precision before column mapping; a
microsecond schema
+ // irreversibly rounds nanos even when the subsequent row scan uses the V2
reader.
+ if (params.format_type == TFileFormatType::FORMAT_PARQUET) {
+ reader = std::make_unique<parquet::ParquetReader>(properties, file,
io_ctx, profile,
+ std::nullopt,
timestamp_tz, varbinary);
+ } else if (params.format_type == TFileFormatType::FORMAT_ORC) {
+ // ORC binary mapping is fixed by its schema reader, without a
separate mapping option.
Review Comment:
Addressed in df32882c846 with the requested unconditional type-mapping
policy: V2 ORC BINARY maps to VARBINARY, including nested leaves, while text
stays STRING. V2 Parquet raw binary mapping is also independent of
enable_mapping_varbinary. Added the ORC VARBINARY decoder with selected-row,
null and empty-payload handling, unit coverage for nested schema/byte
preservation, and regression coverage with the option omitted, false and true.
Branch-4.2 internal tables do not support VARBINARY, so the CTAS test verifies
explicit rejection rather than silently storing TEXT. All 691 focused BE ASAN
tests and five affected HDFS regression suites pass; binary golden results were
generated by actual execution and verified in comparison mode.
##########
be/src/service/internal_service.cpp:
##########
@@ -883,13 +883,19 @@ void
PInternalService::fetch_table_schema(google::protobuf::RpcController* contr
file_slots,
fetch_schema_batch_size, io_ctx.get());
break;
}
- case TFileFormatType::FORMAT_PARQUET: {
- reader = ParquetReader::create_unique(params, range, io_ctx,
nullptr);
- break;
- }
+ case TFileFormatType::FORMAT_PARQUET:
case TFileFormatType::FORMAT_ORC: {
- reader = OrcReader::create_unique(params, range,
fetch_schema_batch_size, "", io_ctx);
- break;
+ std::vector<format::ColumnDefinition> schema;
+ st = format::fetch_file_schema(params, range, io_ctx,
profile.get(), &schema);
Review Comment:
Not changed, as requested by the PR author: file_scanner V1 issues and
backward compatibility changes are outside the scope of this update. The
identified enable_file_scanner_v2=false path remains unsupported here; this
comment is not being claimed as fixed. The added mapping regression explicitly
exercises V2.
--
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]
