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

Remove special handling for 'postgis' schema #114

Merged
merged 2 commits into from
May 13, 2014
Merged

Conversation

teeparham
Copy link
Member

Before merging this, I would like your feedback. Motivation: I don't think this adapter should encourage installing PostGIS into a non-standard schema.

To test this locally:

gem 'activerecord-postgis-adapter', github: 'rgeo/activerecord-postgis-adapter', branch: 'postgis_schema'

By default, the PostGIS extension installs PostGIS tables into the 'public' schema. There is no reason for this adapter to do anything special for a 'postgis' schema. PostGIS tables are excluded from the structure dump via ignore_tables, not via the schema.

The default postgres #structure_dump method handles multiple schemas via 'schema_search_path' in database.yml. If you want install PostGIS tables into a non-default schema, add that schema to your 'schema_search_path' like so:

database.yml:

schema_search_path: 'public, postgis'

https://github.com/rails/docrails/blob/master/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb

http://lists.osgeo.org/pipermail/postgis-users/2009-November/025165.html

Related: #73 and #97. This PR would make those obsolete.

By default, the PostGIS extension installs PostGIS tables into the
'public' schema. There is no reason for this adapter to do anything
special for a 'postgis' schema. PostGIS tables are excluded from the
structure dump via ignore_tables, not via the schema.

The default postgres #structure_dump method handles multiple schemas
via 'schema_search_path' in database.yml. If you want install PostGIS
tables into a non-default schema, add that schema to your
'schema_search_path' like so:

database.yml:
schema_search_path: 'public, postgis'

https://github.com/rails/docrails/blob/master/activerecord/lib/active_re
cord/tasks/postgresql_database_tasks.rb

http://lists.osgeo.org/pipermail/postgis-users/2009-November/025165.html
Remove tasks for non-standard schema creation and alteration.

The #setup_gis_grant_privileges and #setup_gis_schemas methods are not
necessary and possibly insecure.
@teeparham
Copy link
Member Author

@nathanvda @tompesman @nleo @fermion @benebrice @GUI @petergoldstein any thoughts on this?

@nathanvda
Copy link

Afaik I have always installed postgis in the public schema. Furthermore I think it is best to keep as close as possible to the standard postgres adapter as possible. So I am all for it 👍

@petergoldstein
Copy link
Contributor

@teeparham In general I agree with this approach - there should be no special handling for the 'postgis' schema.

We do have the postgis extension installed in a dedicated schema, but so long as the adapter supports that case through the schema_search_path, I'm fine with this change.

@dazuma
Copy link
Member

dazuma commented May 11, 2014

The original reason postgis was installed in a separate schema was to separate it from the user's schema for database schema dump purposes. If postgis was installed into public, its very long list of definitions would get added to the dumps generated by the rake tasks (db:schema:dump? can't remember exactly). Which then would cause confusion and inability to round-trip the schema dump/load process.

That said, it has been a while; this mechanism was defined before postgis could be installed as an extension, and I don't remember if I ever investigated how that change affected any of this. I certainly am not married to the current behavior, and it has caused its share of confusion. I'm in agreement with @petergoldstein: as long as there remains a way to install the extension in a separate dedicated schema, I'm fine with it not being the default.

@teeparham
Copy link
Member Author

Thanks for the feedback!

Pre-2.0 versions of PostGIS included tables and functions that would be exported. AFAIK, this has been fixed with 2.0, and all the extension information is simply exported as 'create extension postigs'.

I created a test rails app using this branch & there were no extra tables exported. I tried with the default rb schema format & with sql. Any extensions you have set up will be dumped, which is good. The PostGIS tables were not exported. So in the basic case, I think this works as desired.

config/database.yml:

development:
  adapter: postgis
  username: tee
  host: localhost

Gemfile:

source 'https://rubygems.org'
gem 'rails', '4.1.1'
gem 'pg'
gem 'activerecord-postgis-adapter', 
  github: 'rgeo/activerecord-postgis-adapter', 
  branch: 'postgis_schema'
rails g model X shape:geometry
rake db:migrate
cat db/schema.rb

config/application.rb (for sql schema):

config.active_record.schema_format = :sql
cat db/structure.sql

See http://gis.stackexchange.com/questions/74147/how-do-i-upgrade-from-postgis-2-0-to-2-1/74150#74150

@dazuma
Copy link
Member

dazuma commented May 12, 2014

@teeparham Great, thanks for checking on that! Sounds good to me.

@nleo
Copy link

nleo commented May 13, 2014

I'm ok with it. Heroku by default supports 2.1

teeparham added a commit that referenced this pull request May 13, 2014
Remove special handling for 'postgis' schema
@teeparham teeparham merged commit 8dcef8b into master May 13, 2014
@teeparham teeparham deleted the postgis_schema branch May 13, 2014 15:04
@fermion
Copy link

fermion commented May 13, 2014

A late 👍 here.

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.

6 participants