EscaperLL opened a new pull request, #69602:
URL: https://github.com/apache/doris/pull/69602
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
Doris accepts `"compression" = "LZ4HC"` and retains the property in FE table
metadata. However, both `TabletMeta::init_schema_from_thrift()` in the local
tablet creation path and `CloudInternalCatalog.createTabletMetaBuilder()` in
the Cloud path omit the LZ4HC conversion case. They fall back to LZ4F when
constructing the tablet schema.
As a result, `SHOW CREATE TABLE` can report LZ4HC while segment writers
select LZ4F from the persisted schema.
This PR adds the missing mapping in both paths, reusing the existing LZ4HC
codec and enum values. Tests cover schema construction and serialization with
LZ4HC and LZ4F control cases. The Cloud test also checks the initial rowset
schema.
The fix applies to schemas constructed through these paths after upgrading
the relevant BE or FE. Existing tablet schemas persisted as LZ4F are not
rewritten.
### Release note
Fix local and Cloud tablet schema creation to preserve requested LZ4HC
compression instead of silently falling back to LZ4F. Existing tablet schemas
are not rewritten.
### Check List (For Author)
- Test
- [x] Regression test
- Existing `compression_p0` load and query suites passed (2 suites,
0 failures), covering loading and querying with the supported codecs.
- [x] Unit Test
- ASAN BE: `TabletMetaTest.*:BlockCompressionTest.*`, 22 tests
passed, including `TabletMetaTest.PreserveCompressionType`.
- FE: the retained earlier run of `CloudInternalCatalogTest` passed
all 8 tests, including `testCreateTabletMetaPreservesCompressionType`; FE unit
tests were not rerun in the latest validation pass.
- The new tests directly verify LZ4HC mapping and serialization,
with LZ4F control cases; the FE test also verifies the initial Cloud rowset
schema.
- [x] Manual test
- Local LZ4F and LZ4HC tables each returned the expected 10,000 rows
with zero content mismatches. Segment footers reported the requested codec for
both top-level columns. Steps and build limitations are below.
- [ ] No need to test or manual test. Explain why:
- Behavior changed:
- [ ] No.
- [x] Yes.
- Newly constructed local and Cloud tablet schemas preserve LZ4HC
instead of falling back to LZ4F.
- Does this need documentation?
- [x] No.
- Corrects an existing compression option.
- [ ] Yes.
### Validation details
The latest validation pass ran on
`b730e2bdf15f785160b8133bf6babfaba864c587`, whose Git tree is identical to PR
head `8892725303254a05a5cfc46992e4f33a35f0f3cb`. The base is
`83f6f9ec033ebde4cea139976fb746152c09fd56`. The FE unit-test result above comes
from a retained earlier run and was not regenerated in this pass.
Validation commands:
- `./build.sh --fe -j 2` passed, including Checkstyle.
- `./run-be-ut.sh --run --filter='TabletMetaTest.*:BlockCompressionTest.*'
-j 4` passed.
- `./run-regression-test.sh -d compression_p0 -s load`, followed by
`./run-regression-test.sh -d compression_p0 -s query`, passed against the
isolated local cluster.
- clang-format 16, build hygiene, and `git diff --check` passed.
Changed-line clang-tidy passed for both modified C++ files using the project
script, the ASAN UT compilation database, and an explicit line filter matching
the PR diff.
Build and coverage limitations:
- With Clang 22.1.8 and the local libstdc++ toolchain, BE validation
required a temporary unrelated Parquet fix: adding `<cmath>` and replacing
`std::powf(2.0F, -14.0F)` with `std::pow(2.0F, -14.0F)`. This patch was
restored after validation and is excluded from the PR.
- The ASAN BE executable linked, but the standard build failed its post-link
glibc 2.17 compatibility check; the check was not bypassed. Runtime checks used
that linked BE and the newly built FE, both reporting revision `b730e2bdf`.
Footer inspection used an earlier-baseline `meta_tool` supporting the existing
LZ4F and LZ4HC enum values. These results establish local functionality, not a
successful complete BE/meta_tool build or release compatibility.
- Cloud validation is limited to FE unit tests; no Cloud cluster run was
performed. Required CI checks must pass on the final PR head before merge.
<details>
<summary>Manual local-storage verification steps</summary>
Create fresh control and test tables in a local-storage deployment:
```sql
CREATE DATABASE IF NOT EXISTS codec_verify;
USE codec_verify;
DROP TABLE IF EXISTS codec_lz4f;
DROP TABLE IF EXISTS codec_lz4hc;
CREATE TABLE codec_lz4f (id BIGINT, value VARCHAR(128))
DUPLICATE KEY(id)
DISTRIBUTED BY HASH(id) BUCKETS 1
PROPERTIES ("replication_num" = "1", "storage_format" = "V2", "compression"
= "LZ4F");
CREATE TABLE codec_lz4hc (id BIGINT, value VARCHAR(128))
DUPLICATE KEY(id)
DISTRIBUTED BY HASH(id) BUCKETS 1
PROPERTIES ("replication_num" = "1", "storage_format" = "V2", "compression"
= "LZ4HC");
INSERT INTO codec_lz4f
SELECT number, RPAD(CAST(number AS STRING), 100, 'x')
FROM numbers("number" = "10000");
INSERT INTO codec_lz4hc
SELECT number, RPAD(CAST(number AS STRING), 100, 'x')
FROM numbers("number" = "10000");
SYNC;
SELECT 'LZ4F' AS codec, COUNT(*), SUM(id), SUM(LENGTH(value)),
SUM(IF(value = RPAD(CAST(id AS STRING), 100, 'x'), 0, 1)) AS
mismatches
FROM codec_lz4f
UNION ALL
SELECT 'LZ4HC' AS codec, COUNT(*), SUM(id), SUM(LENGTH(value)),
SUM(IF(value = RPAD(CAST(id AS STRING), 100, 'x'), 0, 1)) AS
mismatches
FROM codec_lz4hc
ORDER BY codec;
SHOW CREATE TABLE codec_lz4f;
SHOW CREATE TABLE codec_lz4hc;
SHOW TABLETS FROM codec_lz4f;
SHOW TABLETS FROM codec_lz4hc;
```
Both aggregate rows should report `10000`, `49995000`, `1000000`, and `0`
mismatches. `SHOW CREATE TABLE` should retain each requested codec.
For each tablet ID returned above, locate its newest segment under the BE
storage directory, replacing the placeholders with the deployment's paths and
tablet ID:
```bash
find /path/to/be/storage -type f \
-path "*/<tablet_id>/*/*.dat" -printf '%T@ %s %p\n' | sort -nr
/path/to/meta_tool --operation=show_segment_footer
--file=/path/to/segment.dat \
| jq -r '.columns[] | [.column_id, .compression] | @tsv'
```
Both top-level column footers should report LZ4F for the control table and
LZ4HC for the test table. Before the fix, both tables report LZ4F. The input
exceeds the default compression threshold; individual pages may still be
uncompressed when compression is not beneficial.
</details>
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
--
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]
