github-actions[bot] commented on code in PR #66761:
URL: https://github.com/apache/doris/pull/66761#discussion_r3824500305
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/PartitionExprUtil.java:
##########
@@ -252,15 +265,19 @@ private static PartitionValue getPartitionFromDate(Type
partitionColumnType, Dat
return PartitionValue.MAX_VALUE;
}
+ LocalDateTime dateTime = dateLiteral instanceof DateLiteral
+ ? ((DateLiteral) dateLiteral).getTimeFormatter()
+ : ((TimeStampNsLiteral) dateLiteral).toLocalDateTime();
String timeString;
if (partitionColumnType.isDate() || partitionColumnType.isDateV2()) {
- timeString = String.format(DATE_FORMATTER, dateLiteral.getYear(),
dateLiteral.getMonth(),
- dateLiteral.getDay());
+ timeString = String.format(DATE_FORMATTER, dateTime.getYear(),
dateTime.getMonthValue(),
+ dateTime.getDayOfMonth());
} else if (partitionColumnType.isDatetime() ||
partitionColumnType.isDatetimeV2()
+ || partitionColumnType.isTimeStampNs()
Review Comment:
[P1] Preserve nanoseconds in TIMESTAMP_NS auto-partition bounds
This branch formats TIMESTAMP_NS bounds with the seconds-only
`DATETIME_FORMATTER`, even though the missing partition value and
`getRangeEnd()` preserve the fractional origin. For example, with `AUTO
PARTITION BY RANGE (second_floor(ts, 1, TIMESTAMP_NS '1970-01-01
00:00:00.000000001'))`, a row at `1970-01-01 00:00:00.000000000` asks FE to
create `[1969-12-31 23:59:59.000000001, 1970-01-01 00:00:00.000000001)`, but
this code creates `[1969-12-31 23:59:59, 1970-01-01 00:00:00)`. The buffered
row is then equal to the exclusive upper bound, so it still has no partition
when the load retries. Please serialize all nine digits for TIMESTAMP_NS bounds
and add an auto-partition regression with a fractional floor/ceil origin.
##########
be/src/storage/field_type.h:
##########
@@ -96,6 +97,7 @@ constexpr bool field_is_numeric_type(const FieldType&
field_type) {
field_type == FieldType::OLAP_FIELD_TYPE_DATEV2 ||
field_type == FieldType::OLAP_FIELD_TYPE_DATETIME ||
field_type == FieldType::OLAP_FIELD_TYPE_DATETIMEV2 ||
+ field_type == FieldType::OLAP_FIELD_TYPE_TIMESTAMP_NS ||
Review Comment:
[P1] Add TIMESTAMP_NS to the SNII BKD field vocabulary
This makes TIMESTAMP_NS select `SniiBkdIndexColumnWriter`, but the native
BKD format's `kIndexableFieldTypes` still omits `OLAP_FIELD_TYPE_TIMESTAMP_NS`.
Writer initialization succeeds (the size and key coder exist), then
`BkdBuilder::finish()` calls `encode_bkd_index_block()`, where
`resolve_field_type(header.field_type, ...)` fails under `DORIS_CHECK` and
aborts the BE. FE already accepts TIMESTAMP_NS SNII definitions, so this is
reachable through ordinary DDL/load. Please add the stable field type to the
BKD vocabulary and cover SNII write/read/range queries; the new legacy V2
inverted-index test does not exercise this path.
##########
fe/be-java-extensions/java-common/src/main/java/org/apache/doris/common/jni/utils/JavaUdfDataType.java:
##########
@@ -50,6 +50,8 @@ public class JavaUdfDataType {
public static final JavaUdfDataType DATEV2 = new JavaUdfDataType("DATEV2",
TPrimitiveType.DATEV2, 4);
public static final JavaUdfDataType DATETIMEV2 = new
JavaUdfDataType("DATETIMEV2", TPrimitiveType.DATETIMEV2,
8);
+ public static final JavaUdfDataType TIMESTAMP_NS = new
JavaUdfDataType("TIMESTAMP_NS",
Review Comment:
[P1] Add TIMESTAMP_NS support to the JNI VectorTable transport
Registering TIMESTAMP_NS here makes Java UDF creation succeed, but execution
cannot transport the advertised type. `JavaFunctionCall::execute_impl` builds
both argument and result schemas with `JniDataBridge::parse_table_schema()`,
while `get_jni_type()` has no `TYPE_TIMESTAMP_NS` case and emits `unsupported`
(also recursively inside arrays/maps/structs). Adding only that native mapping
is insufficient: Java `ColumnType` has no TIMESTAMP_NS kind, so a
timestamp-like spelling falls into DATETIMEV2 and `VectorColumn` interprets the
signed epoch-nanosecond cell as packed DATETIMEV2 bits. Thus the identity UDF
covered by the new fixture cannot actually execute correctly. Please add an
end-to-end VectorTable type/encoding for epoch nanoseconds, including nested
types, and an execution test.
##########
be/src/exprs/function/function_datetime_floor_ceil.cpp:
##########
@@ -739,6 +754,76 @@ struct DateTimeFloorCeilCore {
trivial_part_ts_arg = calc_arg.microsecond();
trivial_part_ts_res = calc_origin.microsecond();
}
+ } else if constexpr (std::is_same_v<DateValueType, TimeStampNsValue>) {
+ const auto nanos_since_midnight = [](const TimeStampNsValue&
value) {
+ return value.time_part_to_seconds() *
TimeStampNsValue::NANOS_PER_SECOND +
+ value.nanosecond();
+ };
+ const auto nanos_since_date = [&](const TimeStampNsValue& value,
uint8_t month,
+ uint8_t day) {
+ return (value.daynr() - calc_daynr(value.year(), month, day))
* HOUR_PER_DAY *
+ SECOND_PER_HOUR *
TimeStampNsValue::NANOS_PER_SECOND +
+ nanos_since_midnight(value);
+ };
+
+ if constexpr (Flag::Unit == YEAR) {
+ diff = ts_arg.year() - ts_origin.year();
+ trivial_part_ts_arg = nanos_since_date(ts_arg, 1, 1);
+ trivial_part_ts_res = nanos_since_date(ts_origin, 1, 1);
+ }
+ if constexpr (Flag::Unit == QUARTER) {
+ diff = (ts_arg.year() - ts_origin.year()) * 4 +
Review Comment:
[P1] Compute quarters relative to the custom origin
`diff` is origin-relative, but these remainder values are measured from each
value's calendar-quarter start, so the coordinate systems disagree when the
origin is not in January/April/July/October. For `quarter_floor(TIMESTAMP_NS
'2024-04-01 00:00:00', 1, TIMESTAMP_NS '2024-03-01 00:00:00')`, this gets `diff
= 0`, treats April's remainder as less than March's, decrements to -1, and
returns `2023-12-01`; the origin-anchored windows start on March 1 and June 1,
so the correct result is March 1. The FE fold path mirrors the same formula.
Please derive both quotient and remainder from a signed total-month delta
relative to the origin, and add folded/no-fold custom-origin tests.
##########
be/src/exprs/function/array/function_array_range.cpp:
##########
@@ -230,7 +227,7 @@ struct RangeImplUtil {
dest_nested_null_map.push_back(0);
offset++;
move++;
- idx = doris::date_time_add<UNIT::value,
TYPE_DATETIMEV2, Int32>(idx,
+ idx = doris::date_time_add<UNIT::value,
SourceDataPType, Int32>(idx,
Review Comment:
[P1] Do not compute an unused overflowing sequence step
The TIMESTAMP_NS specialization can throw here after it has already emitted
every valid element. For example, `sequence('2262-04-11 23:47:14.854775807',
'2262-04-11 23:47:16.854775807', interval 3 second)` should contain just the
start, but this loop appends it and then unconditionally adds three seconds;
that unused next value is one second beyond the signed endpoint, so
`date_time_add` throws before the loop can compare it with the exclusive end.
The new terminal test avoids the loop because its whole-second difference is
zero. Please stop before advancing when no further element is needed (or use
checked wide arithmetic), and cover overshooting steps at both endpoints.
##########
fe/fe-type/src/main/java/org/apache/doris/catalog/PrimitiveType.java:
##########
@@ -66,6 +66,7 @@ public enum PrimitiveType {
AGG_STATE("AGG_STATE", 16, TPrimitiveType.AGG_STATE, true),
DATEV2("DATEV2", 4, TPrimitiveType.DATEV2, true),
DATETIMEV2("DATETIMEV2", 8, TPrimitiveType.DATETIMEV2, true),
+ TIMESTAMP_NS("TIMESTAMP_NS", 8, TPrimitiveType.TIMESTAMP_NS, true),
Review Comment:
[P1] Add TIMESTAMP_NS to Flight SQL schema discovery
Registering this primitive also exposes it through
`CommandGetTables(includeSchema=true)`, but
`FlightSqlSchemaHelper.getArrowType()` has no TIMESTAMP_NS case and silently
falls through to `ArrowType.Null`. The serialized `table_schema` therefore
advertises scalar and nested TIMESTAMP_NS columns as Null, so Flight SQL
clients cannot type/read them even after the BE Arrow transport is fixed; that
mapper is independent of the BE Arrow/Parquet path. Please map this to a
timezone-naive `ArrowType.Timestamp(TimeUnit.NANOSECOND, null)` and extend the
serialized-schema tests for scalar and nested values.
##########
fe/fe-type/src/main/java/org/apache/doris/catalog/Type.java:
##########
@@ -707,6 +722,7 @@ public boolean isNativeType() {
public boolean isDateType() {
return isScalarType(PrimitiveType.DATE) ||
isScalarType(PrimitiveType.DATETIME)
|| isScalarType(PrimitiveType.DATEV2) ||
isScalarType(PrimitiveType.DATETIMEV2)
+ || isScalarType(PrimitiveType.TIMESTAMP_NS)
Review Comment:
[P1] Keep lossy TIMESTAMP_NS casts out of raw-slot pruning
Including TIMESTAMP_NS in this generic date family makes
`CastExpr.canHashPartition()` treat its casts to DATETIMEV2/TIMESTAMPTZ as
representation-preserving. `BinaryPredicate.getSlotBinding()` then strips the
cast for scan/partition ranges. A stored `1969-12-31 23:59:59.999999500`
satisfies `CAST(ts AS DATETIMEV2(6)) = DATETIMEV2 '1970-01-01 00:00:00.000000'`
because the new cast rounds and carries (the added regression locks that
behavior), but pruning rewrites it to raw `ts = ...000000000` and can exclude
the matching row/tablet/partition. Please restrict `canHashPartition` to truly
order/value-preserving temporal casts instead of admitting TIMESTAMP_NS through
`isDateType`, and add a scan/partition-pruning carry test.
##########
be/src/core/data_type_serde/data_type_timestamp_ns_serde.cpp:
##########
@@ -0,0 +1,270 @@
+// 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 "core/data_type_serde/data_type_timestamp_ns_serde.h"
+
+#include <cctz/time_zone.h>
+
+#include <algorithm>
+#include <cctype>
+#include <limits>
+#include <string>
+
+#include "common/exception.h"
+#include "core/assert_cast.h"
+#include "core/column/column_nullable.h"
+#include "core/column/column_string.h"
+#include "core/value/vdatetime_value.h"
+#include "exprs/function/cast/cast_to_datetimev2_impl.hpp"
+#include "util/mysql_row_buffer.h"
+#include "util/unaligned.h"
+
+namespace doris {
+Status parse_timestamp_ns(StringRef str, int64_t* epoch_nanos,
+ const cctz::time_zone* local_time_zone) {
+ std::string input(str.data, str.size);
+ const size_t dot = input.rfind('.');
+ size_t fraction_begin = std::string::npos;
+ size_t fraction_end = std::string::npos;
+ if (dot != std::string::npos && dot + 1 < input.size() &&
+ std::isdigit(static_cast<unsigned char>(input[dot + 1]))) {
+ fraction_begin = dot + 1;
+ fraction_end = fraction_begin;
+ while (fraction_end < input.size() &&
+ std::isdigit(static_cast<unsigned char>(input[fraction_end]))) {
+ ++fraction_end;
+ }
+ }
+
+ uint32_t nanos = 0;
+ size_t fraction_length = 0;
+ if (fraction_begin != std::string::npos) {
+ fraction_length = fraction_end - fraction_begin;
+ const size_t copied_digits =
+ std::min<size_t>(fraction_length,
TimeStampNsValue::FRACTIONAL_DIGITS);
+ for (size_t i = 0; i < copied_digits; ++i) {
+ nanos = nanos * 10 + static_cast<uint32_t>(input[fraction_begin +
i] - '0');
+ }
+ for (size_t i = copied_digits; i <
TimeStampNsValue::FRACTIONAL_DIGITS; ++i) {
+ nanos *= 10;
+ }
+ }
+
+ if (fraction_length > TimeStampNsValue::FRACTIONAL_DIGITS &&
+ input[fraction_begin + TimeStampNsValue::FRACTIONAL_DIGITS] >= '5') {
+ ++nanos;
+ }
+
+ // Keep the fractional token in place so that the legacy parser validates
its position and all
+ // trailing syntax. Zeroing the digits prevents its scale-0 rounding from
changing the second;
+ // the nanosecond rounding above remains the only source of fractional
rounding.
+ if (fraction_begin != std::string::npos) {
+ std::fill(input.begin() + fraction_begin, input.begin() +
fraction_end, '0');
+ }
+ const StringRef input_ref(input.data(), input.size());
+ DateV2Value<DateTimeV2ValueType> datetime;
+ CastParameters params {.status = Status::OK(), .is_strict = true};
+ CastToDatetimeV2::from_string_strict_mode<DatelikeParseMode::STRICT>(
+ input_ref, datetime, local_time_zone, 0, params);
+ if (!params.status.ok()) {
+ if (dot != std::string::npos) {
+ return Status::InvalidArgument("Invalid TIMESTAMP_NS value '{}'",
+ std::string(str.data, str.size));
+ }
+ return params.status;
+ }
+
+ if (nanos == TimeStampNsValue::NANOS_PER_SECOND) {
+ if (!datetime.date_add_interval<TimeUnit::SECOND>(
+ TimeInterval {TimeUnit::SECOND, 1, false})) {
+ return Status::InvalidArgument("TIMESTAMP_NS value overflows while
rounding '{}'",
+ std::string(str.data, str.size));
+ }
+ nanos = 0;
+ }
+ datetime.set_microsecond(nanos / TimeStampNsValue::NANOS_PER_MICROSECOND);
+ TimeStampNsValue value;
+ if (!value.from_datetime(
+ datetime, static_cast<uint16_t>(nanos %
TimeStampNsValue::NANOS_PER_MICROSECOND))) {
+ return Status::InvalidArgument(
+ "TIMESTAMP_NS value '{}' is outside [{}, {}]",
std::string(str.data, str.size),
+
TimeStampNsValue(std::numeric_limits<int64_t>::min()).to_string(),
+
TimeStampNsValue(std::numeric_limits<int64_t>::max()).to_string());
+ }
+ *epoch_nanos = value.epoch_nanos();
+ return Status::OK();
+}
+
+Status DataTypeTimeStampNsSerDe::from_string_batch(const ColumnString& strings,
+ ColumnNullable& result,
+ const FormatOptions&
options) const {
+ auto& data =
assert_cast<ColumnTimeStampNs&>(result.get_nested_column()).get_data();
+ auto& null_map = result.get_null_map_column().get_data();
+ result.resize(strings.size());
+ for (size_t i = 0; i < strings.size(); ++i) {
+ int64_t value = 0;
+ const auto status = parse_timestamp_ns(strings.get_data_at(i), &value,
options.timezone);
+ null_map[i] = !status.ok();
+ data[i] = TimeStampNsValue(value);
+ }
+ return Status::OK();
+}
+
+Status DataTypeTimeStampNsSerDe::from_string_strict_mode_batch(
+ const ColumnString& strings, IColumn& result, const FormatOptions&
options,
+ const NullMap::value_type* null_map) const {
+ auto& data = assert_cast<ColumnTimeStampNs&>(result).get_data();
+ result.resize(strings.size());
+ for (size_t i = 0; i < strings.size(); ++i) {
+ if (null_map != nullptr && null_map[i]) {
+ continue;
+ }
+ int64_t value = 0;
+ RETURN_IF_ERROR(parse_timestamp_ns(strings.get_data_at(i), &value,
options.timezone));
+ data[i] = TimeStampNsValue(value);
+ }
+ return Status::OK();
+}
+
+Status DataTypeTimeStampNsSerDe::from_string(StringRef& str, IColumn& column,
+ const FormatOptions& options)
const {
+ int64_t value = 0;
+ RETURN_IF_ERROR(parse_timestamp_ns(str, &value, options.timezone));
+
assert_cast<ColumnTimeStampNs&>(column).insert_value(TimeStampNsValue(value));
+ return Status::OK();
+}
+
+Status DataTypeTimeStampNsSerDe::from_string_strict_mode(StringRef& str,
IColumn& column,
+ const FormatOptions&
options) const {
+ return from_string(str, column, options);
+}
+
+Status DataTypeTimeStampNsSerDe::serialize_column_to_json(const IColumn&
column, int64_t start_idx,
+ int64_t end_idx,
BufferWritable& bw,
+ FormatOptions&
options) const {
+ SERIALIZE_COLUMN_TO_JSON();
+}
+
+Status DataTypeTimeStampNsSerDe::serialize_one_cell_to_json(const IColumn&
column, int64_t row_num,
+ BufferWritable& bw,
+ FormatOptions&
options) const {
+ auto [column_ptr, index] = check_column_const_set_readability(column,
row_num);
+ if (_nesting_level > 1) {
+ bw.write('"');
+ }
+ const auto value =
+ assert_cast<const ColumnTimeStampNs&,
TypeCheckOnRelease::DISABLE>(*column_ptr)
+ .get_element(index);
+ const std::string result = value.to_string();
+ bw.write(result.data(), result.size());
+ if (_nesting_level > 1) {
+ bw.write('"');
+ }
+ return Status::OK();
+}
+
+Status DataTypeTimeStampNsSerDe::deserialize_column_from_json_vector(
+ IColumn& column, std::vector<Slice>& slices, uint64_t*
num_deserialized,
+ const FormatOptions& options) const {
+ DESERIALIZE_COLUMN_FROM_JSON_VECTOR();
+ return Status::OK();
+}
+
+Status DataTypeTimeStampNsSerDe::deserialize_one_cell_from_json(
+ IColumn& column, Slice& slice, const FormatOptions& options) const {
+ if (_nesting_level > 1) {
+ slice.trim_quote();
+ }
+ StringRef str(slice.data, slice.size);
+ return from_string(str, column, options);
+}
+
+Status DataTypeTimeStampNsSerDe::deserialize_column_from_jsonb(IColumn& column,
+ const
JsonbValue* jsonb_value,
+ CastParameters&
cast_params) const {
+ DORIS_CHECK(jsonb_value->isString());
+ return parse_column_from_jsonb_string(column, jsonb_value, cast_params);
+}
+
+Status DataTypeTimeStampNsSerDe::deserialize_column_from_jsonb_vector(
+ ColumnNullable& column_to, const ColumnString& column_from,
+ CastParameters& cast_params) const {
+ return DataTypeSerDe::deserialize_column_from_jsonb_vector(column_to,
column_from, cast_params);
+}
+
+Status DataTypeTimeStampNsSerDe::write_column_to_arrow(const IColumn& column,
+ const NullMap* null_map,
+ arrow::ArrayBuilder*
array_builder,
+ int64_t start, int64_t
end,
+ const cctz::time_zone&
ctz) const {
+ return
Status::NotSupported("DataTypeTimeStampNsSerDe::write_column_to_arrow");
Review Comment:
[P1] Support or reject TIMESTAMP_NS on Arrow-backed outputs
Returning `NotSupported` here leaves an accepted query path guaranteed to
fail. FE allows `SELECT ts_col INTO OUTFILE ... FORMAT AS PARQUET`;
`VParquetTransformer::_parse_schema()` first calls `convert_to_arrow_type()`,
which also has no TIMESTAMP_NS case and fails before opening the file, and
adding only that mapping would then reach this unsupported row conversion. The
same boundary is reused by other Arrow consumers. Please map timezone-naive
TIMESTAMP_NS to Arrow `timestamp(NANO)` and transfer the signed
epoch-nanosecond cells (including nested types), with a Parquet round trip;
otherwise reject the type during FE analysis instead of failing in BE.
##########
be/src/information_schema/schema_columns_scanner.cpp:
##########
@@ -134,6 +134,8 @@ std::string
SchemaColumnsScanner::_to_mysql_data_type_string(TColumnDesc& desc)
case TPrimitiveType::DATETIME:
case TPrimitiveType::DATETIMEV2:
return "datetime";
+ case TPrimitiveType::TIMESTAMP_NS:
Review Comment:
[P1] Classify TIMESTAMP_NS as temporal column metadata
This makes TIMESTAMP_NS visible in `information_schema.columns`, but the
precision routing below was not extended. FE fills `columnPrecision = 29` and
`columnScale = 9`; `NUMERIC_PRECISION` and `NUMERIC_SCALE` only exclude
DATETIMEV2, while `DATETIME_PRECISION` only admits DATETIMEV2/TIMESTAMPTZ. An
ordinary TIMESTAMP_NS column therefore reports numeric precision 29, numeric
scale 9, and null datetime precision, so MySQL/JDBC schema introspection
receives numeric metadata for a temporal type and loses its nine-digit datetime
precision. Please route TIMESTAMP_NS with the temporal types, align the
remaining precision fields with that metadata contract, and add an
`information_schema.columns` regression.
--
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]
