github-actions[bot] commented on code in PR #68516:
URL: https://github.com/apache/doris/pull/68516#discussion_r4110176149
##########
be/src/format_v2/table_reader.cpp:
##########
@@ -53,9 +54,29 @@
#include "runtime/file_scan_profile.h"
#include "storage/segment/condition_cache.h"
#include "util/debug_points.h"
+#include "util/hash_util.hpp"
#include "util/string_util.h"
namespace doris::format {
+
+std::optional<std::string> TableReader::_get_int96_timezone_override(
+ const TFileScanRangeParams* params) {
+ if (params == nullptr) {
+ return std::nullopt;
+ }
+ // The timezone field predates the version marker. Honor intermediate FEs
that send it alone,
+ // including an explicit empty value selecting wall-clock semantics.
+ if (params->__isset.hive_parquet_time_zone) {
+ return params->hive_parquet_time_zone;
+ }
+ // Only a plan lacking both an explicit timezone and the new contract uses
the legacy session.
+ if (!params->__isset.parquet_timestamp_semantics_version ||
Review Comment:
[P1] Preserve the base V2 meaning of an old-FE omission. Before this PR,
`create_file_reader` converted an absent field 36 to an empty string; the
scheduler's raw null pointer was then passed to the already-optional
native-reader parameter as an engaged null, selecting wall-clock INT96
decoding. Returning `nullopt` here leaves the new scheduler optional
disengaged, so the same old-FE payload now falls back to the session timezone.
Base FEs omit the empty property and FileScannerV2 is enabled by default, so an
old-FE/new-BE rolling query in America/Los_Angeles shifts raw `16:00` from
`16:00` to `08:00` (the new test currently codifies that regression). Please
preserve the old engaged-empty state or use another reliable legacy
discriminator, and test an actual old serialized payload against base V2
behavior.
##########
be/src/format_v2/table_reader.cpp:
##########
@@ -1245,8 +1276,22 @@ Status TableReader::_init_reader_condition_cache(const
FileScanRequest& file_req
return Status::OK();
}
const auto& file = *_current_file_description;
+ auto cache_digest = _condition_cache_digest;
Review Comment:
[P1] Include the localized predicate identity in this process-global cache
key. `_should_enable_condition_cache` computes a nonzero digest of the
post-mapping conjuncts, but it is only used as a gate and discarded here. For
example, two Hive schemas can reference the same immutable file, both expose
external `x INT` (unique ID `-1`), and use BY_INDEX to map `x` to same-typed
physical `a` versus `b`. Both original `x = 7` predicates hash identically,
while localization produces distinct cacheable `a = 7` and `b = 7` digests.
Because `ExternalCacheKey` has no table/schema identity, an all-false granule
cached for `a = 7` can hit the `b = 7` scan and silently drop a match. Please
domain-separate and mix `localized_digest` into `cache_digest`, with a
same-file/two-mapping cache test.
##########
be/src/core/data_type_serde/data_type_datetimev2_serde.cpp:
##########
@@ -72,20 +72,24 @@ Status decode_timestamp_orc_values(IColumn& nested_column,
const OrcDecodedColum
}
auto& value =
reinterpret_cast<DateV2Value<DateTimeV2ValueType>&>(data[old_data_size + row]);
- orc_serde_utils::TruncatedOrcTimestamp timestamp;
- auto status = orc_serde_utils::truncate_orc_timestamp_to_microseconds(
+ orc_serde_utils::RoundedOrcTimestamp timestamp;
+ auto status = orc_serde_utils::round_orc_timestamp_to_microseconds(
Review Comment:
[P1] Keep ORC precision independent of scanner routing. This changes native
V2 to half-up rounding (`123456789ns -> 123457us`), while the reachable V1
reader still assigns `nanoseconds / 1000` for both DATETIMEV2 and TIMESTAMPTZ,
and the changed Paimon test explicitly expects JNI to return `...123456` while
native returns `...123457`. Ordinary ORC can still select V1 or V2 via
`enable_file_scanner_v2`, so the same file and literal can return a different
value or change equality/range predicate truth solely with the scanner route.
Please preserve truncation until all routes agree, or update/gate every
reachable reader and add a differential same-file/same-literal test.
--
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]
