Repository navigation
Conversation
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary:
Add the legacy semantics and rewrite suites, with their generated outputs, before the opt-in feature. They establish a pristine-master baseline and ensure that columns without the new property retain their existing behavior.
Lock dotted-key flattening and the difference between pushed-down access and in-memory element_at: {"a.b":1} reads back as {"a":{"b":1}}, both pushed-down paths return 1, and exact in-memory element_at(v, 'a.b') returns NULL. Also lock ancestor hiding for {"a":5,"a.b":6}, the legacy inverted-index duplicate-directory error, and index-on/off results across separate segments.
Use extended DESC to distinguish physical ancestor retention from visibility: INSERT SELECT, heavy schema change and whole-value compaction remove the hidden v.a path, while per-path vertical compaction retains it. Pin Variant layout properties and order query results so the outputs remain stable.
### Release note
None
### Check List (For Author)
- Test: Regression test. dotted_key_legacy_semantics and dotted_key_legacy_rewrite passed on pristine master (49bfbde) and on the final feature binary with -parallel 1. Their outputs remained byte-identical; duplicate_json_path also passed as a sanity check.
- Behavior changed: No. This commit records current behavior without changing the implementation.
- Does this need documentation: No. Test coverage only.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s distinct
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary:
Variant V2 stores `{"a.b":1}` and `{"a":{"b":2}}` under the same storage path `a.b`, so a dotted key cannot be told apart from the nested path: `{"a.b":1}` reads back as `{"a":{"b":1}}`, and putting both shapes in one object fails the load with `may contains duplicated entry` (locked by the legacy suites and `duplicate_json_path`).
Add the creation-time Variant property variant_escape_dots_in_keys, default false, to preserve dotted keys independently of nested paths. Encode percent and dot characters per key component, persist a segment-root encoding marker, and decode strictly when the assembler emits object keys. Keep per-path storage machinery in storage syntax and reject incompatible segment markers, ALTER changes in either direction, and NestedGroup combinations.
Plumb the property through FE, optional protocol fields and TabletColumn. Encode logical scan and rowid paths once, key CAST pushdown types by storage column names, and use injective index suffixes only for escaped columns. Write escaped sparse keys in byte order for binary-search lookup. Preserve all keys through whole-value rewrites and retain legacy behavior for columns without the property.
The optional field ids are ColumnPB 36, ColumnMetaPB 28 and TColumn 31, checked against every supported release branch. The pre-existing sparse ordering issue for legacy keys with bytes below '.' is handled by a separate PR.
### Release note
Add the opt-in Variant property variant_escape_dots_in_keys to keep dotted keys distinct from nested paths. Enable it only after every FE and BE is upgraded; tables using it cannot be downgraded.
### Check List (For Author)
- Test: Regression test and Unit Test.
- ASAN BE referenced tests: 1,616 passed, 5 skipped; final focused tests: 203 passed, 2 skipped.
- FE: 817 tests across 40 classes passed after the final FE change.
- New semantics/rewrite suites passed output generation, output review and reruns. Both legacy suites and duplicate_json_path passed; all five targeted suites passed on the final binary, and legacy outputs stayed byte-identical.
- Full variant_p0: 176 of 186 suites passed initially. All 10 failures were rerun separately with -parallel 1; 8 passed after enabling debug points. Remaining failures were OSS InvalidAccessKeyId and an existing schema-change baseline expecting integer 1 instead of boolean true. The HEAD V2 counterpart expects true and passed; no separate pristine-master rebuild was run for this baseline comparison.
- ASAN product builds, formatting and build hygiene passed. Final clang-tidy diagnostics on added lines: zero; existing diagnostics were retained.
- Behavior changed: Yes. New opt-in column property; default behavior is unchanged.
- Does this need documentation: Yes. Document PR to follow.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
eldenmoon
requested review from
924060929,
airborne12,
csun5285,
gavinchou,
liaoxin01 and
morrySnow
as code owners
November 2, 2026 11:32
Member
Author
|
run buildall |
eldenmoon
requested review from
englefly,
luwei16 and
starocean999
as code owners
November 2, 2026 11:32
Contributor
TPC-H: Total hot run time: 28717 ms |
Contributor
TPC-DS: Total hot run time: 151673 ms |
### What problem does this PR solve? Issue Number: N/A Related PR: apache#69995 Problem Summary: The shredder assertion cannot fire because encode_variant_path_component replaces every dot with %2E. The compaction assertion cannot fire because PathInData's string constructor already splits on dots, so no resulting component can contain one. Remove the per-key rescan and redundant temporary path parsing; retain the reader's segment marker checks and the compaction encoding gate. ### Release note None ### Check List (For Author) - Test: Unit Test and Regression test. - ASAN BE build and clang-format passed; clang-tidy reported no diagnostics on changed lines in either file. - BE UT: 139 tests across 6 suites, 137 passed, 2 skipped, 0 failed. Covered PathInDataTest, SchemaUtilTest, VariantCompactionPathsTest, IndexStorageVariantCompactionSchemaTest, VariantColumnWriterReaderTest and VariantShredderTest. - dotted_key_escape_semantics, dotted_key_escape_rewrite, dotted_key_legacy_semantics, dotted_key_legacy_rewrite and duplicate_json_path all passed with -parallel 1, without -genOut. All five output files remained byte-identical. - Behavior changed: No. Remove assertions whose predicates are guaranteed by the codec or parser. - Does this need documentation: No. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
run buildall |
5 of 16 tasks
Member
Author
Review：兼容性、性能、可维护性与冗余审查提交： 按 F-01 · Major：类型 SQL 往返丢失显式
|
Contributor
TPC-H: Total hot run time: 28305 ms |
Contributor
TPC-DS: Total hot run time: 151432 ms |
Contributor
ClickBench: Total hot run time: 25.38 s |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Member
Author
|
/review 除了上面提到的问题，还有哪些问题？尤其注意兼容性 |
Contributor
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Member
Author
Compaction 数据安全补充：F-04 / Blocker（既有缺陷，Regression: no）对同一 head 触发边界普通 non-DOC Variant、 这里的 V3 是表的外置列元数据格式，FE 接受显式 V3，省略/default 则返回 V2；它与 ColumnVariantV2、倒排索引 V3 不是同一个配置。本例不需要索引、ALTER 或混合版本。 如何从“compaction 前可读”变成“之后丢值”令
所以 p 在所有输入中都是可读的 sparse 值。合并统计后 p=3，大于每个 hot 字段的2，top-N 选择会将 p 提升为 materialized。值先被正确读出；sparse merge随后按目标 selected set 排除 p，唯一输出副本交给新子列。 错误发生在 V3 元数据分类： if (rel_path.find("__DORIS_VARIANT_SPARSE__") != std::string::npos) {
classification.pending_sparse[root_uid].emplace_back(col);
}该 substring 检查把普通用户子列 p 当成内部 sparse 列，嵌入 root.children_columns，而没有写普通子列的外部 path-index entry。读端只识别完整 sparse 标记或其 bucket 前缀；遍历 embedded children 时，p 的 第一次 full compaction 后：对应行的 现有校验不会阻止此例：行数不变； 归因与修复这是 Blocker / data-error：静默错误结果，后续重写还会丢掉剩余的有效值副本。但不是本 PR 新引入的 codec 回归：相关 classifier 在 merge base 建议统一 writer/reader 的内部列身份判断，避免 substring 判定。完整路径语法和物理类型校验可修复本例，但 canonical-looking 用户 key 仍需明确的 synthetic-role 元数据或不相交的命名约定。只把 补上以上不同本地热度、两次真实 rewrite、逐行701/702/703和整列内容的回归，不能只检查 count(*)。统计完整且没有行过滤的 DUP 场景还应校验 input-only 路径遗漏；MOW/delete 场景须尊重 survivor 语义。 此前 F-01/F-02/F-03 保持；累计已确认 1 Blocker + 1 Major + 2 Minor，REQUEST_CHANGES。本轮其余普通路径迁移、编码/分片、root 属性传播和 doc 重建没有确认另一条独立丢值路径，但这不构成无条件安全保证。 |
### What problem does this PR solve? Issue Number: N/A Related PR: apache#69995 Problem Summary: An escaped Variant column requires NestedGroup to be off, but both Variant type SQL serializers printed variant_enable_nested_group only when it was true. Re-parsing fills a missing value from the session (default_variant_enable_nested_group), so with that variable set to true, CREATE TABLE LIKE and replaying SHOW CREATE TABLE rejected a valid escaped source table. Print the actual NestedGroup value whenever escaping is enabled. The SQL of non-escaped columns is unchanged. ### Release note None ### Check List (For Author) - Test: Unit Test and Regression test. - FE UT: escaped type SQL round trip with and without a session that defaults NestedGroup to true, for both the catalog and Nereids types; legacy type SQL text unchanged. Added coverage for the escaped-Variant checks in IndexDefinition (CREATE and ALTER INDEX), ColumnDefinition, PropertyAnalyzer, ExpressionTranslator and ColumnToProtobuf. 40 related FE UT classes, 841 tests passed. - Regression: dotted_key_escape_semantics adds CREATE TABLE LIKE and SHOW CREATE TABLE replay under default_variant_enable_nested_group=true. In the semantics and rewrite outputs, only the DESC rows of escaped columns change, each by the added nested-group property. dotted_key_escape_{semantics,rewrite}, dotted_key_legacy_{semantics,rewrite}, duplicate_json_path and eight existing SHOW CREATE / LIKE suites passed. - Behavior changed: Yes. Escaped Variant type SQL now states variant_enable_nested_group = false. - Does this need documentation: No. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
run cloud_p0 |
1 similar comment
Member
Author
|
run cloud_p0 |
Member
Author
|
run external |
Member
Author
|
run vault_p0 |
Member
Author
|
run feut |
Member
Author
|
run nonConcurrent |
Member
Author
|
run buildall |
Member
Author
|
/review ignore above known issue |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Static review of b1a24ab. One new P2 finding: the versioned DDL returned by ADMIN COPY TABLET omits the immutable Variant path policy and recreates escaped tables as legacy. The normal CREATE/scan/write/rewrite mechanisms otherwise match the intended lossless opt-in design under the supported fully upgraded deployment condition.
User focus: known issues were not duplicated. Existing P1 thread 4499322562 remains technically applicable: old BEs ignore the new optional tablet-schema flag, with no enablement/version gate. The author explicitly chose to ignore it and documents mixed-version use as unsupported; it is retained only as an existing blocker. The previous recursive-CTE flag, translated-slot fallback, external CTAS inheritance, and MV view-retarget refresh concerns no longer apply to this head.
Critical checkpoint conclusions (static inspection):
- Goal and proof: literal dotted/percent keys remain distinct from nested paths through canonical values and escaped storage. Unit/regression cases cover these paths; versioned copy-tablet DDL is the new exception and lacks coverage.
- Scope and reuse: changes are focused on table policy, path boundaries and associated tests. Shared PropertyAnalyzer and catalog index helpers avoid separate connector/planner implementations.
- Concurrency: no new threads or lock order; the policy is initialized before publication and cannot be altered. Escaped schema columns bypass the shared ColumnPB-only object pool.
- Lifetimes and initialization: decoded temporary keys are copied into builder-owned storage; schema/iterator ownership remains bounded. No new cross-translation-unit static initialization dependency found.
- Configuration: false remains the default; true is creation-only, persisted and shown. ALTER and NestedGroup incompatibilities fail explicitly.
- Compatibility: legacy marker/default and suffix behavior remain; unknown markers fail. Fully upgraded same-version paths preserve policy. The known mixed-version concern remains in thread 4499322562.
- Thrift/protobuf: read all touched definitions at HEAD and current master/branch-4.2/4.1/4.0/3.1. TTabletSchema 34, TabletSchemaPB 47, TabletSchemaCloudPB 48 and ColumnMetaPB 28 are optional and unused on those branch snapshots; no new ID collision found.
- Parallel paths: inspected local/cloud create, report, restore, rollup/schema change, V2/V3, sparse/DOC, horizontal/vertical compaction, rowstore/rowid, direct/fallback index lookup, CTAS and MV refresh. Versioned DDL is the verified omission.
- Conditions and errors: invalid booleans/escapes, incompatible MV sources, forbidden ALTER and NestedGroup layouts fail at explicit boundaries. Suspected cast-map alias leaves actual-slot residual evaluation and did not establish a correctness finding.
- Test coverage: inspected FE/BE units and all added regression suites, including negative, legacy, cache/index, layout transition, failure-before-publication and MV retargeting cases. The copy-tablet branch needs a focused DDL case.
- Expected results: inspected the changed golden outputs against suite queries and path semantics, including repeated sparse-promotion cases. Their generation provenance and runtime pass claims were not independently validated.
- Observability: existing DDL/task error propagation and corruption messages expose policy/layout failures; no new per-row logging or metrics required by the inspected change.
- Persistence/failover: serialized table properties rebuild the flag on Gson postprocessing; tablet schema and all cloud conversion directions retain it. Existing journal/state transitions remain unchanged; restore signature includes the policy.
- Writes and atomicity: compaction/schema-change continue existing publication and version/delete-bitmap lifecycles. Invalid-marker failure preserves input rowsets in the inspected test; no new transaction protocol found.
- FE/BE transport: all local CreateReplicaTask and cloud tablet-meta producers pass the required flag; scan/load consumers bind it from persisted tablet metadata.
- Performance: component work is cached during shredding and limited to metadata/lookup/reconstruction boundaries; no substantiated material new performance regression found. Display-name cast alias may miss pushdown but retains correctness.
- Other concerns: all initial risks and candidates have explicit accepted/dismissed/duplicate dispositions in the ledger; no unresolved code question or uncovered changed-file scope remains.
Review scope is static only: no builds, tests, benchmarks or runtime reproduction were run. Those prohibited activities are not claimed as validation.
Review converged after 2 round(s).
Existing P0/P1 findings confirmed for this head: #69995 (comment)
|
|
||
| if (olapTable.getVariantEscapeDotsInKeys()) { | ||
| sb.append(",\n\"").append(PropertyAnalyzer.PROPERTIES_VARIANT_ESCAPE_DOTS_IN_KEYS) | ||
| .append("\" = \"true\""); |
Contributor
There was a problem hiding this comment.
[P2] Preserve the policy in copy-tablet DDL too
ADMIN COPY TABLET calls getDdlStmt with a concrete replica version. Its specificVersion != -1 branch emits only replication_num/version_info and returns before this renderer, so the returned CreateTableStmt recreates an escaped source with the default legacy policy. Replaying that DDL and inserting parse_to_variant('{"a.b":1}') stores {"a":{"b":1}}, preventing faithful reproduction. Emit this immutable property in the versioned DDL branch as well and cover getDdlStmt with specificVersion set.
Contributor
|
Codex automated review failed and did not complete. Error: Review recovery stopped: Final review author, head, timestamp, state or summary did not match Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
### Background ADMIN COPY TABLET generates a CREATE TABLE statement for reading a tablet snapshot at a specific version. The debug table must retain the source table's VARIANT key encoding. ### What problem does this PR solve? Issue Number: None Related PR: apache#69995 PR Type: Bugfix Problem Summary: Both specific-version DDL paths returned before emitting variant_escape_dots_in_keys. Reuse the normal DDL property renderer in both paths so debug tables preserve dotted-key lookup semantics. ### Details - **Frequency:** Copying a tablet from a table with variant_escape_dots_in_keys=true. - **Root cause:** The specific-version branches emit a reduced property list and bypass normal OLAP properties. - **Why existing tests missed it:** Copy-tablet DDL was not covered by the escaped-table property tests. - **Fix:** Share the opt-in property renderer across normal DDL and both specific-version branches. - **Other paths and branches:** Cover getDdlStmt and getCreateTableLikeStmt; preserve default-false DDL and snapshot version output. ### Testing - Both new copy-tablet tests fail before the fix on the missing property. - 13 affected FE test classes: 223 passed, 1 existing @disabled test skipped, zero failures or errors. - Both entry points cover true/false, single-bucket DDL, replication count, snapshot version, and executing the generated DDL to restore the encoding and visible version. - FE Checkstyle passed with zero violations. ### Release note ADMIN COPY TABLET preserves variant_escape_dots_in_keys in generated debug-table DDL. ### Check List (For Author) - Behavior changed: Yes (copy-tablet DDL preserves the source encoding). - Does this need documentation: No. - Pre-submission status: Ready. ### AI Disclosure - Tool: OpenAI Codex. - Generated: Code, tests, and commit description. - Human review: Pending.
Member
Author
|
run buildall |
1 similar comment
Member
Author
|
run buildall |
Member
Author
|
/review |
Contributor
There was a problem hiding this comment.
Static review of PR #69995 at 3265322, covering all 122 changed paths. One new P2 is reported inline: automatic CTAS codec inheritance can invalidate a retained legacy VARIANT template. The existing rolling-upgrade P1 (4499322562) still applies and is retained without a duplicate inline. The author documents mixed-version escaped tables as unsupported, but creation is not capability-gated, so old BEs can silently write the legacy namespace. Recommendation: request changes.
The review converged after two rounds: all three reviewers returned NO_NEW_VALUABLE_FINDINGS in round 2, coverage is complete, and every candidate and initial risk has an evidence-backed disposition. No additional user focus was supplied.
Critical checkpoint conclusions:
- Goal/data correctness: the storage-boundary codec separates literal dotted, nested and percent keys; CTAS inferred-template adaptation remains incomplete as described inline. Relevant unit and regression cases exist, but omit this mixed-source typed-template case.
- Scope/reuse: changes are focused on immutable table metadata, storage paths and index identity; shared helpers centralize policy and encoding.
- Concurrency: no new thread or lock order is introduced. Escaped columns bypass the shared column pool before per-schema policy binding; no additional race or deadlock was established.
- Lifecycle/static initialization: builder metadata owns decoded key bytes, canonical finalization sorts children, and cloned slot digests preserve identity. No new ownership cycle or cross-translation-unit initialization dependency was found.
- Configuration: the table property defaults false, persists true, and intentionally rejects ALTER and column-level syntax; dynamic updates are not supported.
- Compatibility: legacy defaults and markers are preserved, unknown markers fail explicitly, and execution values remain canonical. The existing mixed-version P1 remains. Runtime downgrade and final ELF/glibc compatibility were not validated in this static review; no new libc API or dependency change was identified.
- Thrift/protobuf: complete touched definitions were compared on HEAD and live master, branch-4.2, branch-4.1, branch-4.0 and branch-3.1. Added IDs TTabletSchema34, TabletSchemaPB47, TabletSchemaCloudPB48 and ColumnMetaPB28 are optional and unused on those branches; no new collision was found.
- Parallel paths: local/cloud creation, schema change, rollup, restore and repair carry the policy. Normal/vertical writers, sparse/DOC reconstruction, compaction, scanner, rowid/rowstore and scoring/index paths were covered.
- Conditions/error handling: malformed properties, invalid escapes, NestedGroup conflicts and unknown markers fail through existing error boundaries. Decoder exceptions are converted before publishing assembled output; no new ignored Status was found.
- Test coverage: tests cover positive/negative DDL, codec/golden bytes, schema copies, V2/V3, layout thresholds, promotion/demotion, failed compaction, MoW, MVs, indexes and two-phase reads. Tests were inspected only; no builds or tests ran.
- Expected results: all added regression suites and expected blocks were inspected. Identical blocks were compared and JSON-shaped cells across 287 blocks/1453 rows parsed without malformed cells. Actual executions and output-generation provenance were not independently verified.
- Observability: validation errors identify codec/property conflicts; existing path diagnostics, SHOW CREATE and extended DESC expose relevant state. No additional metric requirement was identified.
- Persistence/failover: TableProperty replay reconstructs the flag, tablet/cloud PB conversions preserve it, and segments persist their marker. Existing journal and visible-version transitions are unchanged.
- Writes/atomicity: new read errors propagate before output publication; restore rejects incompatible signatures or uses atomic replacement, and MV refresh validates fixed policy before insertion. Rowset/delete-bitmap publication algorithms are unchanged; failure fixtures check retained authoritative inputs.
- FE-BE propagation: all five local replica-task and four cloud tablet-builder production callers pass policy. Canonical compute values need no additional scan/function flag; storage paths bind policy from tablet schema.
- Performance: metadata caches and existing iteration structure remain; percent decoding is conditional and pool bypass is restricted to escaped columns. Display-name cast collisions can miss pushdown, but SlotId checks and expression fallback prevent the suspected wrong filtering; no separate substantiated performance finding was accepted.
- Other concerns/final sweep: previous copy-tablet DDL, external CTAS, changed-view MV refresh and translated-slot index issues are fixed; the recursive-CTE type concern is obsolete because storage policy is absent from compute types. No unresolved candidate or uncovered changed-file scope remains within the authorized static review. Runtime tests, benchmarks and executable ABI inspection remain outside that scope.
Review converged after 2 round(s).
Existing P0/P1 findings confirmed for this head: #69995 (comment)
Member
Author
|
run cloudut |
Member
Author
Local pipeline review — ✅ PASSschema: doris-repo-review/v1
status: PASS
pr: apache/doris#69995
commit: 326532285e5b7bd942eef91521ef1c0014ed98fb
base: 9e598d38789571a7fc30652aa054f035bd4f1184
reviewed_at: 2026-11-12T16:01+08:00
reviewer: eldenmoon
model: gpt-6-astra
effort: xhigh
findings: {blocker: 0, major: 0, minor: 0, nit: 0}
rounds: 1
converged: trueNotes for maintainers
Reviewed locally with the |
Member
Author
|
run compile |
Member
Author
|
run feut |
Member
Author
|
run p0 |
csun5285
reviewed
Nov 13, 2026
| int32_t doc_hash_shard_count = 64; | ||
|
|
||
| bool enable_nested_group = false; | ||
| bool escape_dots_in_keys = false; |
Contributor
There was a problem hiding this comment.
这个属性放在 TabletSchema 里面了，还有必要在 TabletColumn 存一份吗
| const auto& column = tablet_schema->column(index); | ||
| auto* olap_local_state = static_cast<OlapScanLocalState*>(_local_state); | ||
| const auto& cast_types = olap_local_state->_cast_types_for_variants; | ||
| if (auto it = cast_types.find(slot->col_name()); it != cast_types.end()) { |
Contributor
There was a problem hiding this comment.
如果FE 把Slot 里面的path改写了，be 这些都不动了
Contributor
There was a problem hiding this comment.
查询路径上可以都不用动，FE escape一下path
Member
Author
|
run external |
Member
Author
|
run beut |
…e paths from FE ### What problem does this PR solve? Issue Number: apache#69995 Related PR: apache#69995 Problem Summary: Address review comments on the escaped Variant key property. 1. The escape flag lives only on TabletSchema. Remove the TabletColumn copy, its propagation into derived subcolumns, and the TabletColumnObjectPool bypass. inherit_index now takes the owning schema's flag as a required argument. 2. FE translates Variant slot sub paths of escaped OLAP tables into storage spelling in PlanTranslatorContext.createSlotDesc. BE query code (olap_scanner, segment, predicate_collector, rowid_fetcher, vslot_ref) returns to master; make_variant_storage_path is removed. This also keeps query-cache normalization distinct for v['a.b'] and v['a']['b']. ### Release note None ### Check List (For Author) - Test: BE UT (TabletSchemaTest, schema_util, collection statistics, snii routing, path_in_data, VariantColumnWriterReaderTest), FE UT (VariantEscapeTableTest, QueryCacheNormalizerTest, DescriptorToThriftConverterTest, CreateTableCommandTest, MTMVPlanUtilTest, PropertyAnalyzerTest), variant_p0 and dotted_key_* with fuzzy session variables. - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
run buildall |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: N/A
Related PR: #70001
Problem Summary:
Legacy VARIANT stores
{"a.b":1}and{"a":{"b":2}}in the same dotted path namespace. Literal dotted keys can become nested objects, and an object containing both shapes can fail withduplicated entry. This PR adds an opt-in, lossless storage encoding while retaining legacy behavior for existing tables.Design
variant_escape_dots_in_keys, defaultfalse. It applies to every VARIANT column of the table and to all its indexes (rollups, synchronous MVs); columns added later inherit it.%becomes%25,.becomes%2E. Decode when assembling objects. Logical paths remain component arrays throughout computation and slot identity.["a.b"]a%2Eb["a","b"]a.b["a%2Eb"]a%252Ebfield_patternand SEARCH field syntax retain that storage-path contract. SQL subscripts use logical keys.0legacy,1escaped); readers decode by the marker and reject unknown markers. Rewrites decode and re-encode whole values. Physical index suffixes escape%only for escaped tables.Table property
ALTER TABLE ... SETrejects the key in any form. The column-level formVARIANT<PROPERTIES("variant_escape_dots_in_keys"=...)>is rejected with a message pointing to the table property.true, so other tables' DDL is unchanged.default_variant_enable_nested_group.Derived tables
v['a.b']maps to different storage paths on legacy and escaped tables, so mismatched MVs are rejected rather than rewritten. Refresh keeps the value fixed at creation and fails, leaving the MV out of rewrite, if its base tables change (for example through ALTER VIEW) to a different value.PropertyAnalyzerhelper, called by CTAS and byInternalCatalog.createOlapTablefor async MVs. Escaped-table schema checks (no NestedGroup, valid template andfield_patternstorage paths) run in one function at table creation, schema change and synchronous MV creation.BE transport
The BE receives the flag only when a tablet is created (
TTabletSchema34,TabletSchemaPB47,TabletSchemaCloudPB48) and keeps it in the persisted tablet schema. Loads, scans, schema change, compaction and rowid fetch read it from there, so no load or scan protocol field is needed. On the FE,CreateReplicaTaskand the cloud tablet meta builder take the flag as a required parameter. Escaped VARIANT columns do not use the shared column object pool, whose key is the column PB.Compatibility
Enable the property only after every FE and BE is upgraded; mixed-version clusters are not supported for escaped tables, and downgrading after creating one is unsupported. Existing legacy tables and data are unchanged. Protocol fields are optional:
TTabletSchema34,TabletSchemaPB47,TabletSchemaCloudPB48 andColumnMetaPB28, checked as unused on master, branch-4.2, branch-4.1, branch-4.0 and branch-3.1. Extended DESC displays stored paths such asv.a%2Eb.Release note
Add the table property
variant_escape_dots_in_keys, which stores literal dotted VARIANT keys separately from nested paths. It is set at creation and inherited by CTAS and async MVs over escaped tables. Enable it only after all FE/BE nodes are upgraded.Check List (For Author)
Validation at
f63f7fb5cf5, merged with master4c63661f60f(ASAN BE+FE, FE fuzzy session variables on withfuzzy_test_type=p0unless noted):Full
variant_p0: 210 suites, 209 passed, no.outregeneration. The only failure isoutfile_csv_variant_doc_valuewith object storage HTTP 403InvalidAccessKeyId, which also fails with fuzzy off. The local BE capsmax_sys_mem_available_low_water_mark_bytesat 1 GiB because the shared 1.5 TB host otherwise derives a 151 GB warning water mark and cancels small queries withMEM_LIMIT_EXCEEDED.BE UT: tablet schema, writer/reader (golden segment bytes, cross-encoding sink), binary extract iterator, shredder, pb_convert and rowid fetch: 195 passed, 2 skipped (NestedGroup writing is unavailable in this build). Earlier runs of the broader set (rowset builder, schema change, compaction): 577 passed. Cloud
SchemaKVTest: 9 passed.FE UT: 325 classes referencing the changed classes and methods, 3686 passed. The one failure,
DiskReblanceWhenSchedulerIdle, asserts a tablet distribution after a fixed one-second wait on INT tables; the test and the scheduler code are unchanged from master.Suite adjustments:
dotted_key_escape_common_typeanddotted_key_escape_scorepin the doc mode (and, for score, subcolumn) defaults that fuzzy randomizes; a path that is not materialized scores 0 on both legacy and escaped tables.New coverage: async MV inheritance, explicit true, rejected explicit false and rejected mixed VARIANT sources, int-only legacy join escaped table, refresh; CTAS inheritance through CTE/view/subquery/recursive CTE and explicit overrides; int-only CTAS followed by ADD COLUMN VARIANT; ALTER rejection; NestedGroup rejection (explicit and session default); rollup inheritance; row store with and without two-phase read; sparse path promotion during compaction (
dotted_key_escape_sparse_promotion).Recursive CTE mixing a legacy anchor with an escaped recursive branch (and the reverse) returns literal and nested keys correctly both directly and after CTAS.
Clang-format 16, clang-tidy on changed lines, FE Checkstyle and build hygiene passed.
Behavior changed: Yes; opt-in table property for lossless dotted keys.
Does this need documentation: Yes; website documentation follows the feature PR.
Check List (For Reviewer who merge this PR)
🤖 Generated with Claude Code