-
Notifications
You must be signed in to change notification settings - Fork 35
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
Fix use of ipaddress for INET types with netmask using ip_interface #74
base: master
Are you sure you want to change the base?
Conversation
http://www.postgresql.org/docs/9.1/static/datatype-net-types.html#8.9.1; inet represents an address. I'm skeptical that the interface type is appropriate. I'm willing to commit a patch that provides a proper netmask for inet's so that text casts do not render a netmask along with the IP. I thought that was what the conversation about. |
inet represents an address which accepts values with nonzero bits to the right of the netmask. You would not try to map a signed int to a unsigned int, it would be obvious the types won't match. The changes proposed in #71 do no fix the actual problem. Assume you have imported inet data into your postgres db, inet with nonzero bits to the right of the mask, like 10.0.0.1/24, this data can be a route, the router with the netmask. |
I was going to dismiss this, but I see the inet_in allows a netmask to be specified... I don't remember that being the case before. |
Wow, glossed over the documentation and missed the whole optional subnet part(my mind has been far away from the driver lately =\ ). Looking at ipaddress docs, IPvXInterface seems appropriate. However, I should note that the data is still accessible with the current version of the driver, you just have to go through a text cast to get it "select '...'::inet::text". |
Can we merge then? |
Yes, I believe so. |
Any word on this? |
While I do not see why this is not getting merged, this workaround allows me to use the driver with INET types having a mask:
|
This patch addresses #71 and #66 and includes updated driver tests as requested in #71.