batch serialize/deserialize interface support handling nullmap by guo-shaoge · Pull Request #9825 · pingcap/tiflash

guo-shaoge · 2025-01-24T02:49:54Z

What problem does this PR solve?

Issue Number: close #9836

Problem Summary:
#9756 support the compare semantics family of batch serialize/deserialize. But it didn't handle nullmap.

For example, for column ColumnNullable(ColumnString), if row-i is null, a default value will be serialized.

What is changed and how it works?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

None

Signed-off-by: guo-shaoge <shaoge1994@163.com>

…ize_refine Signed-off-by: guo-shaoge <shaoge1994@163.com>

Signed-off-by: guo-shaoge <shaoge1994@163.com>

gengliqi · 2025-02-14T05:17:38Z

dbms/src/Columns/ColumnArray.cpp

 }

-template <bool for_compare>
+template <bool compare_semantics>


for_compare seems better? Because the caller function name is XXXForCmp

dbms/src/Columns/IColumn.h

gengliqi

Seems countSerializeByteSizeForCmp also needs to add a nullmap because the size may be changed if it's null.

Signed-off-by: guo-shaoge <shaoge1994@163.com>

guo-shaoge · 2025-02-17T09:21:06Z

/retest

dbms/src/Columns/ColumnString.cpp

dbms/src/Columns/ColumnDecimal.cpp

xzhangxian1008

LGTM

Signed-off-by: guo-shaoge <shaoge1994@163.com>

dbms/src/Columns/ColumnDecimal.cpp

gengliqi

Others LGTM

gengliqi · 2025-02-20T09:58:43Z

dbms/src/Columns/ColumnString.cpp

 }

-template <bool has_null, bool has_collator, typename DerivedCollator>
+template <bool compare_semantics, bool has_null, bool need_decode_collator, typename DerivedCollator, bool has_nullmap>


The function defination uses has_collator but here uses need_decode_collator

ti-chi-bot · 2025-02-20T10:01:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gengliqi, xzhangxian1008

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [gengliqi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2025-02-20T10:01:56Z

[LGTM Timeline notifier]

Timeline:

2025-02-19 03:20:12.837546193 +0000 UTC m=+1017855.233768252: ☑️ agreed by xzhangxian1008.
2025-02-20 10:01:55.20583731 +0000 UTC m=+1128357.602059372: ☑️ agreed by gengliqi.

ti-chi-bot · 2025-02-20T10:16:34Z

@guo-shaoge: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

trigger some heavy tests which will not run always when PR updated.

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

guo-shaoge added 2 commits January 24, 2025 10:16

tmp save

f6bb074

Signed-off-by: guo-shaoge <shaoge1994@163.com>

refine

51e24d8

Signed-off-by: guo-shaoge <shaoge1994@163.com>

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/needs-linked-issue size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 24, 2025

guo-shaoge changed the title ~~Batch serialize refine~~ batch serialize/deserialize interface support handling nullmap Jan 24, 2025

guo-shaoge added 3 commits January 24, 2025 11:50

plan 1 code

543f1b8

Signed-off-by: guo-shaoge <shaoge1994@163.com>

refine

370bf0a

Signed-off-by: guo-shaoge <shaoge1994@163.com>

remove changes in count

a13008f

Signed-off-by: guo-shaoge <shaoge1994@163.com>

guo-shaoge force-pushed the batch_serialize_refine branch from 300891b to a13008f Compare January 25, 2025 14:02

guo-shaoge added 4 commits January 26, 2025 11:45

refine

817854b

Signed-off-by: guo-shaoge <shaoge1994@163.com>

refine

d9aeae7

Signed-off-by: guo-shaoge <shaoge1994@163.com>

refine

eb01d9e

Signed-off-by: guo-shaoge <shaoge1994@163.com>

refine

62fbbed

Signed-off-by: guo-shaoge <shaoge1994@163.com>

guo-shaoge force-pushed the batch_serialize_refine branch from 5d11869 to 62fbbed Compare January 26, 2025 10:22

refine

0ce2f53

Signed-off-by: guo-shaoge <shaoge1994@163.com>

ti-chi-bot bot removed the do-not-merge/needs-linked-issue label Jan 27, 2025

guo-shaoge requested review from gengliqi and windtalker January 27, 2025 05:55

guo-shaoge added 3 commits January 27, 2025 17:25

refine

c4c0eb9

Signed-off-by: guo-shaoge <shaoge1994@163.com>

Merge branch 'master' of github.com:pingcap/tiflash into batch_serial…

688ed25

…ize_refine Signed-off-by: guo-shaoge <shaoge1994@163.com>

fix

2a48af6

Signed-off-by: guo-shaoge <shaoge1994@163.com>

guo-shaoge force-pushed the batch_serialize_refine branch from 8d3142c to 2a48af6 Compare February 6, 2025 03:32

guo-shaoge requested a review from xzhangxian1008 February 11, 2025 08:49

gengliqi reviewed Feb 14, 2025

View reviewed changes

guo-shaoge added 4 commits February 14, 2025 16:13

add nullmap for countSerializeByte

c88cbdc

Signed-off-by: guo-shaoge <shaoge1994@163.com>

add has_null

e2face0

Signed-off-by: guo-shaoge <shaoge1994@163.com>

fmt

d57ff7b

Signed-off-by: guo-shaoge <shaoge1994@163.com>

refine count

10a1914

Signed-off-by: guo-shaoge <shaoge1994@163.com>

refine

7a68832

Signed-off-by: guo-shaoge <shaoge1994@163.com>

guo-shaoge removed the request for review from windtalker February 17, 2025 10:46

xzhangxian1008 reviewed Feb 18, 2025

View reviewed changes

dbms/src/Columns/ColumnString.cpp Outdated Show resolved Hide resolved

dbms/src/Columns/ColumnString.cpp Show resolved Hide resolved

dbms/src/Columns/ColumnString.cpp Show resolved Hide resolved

gengliqi reviewed Feb 18, 2025

View reviewed changes

dbms/src/Columns/ColumnDecimal.cpp Outdated Show resolved Hide resolved

xzhangxian1008 approved these changes Feb 19, 2025

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Feb 19, 2025

fix

f94d6cb

Signed-off-by: guo-shaoge <shaoge1994@163.com>

guo-shaoge force-pushed the batch_serialize_refine branch from 2b7c8ff to f94d6cb Compare February 19, 2025 03:23

add nullmap for column decimal cout

5fe79b6

Signed-off-by: guo-shaoge <shaoge1994@163.com>

guo-shaoge force-pushed the batch_serialize_refine branch from d58ce32 to 5fe79b6 Compare February 19, 2025 04:08

guo-shaoge added 2 commits February 19, 2025 12:59

fix

ef3953b

Signed-off-by: guo-shaoge <shaoge1994@163.com>

more unit test

d203047

Signed-off-by: guo-shaoge <shaoge1994@163.com>

guo-shaoge requested a review from gengliqi February 19, 2025 06:47

guo-shaoge added 4 commits February 19, 2025 15:10

fix

f27e192

Signed-off-by: guo-shaoge <shaoge1994@163.com>

refine

2218337

Signed-off-by: guo-shaoge <shaoge1994@163.com>

fmt

5396f00

Signed-off-by: guo-shaoge <shaoge1994@163.com>

fix

59925ee

Signed-off-by: guo-shaoge <shaoge1994@163.com>

gengliqi reviewed Feb 20, 2025

View reviewed changes

dbms/src/Columns/ColumnDecimal.cpp Show resolved Hide resolved

guo-shaoge requested a review from gengliqi February 20, 2025 04:09

gengliqi approved these changes Feb 20, 2025

View reviewed changes

ti-chi-bot bot added the lgtm label Feb 20, 2025

ti-chi-bot bot added approved and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Feb 20, 2025

Merge branch 'master' into batch_serialize_refine

aaebebe

ti-chi-bot bot merged commit ed8f828 into pingcap:master Feb 20, 2025
5 checks passed

guo-shaoge deleted the batch_serialize_refine branch February 21, 2025 02:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

batch serialize/deserialize interface support handling nullmap#9825

batch serialize/deserialize interface support handling nullmap#9825
ti-chi-bot[bot] merged 33 commits intopingcap:masterfrom
guo-shaoge:batch_serialize_refine

guo-shaoge commented Jan 24, 2025 •

edited

Loading

Uh oh!

gengliqi Feb 14, 2025

Uh oh!

Uh oh!

gengliqi left a comment

Uh oh!

guo-shaoge commented Feb 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xzhangxian1008 left a comment

Uh oh!

Uh oh!

gengliqi left a comment

Uh oh!

gengliqi Feb 20, 2025

Uh oh!

ti-chi-bot bot commented Feb 20, 2025

Uh oh!

ti-chi-bot bot commented Feb 20, 2025

Uh oh!

ti-chi-bot bot commented Feb 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

guo-shaoge commented Jan 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

gengliqi Feb 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gengliqi left a comment

Choose a reason for hiding this comment

Uh oh!

guo-shaoge commented Feb 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xzhangxian1008 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gengliqi left a comment

Choose a reason for hiding this comment

Uh oh!

gengliqi Feb 20, 2025

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Feb 20, 2025

Uh oh!

ti-chi-bot bot commented Feb 20, 2025

[LGTM Timeline notifier]

Uh oh!

ti-chi-bot bot commented Feb 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

guo-shaoge commented Jan 24, 2025 •

edited

Loading