Skip to content

Add IReadValueConverter#293

Open
alex-clickhouse wants to merge 5 commits intomainfrom
read-value-converter
Open

Add IReadValueConverter#293
alex-clickhouse wants to merge 5 commits intomainfrom
read-value-converter

Conversation

@alex-clickhouse
Copy link
Copy Markdown
Collaborator

@alex-clickhouse alex-clickhouse commented Apr 17, 2026

Summary

  • Adds IReadValueConverter interface for same-type transformation of values returned by ClickHouseDataReader (e.g., DateTime.SpecifyKind for UTC, string trimming/normalization).
  • Configurable globally via ClickHouseClientSettings.ReadValueConverter or per-query via QueryOptions.ReadValueConverter.
  • Intercepts both GetValue() (boxed path) and GetFieldValue<T>() (generic path). Zero overhead when no converter is set.
  • Converter must preserve the CLR type because column metadata (GetFieldType, GetSchemaTable) is not rerouted through the converter and must stay consistent.

One important consideration for the future: the GetFieldValue path, despite being generic, currently still boxes every value, which has a big perf cost. We are planning to change that in the future by adding Read<T>() methods to all the Type classes. The current design should be compatible with that future change and avoid any additional perf penalties.

Performance

Benchmarked on 500K rows across Int32/DateTime/String with GetValue and GetFieldValue.

  • No converter: when it's null there's essentially zero overhead, no perf penalty.
  • Passthrough converter (converter is set but it just returns all values unchanged): slightly higher, but not significant (well within margin of error). Virtual dispatch cost is negligible.
  • Transforming DateTime converter: GetValue (boxed) allocation doubles only when the converter creates new value-type instances (another boxing); GetFieldValue avoids the extra allocation entirely because the JIT elides the box/unbox round-trip and almost matches baseline perf.

Users who don't set a converter pay nothing; users who do pay only for the conversion work itself.


Alternative Approach

Also considered going for per-column/per-type config instead, something like:

settings.ReadConverters
      .ForType<DateTime>(dt => DateTime.SpecifyKind(dt, DateTimeKind.Utc))
      .ForColumn<string>("email", s => s.Trim())
      .ForClickHouseType<string>("FixedString(5)", s => s.Trim())

This would have some benefits:

  • Looks a bit cleaner/more expressive.
  • Tiny perf benefit: only the columns that have a converter call it
  • Some more potential perf benefits depending on future zero-boxing read path implementation, but nothing substantial

Downside is that it would be quite a bit more complex to set up. And some new questions come up, like what if multiple converters match?

Going with the one-converter approach also doesn't stop us from also adding the per-column converter in the future. But that might be a bit much.

Thoughts?

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@alex-clickhouse alex-clickhouse marked this pull request as ready for review April 17, 2026 13:35
Copilot AI review requested due to automatic review settings April 17, 2026 13:35
Copy link
Copy Markdown
Contributor

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

Adds an IReadValueConverter hook to allow same-CLR-type transformations of values returned from ClickHouseDataReader, configurable globally via ClickHouseClientSettings and overridable per-query via QueryOptions.

Changes:

  • Introduces IReadValueConverter and threads it through ClickHouseClient.ExecuteReaderAsync / ADO.NET reader creation.
  • Applies conversion on GetValue(), GetFieldValue<T>(), and GetValues() while preserving existing metadata APIs.
  • Adds documentation (changelog/release notes), an advanced example, benchmarks, and NUnit coverage for the new behavior.

Reviewed changes

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

Show a summary per file
File Description
examples/README.md Adds the new advanced example to the examples index.
examples/Program.cs Runs the new ReadValueConverter example in the examples harness.
examples/Advanced/Advanced_013_ReadValueConverter.cs Demonstrates global/per-query converters and ADO.NET behavior.
RELEASENOTES.md Documents the new IReadValueConverter feature for the release.
CHANGELOG.md Adds the new feature entry to the changelog.
ClickHouse.Driver/QueryOptions.cs Adds QueryOptions.ReadValueConverter for per-query override.
ClickHouse.Driver/ClickHouseClient.cs Passes the effective converter into reader creation.
ClickHouse.Driver/ADO/Readers/IReadValueConverter.cs Defines the new converter interface.
ClickHouse.Driver/ADO/Readers/ClickHouseDataReader.cs Applies the converter in GetValue, GetValues, and GetFieldValue<T> and captures raw type strings when needed.
ClickHouse.Driver/ADO/ClickHouseCommand.cs Passes the configured converter through the ADO.NET command path.
ClickHouse.Driver/ADO/ClickHouseClientSettings.cs Adds ClickHouseClientSettings.ReadValueConverter and includes it in copy/equality/hash.
ClickHouse.Driver.Tests/ADO/ReadValueConverterTests.cs Adds NUnit tests covering conversion behavior, overrides, and forwarded type strings.
ClickHouse.Driver.Benchmark/ReadValueBenchmark.cs Adds BenchmarkDotNet benchmarks measuring converter overhead.

Comment thread ClickHouse.Driver/ADO/Readers/ClickHouseDataReader.cs
@alex-clickhouse alex-clickhouse changed the title add read value converter Add IReadValueConverter Apr 17, 2026
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