Gabriel39 commented on code in PR #69723:
URL: https://github.com/apache/doris/pull/69723#discussion_r4430211303
##########
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:
Clarifying the supported contract: the official rolling-upgrade procedure
upgrades all BEs before upgrading FEs:
https://doris.apache.org/docs/dev/admin-manual/cluster-management/upgrade/#upgrade-flow-overview
. Old-FE/new-BE COUNT requests without semantic arguments retain their
ordinary-read fallback. The new-FE/old-BE scenario described here reverses that
supported order, so no legacy routing gate is added. The deprecated scanner
option remains a no-op. The PR description now states this direction
explicitly, consistent with the latest review disposition.
##########
be/src/load/delta_writer/push_handler.cpp:
##########
@@ -35,20 +35,15 @@
#include <queue>
#include <shared_mutex>
#include <type_traits>
+#include <utility>
+#include "common/cast_set.h"
#include "common/compiler_util.h" // IWYU pragma: keep
#include "common/config.h"
#include "common/logging.h"
#include "common/status.h"
#include "core/block/block.h"
-#include "core/block/column_with_type_and_name.h"
-#include "core/data_type/data_type_bitmap.h"
-#include "core/data_type/data_type_factory.hpp"
-#include "core/data_type/data_type_nullable.h"
-#include "exprs/function/function_helpers.h"
-#include "exprs/function/simple_function_factory.h"
-#include "exprs/vexpr_context.h"
-#include "format/parquet/vparquet_reader.h"
+#include "format_v2/table/load_reader.h"
#include "io/hdfs_builder.h"
#include "runtime/descriptors.h"
Review Comment:
Revalidated at 83e6d40df16. The source descriptor and required slots follow
validated src_slot_ids order; unused source slots are omitted and unknown,
foreign and duplicate IDs fail explicitly. The omitted/reordered projection,
path-column, NULL and invalid-ID cases pass within all 18 LoadReaderTest tests
in the 565-test ASAN run. This earlier fix remains intact after the rebase.
##########
be/src/format_v2/orc/orc_reader.cpp:
##########
@@ -882,6 +886,180 @@ 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]);
+ }
+ if (_columns.empty()) {
+ return;
+ }
+ // The SDK's depth-first dotted-path map uses last-write wins: root
"a.b" can name
+ // nested a.b instead. Resolve requested names once and retain only
matching physical IDs.
+ std::unordered_map<std::string, uint64_t> sdk_type_ids;
+ for (const auto& [id, column] : _columns) {
+ sdk_type_ids.emplace(column.name, id);
+ }
+ const auto resolve_names = [&](auto&& self, const ::orc::Type& type,
+ const std::string& prefix) -> void {
+ for (uint64_t i = 0; i < type.getSubtypeCount(); ++i) {
+ const auto& child = *type.getSubtype(i);
+ auto path = prefix;
+ if (type.getKind() == ::orc::STRUCT) {
+ path = type.getColumnId() == 0 ? type.getFieldName(i)
+ : prefix + "." +
type.getFieldName(i);
+ if (const auto it = sdk_type_ids.find(path); it !=
sdk_type_ids.end()) {
+ it->second = child.getColumnId();
+ }
+ }
+ self(self, child, path);
+ }
+ };
+ resolve_names(resolve_names, *_reader->_state->root_type, "");
+ std::erase_if(_columns, [&](const auto& entry) {
+ return sdk_type_ids.at(entry.second.name) != entry.first;
+ });
+ }
+
+ bool empty() const { return _columns.empty(); }
+
+ void fillDictFilterColumnNames(std::unique_ptr<::orc::StripeInformation>,
+ std::list<std::string>& names) const
override {
+ for (const auto& [id, column] : _columns) {
+ names.push_back(column.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& [id, column] : _columns) {
+ const auto it = dictionaries.find(column.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 {
+ std::string name;
+ 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.
Review Comment:
Revalidated at 83e6d40df16. The callback validates the zero sentinel,
nondecreasing offsets and dictionary-blob bounds before subtraction or entry
access, retaining CORRUPTION across the SDK boundary.
DictionaryRejectsMalformedOffsetsBeforePruning and all 190 ORC ASAN tests pass.
This earlier fix remains intact.
##########
be/src/format_v2/json/json_reader.cpp:
##########
@@ -540,8 +541,17 @@ Status JsonReader::_open_file_reader(bool need_schema) {
_reader_range.__isset.modification_time ?
_reader_range.modification_time : 0;
auto reader_options =
FileFactory::get_reader_options(_runtime_state->query_options(),
*_file_description);
- auto file_reader = DORIS_TRY(FileFactory::create_file_reader(
- *_system_properties, *_file_description, reader_options,
_profile));
+ // The aligned prefix belongs to this scan, while the original split
end bounds
+ // asynchronous read-ahead. Shared ownership keeps cancellation state
alive for workers.
+ const auto range_end =
+ _reader_range.size > 0
+ ? static_cast<size_t>(_reader_range.start_offset +
_reader_range.size)
+ : std::numeric_limits<size_t>::max();
+ auto file_reader = DORIS_TRY(io::DelegateReader::create_file_reader(
Review Comment:
Revalidated at 83e6d40df16. Filesystem JSON readers drain and publish their
profile before aliases are released; load-owned stream pipes retain external
ownership. All three S3/JsonReaderPrefetchTest.ClosePublishesReadStatisticsOnce
variants pass in the 565-test ASAN run, including repeated close and
schema-read coverage.
--
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]
