github-actions[bot] commented on code in PR #65674:
URL: https://github.com/apache/doris/pull/65674#discussion_r3627308190
##########
be/src/format_v2/parquet/parquet_statistics.cpp:
##########
@@ -1388,127 +1065,150 @@ void collect_request_leaf_schemas(
}
}
-bool build_page_skip_plan_for_leaf(
- const std::shared_ptr<::parquet::RowGroupPageIndexReader>& row_group,
- const ParquetColumnSchema& column_schema, const std::vector<RowRange>&
selected_ranges,
- int64_t row_group_rows, ParquetPageSkipPlan* page_skip_plan) {
- DORIS_CHECK(page_skip_plan != nullptr);
- *page_skip_plan = ParquetPageSkipPlan {};
- if (column_schema.kind != ParquetColumnSchemaKind::PRIMITIVE ||
- column_schema.descriptor == nullptr || column_schema.leaf_column_id <
0 ||
- column_schema.descriptor->max_repetition_level() != 0) {
+template <typename ValueType>
+bool set_native_page_scalar_min_max(const tparquet::ColumnIndex& column_index,
+ const ParquetColumnSchema& column_schema,
size_t page_idx,
+ DecodedValueKind kind,
ParquetColumnStatistics* page_statistics,
+ const cctz::time_zone* timezone) {
+ if (page_idx >= column_index.min_values.size() || page_idx >=
column_index.max_values.size() ||
+ column_index.min_values[page_idx].size() != sizeof(ValueType) ||
+ column_index.max_values[page_idx].size() != sizeof(ValueType)) {
return false;
}
-
- std::shared_ptr<::parquet::OffsetIndex> offset_index;
- try {
- offset_index = row_group->GetOffsetIndex(column_schema.leaf_column_id);
- } catch (const ::parquet::ParquetException&) {
+ const auto min_value =
unaligned_load<ValueType>(column_index.min_values[page_idx].data());
+ const auto max_value =
unaligned_load<ValueType>(column_index.max_values[page_idx].data());
+ if constexpr (std::is_same_v<ValueType, int64_t>) {
+ if (!timestamp_min_max_is_safe(column_schema, min_value, max_value,
timezone)) {
+ return false;
+ }
+ }
+ if (!valid_min_max(min_value, max_value)) {
+ return true;
+ }
+ if (!set_decoded_field(column_schema, kind, min_value,
&page_statistics->min_value, timezone) ||
+ !set_decoded_field(column_schema, kind, max_value,
&page_statistics->max_value, timezone)) {
return false;
- } catch (const std::exception&) {
+ }
+ if (decoded_min_max_is_ordered(*page_statistics)) {
+ page_statistics->has_min_max = true;
+ }
+ return true;
+}
+
+bool build_native_page_statistics(const tparquet::ColumnIndex& column_index,
+ const ParquetColumnSchema& column_schema,
size_t page_idx,
+ int64_t page_rows, ParquetColumnStatistics*
page_statistics,
+ const cctz::time_zone* timezone) {
+ DORIS_CHECK(page_statistics != nullptr);
+ *page_statistics = {};
+ if (!column_index.__isset.null_counts || page_idx >=
column_index.null_pages.size() ||
+ page_idx >= column_index.null_counts.size()) {
return false;
}
- if (offset_index == nullptr) {
+ const int64_t null_count = column_index.null_counts[page_idx];
+ const bool all_null = column_index.null_pages[page_idx];
+ if (page_rows < 0 || null_count < 0 || null_count > page_rows ||
+ all_null != (null_count == page_rows)) {
+ // The caller supplies the exact flat page or row-group span.
Contradictory optional null
+ // metadata must disable pruning instead of turning a partial span
into an all-null proof.
return false;
}
-
- const auto page_count = offset_index->page_locations().size();
- page_skip_plan->leaf_column_id = column_schema.leaf_column_id;
- page_skip_plan->skipped_pages.resize(page_count);
- page_skip_plan->skipped_page_compressed_sizes.resize(page_count);
- const auto& page_locations = offset_index->page_locations();
- for (size_t page_idx = 0; page_idx < page_count; ++page_idx) {
- const RowRange row_range = page_row_range(*offset_index, page_idx,
row_group_rows);
- if (row_range.length == 0 || ranges_intersect(selected_ranges,
row_range)) {
- continue;
+ page_statistics->has_null_count = true;
+ page_statistics->has_null = null_count > 0;
+ page_statistics->has_not_null = !all_null;
+ if (!page_statistics->has_not_null) {
+ return true;
+ }
+ switch (column_schema.type_descriptor.physical_type) {
+ case tparquet::Type::BOOLEAN:
+ return set_native_page_scalar_min_max<uint8_t>(column_index,
column_schema, page_idx,
Review Comment:
[P1] Decode BOOLEAN bounds with Parquet's bit packing
Parquet PLAIN BOOLEAN is bit-packed LSB-first, but this loads the entire
statistics byte and labels its address `BOOL`; NumberSerDe then reinterprets it
as a C++ `bool`. A byte such as `0x02` has a false value bit (the replaced
Arrow decoder read that bit), whereas this path creates a noncanonical `bool`
that may behave as true or otherwise invoke undefined behavior. Footer
statistics reuse this helper and ColumnIndex calls it directly, so the
misdecoded bound can drive false-negative pruning or wrong MIN/MAX. Decode bit
0 into a canonical Boolean before constructing the decoded view, and cover
footer/page-index bytes with high padding bits such as `0x02`.
##########
be/src/format_v2/parquet/parquet_statistics.cpp:
##########
@@ -109,8 +238,15 @@ Status read_decoded_field(const ParquetColumnSchema&
column_schema, DecodedColum
view.fixed_length = column_schema.type_descriptor.fixed_length;
view.timestamp_is_adjusted_to_utc =
column_schema.type_descriptor.timestamp_is_adjusted_to_utc;
view.timezone = timezone;
- return
column_schema.type->get_serde()->read_field_from_decoded_value(*column_schema.type,
-
field, view);
+ // Statistics are pruning proofs, not row materialization. A malformed
non-NULL bound must
+ // disable pruning instead of being converted to NULL under permissive
scan semantics.
+ view.enable_strict_mode = true;
+
RETURN_IF_ERROR(column_schema.type->get_serde()->read_field_from_decoded_value(
Review Comment:
[P1] Validate TIME bounds before publishing a ZoneMap
Strict mode does not validate TIME here:
`DataTypeTimeV2SerDe::read_column_from_decoded_values()` accepts any
INT32/INT64 carrier, while normal Parquet materialization rejects values
outside `[0, 24h)`. Thus a footer or ColumnIndex bound of 90,000,000
TIME_MILLIS becomes 25:00 and can make `time_col < 13:00` prune a row group
containing a valid 12:00 value; `INT64_MIN` also reaches `std::abs(INT64_MIN)`.
Reuse the raw-unit one-day-domain check before publishing min/max, and make
invalid bounds unusable for pruning/aggregation.
##########
be/src/format_v2/parquet/native_schema_desc.cpp:
##########
@@ -0,0 +1,848 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "format_v2/parquet/native_schema_desc.h"
+
+#include <ctype.h>
+
+#include <algorithm>
+#include <ostream>
+#include <utility>
+
+#include "common/cast_set.h"
+#include "common/exception.h"
+#include "common/logging.h"
+#include "core/data_type/data_type_array.h"
+#include "core/data_type/data_type_factory.hpp"
+#include "core/data_type/data_type_map.h"
+#include "core/data_type/data_type_struct.h"
+#include "core/data_type/define_primitive_type.h"
+#include "util/slice.h"
+#include "util/string_util.h"
+
+namespace doris::format::parquet {
+
+static bool is_group_node(const tparquet::SchemaElement& schema) {
+ return schema.num_children > 0;
+}
+
+static bool is_list_node(const tparquet::SchemaElement& schema) {
+ // Writers may emit only the modern logical type, so collection shape must
not depend on the
+ // deprecated converted_type field being duplicated in the footer.
+ return (schema.__isset.converted_type &&
+ schema.converted_type == tparquet::ConvertedType::LIST) ||
+ (schema.__isset.logicalType && schema.logicalType.__isset.LIST);
+}
+
+static bool is_map_node(const tparquet::SchemaElement& schema) {
+ return (schema.__isset.converted_type &&
+ (schema.converted_type == tparquet::ConvertedType::MAP ||
+ schema.converted_type == tparquet::ConvertedType::MAP_KEY_VALUE))
||
+ (schema.__isset.logicalType && schema.logicalType.__isset.MAP);
+}
+
+static bool has_primitive_only_annotation(const tparquet::SchemaElement&
schema) {
+ if (schema.__isset.logicalType) {
+ const auto& logical = schema.logicalType;
+ if (logical.__isset.STRING || logical.__isset.ENUM ||
logical.__isset.DECIMAL ||
+ logical.__isset.DATE || logical.__isset.TIME ||
logical.__isset.TIMESTAMP ||
+ logical.__isset.INTEGER || logical.__isset.UNKNOWN ||
logical.__isset.JSON ||
+ logical.__isset.BSON || logical.__isset.UUID ||
logical.__isset.FLOAT16 ||
+ logical.__isset.GEOMETRY || logical.__isset.GEOGRAPHY) {
+ return true;
+ }
+ }
+ if (!schema.__isset.converted_type) {
+ return false;
+ }
+ return schema.converted_type != tparquet::ConvertedType::MAP &&
+ schema.converted_type != tparquet::ConvertedType::MAP_KEY_VALUE &&
+ schema.converted_type != tparquet::ConvertedType::LIST;
+}
+
+static bool is_repeated_node(const tparquet::SchemaElement& schema) {
+ return schema.__isset.repetition_type &&
+ schema.repetition_type == tparquet::FieldRepetitionType::REPEATED;
+}
+
+static bool is_required_node(const tparquet::SchemaElement& schema) {
+ return schema.__isset.repetition_type &&
+ schema.repetition_type == tparquet::FieldRepetitionType::REQUIRED;
+}
+
+static bool is_optional_node(const tparquet::SchemaElement& schema) {
+ return schema.__isset.repetition_type &&
+ schema.repetition_type == tparquet::FieldRepetitionType::OPTIONAL;
+}
+
+static int num_children_node(const tparquet::SchemaElement& schema) {
+ return schema.__isset.num_children ? schema.num_children : 0;
+}
+
+static Status validate_native_schema_structure(
+ const std::vector<tparquet::SchemaElement>& schemas) {
+ if (schemas.empty()) {
+ return Status::InvalidArgument("Wrong parquet root schema element");
+ }
+
+ const auto& root = schemas[0];
+ if (root.__isset.type || !root.__isset.num_children || root.num_children <
0 ||
+ (root.__isset.repetition_type &&
+ root.repetition_type != tparquet::FieldRepetitionType::REQUIRED)) {
+ // Writers may encode the root's implicit REQUIRED repetition
explicitly, and a zero-child
+ // root is a valid metadata-only schema. Optional or repeated roots
remain ambiguous.
+ return Status::InvalidArgument("Wrong parquet root schema element");
+ }
+
+ struct PendingGroup {
+ size_t remaining_children;
+ size_t depth;
+ };
+ const auto root_children = num_children_node(schemas[0]);
+ if (root_children < 0 || static_cast<size_t>(root_children) >
schemas.size() - 1) {
+ return Status::InvalidArgument("Invalid parquet root child count {}",
root_children);
+ }
+ std::vector<PendingGroup> pending {{static_cast<size_t>(root_children),
0}};
+ for (size_t pos = 1; pos < schemas.size(); ++pos) {
+ while (!pending.empty() && pending.back().remaining_children == 0) {
+ pending.pop_back();
+ }
+ if (pending.empty()) {
+ return Status::InvalidArgument("Schema element {} is not reachable
from the root", pos);
+ }
+
+ const size_t depth = pending.back().depth + 1;
+ --pending.back().remaining_children;
+ const auto& schema = schemas[pos];
+ if (!schema.__isset.repetition_type) {
+ return Status::InvalidArgument("Schema element {} has no
repetition type", pos);
+ }
+ const bool has_children = schema.__isset.num_children &&
schema.num_children > 0;
+ if (schema.__isset.type == has_children) {
+ // Some legacy parquet-cpp files explicitly encode num_children=0
on primitive nodes.
+ // Only a positive count denotes a group, preserving strict
rejection of real dual-kind
+ // nodes without dropping those otherwise valid files.
+ return Status::InvalidArgument("Schema element {} has ambiguous
primitive/group kind",
+ pos);
+ }
+ if (schema.__isset.num_children && schema.num_children < 0) {
+ return Status::InvalidArgument("Schema element {} has a negative
child count", pos);
+ }
+ const int children = num_children_node(schemas[pos]);
+ if (children < 0 || static_cast<size_t>(children) > schemas.size() -
pos - 1) {
+ return Status::InvalidArgument("Invalid child count {} at schema
element {}", children,
+ pos);
+ }
+ if (children > 0) {
+ // Bound the tree before any resize or recursion so an untrusted
footer cannot
+ // turn a tiny schema into an unbounded allocation or parser stack.
+ if (depth > MAX_NATIVE_SCHEMA_DEPTH) {
+ return Status::InvalidArgument("Parquet schema depth {}
exceeds limit {}", depth,
+ MAX_NATIVE_SCHEMA_DEPTH);
+ }
+ pending.push_back({static_cast<size_t>(children), depth});
+ }
+ }
+ while (!pending.empty() && pending.back().remaining_children == 0) {
+ pending.pop_back();
+ }
+ if (!pending.empty()) {
+ return Status::InvalidArgument("Parquet schema ended before all
children were parsed");
+ }
+ return Status::OK();
+}
+
+/**
+ * `repeated_parent_def_level` is the definition level of the first ancestor
node whose repetition_type equals REPEATED.
+ * Empty array/map values are not stored in doris columns, so have to use
`repeated_parent_def_level` to skip the
+ * empty or null values in ancestor node.
+ *
+ * For instance, considering an array of strings with 3 rows like the
following:
+ * null, [], [a, b, c]
+ * We can store four elements in data column: null, a, b, c
+ * and the offsets column is: 1, 1, 4
+ * and the null map is: 1, 0, 0
+ * For the i-th row in array column: range from `offsets[i - 1]` until
`offsets[i]` represents the elements in this row,
+ * so we can't store empty array/map values in doris data column.
+ * As a comparison, spark does not require `repeated_parent_def_level`,
+ * because the spark column stores empty array/map values , and use anther
length column to indicate empty values.
+ * Please reference:
https://github.com/apache/spark/blob/master/sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/ParquetColumnVector.java
+ *
+ * Furthermore, we can also avoid store null array/map values in doris data
column.
+ * The same three rows as above, We can only store three elements in data
column: a, b, c
+ * and the offsets column is: 0, 0, 3
+ * and the null map is: 1, 0, 0
+ *
+ * Inherit the repetition and definition level from parent node, if the parent
node is repeated,
+ * we should set repeated_parent_def_level = definition_level, otherwise as
repeated_parent_def_level.
+ * @param parent parent node
+ * @param repeated_parent_def_level the first ancestor node whose
repetition_type equals REPEATED
+ */
+static void set_child_node_level(NativeFieldSchema* parent, int16_t
repeated_parent_def_level) {
+ for (auto& child : parent->children) {
+ child.repetition_level = parent->repetition_level;
+ child.definition_level = parent->definition_level;
+ child.repeated_parent_def_level = repeated_parent_def_level;
+ }
+}
+
+static bool is_struct_list_node(const tparquet::SchemaElement& schema,
+ const std::string& enclosing_list_name) {
+ // The legacy Parquet exception is exact: accepting every "*_tuple"
wrapper changes a standard
+ // one-child LIST wrapper from ARRAY<T> to ARRAY<STRUCT<T>>.
+ return schema.name == "array" || schema.name == enclosing_list_name +
"_tuple";
+}
+
+static bool has_logical_annotation(const tparquet::SchemaElement& schema) {
+ return schema.__isset.logicalType || schema.__isset.converted_type;
+}
+
+std::string NativeFieldSchema::debug_string() const {
+ std::stringstream ss;
+ ss << "NativeFieldSchema(name=" << name << ", R=" << repetition_level
+ << ", D=" << definition_level;
+ if (children.size() > 0) {
+ ss << ", type=" << data_type->get_name() << ", children=[";
+ for (int i = 0; i < children.size(); ++i) {
+ if (i != 0) {
+ ss << ", ";
+ }
+ ss << children[i].debug_string();
+ }
+ ss << "]";
+ } else {
+ ss << ", physical_type=" << physical_type;
+ ss << " , doris_type=" << data_type->get_name();
+ }
+ ss << ")";
+ return ss.str();
+}
+
+Status NativeFieldDescriptor::parse_from_thrift(
+ const std::vector<tparquet::SchemaElement>& t_schemas) {
+ _fields.clear();
+ _physical_fields.clear();
+ _name_to_field.clear();
+ RETURN_IF_ERROR(validate_native_schema_structure(t_schemas));
+ const auto& root_schema = t_schemas[0];
+ _fields.resize(root_schema.num_children);
+ _next_schema_pos = 1;
+
+ for (int i = 0; i < root_schema.num_children; ++i) {
+ RETURN_IF_ERROR(parse_node_field(t_schemas, _next_schema_pos,
&_fields[i]));
+ if (_name_to_field.find(_fields[i].name) != _name_to_field.end()) {
+ return Status::InvalidArgument("Duplicated field name: {}",
_fields[i].name);
+ }
+ _name_to_field.emplace(_fields[i].name, &_fields[i]);
+ }
+
+ if (_next_schema_pos != t_schemas.size()) {
+ return Status::InvalidArgument("Remaining {} unparsed schema elements",
+ t_schemas.size() - _next_schema_pos);
+ }
+
+ return Status::OK();
+}
+
+Status NativeFieldDescriptor::parse_node_field(
+ const std::vector<tparquet::SchemaElement>& t_schemas, size_t curr_pos,
+ NativeFieldSchema* node_field) {
+ if (curr_pos >= t_schemas.size()) {
+ return Status::InvalidArgument("Out-of-bounds index of schema
elements");
+ }
+ auto& t_schema = t_schemas[curr_pos];
+ if (is_group_node(t_schema)) {
+ // nested structure or nullable list
+ return parse_group_field(t_schemas, curr_pos, node_field);
+ }
+ if (is_repeated_node(t_schema)) {
+ // repeated <primitive-type> <name> (LIST)
+ // produce required list<element>
+ node_field->repetition_level++;
+ node_field->definition_level++;
+ node_field->children.resize(1);
+ set_child_node_level(node_field, node_field->definition_level);
+ auto child = &node_field->children[0];
+ parse_physical_field(t_schema, false, child);
+
+ node_field->name = t_schema.name;
+ node_field->lower_case_name = to_lower(t_schema.name);
+ node_field->data_type =
std::make_shared<DataTypeArray>(make_nullable(child->data_type));
+ _next_schema_pos = curr_pos + 1;
+ node_field->field_id = t_schema.__isset.field_id ? t_schema.field_id :
-1;
+ } else {
+ bool is_optional = is_optional_node(t_schema);
+ if (is_optional) {
+ node_field->definition_level++;
+ }
+ parse_physical_field(t_schema, is_optional, node_field);
+ _next_schema_pos = curr_pos + 1;
+ }
+ return Status::OK();
+}
+
+void NativeFieldDescriptor::parse_physical_field(const
tparquet::SchemaElement& physical_schema,
+ bool is_nullable,
+ NativeFieldSchema*
physical_field) {
+ physical_field->name = physical_schema.name;
+ physical_field->lower_case_name = to_lower(physical_field->name);
+ physical_field->parquet_schema = physical_schema;
+ physical_field->physical_type = physical_schema.type;
+ physical_field->column_id = NATIVE_UNASSIGNED_COLUMN_ID; // Initialize
column_id
+ _physical_fields.push_back(physical_field);
+ physical_field->physical_column_index =
cast_set<int>(_physical_fields.size() - 1);
+ auto type = get_doris_type(physical_schema, is_nullable);
+ physical_field->data_type = type.first;
+ physical_field->is_type_compatibility = type.second;
+ physical_field->field_id = physical_schema.__isset.field_id ?
physical_schema.field_id : -1;
+}
+
+std::pair<DataTypePtr, bool> NativeFieldDescriptor::get_doris_type(
+ const tparquet::SchemaElement& physical_schema, bool nullable) {
+ std::pair<DataTypePtr, bool> ans = {std::make_shared<DataTypeNothing>(),
false};
+ try {
+ if (physical_schema.__isset.logicalType) {
+ ans = convert_to_doris_type(physical_schema.logicalType, nullable);
+ } else if (physical_schema.__isset.converted_type) {
+ ans = convert_to_doris_type(physical_schema, nullable);
+ }
+ } catch (...) {
Review Comment:
[P1] Preserve unsupported DECIMAL conversion failures
A valid `BYTE_ARRAY DECIMAL(77,s)` reaches `create_decimal()`, which throws
because Doris tops out at precision 76; this catch suppresses that failure and
the fallback below assigns STRING/VARBINARY. `fill_native_type_descriptor()`
still labels the leaf decimal, but the scalar reader selects StringSerDe from
the fallback file type and returns its two's-complement payload as raw bytes.
Parquet does not limit BYTE_ARRAY decimal precision, so preserve/record the
logical conversion failure and fail an actual projection instead of silently
changing the column's meaning. Please cover a precision-over-76 BYTE_ARRAY
decimal.
--
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]