Gabriel39 commented on code in PR #68649:
URL: https://github.com/apache/doris/pull/68649#discussion_r4135169722
##########
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:
Fixed in 0842fe5d56e. Both DATE day and bucket partition transforms now use
daynr_to_epoch_days, matching the Arrow/Parquet payload. Added a unit test for
the year-zero boundary and epoch controls, plus a Spark/Iceberg partitioned
write/read regression that checks partition tuples and selective day/bucket
predicates. The unit test passes; the external regression compiles and awaits
execution in external CI.
##########
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:
Fixed in 0842fe5d56e. Added a shared representability check for the entire
raw DATE interval, including the Gregorian-only leap day. ORC and Parquet
MIN/MAX now decline metadata pushdown when the interval can contain an invalid
DATE, even if both endpoints are valid. New tests use -719528, -719469 and
-719468 and verify fallback; representable ranges retain their optimization.
All 240 selected unit tests pass under ASAN.
##########
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:
Fixed in 0842fe5d56e. ORC checks selected DATE domains against file
statistics before enabling SDK SARG; an unknown or unsafe range keeps row
decoding active. Parquet footer and page statistics are unavailable when the
DATE interval can contain an invalid ordinal. Added ORC SARG on/off and strict
Parquet pruning on/off tests that preserve the -719469 error, plus explicit
page-index candidate-retention coverage. These tests fail before the fix and
pass afterward.
##########
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:
Fixed in 0842fe5d56e. Unsafe DATE ranges invalidate physical null statistics
as well as MIN/MAX. COUNT(DATE) now uses definition levels/footer counts only
after statistics prove the range is representable (or entirely NULL); otherwise
it falls back to materialization. Added non-strict IS NULL pruning on/off
coverage and a COUNT fallback check against materialized NULLs. Additional
tests confirm valid required/nullable DATE ranges and all-NULL columns retain
their COUNT shortcut.
--
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]
