github-actions[bot] commented on code in PR #68649:
URL: https://github.com/apache/doris/pull/68649#discussion_r4134974876
##########
be/src/core/data_type_serde/data_type_datev2_serde.cpp:
##########
@@ -217,13 +236,12 @@ Status DataTypeDateV2SerDe::write_column_to_arrow(const
IColumn& column, const N
const auto& col_data = static_cast<const ColumnDateV2&>(column).get_data();
auto& date32_builder = assert_cast<arrow::Date32Builder&>(*array_builder);
for (size_t i = start; i < end; ++i) {
- auto daynr = col_data[i].daynr() - date_threshold;
if (null_map && (*null_map)[i]) {
RETURN_IF_ERROR(checkArrowStatus(date32_builder.AppendNull(),
column, *array_builder));
} else {
- RETURN_IF_ERROR(
- checkArrowStatus(date32_builder.Append(cast_set<int,
int64_t, false>(daynr)),
- column, *array_builder));
+ RETURN_IF_ERROR(checkArrowStatus(
+
date32_builder.Append(daynr_to_epoch_days(col_data[i].daynr())), column,
Review Comment:
[P1] Keep Iceberg DATE partitions aligned with the new file ordinal. The
Iceberg Parquet writer reaches this Arrow SerDe, so `0000-01-01` is now stored
as -719528, but `DateDayPartitionColumnTransform::apply` still uses Doris
`datetime_diff<DAY>` and commits -719527 (`0000-01-02`). That partition
metadata contradicts the file, so a spec-compliant reader can prune away this
row for `d = '0000-01-01'`; the DATE bucket transform also uses the old
ordinal. Align both partition transforms with `daynr_to_epoch_days` and cover a
year-zero partitioned write/read.
##########
be/src/format_v2/orc/orc_reader.cpp:
##########
@@ -487,10 +487,26 @@ bool set_date_zone_map(const ::orc::ColumnStatistics&
statistics, segment_v2::Zo
!date_statistics->hasMaximum()) {
return false;
}
- auto& date_dict = date_day_offset_dict::get();
- return set_validated_zone_map(
-
Field::create_field<TYPE_DATEV2>(date_dict[date_statistics->getMinimum()]),
-
Field::create_field<TYPE_DATEV2>(date_dict[date_statistics->getMaximum()]),
zone_map);
+ // ORC DATE statistics are proleptic-Gregorian day ordinals, the same
domain the row decoder
+ // (DataTypeDateV2SerDe::read_column_from_orc) interprets. Converting them
through
+ // `date_day_offset_dict` instead would put the year-zero window one day
off the rows and let a
+ // pushed-down MIN/MAX report a value no row holds. A bound with no Doris
DATE disables the
+ // statistics, so MIN/MAX falls back to scanning rows.
+ const auto to_date = [](int64_t epoch_days) ->
std::optional<DateV2Value<DateV2ValueType>> {
+ const int64_t daynr = epoch_days_to_daynr(epoch_days);
+ DateV2Value<DateV2ValueType> value;
+ if (daynr == 0 ||
!value.get_date_from_daynr(static_cast<uint64_t>(daynr))) {
+ return std::nullopt;
+ }
+ return value;
+ };
+ const auto min_value = to_date(date_statistics->getMinimum());
Review Comment:
[P1] Disable DATE MIN/MAX pushdown when bounds span the unrepresentable leap
day. A stripe containing -719528, -719469 (0000-02-29), and -719468 has valid
endpoints, so this ORC statistics conversion lets MIN/MAX succeed without
reading the invalid middle row; a row scan now errors. The Parquet footer
MIN/MAX path has the same gap in strict mode. Refuse metadata pushdown when raw
DATE bounds enclose `EPOCH_DAYS_0000_02_29` in both readers, and test an
invalid interior value.
##########
be/src/core/data_type_serde/data_type_datev2_serde.cpp:
##########
@@ -65,22 +62,44 @@ Status decode_date_orc_values(const DataTypeSerDe& serde,
IColumn& column,
orc_serde_utils::orc_decode_row_count(orc_view.rows,
orc_view.selected_rows);
std::vector<int32_t> date_values;
date_values.resize(output_rows);
- auto& date_dict = date_day_offset_dict::get();
for (size_t row = 0; row < output_rows; ++row) {
const auto source_row = orc_serde_utils::orc_source_row_at(row,
orc_view.selected_rows);
- const auto date =
date_dict[cast_set<int>(orc_batch->data[source_row])];
- date_values[row] = cast_set<int32_t>(date.daynr() -
DORIS_DATE_EPOCH_DAYNR);
+ // The payload of a null slot is undefined, so it is zeroed rather
than range-checked;
+ // `read_column_from_decoded_values` never decodes it.
+ if (view.null_map != nullptr && view.null_map[row] != 0) {
+ date_values[row] = 0;
+ continue;
+ }
+ // ORC DATE is days since 1970-01-01 in the proleptic Gregorian
calendar, the same encoding
+ // `decode_epoch_days()` expects, but ORC hands it over in an int64
batch. Reject a value
+ // that does not fit the int32 view here, while the real file value is
still available for
+ // the message, instead of narrowing it blindly.
+ const int64_t file_days = orc_batch->data[source_row];
+ if (file_days < std::numeric_limits<int32_t>::min() ||
+ file_days > std::numeric_limits<int32_t>::max()) {
+ return Status::DataQualityError(
+ "DATE value {} is outside the Doris DATE range
(0000-01-01..9999-12-31, "
+ "0000-02-29 excluded)",
+ file_days);
+ }
+ date_values[row] = static_cast<int32_t>(file_days);
}
view.values = reinterpret_cast<const uint8_t*>(date_values.data());
RETURN_IF_ERROR(orc_serde_utils::read_decoded_values(serde, column,
&view));
return Status::OK();
}
-Status decode_parquet_date(int32_t encoded_date, DateV2Value<DateV2ValueType>*
value) {
+// Shared by every "days since 1970-01-01" source: Parquet, ORC and Arrow
date32/date64. The
+// message must therefore not name one format, and it carries the value so a
broken file can be
+// identified without re-reading it.
+Status decode_epoch_days(int64_t encoded_date, DateV2Value<DateV2ValueType>*
value) {
DORIS_CHECK(value != nullptr);
- const int64_t day_number = static_cast<int64_t>(encoded_date) +
date_threshold;
- if (day_number < 0 ||
!value->get_date_from_daynr(static_cast<uint64_t>(day_number))) {
- return Status::DataQualityError("Parquet DATE value is out of range");
+ const int64_t day_number = epoch_days_to_daynr(encoded_date);
Review Comment:
[P2] Preserve invalid DATE errors when pruning predicates. A one-stripe ORC
file containing -719469 (0000-02-29) and `WHERE d >= DATE '0001-01-01'` can be
discarded by DATE SARG before this decoder runs, returning empty success,
whereas the same scan without SARG errors on that row. Strict Parquet
footer/page pruning has the same behavior when valid bounds enclose -719469.
Keep a DATE granule when its raw bounds may contain an unrepresentable ordinal,
or otherwise ensure the conversion error is raised; add pruning-on/off coverage.
##########
be/src/core/data_type_serde/data_type_datev2_serde.cpp:
##########
@@ -100,7 +119,7 @@ class DateV2ParquetConsumer final : public
ParquetFixedValueConsumer {
_data.resize(old_size + num_values);
for (size_t row = 0; row < num_values; ++row) {
DateV2Value<DateV2ValueType> value;
- const auto status = decode_parquet_date(
+ const auto status = decode_epoch_days(
Review Comment:
[P1] Keep Parquet NULL metadata consistent with DATE conversion failures. In
non-strict nullable scans this decoder turns -719469 (0000-02-29) into NULL,
but a row group with valid bounds around that value and physical null_count=0
is pruned for `WHERE d IS NULL` using has_null=false. `COUNT(d)` pushdown
likewise counts the physical definition level as 1 where a materialized scan
counts 0. Avoid these physical-null shortcuts when DATE conversion can fail, or
prove the raw DATE range is representable; test both queries against row
materialization.
--
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]
