Skip to content

[Test](bitmap) Harden BitmapValue serialization test#65491

Open
linrrzqqq wants to merge 1 commit into
apache:masterfrom
linrrzqqq:bitmap-ser-test
Open

[Test](bitmap) Harden BitmapValue serialization test#65491
linrrzqqq wants to merge 1 commit into
apache:masterfrom
linrrzqqq:bitmap-ser-test

Conversation

@linrrzqqq

Copy link
Copy Markdown
Collaborator

Release note

  • Strengthen BitmapValue serialization tests for deterministic and non-deterministic formats.

  • Keep exact byte assertions for non-SET bitmap encodings.

  • Add fixed historical SET binary inputs and verify deserialized bitmap contents semantically.

  • Avoid pinning bitmap_to_base64 output for SET because hash-set iteration order makes serialized bytes unstable.

@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See 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?

@linrrzqqq

Copy link
Copy Markdown
Collaborator Author

/review

@linrrzqqq

Copy link
Copy Markdown
Collaborator Author

run buildall

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review completed for PR #65491.

I did not find a substantiated blocking issue to raise as an inline comment. The PR is test-only and is focused on making bitmap serialization coverage deterministic where SET payload ordering depends on hash-set iteration, while keeping exact byte checks for deterministic encodings and adding historical SET decode cases.

Critical checkpoint conclusions:

  • Goal and coverage: The changed tests exercise BitmapValue serialization for v1/v2 bitmap encodings, semantic SET round trips, historical SET payload order, SQL bitmap_from_base64 semantics, and doc-suite round-trip behavior. The current code accomplishes the stated test-hardening goal.
  • Scope: The PR is small and limited to bitmap serialization tests and expected output files.
  • Concurrency, lifecycle, persistence, data writes, and FE/BE protocol: No production code paths are changed, so these checkpoints are not materially affected.
  • Configuration/session behavior: The new BitmapValue serde test saves and restores both bitmap config values it mutates. Existing function tests still use config::set_config as before; the new assertions do not introduce a distinct leak.
  • Parallel paths: Unit coverage and regression coverage both exercise bitmap_to_base64/bitmap_from_base64 round trips; fixed historical bytes cover from_base64 decode semantics independently from nondeterministic SET serialization order.
  • Expected results: The added regression outputs match fixed decode inputs. bitmap_to_string sorts SET contents before printing, so the new outputs are deterministic.
  • Performance and observability: Not applicable for this test-only change.

Subagent conclusions:

  • optimizer-rewrite: NO_NEW_VALUABLE_FINDINGS in the initial pass and final convergence round.
  • tests-session-config: NO_NEW_VALUABLE_FINDINGS in the initial pass and final convergence round.
  • No subagent candidates were accepted, dismissed as duplicates, or converted into inline comments.

User focus points: No additional user-provided review focus was supplied.

Validation performed: reviewed the GitHub PR file list and patch, inspected related BitmapValue and bitmap function code paths, verified no existing inline review comments, and ran a scoped git diff --check over the five GitHub-reported PR files. I did not run BE unit tests or regression tests because this checkout is missing .worktree_initialized, thirdparty/installed, and thirdparty/installed/bin/protoc; the review is therefore static plus lightweight diff validation.

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100% (0/0) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 74.78% (30188/40368)
Line Coverage 58.77% (331403/563878)
Region Coverage 55.57% (278219/500634)
Branch Coverage 56.82% (122935/216359)

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 29261 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit c96f4ca2c39fff291e19f355e9a9a5b1f55bb377, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17744	4115	4164	4115
q2	2011	319	202	202
q3	10359	1444	792	792
q4	4682	469	335	335
q5	7490	848	588	588
q6	184	174	138	138
q7	770	833	632	632
q8	9363	1445	1521	1445
q9	5601	4383	4350	4350
q10	6760	1775	1519	1519
q11	515	337	312	312
q12	725	542	435	435
q13	18054	3370	2727	2727
q14	263	256	256	256
q15	q16	782	789	709	709
q17	1023	926	928	926
q18	6896	5723	5495	5495
q19	1306	1284	980	980
q20	717	683	557	557
q21	6065	2687	2453	2453
q22	448	355	295	295
Total cold run time: 101758 ms
Total hot run time: 29261 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	4349	4275	4290	4275
q2	293	313	213	213
q3	4596	4911	4395	4395
q4	2053	2153	1357	1357
q5	4426	4322	4316	4316
q6	226	175	127	127
q7	1747	1611	2058	1611
q8	2558	2137	2179	2137
q9	8082	8132	7817	7817
q10	4756	4773	4231	4231
q11	574	420	373	373
q12	764	784	561	561
q13	3261	3565	2954	2954
q14	324	319	280	280
q15	q16	701	738	681	681
q17	1386	1306	1303	1303
q18	8293	7264	7202	7202
q19	1132	1099	1091	1091
q20	2208	2189	1937	1937
q21	5235	4508	4338	4338
q22	529	446	392	392
Total cold run time: 57493 ms
Total hot run time: 51591 ms

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants