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

Add support for macaddr, inet, and cidr types to PostgreSQL adapter #6192

Merged
merged 3 commits into from May 16, 2012
Merged

Add support for macaddr, inet, and cidr types to PostgreSQL adapter #6192

merged 3 commits into from May 16, 2012

Conversation

danmcclain
Copy link
Contributor

Adds migration and schema dump support for macaddr, inet and cidr column types in the PostgreSQL adapter.

Converst inet and cidr types to NetAddr::CIDR objects, instead of using IPAddr, since NetAddr::CIDR handles subnets better.

@@ -24,4 +24,5 @@ Gem::Specification.new do |s|
s.add_dependency('arel', '~> 3.0.2')

s.add_dependency('active_record_deprecated_finders', '0.0.1')
s.add_dependency('netaddr', '~> 1.5.0')
Copy link
Member

Choose a reason for hiding this comment

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

I think that we should not add netaddr gem as Active Record dependency because we will only use it with PostgreSQL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the one issue I thought the pull request would definitely have. I was unsure of how to make netaddr a PostgreSQL only dependency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am going to update this so that it will try to use NetAddr if the user has included it, if not, it will fall back to IPAddr.

Copy link
Member

Choose a reason for hiding this comment

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

I'll fix it. We should just do a gem netaddr in the pg adapter code.

@danmcclain
Copy link
Contributor Author

I have updated my implementation to remove NetAddr completely, after discovering how to extract the subnet mask from IPAddr

@danmcclain
Copy link
Contributor Author

ping @tenderlove

@jeremyf
Copy link
Contributor

jeremyf commented May 9, 2012

The tests all pass. I'm not familiar with the adapters, but am wondering if there is a way to push the Gemfile requirements onto the Postgres adapter gem (I'm assuming there's such a thing as Postgres adapter gem).

@tenderlove can you review?

@rafaelfranca
Copy link
Member

@danmcclain looks great now.

@jeremyf the only way to push requirements to the PostgreSQL adapter is adding as requirement of the pg gem or other PostgreSQL gem.

@bcardarella
Copy link
Contributor

this looks pretty good, 👍 on merge

tenderlove added a commit that referenced this pull request May 16, 2012
…ostgresql_adapter

Add support for macaddr, inet, and cidr types to PostgreSQL adapter
@tenderlove tenderlove merged commit 835df6f into rails:master May 16, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants