Skip to content

Remove strange code#40195

Merged
nickitat merged 10 commits intomasterfrom
remove-strange-code
Sep 12, 2022
Merged

Remove strange code#40195
nickitat merged 10 commits intomasterfrom
remove-strange-code

Conversation

@alexey-milovidov
Copy link
Copy Markdown
Member

@alexey-milovidov alexey-milovidov commented Aug 14, 2022

Changelog category (leave one):

  • Bug Fix

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Fix memory safety issues with functions encrypt and contingency if Array of Nullable is used as an argument. This fixes #41004.

Remove incorrect code introduced in #12550.

The code was incorrect because the value of getDataAt for the consecutive array elements should point to consecutive memory locations to enable the implementation of getDataAt for ColumnArray of simple values.

Note that the attempt to fix it in #40168 was also incorrect because the values of getDataAt should be different for empty strings and NULLs.

This error does not lead to any user-visible issues.

This partially reverts #12550.
This fixes #41004.

@robot-ch-test-poll robot-ch-test-poll added the pr-not-for-changelog This PR should not be mentioned in the changelog label Aug 14, 2022
@alexey-milovidov alexey-milovidov marked this pull request as ready for review September 12, 2022 04:20
@nickitat nickitat self-assigned this Sep 12, 2022
@alexey-milovidov
Copy link
Copy Markdown
Member Author

Wow, it's green!

@nickitat let's merge ASAP.

@alexey-milovidov alexey-milovidov added the pr-must-backport Pull request should be backported intentionally. Use this label with great care! label Sep 12, 2022
nickitat pushed a commit that referenced this pull request Sep 13, 2022
robot-clickhouse pushed a commit that referenced this pull request Sep 13, 2022
robot-clickhouse pushed a commit that referenced this pull request Sep 13, 2022
@robot-clickhouse robot-clickhouse added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Sep 13, 2022
nickitat pushed a commit that referenced this pull request Sep 15, 2022
nickitat pushed a commit that referenced this pull request Sep 15, 2022
alexey-milovidov added a commit that referenced this pull request Sep 18, 2022
@filimonov
Copy link
Copy Markdown
Contributor

This error does not lead to any user-visible issues.

Unfortunately that's not true. see #51541

@alexey-milovidov
Copy link
Copy Markdown
Member Author

We should not allow any incorrect (buggy, unsafe) code in our codebase.

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

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Buffer overflow in AggregateFunctionCrossTab

5 participants