github-actions[bot] commented on code in PR #66200:
URL: https://github.com/apache/doris/pull/66200#discussion_r3670579968
##########
be/src/format_v2/parquet/reader/native/column_chunk_reader.cpp:
##########
@@ -804,6 +825,85 @@ class FixedWidthPredicateConsumer final : public
ParquetFixedValueConsumer {
int _column_id;
IColumn::Filter* _matches;
IColumn* _projected_column;
+ IColumn::Filter* _conversion_nulls;
+};
+
+class BinaryPredicateConsumer final : public ParquetFixedValueConsumer,
+ public ParquetBinaryValueConsumer {
+public:
+ BinaryPredicateConsumer(const VExprSPtrs& conjuncts, DataTypePtr
data_type, int column_id,
+ IColumn::Filter* matches, IColumn*
projected_column,
+ std::vector<StringRef>* refs,
std::vector<StringRef>* projected_refs)
+ : _conjuncts(conjuncts),
+ _data_type(std::move(data_type)),
+ _column_id(column_id),
+ _matches(matches),
+ _projected_column(projected_column),
+ _refs(refs),
+ _projected_refs(projected_refs) {
+ DORIS_CHECK(_matches != nullptr);
+ DORIS_CHECK(_refs != nullptr);
+ DORIS_CHECK(_projected_refs != nullptr);
+ }
+
+ Status consume(const uint8_t* values, size_t num_values, size_t
value_width) override {
+ _refs->resize(num_values);
+ for (size_t row = 0; row < num_values; ++row) {
+ (*_refs)[row] = StringRef(reinterpret_cast<const char*>(values +
row * value_width),
+ value_width);
+ }
+ return consume(_refs->data(), _refs->size());
+ }
+
+ Status consume_plain_byte_array(
+ const char* encoded_data, const uint32_t* payload_offsets,
+ const uint32_t* value_offsets, size_t num_values,
+ const std::vector<ParquetSelectionRange>& value_spans) override {
+ _refs->resize(num_values);
+ size_t covered = 0;
+ for (const auto& span : value_spans) {
+ DORIS_CHECK_EQ(span.first, covered);
+ for (size_t row = span.first; row < span.first + span.count;
++row) {
+ (*_refs)[row] = StringRef(encoded_data + payload_offsets[row],
+ value_offsets[row + 1] -
value_offsets[row]);
+ }
+ covered += span.count;
+ }
+ DORIS_CHECK_EQ(covered, num_values);
+ return consume(_refs->data(), _refs->size());
+ }
+
+ Status consume(const StringRef* values, size_t num_values) override {
+ const size_t old_size = _matches->size();
+ _matches->resize_fill(old_size + num_values, 1);
+ for (const auto& conjunct : _conjuncts) {
+ RETURN_IF_ERROR(conjunct->execute_on_raw_binary_values(
+ values, num_values, _data_type, _column_id,
_matches->data() + old_size));
+ }
+ if (_projected_column != nullptr) {
+ _projected_refs->clear();
+ _projected_refs->reserve(num_values);
+ for (size_t row = 0; row < num_values; ++row) {
+ if ((*_matches)[old_size + row] != 0) {
+ _projected_refs->push_back(values[row]);
+ }
+ }
+ if (!_projected_refs->empty()) {
+ _projected_column->insert_many_strings(_projected_refs->data(),
Review Comment:
[P1] Preserve non-NULL empty strings in direct projection
`DELTA_BYTE_ARRAY` selected decode represents an empty but physically
present value as `StringRef(nullptr, 0)`. Passing that survivor to a nullable
output column here makes `ColumnNullable::insert_many_strings()` set the
null-map bit, so a projected `''` becomes SQL NULL. This path is reachable
because raw-binary filtering admits `DELTA_BYTE_ARRAY` and projected predicate
columns pass the nullable file-block column. Please insert into the nested
string column while appending zero null bits (or otherwise normalize the empty
ref), and add projected empty-value coverage.
##########
be/src/exprs/vtopn_pred.h:
##########
@@ -121,14 +121,64 @@ class VTopNPred : public VExpr {
RETURN_IF_ERROR(_function->execute(nullptr, temp_block, arguments,
num_columns_without_result,
temp_block.rows()));
result_column =
std::move(temp_block.get_by_position(num_columns_without_result).column);
- if (is_nullable() && _predicate->nulls_first()) {
- // null values are always not filtered
- result_column = change_null_to_true(std::move(result_column));
+ if (auto mutable_result = IColumn::mutate(std::move(result_column));
Review Comment:
[P1] Collapse NULL inside constant TopN results
When partition-only TopN runs on an external partition value, both operands
are constants, so the comparison result is
`ColumnConst(ColumnNullable(UInt8))`. This branch only recognizes an outer
`ColumnNullable`; the NULL result survives, and `execute_filter(...,
accept_null=false)` treats it as false, allowing a NULL partition to be pruned
even with `NULLS FIRST`. The removed `change_null_to_true()` helper recursed
through `ColumnConst`. Please unwrap/materialize the constant (or recurse and
rewrap it with the original row count) and cover this case.
--
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]
