Gabriel39 commented on code in PR #69723:
URL: https://github.com/apache/doris/pull/69723#discussion_r4420467095
##########
be/src/format_v2/orc/orc_reader.cpp:
##########
@@ -882,6 +886,150 @@ struct OrcReaderScanState {
bool row_reader_created = false;
};
+// Dictionary rejection is independent of SARG min/max pruning: an absent
value can lie
+// between both bounds. Keep residual predicates intact; this callback only
rejects stripes.
+class OrcReader::StringDictFilterImpl final : public ::orc::StringDictFilter {
+public:
+ explicit StringDictFilterImpl(OrcReader* reader) : _reader(reader) {
+ const auto& request = *_reader->_request;
+ // Phase-two fetches must materialize every requested physical row.
+ if (request.row_ids.has_value()) {
+ return;
+ }
+ const auto safe_count =
+ std::min(request.metadata_pruning_safe_conjunct_count,
request.conjuncts.size());
+ for (size_t i = 0; i < safe_count; ++i) {
+ _add_predicate(request.conjuncts[i]);
+ }
+ }
+
+ bool empty() const { return _columns.empty(); }
+
+ void fillDictFilterColumnNames(std::unique_ptr<::orc::StripeInformation>,
+ std::list<std::string>& names) const
override {
+ for (const auto& [name, column] : _columns) {
+ names.push_back(name);
+ }
+ }
+
+ void onStringDictsLoaded(
+ std::unordered_map<std::string, ::orc::StringDictionary*>&
dictionaries,
+ bool* stripe_filtered) const override {
+
SCOPED_RAW_TIMER(&_reader->_reader_statistics.dict_filter_rewrite_time);
+ *stripe_filtered = false;
+ for (const auto& [name, column] : _columns) {
+ const auto it = dictionaries.find(name);
+ // A later stripe may use direct encoding for a column that
previously had a dictionary.
+ if (it == dictionaries.end()) {
+ continue;
+ }
+ bool has_match = false;
+ THROW_IF_ERROR(_has_match(column, *it->second, &has_match));
+ if (!has_match) {
+ *stripe_filtered = true;
+ if (_reader->_profile != nullptr) {
+
COUNTER_UPDATE(_reader->_orc_profile.dictionary_stripes_filtered, 1);
+ }
+ return;
+ }
+ }
+ }
+
+private:
+ struct DictionaryColumn {
+ size_t position = 0;
+ DataTypePtr type;
+ VExprContextSPtrs conjuncts;
+ };
+
+ void _add_predicate(const VExprContextSPtr& conjunct) {
+ const auto& root = conjunct->root();
+ const auto& children = root->children();
+ // Only direct slot/literal comparisons and IN lists are stable
dictionary predicates.
+ // Casts, multi-column expressions and mutable runtime filters retain
row-level evaluation.
+ if ((root->node_type() != TExprNodeType::BINARY_PRED &&
+ root->node_type() != TExprNodeType::IN_PRED) ||
+ root->is_rf_wrapper() || children.size() < 2 ||
!children[0]->is_slot_ref() ||
+ !std::ranges::all_of(children.begin() + 1, children.end(),
+ [](const auto& child) { return
child->is_literal(); })) {
+ return;
+ }
+ const auto* slot = assert_cast<const VSlotRef*>(children[0].get());
+ const auto& request = *_reader->_request;
+ const auto column = std::ranges::find_if(request.local_positions,
[&](const auto& entry) {
+ return static_cast<int64_t>(entry.second.value()) ==
slot->column_id();
+ });
+ if (column == request.local_positions.end() ||
is_virtual_column(column->first)) {
+ return;
+ }
+ const auto id = column->first.value();
+ const auto& type = *_reader->_state->root_type->getSubtype(id);
+ // CHAR padding and binary logical annotations need their own
physical-to-value rules.
+ if (type.getKind() != ::orc::STRING && type.getKind() !=
::orc::VARCHAR) {
+ return;
+ }
+ auto& target = _columns[_reader->_state->root_type->getFieldName(id)];
Review Comment:
Addressed in af7cb5253fc. Dictionary pruning now verifies that the SDK name
lookup resolves to the intended physical type ID and skips ambiguous names.
DictionaryNamesPreservePhysicalColumnIdentity covers both root-field orderings
and an unambiguous dotted-name control. The collision reproduced an incorrect
zero-row result before the fix; all 187 ORC tests pass with the fix.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java:
##########
@@ -756,13 +756,10 @@ private LogicalAggregate<? extends Plan>
storageLayerAggregate(
// COUNT(nullable_col); synthetic rows would make one
upper aggregate wrong.
return canNotPush;
}
- // Nullable file COUNT is exact only when this query is routed
to FileScannerV2,
- // which carries the semantic argument and counts definition
levels. Gating on the
- // session switch keeps V1 on its original full-column
evaluation path.
+ // File scans preserve the COUNT argument and count its
definition levels,
+ // so nullable COUNT can use exact storage-layer aggregation.
boolean supportsNullableFileCount = logicalScan instanceof
LogicalFileScan
- && mergeOp == PushDownAggOp.COUNT
- && cascadesContext.getConnectContext() != null
- &&
cascadesContext.getConnectContext().getSessionVariable().enableFileScannerV2;
+ && mergeOp == PushDownAggOp.COUNT;
if (column.isAllowNull() && checkNullSlots.contains(slot)
Review Comment:
Mixed-version FE/BE upgrade compatibility is outside the supported scope of
this change. The deprecated scanner option intentionally remains a no-op, and
nullable COUNT pushdown targets the current backend contract. No compatibility
gate is added.
--
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]
