Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Postgres network types (inet) #888

Open
wbew opened this issue Jul 18, 2022 · 14 comments · May be fixed by #2395
Open

Support Postgres network types (inet) #888

wbew opened this issue Jul 18, 2022 · 14 comments · May be fixed by #2395

Comments

@wbew
Copy link

wbew commented Jul 18, 2022

Motivation

I would like to use Postgres network types (i.e. inet) in my Model definitions. I understand there may be workarounds, so this is not urgent but would be a quality-of-life improvement.

Proposed Solutions

I believe this is nearly/already supported in sea-query: SeaQL/sea-query#187. From my understanding, supporting this in sea-orm requires updating the logic that converts a query result into an instance of a Model: perhaps here.

Additional Information

Currently, attempts to use inet will result in a runtime error.

@billy1624
Copy link
Member

Hey @001wwang, sorry for the delay. Network types are supported on SeaQuery, however, it's only supported by PostgreSQL but not by MySQL and SQLite.

So, the questions was do we want to bring the network types support to SeaORM for PostgreSQL only? Or, support it for MySQL and SQLite as well. We can simply serialize network types as string for MySQL and SQLite.

Thoughts?

CC @tyt2y3

@wbew
Copy link
Author

wbew commented Jul 25, 2022

No worries at all, this is not a blocker for us. We only use PostgreSQL so I'll hold off on any judgement for MySQL and SQLite. Thanks for the context.

@ikrivosheev
Copy link
Member

@billy1624 I think support network types only for PostgreSQL (for example SqlAlchemy, python ORM, support it only for PostgreSQL).

@ikrivosheev
Copy link
Member

@tyt2y3 @billy1624 if it's, I can prepare PR)

@tyt2y3
Copy link
Member

tyt2y3 commented Aug 7, 2022

Let me think about it

@billy1624 billy1624 moved this from Triage to Next Up in SeaQL Dev Tracker Aug 10, 2022
@billy1624
Copy link
Member

Any updates? @tyt2y3

@hut8
Copy link
Contributor

hut8 commented Nov 28, 2022

I would like this feature! Question about implementation: to what Rust type would inet map? std::net::IpAddr seems like a logical choice, but Postgres's inet type also can contain the subnet. So I guess we would have to just discard that, right? Is there a better type?

@Razican
Copy link

Razican commented Nov 28, 2022

One option would be ipnetwork, which is already supported by SQLx (and diesel), but it's not stable, and SQLx itself is not able keep the version up to date: launchbadge/sqlx#2148

Another option is ipnet, which has a stable API, many downloads and updates, and it's supported by diesel too, for example.

@billy1624 billy1624 moved this from Next Up to Triage in SeaQL Dev Tracker Jan 13, 2023
@holmofy
Copy link
Contributor

holmofy commented Aug 31, 2023

What is the current progress on this issue? I found that sqlx already provides inet type support for postgresql :https://github.com/launchbadge/sqlx/blob/main/sqlx-postgres/src/types/ipnetwork.rs

@tyt2y3 tyt2y3 removed the good first issue Good for newcomers label Sep 1, 2023
@tyt2y3
Copy link
Member

tyt2y3 commented Sep 1, 2023

The issue is still outstanding. No concrete plan for it yet.

@oovm
Copy link

oovm commented Sep 22, 2024

with-ipnetwork doesn't seem to exist in cargo.toml?

#[cfg(feature = "with-ipnetwork")]
"INET" | "CIDR" => Value::IpNetwork(Some(Box::new(
row.try_get(c.ordinal()).expect("Failed to get ip address"),
))),
#[cfg(feature = "with-ipnetwork")]
"INET[]" | "CIDR[]" => Value::Array(
sea_query::ArrayType::IpNetwork,
Some(Box::new(
row.try_get::<Vec<ipnetwork::IpNetwork>, _>(c.ordinal())
.expect("Failed to get ip address array")
.iter()
.map(|val| Value::IpNetwork(Some(Box::new(val.clone()))))
.collect(),
)),
),

@lpotthast
Copy link

lpotthast commented Sep 27, 2024

With roughly this set of dependencies and features

sea-orm = { version = "1.1.0-rc.1", features = [
    "debug-print",
    "runtime-tokio-rustls",
    "sqlx",
    "sqlx-postgres",
] }
sea-query = { version = "0.32.0-rc.1", features = ["with-ipnetwork"] }
sea-query-binder = { version = "0.7.0-rc.2", features = ["with-ipnetwork"] }

where, sea-query and sea-query-binder are only needed because sea-orm has no "with-ipnetwork" feature that would get passed onwards,
I am able to define a wrapper new-type

#[derive(Debug, Eq, Clone, PartialEq)]
pub struct IpNetworkWrapper(pub ipnetwork::IpNetwork);

and use it with DeriveEntityModel.
Inserting works fine, but parsing a DB response I cannot get to work.
How should a TryGetable implementation look like?

@tyt2y3
Copy link
Member

tyt2y3 commented Sep 28, 2024

@Leo1003 Leo1003 linked a pull request Oct 16, 2024 that will close this issue
5 tasks
@Leo1003
Copy link

Leo1003 commented Oct 16, 2024

I found it is hard to implement TryGetable without having access to the underlying row data in QueryResult.

Although sqlx::Decode is implemented for ipnetwork::IpNetwork, we cannot get an appropriate type from QueryResult to use it. I think it is hard to support new SQL type outside of sea_orm.

Therefore, I tried to implement it in sea_orm directly and submit a draft PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Triage
Development

Successfully merging a pull request may close this issue.

10 participants