github-actions[bot] commented on code in PR #61539:
URL: https://github.com/apache/doris/pull/61539#discussion_r3042769449


##########
be/src/storage/segment/variant/variant_column_writer_impl.cpp:
##########
@@ -256,62 +258,71 @@ void build_sparse_subcolumns(const ColumnVariant& 
variant, const DocValuePathSta
     }
 }
 
-SubcolumnWritePlan build_subcolumn_write_plan(const ColumnVariant& variant, 
size_t num_rows,
-                                              int64_t 
variant_doc_materialization_min_rows) {
-    SubcolumnWritePlan plan;
-    // Below threshold: skip materialization and let finalize() compute stats 
on demand.
-    if (num_rows < static_cast<size_t>(variant_doc_materialization_min_rows)) {
-        return plan;
-    }
-
+// Materialize doc_value into subcolumns (sparse or dense) and populate plan 
entries.
+void materialize_and_populate_entries(const ColumnVariant& variant, 
SubcolumnWritePlan& plan) {
     if (config::enable_variant_doc_sparse_write_subcolumns) {
         build_doc_value_stats(variant, &plan.stats);
         build_sparse_subcolumns(variant, plan.stats, &plan.sparse_subcolumns);
         plan.entries.reserve(plan.sparse_subcolumns.size());
         for (auto& [path, sparse] : plan.sparse_subcolumns) {
             SubcolumnWriteEntry entry;
-            // StringRef points to variant storage; valid for the plan's 
lifetime.
             entry.path = std::string_view(path.data, path.size);
             entry.subcolumn = &sparse.subcolumn;
             entry.rowids = &sparse.rowids;
             plan.entries.push_back(entry);
         }
-        return plan;
+    } else {
+        build_doc_value_stats(variant, &plan.stats);
+        plan.dense_subcolumns = 
variant_util::materialize_docs_to_subcolumns_map(variant);
+        plan.entries.reserve(plan.dense_subcolumns.size());
+        for (auto& [path, subcolumn] : plan.dense_subcolumns) {
+            SubcolumnWriteEntry entry;
+            entry.path = path;
+            entry.subcolumn = &subcolumn;
+            entry.rowids = nullptr;
+            plan.entries.push_back(entry);
+        }
     }
+}
 
-    build_doc_value_stats(variant, &plan.stats);
-    plan.dense_subcolumns =
-            variant_util::materialize_docs_to_subcolumns_map(variant, 
plan.stats.size());
-    plan.entries.reserve(plan.dense_subcolumns.size());
-    for (auto& [path, subcolumn] : plan.dense_subcolumns) {
-        SubcolumnWriteEntry entry;
-        entry.path = path;
-        entry.subcolumn = &subcolumn;
-        entry.rowids = nullptr;
-        plan.entries.push_back(entry);
+SubcolumnWritePlan build_subcolumn_write_plan(const ColumnVariant& variant, 
size_t num_rows,
+                                              int64_t 
variant_doc_materialization_min_rows,

Review Comment:
    already means "no limit / materialize all subcolumns" in the existing 
variant codepaths (, , etc.). With this new comparison,  is always false, so 
doc-mode batches below  will never downgrade when the user configured . That 
silently changes the meaning of an existing schema value. The downgrade check 
should preserve the old  semantics.



-- 
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