github-actions[bot] commented on code in PR #69163:
URL: https://github.com/apache/doris/pull/69163#discussion_r4290583570
##########
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:
[P1] Keep precision-reducing casts out of nanos pushdown
For this retained scan predicate:
```text
Filter(CAST(ts AS DATETIMEV2(6)) = '2024-01-02 03:04:05.123457')
IcebergScan(ts: TIMESTAMP_NS)
```
`convertDorisExprToSlotRef` strips the cast, and this new branch converts
the right-hand value into the exact nanos filter `ts = .123457000`. A file or
identity partition containing only `ts='2024-01-02 03:04:05.123456789'` is then
pruned, although the actual DATETIMEV2 cast rounds it to `.123457` and the row
matches. The optimizer deliberately retains this cast for TIMESTAMP_NS. Skip
pushdown for a lossy cast, or translate it into the correct rounded interval.
##########
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:
[P2] Use Doris timestamp parsing for static partition literals
For an identity `TIMESTAMP_NS` partition, `INSERT OVERWRITE TABLE t
PARTITION(ts='20240102') SELECT 1` passes the normal sink cast, but
`setStaticPartitionToContext` retains the original string. At commit,
constructing partition metadata (or the overwrite filter) calls
`parsePartitionValueFromString -> parseNanos` and reaches this ISO-only parser,
which rejects that compact date (and `20240102030405`), so the write fails
after producing files. Normalize static SQL literals with the same timestamp
parser used by the sink. The new `ColumnDefinition` normalization only covers
DDL defaults, so it does not fix this path.
##########
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:
[P2] Preserve the ORC varbinary mapping option
For an ORC TVF with `enable_mapping_varbinary=true`, schema discovery now
uses the V2 `OrcReader`, whose `_convert_to_doris_type` always maps `BINARY` to
`DataTypeString`. The old schema reader honored this explicit option; the new
helper computes `varbinary` but passes it only to Parquet. Consequently `DESC
FUNCTION` and CTAS now expose/store TEXT instead of VARBINARY, including nested
binary fields. Propagate the option through ORC schema conversion too.
##########
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:
[P2] Align ORC nanos inference with scanner selection
This now infers `TIMESTAMP_NS` for an ORC timestamp annotated
`iceberg.timestamp-unit=NANOS`, even when `enable_file_scanner_v2=false`. An
ordinary ORC TVF still selects `FileScanner`: `should_use_file_scanner_v2` only
forces the timestamp contract for Parquet/hybrid/Iceberg scans. The legacy ORC
reader reports the source as `DATETIMEV2(6)`, and
`ColumnTypeConverter::get_converter` rejects conversion to the newly inferred
`TIMESTAMP_NS`, so `SELECT ts FROM S3(..., "format"="orc")` now fails under
that setting. Ensure these inferred schemas select a reader that supports them,
including nested leaves.
--
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]
