Skip to content

Fix ClickHouse datatype handling and update test cases for consistency#188

Merged
wyhaya merged 1 commit into
mainfrom
fix-clickhouse-datatype
Jun 3, 2026
Merged

Fix ClickHouse datatype handling and update test cases for consistency#188
wyhaya merged 1 commit into
mainfrom
fix-clickhouse-datatype

Conversation

@wyhaya
Copy link
Copy Markdown
Member

@wyhaya wyhaya commented Jun 3, 2026

No description provided.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts the ClickHouse connector to preserve ClickHouse’s native datatype strings (instead of lowercasing them) and updates decoding/tests to match, improving correctness and consistency between returned column metadata and value decoding.

Changes:

  • Preserve meta.type casing in QueryColumn.datatype (stop lowercasing ClickHouse types).
  • Update ClickHouse value decoding to match ClickHouse’s canonical type names (e.g., Int32, Nullable(UInt64)).
  • Update integration/unit tests to assert the preserved datatype strings and reuse a shared password constant.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src-crates/clickhouse/src/lib.rs Stops lowercasing returned column datatypes and updates integration tests to assert preserved type strings.
src-crates/clickhouse/src/decode.rs Updates decoding logic and unit tests to match ClickHouse’s canonical (cased) datatype names.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +533 to +534
assert_eq!(result.columns[0].datatype, "Int32");
assert_eq!(result.columns[1].datatype, "Nullable(UInt64)");
@wyhaya wyhaya merged commit 406b1b4 into main Jun 3, 2026
1 check passed
@wyhaya wyhaya deleted the fix-clickhouse-datatype branch June 3, 2026 08:38
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