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.

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

groupArray returns cannot be nullable #48593

Merged
merged 3 commits into from
Apr 14, 2023

Conversation

lgbo-ustc
Copy link
Contributor

@lgbo-ustc lgbo-ustc commented Apr 10, 2023

Changelog category (leave one):

  • Bug Fix

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

Fixed Nested type Array(T) cannot be inside Nullable type for groupArray function.

closed #48462

@robot-ch-test-poll4 robot-ch-test-poll4 added the pr-bugfix Pull request with bugfix, not backported by default label Apr 10, 2023
@nickitat nickitat self-assigned this Apr 10, 2023
@nickitat nickitat added the can be tested Allows running workflows for external contributors label Apr 10, 2023
@lgbo-ustc lgbo-ustc force-pushed the group_array_nullable branch 2 times, most recently from d9b900e to 572bc5b Compare April 12, 2023 08:11
@lgbo-ustc
Copy link
Contributor Author

lgbo-ustc commented Apr 13, 2023

change the result of groupArray(NULL) from null to []

SELECT groupArray(NULL)

┌─groupArray(NULL)─┐
│ []               │
└──────────────────┘

SELECT toTypeName(groupArray(NULL))

┌─toTypeName(groupArray(NULL))─┐
│ Array(Nullable(Nothing))     │
└──────────────────────────────┘

Copy link
Member

@nickitat nickitat left a comment

Choose a reason for hiding this comment

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

let's pray we didn't break backward compatibility

@robot-ch-test-poll3 robot-ch-test-poll3 merged commit ea5339e into ClickHouse:master Apr 14, 2023
138 checks passed
@azat
Copy link
Collaborator

azat commented May 17, 2023

This does break the binary format (updated test references shows that there are problems), so I'm suggesting to revert it for now - #49971 (yes one release already affected, but I don't see a way to fix this in a binary compatible way now, and I hope that not a lot people affected by this), and fix it without breaking compatibility (also I found more changes of the on disk format for aggregate functions over Nullable, that comes from #11593)

Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
can be tested Allows running workflows for external contributors pr-bugfix Pull request with bugfix, not backported by default
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should returns_default_when_only_null be true for groupArray
5 participants