github-actions[bot] commented on code in PR #69163:
URL: https://github.com/apache/doris/pull/69163#discussion_r4292145653
##########
be/src/format_v2/orc/orc_reader.cpp:
##########
@@ -1866,24 +2001,104 @@ Status
OrcReader::_select_stripe_ranges_by_statistics() {
return Status::OK();
}
-void OrcReader::_apply_current_stripe_range() {
- if (!_state->stripe_pruning_applied ||
_state->selected_stripe_ranges.empty()) {
- return;
+Status OrcReader::_prepare_timestamp_stripe_ranges() {
+ if (!_state->timestamp_plan.requires_stripe_reader()) {
+ return Status::OK();
+ }
+ std::vector<uint64_t> stripes;
+ std::set<uint64_t> unsafe_date_stripes;
+ if (_state->stripe_ranges_applied) {
+ for (const auto& range : _state->selected_stripe_ranges) {
+ for (auto stripe = range.first_stripe; stripe < range.last_stripe;
++stripe) {
+ stripes.push_back(stripe);
+ if (range.disable_sarg) {
+ unsafe_date_stripes.insert(stripe);
+ }
+ }
+ }
+ } else {
+ RETURN_IF_ERROR(_collect_split_stripes(&stripes));
+ }
+ _state->selected_stripe_ranges.clear();
+ _state->stripe_ranges_applied = true;
+ _reader_statistics.read_row_groups = cast_set<int32_t>(stripes.size());
+ if (stripes.empty()) {
+ return Status::OK();
+ }
+ try {
+ uint64_t first_row = 0;
+ size_t selected = 0;
+ std::string previous_timezone;
+ for (uint64_t index = 0; index <= stripes.back(); ++index) {
+ const auto stripe = _state->reader->getStripe(index);
+ const auto end_row = first_row + stripe->getNumberOfRows();
+ if (index == stripes[selected]) {
+ const auto& name = stripe->getWriterTimezone();
Review Comment:
[P2] Avoid eagerly reading every stripe footer
In the pinned SDK, `getWriterTimezone()` loads and parses the stripe footer,
and each `getStripe()` creates a fresh object whose parsed footer is discarded
here. A large cold S3/HDFS scan with a NANOS sibling and any projected plain
timestamp therefore performs a synchronous footer read for every selected
stripe before its first row, even for `LIMIT 1` or row IDs confined to one
stripe. Physical splitting also opens the parent reader first. Discover
timezone runs incrementally or retain reusable footer metadata so unvisited
stripes do not determine startup cost.
##########
be/src/format_v2/orc/orc_reader.cpp:
##########
@@ -1152,7 +1255,12 @@ DataTypePtr OrcReader::_convert_to_doris_type(const
::orc::Type& type) const {
data_type = std::make_shared<DataTypeDateV2>();
break;
case ::orc::TypeKind::TIMESTAMP:
- data_type = std::make_shared<DataTypeDateTimeV2>(6);
+ if (type.hasAttributeKey("iceberg.timestamp-unit") &&
+ type.getAttributeValue("iceberg.timestamp-unit") == "NANOS") {
+ data_type = std::make_shared<DataTypeTimeStampNs>();
Review Comment:
[P2] Preserve nanos range validation in metadata COUNT
For an Iceberg v3 table with an optional `timestamp_ns` column backed by an
externally supplied annotated ORC file, a non-null civil value such as
`1500-01-01` is rejected by the new nanos SerDe. However, `SELECT COUNT(ts)`
with V2 aggregate pushdown uses a trivial same-type mapping and reaches
`get_aggregate_result`, which validates only DATE leaves before returning
`getNumberOfValues()`. It therefore succeeds where scanning the argument
reports `DataQualityError`. Extend the existing COUNT validation/fallback to
NANOS timestamp leaves, including nested arguments, while keeping `COUNT(*)`
metadata-only.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -2407,11 +2409,12 @@ protected List<FileScanTask>
loadFileScanTasksWithManifestCache(
}
ExternalTable targetExternalTable = (ExternalTable)
source.getTargetTable();
Schema scanSchema = scan.schema();
+ Set<Integer> unsafeNanosFields =
IcebergTimestampUtils.unsafeNanosFields(icebergTable, scanSchema);
Review Comment:
[P1] Preserve equality deletes after DATE-to-nanos promotion
With `meta.cache.iceberg.manifest.enable=true`, the new unsafe-field set
only disables query pruning. In an unpartitioned v3 table, let an old required
`DATE ts` file contain `2024-01-02` (four-byte bounds `19724`), then promote
`ts` to `timestamp_ns` and add a later equality delete for that midnight
(bounds `1704153600000000000`). This planner's `DeleteFileIndex` uses the
latest field type: `Conversions.fromByteBuffer` widens the DATE bounds to
`19724L`, `rangesOverlap` returns false, and the delete is omitted, so `SELECT`
returns the deleted row even without a `ts` predicate. Apply the retained-DATE
safeguard to equality-delete applicability too, or decode historical bounds
into the promoted unit. This promotion and the historical bound representation
are defined by the [Iceberg
specification](https://iceberg.apache.org/spec/#schema-evolution).
--
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]
