github-actions[bot] commented on code in PR #69723:
URL: https://github.com/apache/doris/pull/69723#discussion_r4420555519
##########
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:
[P1] Preserve the broker source-slot projection.
`TBrokerScanRangeParams.src_slot_ids` is the file tuple's selected slot list
and may be a subset or a different order from `source->slots()`. This loop
instead synthesizes `required_slots` from every source slot and marks the first
N as physical, so V2 can read the wrong Parquet columns and apply destination
expressions to the wrong values. Build the required slots in `src_slot_ids`
order (while validating each ID) and add a test with an extra/reordered source
slot.
##########
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:
[P2] Bound-check ORC dictionary offsets before pruning.
`dictionaryOffset.size() - 1` underflows when a corrupt/empty dictionary has no
sentinel, and the wrapped loop then calls `getValueByIndex` out of range.
Return a corruption status or disable dictionary pruning and retain the
residual predicates unless the offset vector is nonempty, monotonic, and fully
bounded.
--
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]
