- Notifications
You must be signed in to change notification settings - Fork516
Add ToSql / FromSql for IpInet and IpCidr from cidr crate#877
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
postgres-types/src/cidr_02.rs Outdated
| fn from_sql(_: &Type, raw: &[u8]) -> Result<Self, Box<dyn Error + Sync + Send>> { | ||
| let inet = types::inet_from_sql(raw)?; | ||
| Ok(IpCidr::new(inet.addr(), inet.netmask()) | ||
| .expect("postgres cidr type has zeroed host portion")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why panic here instead of returning an error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
the type only accepts postgrescidr type which postgres guarantees to be a validcidr (i.e. mask length within the limits for the address type and zeroed host portion) which is the same as the requirements to create anIpCidr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If you assume the value passed to from_sql is always correct, then there's no need for any of these implementations to return an error. The library should not assume that the input is valid.
postgres-types/src/cidr_02.rs Outdated
| fn from_sql(_: &Type, raw: &[u8]) -> Result<Self, Box<dyn Error + Sync + Send>> { | ||
| let inet = types::inet_from_sql(raw)?; | ||
| Ok(IpInet::new(inet.addr(), inet.netmask()) | ||
| .expect("postgres enforces maximum length of netmask")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Similar, postgres guarantees that theinet type has a mask length corresponding to the address type (<= 32 for IPv4, <=128 for IPv6)
Thanks! |
No description provided.