github-actions[bot] commented on code in PR #68934:
URL: https://github.com/apache/doris/pull/68934#discussion_r4246410050
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergSchemaBuilder.java:
##########
@@ -360,10 +362,16 @@ public static Literal<?> parseDefaultLiteral(String
value, Type type) {
} catch (NumberFormatException e) {
throw new IllegalArgumentException("Invalid Int string: "
+ value, e);
}
+ case TIMESTAMP_NANO:
+ // ADD COLUMN carries the validated SQL datetime, not an epoch
integer.
+ if (!value.matches("[+-]?[0-9]+")) {
Review Comment:
[P2] Parse compact SQL defaults as civil timestamps. `ALTER TABLE t ADD
COLUMN ts TIMESTAMP_NS DEFAULT '20240102'` passes FE validation as 2024-01-02
midnight, and the original text reaches this method unchanged. This numeric
branch instead commits `Expressions.nanos(20240102L)`, making both initial and
write defaults `1970-01-01 00:00:00.020240102`. Normalize the validated
temporal literal or parse its SQL formats here, and cover compact date/datetime
defaults. This is a different outcome from the earlier canonical-text
rejection: the new discriminator succeeds with the wrong persisted value.
##########
be/src/core/data_type_serde/orc_serde_utils.cpp:
##########
@@ -23,6 +23,32 @@
namespace doris::orc_serde_utils {
+bool has_nanosecond_timestamp(const ::orc::Type& type) {
+ if (type.getKind() == ::orc::TIMESTAMP &&
type.hasAttributeKey("iceberg.timestamp-unit") &&
+ type.getAttributeValue("iceberg.timestamp-unit") == "NANOS") {
+ return true;
+ }
+ for (uint64_t child = 0; child < type.getSubtypeCount(); ++child) {
+ if (has_nanosecond_timestamp(*type.getSubtype(child))) {
+ return true;
+ }
+ }
+ return false;
+}
+
+int64_t corrected_orc_timestamp_seconds(const OrcDecodedColumnView& view,
int64_t seconds,
+ int64_t nanoseconds) {
+ if (view.writer_epoch_offset == 0 || nanoseconds <= 999999 ||
Review Comment:
[P2] Preserve sparse follower timestamps in the GMT carrier. The [pinned ORC
selected
decoder](https://github.com/apache/doris-thirdparty/blob/5860f94768b94406f31698ac66f4cc3d2a781324/c%2B%2B/src/ColumnReader.cc#L405-L431)
tests `nanoBuffer[i]` instead of `nanoBuffer[idx]`. For a New York writer with
plain timestamp seconds `[1,1]`, fractions `[500000123,1]`, an unprojected
NANOS sibling and an integer predicate selecting row 1, GMT shifts the carrier
negative. The SDK tests row 0's packed fraction and wrongly subtracts a second;
this guard preserves it. The plain result becomes `1969-12-31 19:00:00.000000`
instead of `19:00:01.000000`. Dense decoding and the prior matching-zone reader
are correct. Fix or bypass that selected-index path and test lazy/dense
equivalence; this differs from the existing dense epoch-zero issue.
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonArrowBatchAdapter.java:
##########
@@ -204,6 +204,10 @@ private static FieldType timestampField(
TimeUnit unit = precision > 3
? TimeUnit.MICROSECOND
: precision > 0 ? TimeUnit.MILLISECOND : TimeUnit.SECOND;
+ if (timeZone == null && precision > 6) {
+ // The transport must retain the sub-microsecond digits of
TIMESTAMP_NS inputs.
+ unit = TimeUnit.NANOSECOND;
Review Comment:
[P2] Normalize nanoseconds to the Paimon target precision. Existing Paimon
`TIMESTAMP(7/8)` fields now map to `TIMESTAMP_NS`, so inserting `'2024-01-02
03:04:05.123456789'` retains all nine digits in the sink. This branch emits
nanoseconds, `timestampAdapter.getTimestamp` ignores target precision, and
`PaimonWriteSchema.tableRow` copies the value unchanged. Paimon's
serializer/INT96 writer therefore persists `.123456789` instead of
`.123456700`/`.123456780`, also changing timestamp-key equality across writers.
Apply target precision normalization, including nested fields, and test writes
to existing precision-7/8 schemas. Paimon's [timestamp
cast](https://github.com/apache/paimon/blob/release-1.4.2/paimon-common/src/main/java/org/apache/paimon/casting/TimestampToTimestampCastRule.java#L72-L75)
supplies the expected narrowing semantics.
##########
be/src/core/data_type_serde/orc_serde_utils.cpp:
##########
@@ -23,6 +23,32 @@
namespace doris::orc_serde_utils {
+bool has_nanosecond_timestamp(const ::orc::Type& type) {
+ if (type.getKind() == ::orc::TIMESTAMP &&
type.hasAttributeKey("iceberg.timestamp-unit") &&
+ type.getAttributeValue("iceberg.timestamp-unit") == "NANOS") {
+ return true;
+ }
+ for (uint64_t child = 0; child < type.getSubtypeCount(); ++child) {
+ if (has_nanosecond_timestamp(*type.getSubtype(child))) {
+ return true;
+ }
+ }
+ return false;
+}
+
+int64_t corrected_orc_timestamp_seconds(const OrcDecodedColumnView& view,
int64_t seconds,
+ int64_t nanoseconds) {
+ if (view.writer_epoch_offset == 0 || nanoseconds <= 999999 ||
+ view.file_type->getKind() != ::orc::TIMESTAMP) {
+ return seconds;
+ }
+ // ORC 1.9 applies the negative-fraction correction after changing
timezones. Undo it
+ // in the GMT carrier and apply it at the writer's epoch boundary instead,
preserving
+ // fractions when the timezone shift crosses zero. TIMESTAMP_INSTANT is
always UTC.
+ const int64_t encoded_seconds = seconds < 0 ? seconds + 1 : seconds;
Review Comment:
[P2] Restore the writer second before applying timezone rules. For an
external New York stripe, `1969-10-26 01:59:59.500000000` has writer second
`-5767201`. ORC increments it for fractional encoding; the new GMT reader then
selects UTC-05 at `-5767200`, after the fallback, rather than UTC-04. This
helper cannot recover the lost offset and returns `00:59:59.500000000`; the
prior matching-zone reader returned `01:59:59.500000000`. An unprojected
nanosecond sibling also triggers this. Correct timezone conversion in the
original writer-second domain and cover a historical transition. Unlike the
existing constant-offset epoch-zero finding, this loses a timezone rule. See
the [pinned conversion
order](https://github.com/apache/doris-thirdparty/blob/5860f94768b94406f31698ac66f4cc3d2a781324/c%2B%2B/src/ColumnReader.cc#L370-L386).
##########
be/src/core/data_type_serde/data_type_timestamp_ns_serde.cpp:
##########
@@ -345,6 +398,112 @@ Status
DataTypeTimeStampNsSerDe::deserialize_column_from_jsonb_vector(
source_null_map);
}
+Status DataTypeTimeStampNsSerDe::read_parquet_dictionary(
+ IColumn& column, ParquetDecodeSource& source, const
ParquetDecodeContext& context) const {
+ TimestampNsParquetConsumer consumer(column, context);
+ RejectTimestampNsBinaryConsumer binary;
+ return source.decode_dictionary(consumer, binary);
+}
+
+Status DataTypeTimeStampNsSerDe::read_column_from_parquet(
+ IColumn& column, ParquetDecodeSource& source, const
ParquetDecodeContext& context,
+ size_t num_values, ParquetMaterializationState& state) const {
+ TimestampNsParquetConsumer consumer(column, context, &state);
+ if (context.encoding != ParquetValueEncoding::DICTIONARY) {
+ return source.decode_fixed_values(num_values, consumer);
+ }
+ if (state.dictionary_generation != source.dictionary_generation()) {
+ state.typed_dictionary = column.clone_empty();
+ auto* null_map =
state.begin_dictionary_conversion(source.dictionary_size());
+ TimestampNsParquetConsumer dictionary(*state.typed_dictionary,
context, &state);
+ RejectTimestampNsBinaryConsumer binary;
+ auto status = source.decode_dictionary(dictionary, binary);
+ state.end_dictionary_conversion(null_map);
+ RETURN_IF_ERROR(status);
+ DORIS_CHECK_EQ(state.typed_dictionary->size(),
source.dictionary_size());
+ state.dictionary_generation = source.dictionary_generation();
+ }
+ return state.materialize_dictionary(column, source, num_values);
+}
+
+Status DataTypeTimeStampNsSerDe::read_column_from_decoded_values(
+ IColumn& column, const DecodedColumnView& view) const {
+ if (view.value_kind != DecodedValueKind::INT64 && view.value_kind !=
DecodedValueKind::INT96) {
+ return Status::NotSupported("TIMESTAMP_NS expects decoded INT64 or
INT96 values");
+ }
+ if (view.values == nullptr &&
decoded_column_view_has_non_null_value(view)) {
+ return Status::Corruption("Decoded TIMESTAMP_NS value buffer is null");
+ }
+ auto unit = ParquetTimeUnit::MICROS;
+ if (view.time_unit == DecodedTimeUnit::MILLIS) {
+ unit = ParquetTimeUnit::MILLIS;
+ } else if (view.time_unit == DecodedTimeUnit::NANOS) {
+ unit = ParquetTimeUnit::NANOS;
+ }
+ auto& data = assert_cast<ColumnTimeStampNs&>(column).get_data();
+ const auto old_size = data.size();
+ for (int64_t row = 0; row < view.row_count; ++row) {
+ if (decoded_column_view_row_is_null(view, row)) {
+ data.emplace_back();
+ continue;
+ }
+ int64_t nanos;
+ auto status =
+ view.value_kind == DecodedValueKind::INT96
+ ? parquet_int96_timestamp_nanos(
+ unaligned_load<ParquetInt96Timestamp>(
+ view.values + row *
sizeof(ParquetInt96Timestamp)),
+ &nanos)
+ : parquet_timestamp_nanos(
+ unit,
+ unaligned_load<int64_t>(view.values + row *
sizeof(int64_t)),
+ &nanos);
+ if (!status.ok()) {
+ if (decoded_column_view_can_null_on_conversion_failure(view)) {
+ decoded_column_view_insert_null_on_conversion_failure(column,
view, row);
+ continue;
+ }
+ data.resize(old_size);
+ return status;
+ }
+ data.emplace_back(nanos);
+ }
+ return Status::OK();
+}
+
+Status DataTypeTimeStampNsSerDe::read_column_from_orc(IColumn& column,
+ const
OrcDecodedColumnView& view) const {
+ const auto& batch = assert_cast<const
orc::TimestampVectorBatch&>(*view.batch);
+ auto& data = assert_cast<ColumnTimeStampNs&>(column).get_data();
+ const auto old_size = data.size();
+ const auto rows = orc_serde_utils::orc_decode_row_count(view.rows,
view.selected_rows);
+ const auto timezone = view.timezone == nullptr ? cctz::utc_time_zone() :
*view.timezone;
+ for (size_t row = 0; row < rows; ++row) {
+ const auto source = orc_serde_utils::orc_source_row_at(row,
view.selected_rows);
+ if (orc_serde_utils::orc_row_is_null(batch, source)) {
+ data.emplace_back();
+ continue;
+ }
+ const auto nanos = batch.nanoseconds[source];
+ if (nanos < 0 || nanos >= TimeStampNsValue::NANOS_PER_SECOND) {
+ data.resize(old_size);
+ return Status::DataQualityError("Invalid ORC timestamp
nanoseconds: {}", nanos);
+ }
+ DateV2Value<DateTimeV2ValueType> civil;
+ const int64_t seconds =
+ orc_serde_utils::corrected_orc_timestamp_seconds(view,
batch.data[source], nanos);
+ auto status = orc_serde_utils::orc_timestamp_to_datetime(seconds,
nanos / 1000, timezone,
Review Comment:
[P2] Preserve ancient offsets from the C++ ORC UTC carrier. The [pinned C++
API](https://github.com/apache/doris-thirdparty/blob/5860f94768b94406f31698ac66f4cc3d2a781324/c%2B%2B/include/orc/Vector.hh#L403-L406)
requires caller-converted UTC seconds. A New York writer can therefore store
`1800-01-01 00:00:00` as `-5364644638`, with zero nanos and the historical
UTC-04:56:02 offset. File-wide GMT reading uses ORC's ancient EST offset
instead, so this conversion returns `1799-12-31 23:56:02`. The prior
matching-zone reader preserved the correct ordinary timestamp in the same mixed
file. An unprojected NANOS sibling also activates the regression. Preserve that
supported writer carrier and add an external C++ historical fixture;
fractional-second and sparse-index fixes cannot repair this whole-second case.
--
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]
