Skip to content

feat: add interval type support for RowBinaryWithNamesAndTypes - #277

Merged
slvrtrn merged 2 commits into
ClickHouse:mainfrom
timvermeulen:rbwnat-intervals
Aug 25, 2025
Merged

feat: add interval type support for RowBinaryWithNamesAndTypes#277
slvrtrn merged 2 commits into
ClickHouse:mainfrom
timvermeulen:rbwnat-intervals

Conversation

@timvermeulen

Copy link
Copy Markdown
Contributor

Summary

Adds Interval type support for RowBinaryWithNamesAndTypes.


I added conversions to/from SerdeType::I64 because that seems to be how ClickHouse stores interval values, though https://clickhouse.com/docs/sql-reference/data-types/special-data-types/interval says "Time interval as an unsigned integer value.", so let me know if SerdeType::I64 is the appropriate type here.

@mshustov
mshustov requested a review from slvrtrn August 25, 2025 08:45
@slvrtrn

slvrtrn commented Aug 25, 2025

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

I added conversions to/from SerdeType::I64 because that seems to be how ClickHouse stores interval values

yeah, seems to be a correct assumption:

curl -XPOST "http://localhost:8123?default_format=RowBinaryWithNamesAndTypes" \
  --data-binary "SELECT toIntervalYear(-42) AS y" > out.bin

yields

image

Since you are changing validation too, could you add more integration tests for that? See https://github.com/ClickHouse/clickhouse-rs/blob/main/tests/it/rbwnat.rs on the current main branch.

I think just a few SELECT queries with min int64, zero, max int64 values for all interval units will be enough.

NB: it is possible to run something like

CREATE TABLE foobar
(
    `i` IntervalYear
)
ENGINE = Memory

which is described as

DESCRIBE TABLE foobar
   ┌─name─┬─type─────────┬─default_type─┬─default_expression─┬─comment─┬─codec_expression─┬─ttl_expression─┐
1. │ i    │ IntervalYear │              │                    │         │                  │                │
   └──────┴──────────────┴──────────────┴────────────────────┴─────────┴──────────────────┴────────────────┘

so we'll need to add integration tests for insert after #244, too

@slvrtrn

slvrtrn commented Aug 25, 2025

Copy link
Copy Markdown
Contributor

Clippy issues are fixed on main.

@timvermeulen

Copy link
Copy Markdown
Contributor Author

Thanks for the pointers! I've added an integration test.

@slvrtrn
slvrtrn merged commit 44525e4 into ClickHouse:main Aug 25, 2025
13 of 14 checks passed
@timvermeulen
timvermeulen deleted the rbwnat-intervals branch August 25, 2025 16:30
@slvrtrn slvrtrn added this to the 0.14.0 milestone Sep 22, 2025
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