Skip to content

Conversation

vassilevsky
Copy link
Contributor

We have columns of type point in our PostgreSQL database:

#<ActiveRecord::ConnectionAdapters::PostgreSQLColumn:0x007ffd7b829aa8 @coder=nil, @default=nil, @limit=nil, @name="point", @null=false, @precision=nil, @primary=false, @scale=nil, @sql_type="point", @type=:string> 

http://www.postgresql.org/docs/9.4/interactive/datatype-geometric.html

They are annotated as string in our models. This is misleading: point is actually 2 floats; not even close to a string.

I thought that SQL type should take precedence.

I understand that this change may be not enough or is completely wrong, and would welcome suggestions on the best solution.

@ctran
Copy link
Owner

ctran commented Mar 2, 2015

A small change but with big impact. I'm not sure how other would like this.

@vassilevsky
Copy link
Contributor Author

I think that people will like more correct annotations.

@alexch
Copy link
Collaborator

alexch commented Mar 17, 2015 via email

@ctran
Copy link
Owner

ctran commented Apr 10, 2015

What I meant is I don't know much this will change over the existing annotations. @alexch's suggestion make sense if you don't mind making that change.

@vassilevsky
Copy link
Contributor Author

I tested it on Rails 4.2.1 now.

rails new maps -d postgresql -T cd maps rails g model marker name location:point
class CreateMarkers < ActiveRecord::Migration def change create_table :markers do |t| t.string :name t.point :location t.timestamps null: false end end end
rake db:create db:migrate pgcli maps_development 
maps_development> \d markers +------------+-----------------------------+------------------------------------------------------+ | Column | Type | Modifiers | |------------+-----------------------------+------------------------------------------------------| | id | integer | not null default nextval('markers_id_seq'::regclass) | | name | character varying | | | location | point | | | created_at | timestamp without time zone | not null | | updated_at | timestamp without time zone | not null | +------------+-----------------------------+------------------------------------------------------+ 
# ... ActiveRecord::Schema.define(version: 20150412093810) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" create_table "markers", force: :cascade do |t| t.string "name" t.point "location" t.datetime "created_at", null: false t.datetime "updated_at", null: false end end

I think that the proposed change will not cause much trouble in people's model annotations. First of all, not many people use those exotic types :) And if they do, and their strings change to points, then, after a brief wtf moment, they should feel better informed and therefore happy. Right?

ctran added a commit that referenced this pull request Apr 13, 2015
Prefer SQL column type over normalized AR type
@ctran ctran merged commit 8aa0be7 into ctran:develop Apr 13, 2015
@ctran ctran self-assigned this Apr 13, 2015
@ctran ctran added this to the v2.6.9 milestone Apr 13, 2015
@ctran ctran added the feature label Apr 13, 2015
@vassilevsky
Copy link
Contributor Author

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 participants