github-actions[bot] commented on code in PR #66169:
URL: https://github.com/apache/doris/pull/66169#discussion_r3749048092
##########
be/src/util/block_compression.cpp:
##########
@@ -1615,6 +1641,84 @@ Status
get_block_compression_codec(segment_v2::CompressionTypePB type,
return Status::OK();
}
+// 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
Review 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.
##########
fe/fe-catalog/src/main/java/org/apache/doris/catalog/Column.java:
##########
@@ -389,6 +395,8 @@ public Column(Column column) {
this.clusterKeyId = column.getClusterKeyId();
this.generatedColumnInfo = column.generatedColumnInfo;
this.sessionVariables = column.sessionVariables;
+ this.compressionType = column.compressionType;
Review 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.
##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java:
##########
@@ -186,6 +186,9 @@ private boolean processAddColumn(AddColumnOp addColumnOp,
OlapTable olapTable,
Map<Long, IntSupplier>
colUniqueIdSupplierMap)
throws DdlException {
Column column = addColumnOp.getColumn();
+ if (column.hasCompressionOverride()) {
+ throw new DdlException("Per-column compression is not supported
for ADD COLUMN");
Review 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
