Skip to content

Correct nullability checker for LowCardinality nested columns#14591

Merged
alexey-milovidov merged 4 commits intoClickHouse:masterfrom
myrrc:bugfix/lc-nullability-checker
Sep 9, 2020
Merged

Correct nullability checker for LowCardinality nested columns#14591
alexey-milovidov merged 4 commits intoClickHouse:masterfrom
myrrc:bugfix/lc-nullability-checker

Conversation

@myrrc
Copy link
Copy Markdown
Contributor

@myrrc myrrc commented Sep 8, 2020

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Bug Fix

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

Added the checker as neither calling lc->isNullable() nor calling ls->getDictionaryPtr()->isNullable() would return the correct result.

@robot-clickhouse robot-clickhouse added the pr-bugfix Pull request with bugfix, not backported by default label Sep 8, 2020
@myrrc
Copy link
Copy Markdown
Contributor Author

myrrc commented Sep 8, 2020

Related: #14590

@alexey-milovidov
Copy link
Copy Markdown
Member

Maybe add a test?

@alexey-milovidov alexey-milovidov self-assigned this Sep 9, 2020
@alexey-milovidov
Copy link
Copy Markdown
Member

It's 100% green. Let's add a specific test (to make sure it won't be introduced later) and merge after fast test check.

@alexey-milovidov alexey-milovidov merged commit c6d0944 into ClickHouse:master Sep 9, 2020
@myrrc myrrc deleted the bugfix/lc-nullability-checker branch September 9, 2020 16:25
robot-clickhouse pushed a commit that referenced this pull request Sep 9, 2020
robot-clickhouse pushed a commit that referenced this pull request Sep 9, 2020
alexey-milovidov added a commit that referenced this pull request Sep 10, 2020
Backport #14591 to 20.8: Correct nullability checker for LowCardinality nested columns
alexey-milovidov added a commit that referenced this pull request Sep 14, 2020
Backport #14591 to 20.9: Correct nullability checker for LowCardinality nested columns
@alexey-milovidov
Copy link
Copy Markdown
Member

The message for changelog is not actually useful.

@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented Nov 5, 2020

I write

Fix function `has` with `LowCardinality` of `Nullable`.

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

Labels

pr-bugfix Pull request with bugfix, not backported by default

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants