github-actions[bot] commented on code in PR #65498:
URL: https://github.com/apache/doris/pull/65498#discussion_r3567612022
##########
be/src/format_v2/column_mapper.cpp:
##########
@@ -990,6 +990,18 @@ static bool mapping_can_use_file_column_directly(const
ColumnMapping& mapping) {
return !needs_complex_rematerialize(mapping);
}
+static FilterConversionType direct_filter_conversion(const ColumnMapping&
mapping) {
+ DORIS_CHECK(mapping.table_type != nullptr);
+ // FileScanOperator deliberately keeps VARBINARY predicates above external
readers. Their
+ // physical binary representations are not uniformly supported by
reader-side expression and
+ // metadata filtering, so localizing a late runtime filter here can
incorrectly reject rows.
+ if (remove_nullable(mapping.table_type)->get_primitive_type() ==
TYPE_VARBINARY) {
+ return FilterConversionType::FINALIZE_ONLY;
Review Comment:
This keeps projected VARBINARY filters above the reader, but it breaks
predicate-only VARBINARY filters. Hidden filter mappings are created after the
visible `projected_columns` loop, and `FINALIZE_ONLY` makes
`filter_conversion_has_local_source()` false, so `localize_filters()` skips the
hidden VARBINARY mapping instead of adding it as a predicate column. It is also
never added as a non-predicate column, and `TableReader::finalize_chunk()` only
materializes visible `mappings()`, not hidden mappings. A query that filters on
a non-projected VARBINARY column therefore reaches the scanner-side fallback
without the VARBINARY slot in the table block. Please make predicate-only
VARBINARY filters materialize a column for final evaluation while still
avoiding reader-local predicate pushdown, and add a regression where the output
projects a different column and the VARBINARY column appears only in the filter.
##########
be/src/format_v2/parquet/reader/map_column_reader.cpp:
##########
@@ -126,32 +143,50 @@ Status MapColumnReader::build_nested_column(int64_t
length_upper_bound, MutableC
}
const bool parent_is_null = def_level < _definition_level - 1;
- if (parent_is_null && parent_null_map == nullptr) {
+ if (parent_is_null && !_type->is_nullable()) {
return Status::Corruption("Parquet MAP column {} contains null for
non-nullable MAP",
_name);
}
parent_nulls.push_back(parent_is_null);
entry_counts.push_back(def_level >= _definition_level ? 1 : 0);
- ++*values_read;
+ ++*values_processed;
}
set_nested_build_level_cursor(level_idx);
uint64_t total_entries = 0;
for (const auto entry_count : entry_counts) {
total_entries += entry_count;
}
- const size_t key_start = key_column->size();
int64_t key_value_count = 0;
-
RETURN_IF_ERROR(_key_reader->build_nested_column(static_cast<int64_t>(total_entries),
- key_column,
&key_value_count));
+ size_t key_start = 0;
+ if (column != nullptr) {
+ key_start = key_column->size();
+
RETURN_IF_ERROR(_key_reader->build_nested_column(static_cast<int64_t>(total_entries),
+ key_column,
&key_value_count));
+ } else if (auto* scalar_key_reader =
dynamic_cast<ScalarColumnReader*>(_key_reader.get())) {
+ // MAP keys are required even if a projected Doris key type is
nullable. Validate each
+ // actual entry directly from the key level stream while advancing
past empty/null maps.
+ for (const int64_t key_level_idx : map_level_indices) {
+ if (def_levels[key_level_idx] >= _definition_level) {
+
RETURN_IF_ERROR(scalar_key_reader->validate_nested_value(key_level_idx, true));
+ ++key_value_count;
+ }
+ }
+ scalar_key_reader->set_nested_build_level_cursor(level_idx);
+ } else {
+
RETURN_IF_ERROR(_key_reader->consume_nested_column(static_cast<int64_t>(total_entries),
Review Comment:
`consume_nested_column()` is documented to preserve the same null/alignment
validation as `build_nested_column()`, but this non-scalar key branch only
checks that the complex key reader consumed the expected count. MAP schemas
intentionally accept optional key fields for compatibility, and the
materialized path still rejects actual null keys by checking the appended key
column. With a nullable STRUCT/LIST MAP key, skipping a row now takes this
branch; the complex key reader accepts the null key as a valid nullable value
and returns the expected count, so malformed data is silently accepted whenever
that row is skipped. Please make the consume-only MAP key path validate that
every key entry is non-null regardless of key reader kind, and add a skip
regression for an optional complex key containing a null key.
--
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]
