Gabriel39 commented on code in PR #66575:
URL: https://github.com/apache/doris/pull/66575#discussion_r3747156793
##########
be/src/format_v2/parquet/parquet_statistics.cpp:
##########
@@ -701,11 +701,15 @@ std::optional<ResolvedVariantShredding>
resolve_variant_shredding(
if (wrapper == nullptr || wrapper->kind !=
ParquetColumnSchemaKind::VARIANT) {
return std::nullopt;
}
+ std::vector<const ParquetColumnSchema*> fallback_values;
for (const auto& component : predicate.path) {
+ const auto* fallback = child_named(*wrapper, "value");
const auto* typed_object = child_named(*wrapper, "typed_value");
- if (typed_object == nullptr || typed_object->kind !=
ParquetColumnSchemaKind::STRUCT) {
+ if (fallback == nullptr || fallback->kind !=
ParquetColumnSchemaKind::PRIMITIVE ||
+ typed_object == nullptr || typed_object->kind !=
ParquetColumnSchemaKind::STRUCT) {
return std::nullopt;
}
+ fallback_values.push_back(fallback);
Review Comment:
Fixed in the follow-up. The safety proof now checks only the terminal field
fallback; ancestor residual keys remain independent.
ResidualStatisticsGuardPhysicalLeafProjection covers a non-null ancestor
residual with a complete requested leaf.
##########
be/src/format_v2/parquet/parquet_scan.cpp:
##########
@@ -1129,6 +1154,22 @@ Status ParquetScanScheduler::open_next_row_group(
const auto& row_group_metadata =
file_context.native_metadata->to_thrift().row_groups[row_group_idx];
+ _current_row_group_request = std::make_unique<format::FileScanRequest>();
+ _current_row_group_request->predicate_columns = request.predicate_columns;
+ _current_row_group_request->non_predicate_columns =
request.non_predicate_columns;
+ _current_row_group_request->count_star_placeholder_columns =
+ request.count_star_placeholder_columns;
+ VariantRowGroupProjectionCounts variant_projection_counts;
+ if (file_context.contains_variant) {
+ const auto predicate_counts =
finalize_variant_projections_for_row_group(
Review Comment:
Fixed in the follow-up. Row-group physical projection decisions are prepared
before footer pruning, and the finalized physical leaf IDs are used
consistently for footer accounting, deferred metadata probes, page indexes, and
reader materialization.
##########
be/test/format_v2/parquet/parquet_reader_test.cpp:
##########
@@ -78,13 +79,65 @@
#include "storage/index/zone_map/zonemap_filter_result.h"
#include "storage/segment/condition_cache.h"
#include "storage/utils.h"
+#include "util/coding.h"
#include "util/defer_op.h"
+#include "util/thrift_util.h"
namespace doris {
namespace {
constexpr int64_t ROW_COUNT = 5;
+void duplicate_variant_fixture_with_unsafe_second_row_group(const std::string&
source_path,
+ const std::string&
output_path) {
+ std::filesystem::copy_file(source_path, output_path,
+
std::filesystem::copy_options::overwrite_existing);
+ std::ifstream input(output_path, std::ios::binary | std::ios::ate);
+ DORIS_CHECK(input.good());
+ const auto input_size = static_cast<std::streamoff>(input.tellg());
+ DORIS_CHECK(input_size >= static_cast<std::streamoff>(8));
+ std::vector<uint8_t> file_bytes(cast_set<size_t>(input_size));
+ input.seekg(0);
+ input.read(reinterpret_cast<char*>(file_bytes.data()),
cast_set<std::streamsize>(input_size));
+ DORIS_CHECK(input.good());
+ DORIS_CHECK(memcmp(file_bytes.data() + file_bytes.size() - 4, "PAR1", 4)
== 0);
+
+ const uint32_t footer_size = decode_fixed32_le(file_bytes.data() +
file_bytes.size() - 8);
+ DORIS_CHECK(footer_size <= file_bytes.size() - 8);
+ const size_t footer_offset = file_bytes.size() - 8 - footer_size;
+ uint32_t thrift_size = footer_size;
+ tparquet::FileMetaData metadata;
+ DORIS_CHECK(
+ deserialize_thrift_msg(file_bytes.data() + footer_offset,
&thrift_size, true, &metadata)
+ .ok());
+ DORIS_CHECK(metadata.row_groups.size() == 1);
+ DORIS_CHECK(metadata.row_groups[0].columns.size() > 2);
+ // Reuse the immutable chunks to isolate the scheduler decision: each
row-group reader owns an
+ // independent cursor, while the second footer entry deliberately cannot
prove an empty residual.
+ auto second_row_group = metadata.row_groups[0];
+ auto& root_residual = second_row_group.columns[2].meta_data.statistics;
+ DORIS_CHECK(root_residual.__isset.null_count);
+ root_residual.__set_null_count(second_row_group.num_rows - 1);
Review Comment:
Fixed in the follow-up. SwitchesVariantLeafProjectionPerRowGroup writes a
real second row group whose terminal residual contains a string fallback and
verifies the reconstructed values are [1, "n/a"], with one leaf and one
full-projection row-group counter.
##########
regression-test/suites/external_table_p0/paimon/test_paimon_catalog_variant.groovy:
##########
@@ -252,12 +254,52 @@ suite("test_paimon_catalog_variant",
"p0,external,doris,external_docker,external
select id,
cast(payload['name'] as string),
cast(payload['age'] as int),
- cast(payload['extra'] as string)
+ cast(payload['extra'] as string),
+ cast(payload['profile']['address']['city'] as string),
+ cast(payload['profile']['address']['zip'] as int)
from variant_shredded
where cast(payload['age'] as int) >= 20
order by id
"""
+ order_qt_native_shredded_deep_object_projection """
+ select id,
+ cast(payload['profile']['address']['zip'] as int),
+ cast(payload['profile']['address']['rank'] as int)
+ from variant_shredded
+ order by id
+ """
+
+ sql """set enable_profile = true"""
+ sql """set profile_level = 2"""
+ String deepProjectionToken =
+ "paimon_variant_deep_object_projection_" +
UUID.randomUUID().toString()
+ List<List<Object>> deepProjectionRows = sql """
+ select '${deepProjectionToken}', id,
+ cast(payload['profile']['address']['zip'] as int),
+ cast(payload['profile']['address']['rank'] as int)
+ from variant_shredded
+ order by id
+ """
+ assertEquals(2, deepProjectionRows.size())
+ String deepProjectionProfile = new
ProfileAction(context).getProfileBySql(
+ deepProjectionToken, ["VariantLeafProjections"], 30000L, 500L)
Review Comment:
Fixed in the follow-up. The fully shredded fixture now asserts
VariantLeafProjectionRowGroupColumns == 1 and
VariantFullProjectionRowGroupColumns == 0, while the mixed fixture separately
asserts one leaf and one full fallback.
--
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]