Skip to content

Fix: Enum8 and AggregateFunction dropped#25

Merged
alex-clickhouse merged 3 commits into
ClickHouse:mainfrom
Felixzed:feat/enum8-aggregatefunction-support
May 26, 2026
Merged

Fix: Enum8 and AggregateFunction dropped#25
alex-clickhouse merged 3 commits into
ClickHouse:mainfrom
Felixzed:feat/enum8-aggregatefunction-support

Conversation

@Felixzed
Copy link
Copy Markdown
Contributor

@Felixzed Felixzed commented May 20, 2026

See: #24
This was made with the help of an LLM
Previously, it seems like type mapping of Enum8 and AggregateFunction got stuck on some code that unwrapped LowCardinality() and Nullable() and defaulted to a String type.

@Felixzed Felixzed force-pushed the feat/enum8-aggregatefunction-support branch from e28cb7b to af1c214 Compare May 20, 2026 16:49
@Felixzed Felixzed marked this pull request as ready for review May 20, 2026 16:53
@alex-clickhouse
Copy link
Copy Markdown
Collaborator

alex-clickhouse commented May 21, 2026

Hi, thanks for the PR!

Ok, I think the right thing to do is to change the failing tests actually. The behavior is correct.

Also, please add a note in CHANGELOG and RELEASENOTES.

We should also have some more comprehensive testing to catch this sort of thing earlier, will add that in another PR.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

❌ Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...se/Storage/Internal/ClickHouseTypeMappingSource.cs 81.81% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@alex-clickhouse
Copy link
Copy Markdown
Collaborator

Thank you for the contribution!

@alex-clickhouse alex-clickhouse merged commit c5947b3 into ClickHouse:main May 26, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants