You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "eldenmoon (via GitHub)" <gi...@apache.org> on 2026/04/27 02:06:03 UTC
[PR] [variant] add NestedGroup path metadata support [doris]
eldenmoon opened a new pull request, #62848:
URL: https://github.com/apache/doris/pull/62848
Add NestedGroup path metadata through Variant storage and query planning.
- Write indexed NestedGroup paths into Variant subcolumn metadata so top-array nested fields keep positional nulls.
- Teach ColumnVariant and DataTypeVariant to carry nested-group typed paths through sparse/subcolumn IO, casting, element access, and compaction.
- Align FE Variant type SQL, property analysis, and expression-to-thrift translation with NestedGroup path planning.
- Regenerate nested_in_top_array expected output for the NestedGroup positional layout.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "eldenmoon (via GitHub)" <gi...@apache.org>.
eldenmoon commented on PR #62848:
URL: https://github.com/apache/doris/pull/62848#issuecomment-4323824593
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "eldenmoon (via GitHub)" <gi...@apache.org>.
eldenmoon commented on PR #62848:
URL: https://github.com/apache/doris/pull/62848#issuecomment-4393629304
skip check_coverage
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "eldenmoon (via GitHub)" <gi...@apache.org>.
eldenmoon commented on code in PR #62848:
URL: https://github.com/apache/doris/pull/62848#discussion_r3198542568
##########
be/src/exec/common/variant_util.cpp:
##########
@@ -259,6 +260,132 @@ bool glob_match_re2(const std::string& glob_pattern, const std::string& candidat
return RE2::FullMatch(candidate_path, *compiled);
}
+bool is_regular_ng_compaction_subpath(const PathInData& path) {
+ const std::string& relative_path = path.get_path();
+ return !relative_path.empty() && !path.get_is_typed() && !path.has_nested_part() &&
Review Comment:
done
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "eldenmoon (via GitHub)" <gi...@apache.org>.
eldenmoon commented on PR #62848:
URL: https://github.com/apache/doris/pull/62848#issuecomment-4323826996
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [variant] add NestedGroup path metadata support [doris]
Posted by "Thearas (via GitHub)" <gi...@apache.org>.
Thearas commented on PR #62848:
URL: https://github.com/apache/doris/pull/62848#issuecomment-4323720091
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR).
Please clearly describe your PR:
1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62848:
URL: https://github.com/apache/doris/pull/62848#issuecomment-4325368320
# FE UT Coverage Report
Increment line coverage ` 0.00% (0/23)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/62848_b5c0d49897f6d97b0db6c7a646a87dd33db0e4c2/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/62848_b5c0d49897f6d97b0db6c7a646a87dd33db0e4c2/fe_report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #62848:
URL: https://github.com/apache/doris/pull/62848#issuecomment-4327118596
PR approved by anyone and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #62848:
URL: https://github.com/apache/doris/pull/62848#discussion_r3144597909
##########
fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java:
##########
@@ -2156,8 +2156,8 @@ public static int analyzeVariantSparseHashShardCount(Map<String, String> propert
String bucketNumStr = properties.get(PROPERTIES_VARIANT_SPARSE_HASH_SHARD_COUNT);
try {
bucketNum = Integer.parseInt(bucketNumStr);
- if (bucketNum < 1 || bucketNum > 1024) {
- throw new AnalysisException("variant_sparse_hash_shard_count must between 1 and 1024 ");
+ if (bucketNum < 0 || bucketNum > 1024) {
+ throw new AnalysisException("variant_sparse_hash_shard_count must between 0 and 1024 ");
Review Comment:
Error message grammar/formatting: use "must be between" (and avoid the trailing space) for consistency and clarity.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/types/VariantType.java:
##########
@@ -185,6 +185,11 @@ public String toSql() {
.append(String.valueOf(variantSparseHashShardCount))
.append("\"");
}
+ if (enableNestedGroup) {
+ sb.append(",");
+ sb.append("\"variant_enable_nested_group\" = \"")
+ .append(String.valueOf(enableNestedGroup)).append("\"");
+ }
Review Comment:
VariantType.toSql() now serializes "variant_enable_nested_group" when enableNestedGroup is true, but the Nereids DDL parser currently rejects this property (LogicalPlanBuilder throws NotSupportedException). This makes type-to-SQL roundtrips / SHOW CREATE outputs potentially non-executable. Consider omitting this property from toSql() until it’s supported in DDL (or alternatively, support parsing it).
```suggestion
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "eldenmoon (via GitHub)" <gi...@apache.org>.
eldenmoon commented on code in PR #62848:
URL: https://github.com/apache/doris/pull/62848#discussion_r3192672011
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/types/VariantType.java:
##########
@@ -185,6 +185,11 @@ public String toSql() {
.append(String.valueOf(variantSparseHashShardCount))
.append("\"");
}
+ if (enableNestedGroup) {
Review Comment:
It was expected
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "eldenmoon (via GitHub)" <gi...@apache.org>.
eldenmoon commented on code in PR #62848:
URL: https://github.com/apache/doris/pull/62848#discussion_r3198544689
##########
be/src/exec/common/variant_util.cpp:
##########
@@ -259,6 +260,132 @@ bool glob_match_re2(const std::string& glob_pattern, const std::string& candidat
return RE2::FullMatch(candidate_path, *compiled);
}
+bool is_regular_ng_compaction_subpath(const PathInData& path) {
+ const std::string& relative_path = path.get_path();
+ return !relative_path.empty() && !path.get_is_typed() && !path.has_nested_part() &&
+ !segment_v2::contains_nested_group_marker(relative_path) &&
+ !segment_v2::is_root_nested_group_path(relative_path) &&
+ relative_path != SPARSE_COLUMN_PATH &&
+ relative_path.find(DOC_VALUE_COLUMN_PATH) == std::string::npos;
+}
+
+bool should_keep_existing_ng_compaction_subcolumn(const TabletColumn& column) {
+ if (!column.is_extracted_column()) {
+ return false;
+ }
+ return is_regular_ng_compaction_subpath(column.path_info_ptr()->copy_pop_front());
+}
+
+TabletIndexes clone_tablet_indexes(const std::vector<const TabletIndex*>& indexes) {
+ TabletIndexes cloned_indexes;
+ cloned_indexes.reserve(indexes.size());
+ for (const auto* index : indexes) {
+ cloned_indexes.emplace_back(std::make_shared<TabletIndex>(*index));
+ }
+ return cloned_indexes;
+}
+
+void keep_existing_ng_compaction_subcolumn(const TabletSchemaSPtr& source_schema,
+ const TabletColumnPtr& extracted_column,
+ TabletSchemaSPtr& output_schema,
+ TabletSchema::PathsSetInfo& paths_set_info) {
+ DCHECK(extracted_column->is_extracted_column());
+ DCHECK(should_keep_existing_ng_compaction_subcolumn(*extracted_column));
+
+ auto relative_path = extracted_column->path_info_ptr()->copy_pop_front();
+ const std::string path = relative_path.get_path();
+ output_schema->append_column(*extracted_column);
+
+ auto indexes = clone_tablet_indexes(source_schema->inverted_indexs(*extracted_column));
+ if (extracted_column->path_info_ptr()->get_is_typed()) {
+ TabletSchema::SubColumnInfo sub_column_info {.column = *extracted_column,
+ .indexes = std::move(indexes)};
+ paths_set_info.typed_path_set.emplace(path, std::move(sub_column_info));
+ return;
+ }
+
+ paths_set_info.sub_path_set.emplace(path);
+ if (!indexes.empty()) {
+ paths_set_info.subcolumn_indexes.emplace(path, std::move(indexes));
+ }
+}
+
+using ExistingNgCompactionSubcolumns = std::unordered_map<int32_t, std::vector<TabletColumnPtr>>;
+
+struct NestedGroupCompactionMaterializationPlan {
+ std::vector<TabletColumnPtr> preserved_regular_subcolumns;
+ PathSet materialized_regular_paths;
+ PathToDataTypes additional_regular_path_to_data_types;
+};
+
+bool should_preserve_existing_ng_compaction_subcolumns(
+ const TabletColumnPtr& column,
+ const std::unordered_map<int32_t, VariantExtendedInfo>& uid_to_variant_extended_info) {
+ const auto info_it = uid_to_variant_extended_info.find(column->unique_id());
+ return column->variant_enable_nested_group() ||
+ (info_it != uid_to_variant_extended_info.end() && info_it->second.has_nested_group);
+}
+
+std::unordered_set<int32_t> collect_ng_compaction_root_uids(
+ const TabletSchemaSPtr& target,
+ const std::unordered_map<int32_t, VariantExtendedInfo>& uid_to_variant_extended_info) {
+ std::unordered_set<int32_t> root_uids;
+ for (const TabletColumnPtr& column : target->columns()) {
+ if (column->is_variant_type() && should_preserve_existing_ng_compaction_subcolumns(
+ column, uid_to_variant_extended_info)) {
+ root_uids.insert(column->unique_id());
+ }
+ }
+ return root_uids;
+}
+
+ExistingNgCompactionSubcolumns collect_existing_ng_compaction_subcolumns(
+ const TabletSchemaSPtr& target, const std::unordered_set<int32_t>& ng_root_uids) {
+ ExistingNgCompactionSubcolumns uid_to_existing_subcolumns;
+ for (const TabletColumnPtr& column : target->columns()) {
+ if (!column->is_extracted_column() || !ng_root_uids.contains(column->parent_unique_id()) ||
+ !should_keep_existing_ng_compaction_subcolumn(*column)) {
+ continue;
+ }
+ uid_to_existing_subcolumns[column->parent_unique_id()].push_back(column);
+ }
+ return uid_to_existing_subcolumns;
+}
+
+NestedGroupCompactionMaterializationPlan build_nested_group_compaction_materialization_plan(
+ const std::vector<TabletColumnPtr>& existing_subcolumns,
+ const VariantExtendedInfo& extended_info) {
+ NestedGroupCompactionMaterializationPlan plan;
+ plan.preserved_regular_subcolumns = existing_subcolumns;
+ for (const auto& column : existing_subcolumns) {
+ plan.materialized_regular_paths.emplace(
+ column->path_info_ptr()->copy_pop_front().get_path());
+ }
+ for (const auto& [path, data_types] : extended_info.path_to_data_types) {
Review Comment:
It was expected
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62848:
URL: https://github.com/apache/doris/pull/62848#issuecomment-4326115301
# BE Regression && UT Coverage Report
Increment line coverage `51.50% (223/433)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/62848_b5c0d49897f6d97b0db6c7a646a87dd33db0e4c2_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/62848_b5c0d49897f6d97b0db6c7a646a87dd33db0e4c2_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.80% (27699/37533) |
| Line Coverage | 57.60% (299839/520560) |
| Region Coverage | 54.78% (249468/455368) |
| Branch Coverage | 56.36% (108034/191682) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "eldenmoon (via GitHub)" <gi...@apache.org>.
eldenmoon commented on code in PR #62848:
URL: https://github.com/apache/doris/pull/62848#discussion_r3192785201
##########
be/src/exec/common/variant_util.cpp:
##########
@@ -259,6 +260,132 @@ bool glob_match_re2(const std::string& glob_pattern, const std::string& candidat
return RE2::FullMatch(candidate_path, *compiled);
}
+bool is_regular_ng_compaction_subpath(const PathInData& path) {
+ const std::string& relative_path = path.get_path();
+ return !relative_path.empty() && !path.get_is_typed() && !path.has_nested_part() &&
+ !segment_v2::contains_nested_group_marker(relative_path) &&
+ !segment_v2::is_root_nested_group_path(relative_path) &&
Review Comment:
no `__D0_root__.child`
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62848:
URL: https://github.com/apache/doris/pull/62848#issuecomment-4324093712
# FE UT Coverage Report
Increment line coverage `` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/62848_ef0874d9e4a60596dd2e193224242ce16cc537a7/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/62848_ef0874d9e4a60596dd2e193224242ce16cc537a7/fe_report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62848:
URL: https://github.com/apache/doris/pull/62848#issuecomment-4326059611
# BE UT Coverage Report
Increment line coverage `44.13% (203/460)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/b5c0d49897f6d97b0db6c7a646a87dd33db0e4c2_b5c0d49897f6d97b0db6c7a646a87dd33db0e4c2/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/b5c0d49897f6d97b0db6c7a646a87dd33db0e4c2_b5c0d49897f6d97b0db6c7a646a87dd33db0e4c2/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 53.32% (20428/38311) |
| Line Coverage | 36.91% (192610/521891) |
| Region Coverage | 33.23% (149876/451056) |
| Branch Coverage | 34.34% (65582/190960) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "eldenmoon (via GitHub)" <gi...@apache.org>.
eldenmoon commented on PR #62848:
URL: https://github.com/apache/doris/pull/62848#issuecomment-4324849618
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "airborne12 (via GitHub)" <gi...@apache.org>.
airborne12 commented on code in PR #62848:
URL: https://github.com/apache/doris/pull/62848#discussion_r3159611897
##########
be/src/exec/common/variant_util.cpp:
##########
@@ -259,6 +260,132 @@ bool glob_match_re2(const std::string& glob_pattern, const std::string& candidat
return RE2::FullMatch(candidate_path, *compiled);
}
+bool is_regular_ng_compaction_subpath(const PathInData& path) {
+ const std::string& relative_path = path.get_path();
+ return !relative_path.empty() && !path.get_is_typed() && !path.has_nested_part() &&
Review Comment:
This predicate filters out typed paths (`!path.get_is_typed()`), but the NG compaction branch later relies on this path to preserve existing extracted subcolumns. As a result, typed/predefined paths under an NG root are excluded before `keep_existing_ng_compaction_subcolumn()` can add them to `typed_path_set`, and the NG branch then skips the generic `get_compaction_typed_columns()` path. This can drop typed extracted columns and their indexes from the compaction output schema. Please preserve typed extracted paths explicitly in the NG branch, or run the generic typed-path preservation before the NG `continue`.
##########
be/src/exec/common/variant_util.cpp:
##########
@@ -259,6 +260,132 @@ bool glob_match_re2(const std::string& glob_pattern, const std::string& candidat
return RE2::FullMatch(candidate_path, *compiled);
}
+bool is_regular_ng_compaction_subpath(const PathInData& path) {
+ const std::string& relative_path = path.get_path();
+ return !relative_path.empty() && !path.get_is_typed() && !path.has_nested_part() &&
+ !segment_v2::contains_nested_group_marker(relative_path) &&
+ !segment_v2::is_root_nested_group_path(relative_path) &&
Review Comment:
This only rejects the exact root marker path. `strip_root_nested_group_path()` treats both `__D0_root__` and `__D0_root__.*` as internal root-NG paths, so `__D0_root__.child` can still pass this regular-subpath predicate and be materialized as a user regular path. Please add/use a child-aware helper, e.g. `is_root_nested_group_path_or_child()`, in both compaction and reader path classification.
##########
be/src/core/column/column_variant.cpp:
##########
@@ -413,6 +420,11 @@ void ColumnVariant::Subcolumn::insert_range_from(const Subcolumn& src, size_t st
Status st = variant_util::cast_column({column, column_type, ""},
least_common_type.get(), &casted_column);
if (!st.ok()) {
+ if (is_nested_group_type(least_common_type.get()) &&
Review Comment:
The earlier branch only discards true scalar sources, but this cast-failure fallback no longer checks `src_is_scalar`. With `variant_nested_group_discard_scalar_on_conflict=true`, a non-scalar array/object source that fails casting to the NG-compatible type will be silently replaced with defaults. Please carry the scalar-source decision into both cast-failure branches and surface the cast error for non-scalar sources.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/types/VariantType.java:
##########
@@ -185,6 +185,11 @@ public String toSql() {
.append(String.valueOf(variantSparseHashShardCount))
.append("\"");
}
+ if (enableNestedGroup) {
Review Comment:
This serializes `variant_enable_nested_group=true`, but `LogicalPlanBuilder` currently rejects that property, while catalog `VariantType.toSql()` intentionally omits it. This makes Nereids-generated type SQL non-round-trippable. Please either omit this property here until DDL support is enabled, or support the property end-to-end in the parser/validator. If `enableNestedGroup` remains storage/API-affecting metadata, it should also be included consistently in `equals()` / `hashCode()`.
##########
be/src/exec/common/variant_util.cpp:
##########
@@ -259,6 +260,132 @@ bool glob_match_re2(const std::string& glob_pattern, const std::string& candidat
return RE2::FullMatch(candidate_path, *compiled);
}
+bool is_regular_ng_compaction_subpath(const PathInData& path) {
+ const std::string& relative_path = path.get_path();
+ return !relative_path.empty() && !path.get_is_typed() && !path.has_nested_part() &&
+ !segment_v2::contains_nested_group_marker(relative_path) &&
+ !segment_v2::is_root_nested_group_path(relative_path) &&
+ relative_path != SPARSE_COLUMN_PATH &&
+ relative_path.find(DOC_VALUE_COLUMN_PATH) == std::string::npos;
+}
+
+bool should_keep_existing_ng_compaction_subcolumn(const TabletColumn& column) {
+ if (!column.is_extracted_column()) {
+ return false;
+ }
+ return is_regular_ng_compaction_subpath(column.path_info_ptr()->copy_pop_front());
+}
+
+TabletIndexes clone_tablet_indexes(const std::vector<const TabletIndex*>& indexes) {
+ TabletIndexes cloned_indexes;
+ cloned_indexes.reserve(indexes.size());
+ for (const auto* index : indexes) {
+ cloned_indexes.emplace_back(std::make_shared<TabletIndex>(*index));
+ }
+ return cloned_indexes;
+}
+
+void keep_existing_ng_compaction_subcolumn(const TabletSchemaSPtr& source_schema,
+ const TabletColumnPtr& extracted_column,
+ TabletSchemaSPtr& output_schema,
+ TabletSchema::PathsSetInfo& paths_set_info) {
+ DCHECK(extracted_column->is_extracted_column());
+ DCHECK(should_keep_existing_ng_compaction_subcolumn(*extracted_column));
+
+ auto relative_path = extracted_column->path_info_ptr()->copy_pop_front();
+ const std::string path = relative_path.get_path();
+ output_schema->append_column(*extracted_column);
+
+ auto indexes = clone_tablet_indexes(source_schema->inverted_indexs(*extracted_column));
+ if (extracted_column->path_info_ptr()->get_is_typed()) {
+ TabletSchema::SubColumnInfo sub_column_info {.column = *extracted_column,
+ .indexes = std::move(indexes)};
+ paths_set_info.typed_path_set.emplace(path, std::move(sub_column_info));
+ return;
+ }
+
+ paths_set_info.sub_path_set.emplace(path);
+ if (!indexes.empty()) {
+ paths_set_info.subcolumn_indexes.emplace(path, std::move(indexes));
+ }
+}
+
+using ExistingNgCompactionSubcolumns = std::unordered_map<int32_t, std::vector<TabletColumnPtr>>;
+
+struct NestedGroupCompactionMaterializationPlan {
+ std::vector<TabletColumnPtr> preserved_regular_subcolumns;
+ PathSet materialized_regular_paths;
+ PathToDataTypes additional_regular_path_to_data_types;
+};
+
+bool should_preserve_existing_ng_compaction_subcolumns(
+ const TabletColumnPtr& column,
+ const std::unordered_map<int32_t, VariantExtendedInfo>& uid_to_variant_extended_info) {
+ const auto info_it = uid_to_variant_extended_info.find(column->unique_id());
+ return column->variant_enable_nested_group() ||
+ (info_it != uid_to_variant_extended_info.end() && info_it->second.has_nested_group);
+}
+
+std::unordered_set<int32_t> collect_ng_compaction_root_uids(
+ const TabletSchemaSPtr& target,
+ const std::unordered_map<int32_t, VariantExtendedInfo>& uid_to_variant_extended_info) {
+ std::unordered_set<int32_t> root_uids;
+ for (const TabletColumnPtr& column : target->columns()) {
+ if (column->is_variant_type() && should_preserve_existing_ng_compaction_subcolumns(
+ column, uid_to_variant_extended_info)) {
+ root_uids.insert(column->unique_id());
+ }
+ }
+ return root_uids;
+}
+
+ExistingNgCompactionSubcolumns collect_existing_ng_compaction_subcolumns(
+ const TabletSchemaSPtr& target, const std::unordered_set<int32_t>& ng_root_uids) {
+ ExistingNgCompactionSubcolumns uid_to_existing_subcolumns;
+ for (const TabletColumnPtr& column : target->columns()) {
+ if (!column->is_extracted_column() || !ng_root_uids.contains(column->parent_unique_id()) ||
+ !should_keep_existing_ng_compaction_subcolumn(*column)) {
+ continue;
+ }
+ uid_to_existing_subcolumns[column->parent_unique_id()].push_back(column);
+ }
+ return uid_to_existing_subcolumns;
+}
+
+NestedGroupCompactionMaterializationPlan build_nested_group_compaction_materialization_plan(
+ const std::vector<TabletColumnPtr>& existing_subcolumns,
+ const VariantExtendedInfo& extended_info) {
+ NestedGroupCompactionMaterializationPlan plan;
+ plan.preserved_regular_subcolumns = existing_subcolumns;
+ for (const auto& column : existing_subcolumns) {
+ plan.materialized_regular_paths.emplace(
+ column->path_info_ptr()->copy_pop_front().get_path());
+ }
+ for (const auto& [path, data_types] : extended_info.path_to_data_types) {
Review Comment:
This plan depends on `extended_info.path_to_data_types`, but `get_extended_compaction_schema()` only populates `extended_info` when some target Variant column passes `should_check_variant_path_stats()`. That helper returns false for NG Variant columns, so a table with only NG Variant columns will not collect this type metadata at all, while adding an unrelated non-NG Variant column changes the NG materialization behavior. Please split flat path-stat aggregation from NG type-metadata aggregation and collect the latter whenever NG roots need compaction planning.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "eldenmoon (via GitHub)" <gi...@apache.org>.
eldenmoon commented on PR #62848:
URL: https://github.com/apache/doris/pull/62848#issuecomment-4386445988
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62848:
URL: https://github.com/apache/doris/pull/62848#issuecomment-4325802896
# FE Regression Coverage Report
Increment line coverage ` 11.94% (8/67)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/62848_b5c0d49897f6d97b0db6c7a646a87dd33db0e4c2_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/62848_b5c0d49897f6d97b0db6c7a646a87dd33db0e4c2_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "airborne12 (via GitHub)" <gi...@apache.org>.
airborne12 merged PR #62848:
URL: https://github.com/apache/doris/pull/62848
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "eldenmoon (via GitHub)" <gi...@apache.org>.
eldenmoon commented on code in PR #62848:
URL: https://github.com/apache/doris/pull/62848#discussion_r3192754143
##########
be/src/core/column/column_variant.cpp:
##########
@@ -413,6 +420,11 @@ void ColumnVariant::Subcolumn::insert_range_from(const Subcolumn& src, size_t st
Status st = variant_util::cast_column({column, column_type, ""},
least_common_type.get(), &casted_column);
if (!st.ok()) {
+ if (is_nested_group_type(least_common_type.get()) &&
Review Comment:
It was expected
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Refactor](Variant) add NestedGroup path metadata support [doris]
Posted by "eldenmoon (via GitHub)" <gi...@apache.org>.
eldenmoon commented on PR #62848:
URL: https://github.com/apache/doris/pull/62848#issuecomment-4340344406
skip check_coverage
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org