Skip to content
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

Fix bad cast from LowCardinality column when using short circuit function execution #43311

Merged
merged 5 commits into from
Nov 30, 2022
Merged

Fix bad cast from LowCardinality column when using short circuit function execution #43311

merged 5 commits into from
Nov 30, 2022

Conversation

Avogar
Copy link
Member

@Avogar Avogar commented Nov 16, 2022

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in official stable or prestable release)

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

Fix bad cast from LowCardinality column when using short circuit function execution. Proper fix of #42937

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/

@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-bugfix Pull request with bugfix, not backported by default label Nov 16, 2022
@Avogar
Copy link
Member Author

Avogar commented Nov 16, 2022

After this change ColumnFunction became more complex when it's used as a short-circuit function argument. I think we can refactor it in the future and introduce a new column for this purpose that will share some interface with ColumnFunction.

@Avogar Avogar mentioned this pull request Nov 16, 2022
Closed
devcrafter
devcrafter approved these changes Nov 21, 2022
View reviewed changes
Copy link
Member

@devcrafter devcrafter left a comment

Choose a reason for hiding this comment

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

LGTM with minor comment

tests/queries/0_stateless/02481_low_cardinality_with_short_circuit_functins.sql Show resolved Hide resolved
@Avogar Avogar merged commit 3a0cf5b into ClickHouse:master Nov 30, 2022
@den-crane den-crane mentioned this pull request May 18, 2023
Closed
@den-crane
Copy link
Contributor

@Avogar can it be backported to 22.8?
#46128

@Avogar
Copy link
Member Author

Avogar commented May 19, 2023

#50017

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

@devcrafter devcrafter devcrafter approved these changes

Assignees

@devcrafter devcrafter

Labels
pr-bugfix Pull request with bugfix, not backported by default
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Avogar @den-crane @devcrafter @robot-ch-test-poll2