Skip to content

Fix crash by using alignedAlloc rather than alloc #10554

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
ti-chi-bot merged 1 commit into from
Nov 28, 2025
Merged

Fix crash by using alignedAlloc rather than alloc #10554

ti-chi-bot merged 1 commit into from
Nov 28, 2025
+14 −107

Conversation

@ChangRui-Ryan
Copy link
Contributor

@ChangRui-Ryan ChangRui-Ryan commented Nov 17, 2025 •
edited
Loading

What problem does this PR solve?

Issue Number: close #10553

Problem Summary:

What is changed and how it works?

need to use alignedAlloc for Complex Type

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

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

None

@ti-chi-bot ti-chi-bot bot added do-not-merge/needs-triage-completed release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed do-not-merge/needs-triage-completed labels Nov 17, 2025
windtalker
windtalker reviewed Nov 26, 2025
View reviewed changes
dbms/src/AggregateFunctions/AggregateFunctionGroupUniqArray.h Outdated

void deserialize(AggregateDataPtr __restrict place, ReadBuffer & buf, Arena * arena) const override
{
LOG_INFO(Logger::get(), "Deserialize groupUniqArray with PLACE {}", static_cast<void *>(place));
Copy link
Contributor

@windtalker windtalker Nov 26, 2025

Choose a reason for hiding this comment

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

maybe use debug level?

Copy link
Contributor Author

@ChangRui-Ryan ChangRui-Ryan Nov 27, 2025

Choose a reason for hiding this comment

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

I forgot to delete this line of code after finishing debugging – it can be deleted.

dbms/src/Interpreters/AggregationCommon.h
return key;
}


Copy link
Contributor

@windtalker windtalker Nov 26, 2025

Choose a reason for hiding this comment

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

these are deleted because they are not used?

Copy link
Contributor Author

@ChangRui-Ryan ChangRui-Ryan Nov 27, 2025

Choose a reason for hiding this comment

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

Yes, initially I noticed that the alloc method was used here and wanted to analyze whether it should be replaced with alignedAlloc, but found that it wasn’t being called at all—so I simply deleted the entire section.

windtalker
windtalker approved these changes Nov 27, 2025
View reviewed changes
Copy link
Contributor

@windtalker windtalker left a comment

Choose a reason for hiding this comment

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

lgtm

@ti-chi-bot ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Nov 27, 2025
@ti-chi-bot ti-chi-bot bot added the lgtm label Nov 28, 2025
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Nov 28, 2025

[APPROVALNOTIFIER] This PR is APPROVED

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

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,windtalker]

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 ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Nov 28, 2025
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Nov 28, 2025

[LGTM Timeline notifier]

Timeline:

  • 2025-11-27 09:04:38.831481124 +0000 UTC m=+780042.480675581: ☑️ agreed by windtalker.
  • 2025-11-28 08:05:53.910721768 +0000 UTC m=+862917.559916235: ☑️ agreed by gengliqi.

@ti-chi-bot ti-chi-bot bot merged commit e2fae1d into pingcap:master Nov 28, 2025
7 checks passed
@ti-chi-bot ti-chi-bot bot added the needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. label Dec 3, 2025
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Dec 3, 2025

In response to a cherrypick label: new pull request created to branch release-8.5: #10585.

@ti-chi-bot ti-chi-bot mentioned this pull request Dec 3, 2025
Merged
12 tasks
@ti-chi-bot ti-chi-bot bot added needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. labels Dec 3, 2025
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Dec 3, 2025

In response to a cherrypick label: new pull request created to branch release-8.1: #10587.

@ti-chi-bot ti-chi-bot mentioned this pull request Dec 3, 2025
Open
12 tasks
ti-chi-bot pushed a commit to ti-chi-bot/tiflash that referenced this pull request Dec 3, 2025
@ChangRui-Ryan @ti-chi-bot
This is an automated cherry-pick of pingcap#10554
b008d17 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Dec 3, 2025

In response to a cherrypick label: new pull request created to branch release-7.5: #10588.
But this PR has conflicts, please resolve them!

@ti-chi-bot ti-chi-bot mentioned this pull request Dec 3, 2025
Open
12 tasks
@JaySon-Huang
Copy link
Contributor

JaySon-Huang commented Dec 4, 2025

/cherry-pick release-nextgen-20251011

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Dec 4, 2025

@JaySon-Huang: new pull request created to branch release-nextgen-20251011: #10594.

Details

In response to this:

/cherry-pick release-nextgen-20251011

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.

@ti-chi-bot ti-chi-bot mentioned this pull request Dec 4, 2025
Merged
12 tasks
ti-chi-bot bot pushed a commit that referenced this pull request Dec 4, 2025
@ti-chi-bot @ChangRui-Ryan @JaySon-Huang
Fix crash by using alignedAlloc rather than alloc (#10554) (#10594)
162e7c2 
close #10553

need to use alignedAlloc for Complex Type

Signed-off-by: JaySon-Huang <tshent@qq.com>

Co-authored-by: ChangRui-Ryan <changrui82@gmail.com>
Co-authored-by: JaySon-Huang <tshent@qq.com>
ti-chi-bot bot pushed a commit that referenced this pull request Dec 5, 2025
@ti-chi-bot @ChangRui-Ryan
Fix crash by using alignedAlloc rather than alloc (#10554) (#10585)
e3fe665 
close #10553

need to use alignedAlloc for Complex Type

Co-authored-by: ChangRui-Ryan <changrui82@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@windtalker windtalker windtalker approved these changes

@gengliqi gengliqi gengliqi approved these changes

Assignees

No one assigned

Labels

approved lgtm needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

group_concat with spill may crash

5 participants

@ChangRui-Ryan @ti-chi-bot @JaySon-Huang @windtalker @gengliqi