Skip to content

Fix if, multiIf's nullable bug.#5238

Merged
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
janplus:if-nullable-bug
May 12, 2019
Merged

Fix if, multiIf's nullable bug.#5238
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
janplus:if-nullable-bug

Conversation

@janplus
Copy link
Contributor

@janplus janplus commented May 10, 2019

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

For changelog. Remove if this is non-significant change.

Category (leave one):

  • Improvement
  • Backward Incompatible Change

Short description (up to few sentences):
The if, multiIf functions should not rely on the condition's Nullable, but should only rely on the branches.

You may check the MySQL & Postgresql's results here:
http://sqlfiddle.com/#!9/25e4d/4
http://sqlfiddle.com/#!17/486b3/3

@alexey-milovidov alexey-milovidov added can be tested pr-improvement Pull request with some product improvements sql-compatibility labels May 11, 2019
@alexey-milovidov
Copy link
Member

That's great!

@alexey-milovidov alexey-milovidov merged commit 9ce2dd8 into ClickHouse:master May 12, 2019
@alexey-milovidov
Copy link
Member

Although it doesn't seem logical for me.

@janplus
Copy link
Contributor Author

janplus commented May 12, 2019

Hmm, the data analyst in my company reported this bug to me. I also double checked this logic, and found that MySQL, Postgresql, IBM DB2 all leverage the same logic, so I decide to behave the same way.

@janplus janplus deleted the if-nullable-bug branch May 12, 2019 14:02
@alexey-milovidov
Copy link
Member

Hmm, the data analyst in my company reported this bug to me. I also double checked this logic, and found that MySQL, Postgresql, IBM DB2 all leverage the same logic, so I decide to behave the same way.

This is the right decision, thank you!

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

Labels

pr-improvement Pull request with some product improvements sql-compatibility

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants