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]

Reply via email to