github-actions[bot] commented on code in PR #68024:
URL: https://github.com/apache/doris/pull/68024#discussion_r4043133526
##########
be/src/format_v2/orc/orc_search_argument.cpp:
##########
@@ -1248,17 +1248,17 @@ ::orc::Literal ceil_timestamp_literal_to_millis(const
::orc::Literal& literal) {
return shift_timestamp_literal(literal, NANOS_PER_MILLISECOND - remainder);
}
-std::pair<::orc::Literal, ::orc::Literal> timestamp_rounding_bounds(const
::orc::Literal& literal) {
- constexpr int32_t HALF_MICROSECOND_NANOS = 500;
- // All raw ORC values in [lower, upper) round half-up to this Doris
microsecond. Round these
+std::pair<::orc::Literal, ::orc::Literal> timestamp_truncation_bounds(
+ const ::orc::Literal& literal) {
+ constexpr int32_t NANOS_PER_MICROSECOND = 1000;
+ // All raw ORC values in [lower, upper) truncate to this Doris
microsecond. Expand these
// boundaries toward the side that enlarges the SARG match set because ORC
statistics retain
// only millisecond precision.
- return {shift_timestamp_literal(literal, -HALF_MICROSECOND_NANOS),
- shift_timestamp_literal(literal, HALF_MICROSECOND_NANOS)};
+ return {literal, shift_timestamp_literal(literal, NANOS_PER_MICROSECOND)};
Review Comment:
[P2] Re-enable safe pruning for exact-epoch predicates
With the new truncation preimage `[L, L + 1us)`, epoch now maps to `[0ns,
1000ns)`; the old `-500ns` half-up rationale in the plain-`TIMESTAMP` guards at
lines 984-990 and 1067-1070 no longer applies. Keeping the `(second == 0 &&
nanos == 0)` exclusion prevents this predicate from pruning any stripe (and
makes an `IN` containing epoch lose its entire envelope), so it causes a full
scan whenever this is the only usable filter, even though ORC's reconstructed
negative statistics still compare below `(0,0)`. Please retain the
negative-seconds fallback, remove the exact-zero exclusions, and add an epoch
pruning test with a negative-only stripe.
##########
be/src/format_v2/orc/orc_reader.cpp:
##########
@@ -502,22 +502,12 @@ std::optional<DateV2Value<DateTimeV2ValueType>>
datetime_v2_from_orc_millis(
}
const auto extra_nanos = std::max<int32_t>(nanos_tail, 0);
constexpr int64_t NANOS_PER_MICROSECOND = 1000;
- constexpr int64_t MICROS_PER_SECOND = 1000000;
// Stripe statistics split the timestamp into milliseconds and the
remaining nanoseconds. Use
- // the same half-up rule as row decoding so zone-map pruning observes
identical values.
- const auto rounded_extra_microseconds =
- (extra_nanos + NANOS_PER_MICROSECOND / 2) / NANOS_PER_MICROSECOND;
- const auto microseconds_with_carry = millis_remainder * 1000 +
rounded_extra_microseconds;
- // Calendar bounds depend on the target timezone, so only reject
arithmetic overflow here and
- // let the converted value below decide whether the statistic is
representable by Doris.
- int64_t rounded_seconds;
- if (__builtin_add_overflow(seconds, microseconds_with_carry /
MICROS_PER_SECOND,
- &rounded_seconds)) {
- return std::nullopt;
- }
- const auto microseconds = cast_set<uint64_t>(microseconds_with_carry %
MICROS_PER_SECOND);
+ // the same truncation as row decoding so zone-map pruning observes
identical values.
+ const auto microseconds =
Review Comment:
[P1] Reject legacy timestamp stats without exact nanos tails
This converter is being changed to match row decoding, but the
writer-version gate does not prove that these nanos-tail fields were
serialized. ORC 1.6.3 files can report `ORC_14` (so they pass the `< ORC_135`
check) while lacking the fields added by ORC-611; the pinned reader substitutes
conservative tails of `0`/`999999`. For a one-row value ending in `.123456789`,
this path can therefore return exact `MIN=.123000` and `MAX=.123999`, while row
decoding returns `.123456` (and `TIMESTAMP_INSTANT` has no corresponding
version gate). Please fall back unless exact tails are known to be present, and
cover a legacy/no-tail fixture against a row scan.
--
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]
