Skip to content

Conversation

@jonasrichard
Copy link
Contributor

PR Info

New Features

  • Provide a new type RangeType to implement Postgres range type
  • Provide range type as an element type of Postgres Array

Doubts

  • Not sure where to implement hashable-value features. I put that in the with_range.rs
  • It implements time ranges with Chrono and Time I left out Jiff now
  • Not sure how it plays together with with-json
  • How to make RangeType from std Rust types (tuple, std range, how to describe exclusive and inclusive bounds?)
  • A refactor PR is going on which may affect this branch (Refactor Value::Array and add Value::Enum #1004)

@jonasrichard jonasrichard marked this pull request as draft November 4, 2025 13:53
@jonasrichard jonasrichard marked this pull request as ready for review November 4, 2025 14:37
@jonasrichard
Copy link
Contributor Author

jonasrichard commented Nov 4, 2025

The current dilemma is that I need to implement ToSql but in with_range.rs in src, because I think I cannot implement that in sea-query-postgres trait.

@tyt2y3
Copy link
Member

tyt2y3 commented Nov 4, 2025

i see, shall we move that to a separate type crate? something like sea-query-types / sea-query-postgres-types?

@jonasrichard
Copy link
Contributor Author

I will create a create for postgres only now, so the 2nd. And addressing the sea-query-sqlx compilation error.

@tyt2y3
Copy link
Member

tyt2y3 commented Nov 5, 2025

sounds good, thank you! I will take a closer look

@jonasrichard
Copy link
Contributor Author

@tyt2y3 I started to move Range to the sea-query-postgres-types crate but I am struggling a bit.

  • I cannot reach this crate from the root crate sea-query. I think I would like to re-export these two types from sea-query-postgres crate and that crate is enough to be visible from sea-query.
  • Another one, I tried to put a serde feature - because it is needed by the type definition - in the Cargo.toml of sea-query-postgres-types and say: use the same features as the root crate. Hope it is correct.
  • I included serde in types crate as well, I feel that it is a duplication to say the serde version twice (this and in the root crate).

If you can tell me what to change to fix the 1st point I can go on.

@tyt2y3
Copy link
Member

tyt2y3 commented Nov 5, 2025

thank you. the problem is sea-query-postgres depends on sea-query so it won't work due to circular dependency.
the crate dependency graph should be:

  1. sea-query-postgres-types: defines Range and implement ToSql for it (behind a feature flag driver-postgres)
  2. sea-query-postgres: depends on sea-query-postgres-types and turn on the feature flag
  3. sea-query: depends on sea-query-postgres-types but not turn on the feature flag

dont worry if you can't get CI to pass 100%, I will be able to fix that

I included serde in types crate as well, I feel that it is a duplication to say the serde version twice (this and in the root crate).

this is un-avoidable so I'll set one of them to a very loose requirement, i.e. version = "1"

@jonasrichard
Copy link
Contributor Author

jonasrichard commented Nov 6, 2025

The next obstactle is how to implement ToSql. It seems that sqlx::PgRange<T> implements Encode rather, and all the constituent types (like i32 range, i64 range, etc) needs to implement Encode.

https://docs.rs/sqlx/latest/sqlx/postgres/types/struct.PgRange.html#impl-Encode%3C'q,+Postgres%3E-for-PgRange%3CT%3E

I don't think this goes to a good direction, though.

The other option is to implement the ToSql trait with using postgres-protocol, which used by postgres-types.

https://docs.rs/postgres-protocol/latest/postgres_protocol/types/enum.Range.html

Although I need to figure out how to construct that value for the sake of conversion. Because it has a lifetime and I don't want to have a RangeType with lifetimes.

Also I am leaning toward the generic RangeType implementation, and during ToSql conversion we can restrict the types we accept. So we will have 6 different impl block like impl ToSql for RangeType<i32> and so on.

@jonasrichard
Copy link
Contributor Author

Finally I implemented by postgres-protocol so we don't need to sqlx type.

@jonasrichard
Copy link
Contributor Author

Okay, I think I am ready with that. I managed to run almost all checks with build-tools/rustclippy.sh

@jonasrichard
Copy link
Contributor Author

The next obstactle - quite a big one - is to implement sqlx::Encode with RangeType.

The problem is that in sea-query-sqlx we either make a PgRange conversion, which is not possible since our Value type is not generic. Or we need to implement Encode trait by hand in sea-query-postgres-types which is not easy since all the supporting types are private in sqlx.

https://docs.rs/sqlx-postgres/0.8.6/src/sqlx_postgres/types/range.rs.html#291-293

I think this is the last step since other cases I feel are covered.

@tyt2y3
Copy link
Member

tyt2y3 commented Nov 7, 2025

thank you for the great work so far! for you personal use case, do you use postgres or sqlx?

@jonasrichard
Copy link
Contributor Author

I started to use this for a postgres database for a proof of concept, but to be honest I just wanted to contribute to know better Rust.

Copy link
Member

@tyt2y3 tyt2y3 left a comment

Choose a reason for hiding this comment

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

thank you! if this works for you let's get this merged for now.
can create a followup PR for SQLx.

I think the strategy is to convert our own RangeType to sqlx's Range on the fly and call bind()

@tyt2y3 tyt2y3 merged commit 4560e93 into SeaQL:master Nov 8, 2025
26 checks passed
@tyt2y3
Copy link
Member

tyt2y3 commented Nov 8, 2025

I've pushed some commits to adjust the crate structure

@jonasrichard jonasrichard deleted the add-postgres-range-type branch November 9, 2025 19:59
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.

Question: support for range types

2 participants