[feature](compression) Support per-column compression for non-cloud - #66169
Open
zwy991114 wants to merge 5 commits into
Open
[feature](compression) Support per-column compression for non-cloud#66169zwy991114 wants to merge 5 commits into
zwy991114 wants to merge 5 commits into
Conversation
zwy991114
requested review from
924060929,
airborne12,
csun5285,
eldenmoon,
englefly,
gavinchou,
liaoxin01,
morrySnow,
starocean999 and
yiguolei
as code owners
July 28, 2026 10:47
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Author
|
run buildall |
1 similar comment
Contributor
|
run buildall |
Contributor
|
/review |
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Requesting changes for seven P1 findings and one P2 finding. No additional user-provided review focus was supplied, so I reviewed the whole PR.
Critical checkpoint conclusions:
- Goal and scope: the PR coherently adds a per-column codec and optional level across SQL, FE catalog/protocol, tablet schema, and both segment writers. The ordinary non-cloud scalar path is largely connected, but cloud/full schema change, compression-only MODIFY, light CCR SQL, complex AGG_STATE storage, codec lifetime/accounting, and test gaps mean the goal is not safely complete.
- Size and focus: the cross-module scope is necessary for this feature and unrelated production changes were not found.
- Concurrency and lifecycle: codec pools are mutex-protected, writer ownership keeps the observer pointer valid, and no new race, lock-order, or deadlock issue was found. The new owned-codec lifecycle does create the retained-workspace multiplier and allocation/free tracker mismatch called out inline.
- Configuration and compatibility: no configuration item is added. Optional thrift/protobuf/footer fields remain readable by compatible readers; old BEs can ignore the write policy during a rolling upgrade and emit the table codec, which is storage-policy drift rather than unreadable data.
- Parallel paths and conditions: legacy, vertical, row-binlog, schema-change, and compaction writers implement small-segment suppression > explicit override > table default when every physical schema node carries the policy. The cloud protobuf omission and complex AGG_STATE child/auxiliary metas are the exceptions. Compression in general Column.equals() also reaches routing/type guards that should not treat it as a logical change.
- Tests and results: none of the changed tests observes requested-level application; the regression write is below the compression-suppression threshold, and the suite lacks the required generated result contract. Per the review-only task contract I did not run local builds/tests; this checkout also lacks .worktree_initialized, thirdparty/installed, and protoc. Live CI currently has compile, FE UT, macOS BE UT, style, license, and secret checks passing; BE UT is ERROR, Cloud UT is FAILURE, several regression/performance checks are pending, and the PR title checker is failing.
- Observability and persistence: SHOW CREATE from the catalog column renders the policy, but reconstructed command SQL omits it. Local edit-log replay retains schema objects; the light CCR binlog executes the incomplete rawSql and loses the clause. Full cloud shadow-tablet protobuf construction separately drops the fields.
- Data writes and transactions: no visible-version, transaction, delete-bitmap, or atomicity mechanism is changed. Segments remain readable because the actual codec is stored in the footer, but affected paths silently write a different policy from the DDL.
- Performance: a private reusable native workspace per levelled column/open segment is an unbounded schema-width multiplier and is not included in segment buffer estimation.
- Other: no security-sensitive behavior, new metric, or dynamic configuration requirement was introduced.
PR metadata also needs cleanup: the title does not match the required bracketed format (for example, [feature](compression) Support per-column compression for non-cloud); Issue Number and Related PR still contain #xxx; and this user-visible feature is marked both Release note: None and Behavior changed: No while documentation is checked without a link.
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
TPC-H: Total hot run time: 29478 ms |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
TPC-DS: Total hot run time: 177512 ms |
Contributor
ClickBench: Total hot run time: 25.77 s |
zwy991114
changed the title
zwy991114
force-pushed
the
feature/per-column-compression-codec
branch
from
July 29, 2026 08:28
99ea75a to
3c2c514
Compare
Author
|
run buildall |
Author
|
/review |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 29696 ms |
Contributor
TPC-DS: Total hot run time: 177250 ms |
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
There was a problem hiding this comment.
Automated review of a5ef0e05db837749f8bde5a12186bd666badb0f9 against 8305fe71867fb2acab5a769fe9a098ada59aff50.
I found no additional unique inline finding after reviewing all 29 changed files and suppressing duplicates against the live review threads. This is not an approval: the existing threads and maintainer requirements still need resolution, notably the requested removal of schema-change support for now, mixed-version upgrade/downgrade behavior, compression metadata/default semantics, native ZSTD workspace accounting, production-path level oracles, and regression-test form.
The review focus file adds no extra guidance, so I reviewed the whole PR.
Syntax research requested by the maintainer also remains a product decision. Established systems use different conventions: ClickHouse documents per-column CODEC(ZSTD(3)), Impala/Kudu documents per-column COMPRESSION LZ4, and StarRocks documents compression as a table property. The proposed COMPRESSION 'zstd:9' form should therefore be aligned deliberately rather than treated as an established common syntax. Sources: ClickHouse, Impala/Kudu, StarRocks.
Required checkpoint conclusions:
- Goal and proof: the code connects non-cloud scalar-column DDL through FE persistence/thrift and BE tablet/footer metadata to level-aware codecs. Unit coverage spans parsing, persistence, both writer metadata builders, codecs, and round trips, but the existing live test-oracle thread correctly notes that the production finalized-footer path and the small-segment regression path still do not prove the requested level is applied.
- Scope and clarity: the core create/write path is reasonably localized. ADD/MODIFY and compression-only partition/distribution schema-change handling broaden the feature beyond the maintainer's requested current scope and should be removed or deferred as already requested.
- Concurrency: the process-wide codec registry is mutex-protected; codec construction is outside the lock and duplicate insertion is reconciled under the lock. Returned codec pointers are stable while the production registry lives. The test-only clear hook is safe only under its current serial, no-live-observer use.
- Lifecycle and static order: leveled codecs and their context pools are process-lifetime objects, avoiding per-column teardown. No new circular ownership or production observer race was found. The existing native allocation/free accounting thread remains unresolved because ZSTD may allocate workspace lazily during compression outside the intended tracker scope.
- Configuration: no new dynamic configuration item is added. An absent/non-positive column level follows the existing codec singleton/configured-default path; explicit valid levels select bounded leveled instances.
- Compatibility: absent optional protobuf/thrift fields normalize to inheritance of the table codec, and current enum identities are compile-time guarded. A mixed-version old BE can ignore and persist away a new FE's per-column policy, so rolling upgrade/downgrade support is not established; this matches the maintainer's existing request for testing and disclosure.
- Parallel paths: ordinary legacy and vertical writers, row-binlog inheritance, segcompaction, and vertical compaction reach the changed metadata initialization. Cloud DDL and complex/AGG_STATE layouts are rejected instead of silently dropping the policy. No additional missed supported writer path was found.
- Conditions and error handling: codec/type/level validation fails explicitly, and the non-cloud/complex-type gates explain the unsupported boundaries. Small-segment suppression precedence is documented in code but is already disputed in a live maintainer thread; no duplicate comment added.
- Tests: positive and negative FE/unit coverage exists, and direct codec tests now compare byte streams across levels. Existing threads still cover the missing production-footer oracle, three-row suppression blind spot, upgrade/downgrade coverage, and regression-suite contract violations.
- Expected results: the checked-in
.outcontains deterministic ordered query results, but SHOW assertions and final cleanup remain outside the required generated-result/debug-preservation style already reported inline. - Observability: this does not add a new distributed state machine, and existing status errors plus stored footer metadata are sufficient for the local write path. No new metric requirement was identified.
- Persistence and failover: catalog
Columnstate is Gson-persisted, schema-change records retainColumnobjects, and current FE-to-BE creation persists the fields intoColumnPB. No new master-failover ordering defect was found; mixed-version policy loss remains the compatibility blocker above. - Data writes and crash safety: transaction boundaries and segment commit behavior are unchanged; each segment footer records the codec needed for later reads. No new atomicity or crash-consistency defect was found beyond the existing memory-accounting concern.
- FE/BE variables: codec and level flow through
TColumn,ColumnPB,TabletColumn, and bothColumnMetaPBbuilders. Cloud is explicitly gated. The old-component drop case is the unresolved mixed-version issue. - Performance: sharing codecs by
(type, level)removes per-column heavyweight pools, and validated levels bound the registry to a small fixed key set. The existing tracker/workspace issue still prevents treating memory accounting as complete. - Other issues: no additional correctness, security-boundary, nullable-column, planner-tree, or MoW-specific issue applies to this change. Documentation and release/behavior declarations should be updated as already requested by the maintainer.
No builds or tests were run locally because the authoritative review contract requires static analysis only. At review time, style/license/secret/cloud checks passed; BE, FE, compile, performance, macOS, and automated-review checks were still pending.
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 29176 ms |
Contributor
TPC-DS: Total hot run time: 165780 ms |
Contributor
ClickBench: Total hot run time: 25.29 s |
…x codec lifecycle
Author
|
run buildall |
Author
ClickHouse当前PR最接近ClickHouse 的设计：压缩配置属于列定义，并且算法参数使用函数式语法。 它还支持组合预处理 Codec，例如 Delta 后接 ZSTD，并支持通过 ADD COLUMN、MODIFY COLUMN 修改 Codec。(clickhouse.com (https://clickhouse.com/docs/get-started/sample-datasets/wikistat)) Amazon RedshiftRedshift 使用列级 ENCODE： 也可以让系统自动选择： Redshift 的 encoding 是算法或数据编码类型，不暴露 ZSTD 数字等级。(docs.aws.amazon.com (https://docs.aws.amazon.com/redshift/latest/dg/r_CREATE_TABLE_NEW.html)) StarRocksStarRocks 目前是表级属性： 支持 LZ4、ZSTD、zlib 和 Snappy；只能建表时设置，不能按列或通过 ALTER 修改，也不暴露压缩等级。(docs.starrocks.io (https://docs.starrocks.io/docs/table_design/data_compression/)) DuckDBDuckDB 的原生表通常不在列定义中指定压缩算法，显式算法主要用于 Parquet 输出： 这是文件级配置，不属于表的持久化 Schema。(duckdb.org (https://duckdb.org/docs/current/data/parquet/overview)) |
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 29089 ms |
Contributor
TPC-DS: Total hot run time: 166789 ms |
Contributor
ClickBench: Total hot run time: 25.08 s |
csun5285
reviewed
Aug 7, 2026
morrySnow
reviewed
Aug 7, 2026
csun5285
reviewed
Aug 10, 2026
Author
|
run buildall |
1 similar comment
Contributor
|
run buildall |
Contributor
|
/review |
Contributor
There was a problem hiding this comment.
Requesting changes for three new P1 findings. Two normal full-review passes plus a separate risk-focused pass converged after Round 2 with NO_NEW_VALUABLE_FINDINGS; existing live threads and prior review summaries were duplicate-suppressed. No additional user-provided focus was supplied, so the whole 29-file PR was reviewed.
Critical checkpoint conclusions:
- Goal and proof: the PR largely connects per-column ZSTD/LZ4HC policy from DDL through FE catalog/protocol metadata, TabletSchema, both segment writers, and level-aware codecs. The goal is not complete because SQL materialized-view projection drops the policy, a rejected compound ALTER can partially mutate live metadata, and the replacement codec registry can retain aggregate historical concurrency peaks. Tests prove parsing, metadata round trips, both writer metadata builders, and actual first/reused-context level effects with compressed-byte comparisons; the three reported scenarios are not covered.
- Size, clarity, and focus: the cross-module surface is necessary and no unrelated production change was found. Current serializers and copy-based paths are straightforward; the fresh SQL-MV constructor and clause-local ALTER rejection are the two missed parallel/control-flow paths.
- Concurrency: concurrent segment/page compression shares the new registry. Registry mutation is mutex-protected, codec construction stays outside the registry lock with insertion reconciled under lock, and per-codec context pools have their own locks; no race, lock-order, or deadlock defect was found. The remaining concurrency-related problem is retained capacity, reported inline.
- Lifecycle: registry-owned codec pointers remain stable across map rehash and production instances intentionally live for the process. Context reset/error destruction and native allocation/free tracker symmetry are sound. That lifetime nevertheless makes every used level's unbounded idle-context peak permanent, which is the reported lifecycle/performance defect; no separate cross-TU static-order issue was found.
- Configuration: no configuration item is added, so dynamic-config propagation does not apply. Explicit levels are FE-bounded and absent/non-positive levels correctly select existing codec defaults.
- Compatibility: the optional Thrift/protobuf/footer fields preserve byte readability because decompression needs the stored codec, not the level, and enum identities are compile-time guarded. The previously raised old-BE rolling-upgrade policy-loss concern remains valid and was not duplicated here; current new-BE local/cloud producers carry both fields.
- Parallel paths and conditions: SegmentWriter, VerticalSegmentWriter, inherited row-binlog, load, partial update, schema change, segment compaction, vertical compaction, and cloud rowset output reach the updated metadata builders for supported scalar/nullable columns. Complex, VARIANT, and AGG_STATE overrides are rejected. DEFAULT parsing, new non-reserved codec keywords, type/range checks, and SQL reconstruction are sound. SQL materialized views and compound-ALTER preflight are the reported exceptions.
- Tests and results: changed FE/BE tests use deterministic assertions, the regression suite follows drop-before-use, ordered-result, expected-error, and generated-output form, and BE tests include real compressed-byte level oracles. Missing SQL-MV, compound-ALTER atomicity, and aggregate multi-level retention tests are requested inline. This was a static review; builds and tests were not run under the review-task contract.
- Observability: no new distributed state machine or user-tunable runtime service is introduced. Existing stored footer metadata and status/error surfaces are sufficient once the policy is propagated correctly; no new metric or log requirement was identified.
- Persistence, transactions, and data writes: catalog Gson/edit-log/image state, local ColumnPB, cloud ColumnPB, rowset TabletSchema, and segment footer persistence were traced. Visible-version, commit, MoW delete-bitmap, and crash-readability behavior are unchanged. The compound-ALTER finding is a failure-atomicity violation because live bloom-filter metadata changes without a job or edit log.
- FE/BE variables: codec and level are passed through current
TColumn,ColumnPB,TabletColumn, andColumnMetaPBpaths in both deployment modes. The SQL-MV issue loses them before those producers, rather than inside the wire conversion. - Performance: sharing by
(codec, level)removes the former per-column workspace multiplier, but the new process-lifetime registry has no aggregate idle-context/byte bound, so sequential multi-level peaks accumulate. No other material CPU, allocation, or repeated-scan regression was found. - Other: no additional correctness, nullable-layout, security-boundary, configuration, transaction-visibility, documentation-path, or observability issue survived the final sweep. The linked documentation PR covers this user-visible feature; PR description syntax/scope metadata should be kept aligned with the final implementation.
| this.clusterKeyId = column.getClusterKeyId(); | ||
| this.generatedColumnInfo = column.generatedColumnInfo; | ||
| this.sessionVariables = column.sessionVariables; | ||
| this.compressionType = column.compressionType; |
Contributor
There was a problem hiding this comment.
[P1] Preserve the override when a SQL materialized view projects a base column. This copy constructor now carries the new fields, but both MaterializedViewHandler.checkAndPrepareMaterializedView() branches build the MV schema through MVColumnItem.toMVColumn(), whose fresh new Column(...) never copies them from a direct SlotRef. For example, an MV selecting k, v from a wider table with v ... COMPRESSION ZSTD(9) reaches both the local and cloud tablet serializers with no override, so the physical rollup silently falls back to the table codec; ordinary ADD ROLLUP already preserves it through this copy path. Copy the policy for direct base-column items and add local/cloud SQL-MV schema coverage.
| throws DdlException { | ||
| Column column = addColumnOp.getColumn(); | ||
| if (column.hasCompressionOverride()) { | ||
| throw new DdlException("Per-column compression is not supported for ADD COLUMN"); |
Contributor
There was a problem hiding this comment.
[P1] Preflight this restriction before processing any clause in the statement. Schema-change clauses are compatible and executed in source order, but some earlier clauses mutate live catalog state: processDropColumn() immediately calls olapTable.setBloomFilterInfo() when dropping a bloom-filter column. Thus ALTER TABLE t DROP COLUMN bf_col, ADD COLUMN x INT COMPRESSION ZSTD(9) clears bf_col from the live bloom-filter set and then throws here, before any schema job or edit log, while the column itself remains in the table schema. Validate every ADD/ADD-COLUMNS/MODIFY compression restriction before the clause loop and cover a failing compound ALTER so the operation stays atomic.
| // Process-wide registry of level-aware codecs, keyed by (type, level). All | ||
| // column writers that request the same codec+level share one instance, so its | ||
| // internal context pool is reused according to actual write concurrency rather | ||
| // than allocated once per column. Instances live for the process lifetime (like |
Contributor
There was a problem hiding this comment.
[P1] Bound idle contexts across these process-lifetime level pools. Sharing by (codec, level) fixes the prior per-column pool, but every level has its own unbounded context vector and each returned context retains its native workspace plus a reusable buffer of up to 8 MiB. Because all 22 ZSTD and 12 LZ4HC level instances live forever, sequential bursts of N concurrent writers at different levels retain the sum of all historical peaks even after those tables are idle. Keep codec objects pointer-stable, but cap/shrink/discard excess idle contexts across keys, and test concurrent warm/release at several levels rather than only serial reuse of one already-warm pool.
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 29745 ms |
Contributor
TPC-DS: Total hot run time: 159106 ms |
Contributor
ClickBench: Total hot run time: 23.87 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
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
4 participants
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: close #xxx
Related PR: #xxx
Problem Summary:
Doris only supports a single compression codec configured at the table level (via PROPERTIES("compression"=...)), applied uniformly to every column. In real workloads, columns have very different data characteristics — highly redundant text columns (URLs, referers, search phrases) compress far better under a high-level ZSTD/LZ4HC codec, while numeric/low-cardinality columns gain little and pay the CPU cost. There was no way to tune the codec per column.
This PR adds per-column generic compression codec support for non-cloud (OLAP) tables. Users can specify a codec and optional level directly on a column:
Compression ratio test on ClickBench hits:
Loaded the ClickBench hits dataset into two tables with identical schema. One table (baseline) uses the ZSTD level 3 default on every column; the other overrides five heavy text columns with COMPRESSION 'zstd:19'. After forcing full compaction on both, per-column sizes were read from information_schema.column_data_sizes.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)