Skip to content

Fix PartialSortingTransform by expanding column to full#9151

Closed
azat wants to merge 3 commits intoClickHouse:masterfrom
azat:PartialSortingTransform-full-column
Closed

Fix PartialSortingTransform by expanding column to full#9151
azat wants to merge 3 commits intoClickHouse:masterfrom
azat:PartialSortingTransform-full-column

Conversation

@azat
Copy link
Copy Markdown
Member

@azat azat commented Feb 16, 2020

Changelog category (leave one):

  • Bug Fix

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix PartialSortingTransform by expanding column to full

@azat azat requested a review from KochetovNicolai February 16, 2020 21:07
@alexey-milovidov
Copy link
Copy Markdown
Member

@KochetovNicolai I thought PartialSortingTransform should leave const columns as is. Otherwise we will have performance degradation. Also look at MSan test.

@KochetovNicolai
Copy link
Copy Markdown
Member

KochetovNicolai commented Feb 17, 2020

PartialSortingTransform should leave const columns as is

It's true. The problem should have been in pipeline itself.

@KochetovNicolai
Copy link
Copy Markdown
Member

Msan points to DB::StringEqualsImpl<false>::string_vector_string_vector. I think I've seen it before. Can't get the reason why it happened.

@qoega
Copy link
Copy Markdown
Member

qoega commented Feb 17, 2020

@KochetovNicolai have you tried to build with with CXXFLAGS='-ggdb -fno-omit-frame-pointer -fsanitize-memory-track-origins=2'?

@alexey-milovidov alexey-milovidov added the st-discussion When the implementation aspects are not clear or when the PR is on hold due to questions. label Feb 19, 2020
KochetovNicolai added a commit that referenced this pull request Feb 20, 2020
@alexey-milovidov
Copy link
Copy Markdown
Member

The issue was accidentially fixed in #8929

@azat azat deleted the PartialSortingTransform-full-column branch February 23, 2020 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

st-discussion When the implementation aspects are not clear or when the PR is on hold due to questions.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants