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 'limits' from columns #6

Open
jacobvosmaer opened this issue May 12, 2015 · 6 comments
Open

Remove 'limits' from columns #6

jacobvosmaer opened this issue May 12, 2015 · 6 comments

Comments

@jacobvosmaer
Copy link

After passing through the converter, you get a PG database with 'limits' in some places.

diff --git a/db/schema.rb b/db/schema.rb
index 04abf9b..9da616f 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -13,9 +13,6 @@

 ActiveRecord::Schema.define(version: 20150502064022) do

-  # These are extensions that must be enabled in order to support this database
-  enable_extension "plpgsql"
-
   create_table "application_settings", force: true do |t|
     t.integer  "default_projects_limit"
     t.boolean  "signup_enabled"
@@ -186,9 +183,9 @@ ActiveRecord::Schema.define(version: 20150502064022) do

   create_table "merge_request_diffs", force: true do |t|
     t.string   "state"
-    t.text     "st_commits"
-    t.text     "st_diffs"
-    t.integer  "merge_request_id", null: false
+    t.text     "st_commits",       limit: 2147483647
+    t.text     "st_diffs",         limit: 2147483647
+    t.integer  "merge_request_id",                    null: false
     t.datetime "created_at"
     t.datetime "updated_at"
   end
@@ -269,8 +266,8 @@ ActiveRecord::Schema.define(version: 20150502064022) do
     t.string   "line_code"
     t.string   "commit_id"
     t.integer  "noteable_id"
-    t.boolean  "system",        default: false, null: false
-    t.text     "st_diff"
+    t.boolean  "system",                           default: false, null: false
+    t.text     "st_diff",       limit: 2147483647
   end

   add_index "notes", ["author_id"], name: "index_notes_on_author_id", using: :btree
@@ -338,22 +335,22 @@ ActiveRecord::Schema.define(version: 20150502064022) do
     t.datetime "created_at"
     t.datetime "updated_at"
     t.integer  "creator_id"
-    t.boolean  "issues_enabled",         default: true,     null: false
-    t.boolean  "wall_enabled",           default: true,     null: false
-    t.boolean  "merge_requests_enabled", default: true,     null: false
-    t.boolean  "wiki_enabled",           default: true,     null: false
+    t.boolean  "issues_enabled",                    default: true,     null: false
+    t.boolean  "wall_enabled",                      default: true,     null: false
+    t.boolean  "merge_requests_enabled",            default: true,     null: false
+    t.boolean  "wiki_enabled",                      default: true,     null: false
     t.integer  "namespace_id"
-    t.string   "issues_tracker",         default: "gitlab", null: false
+    t.string   "issues_tracker",                    default: "gitlab", null: false
     t.string   "issues_tracker_id"
-    t.boolean  "snippets_enabled",       default: true,     null: false
+    t.boolean  "snippets_enabled",                  default: true,     null: false
     t.datetime "last_activity_at"
     t.string   "import_url"
-    t.integer  "visibility_level",       default: 0,        null: false
-    t.boolean  "archived",               default: false,    null: false
+    t.integer  "visibility_level",                  default: 0,        null: false
+    t.boolean  "archived",                          default: false,    null: false
     t.string   "avatar"
     t.string   "import_status"
-    t.float    "repository_size",        default: 0.0
-    t.integer  "star_count",             default: 0,        null: false
+    t.float    "repository_size",        limit: 24, default: 0.0
+    t.integer  "star_count",                        default: 0,        null: false
     t.string   "import_type"
     t.string   "import_source"
   end
@@ -395,15 +392,15 @@ ActiveRecord::Schema.define(version: 20150502064022) do

   create_table "snippets", force: true do |t|
     t.string   "title"
-    t.text     "content"
-    t.integer  "author_id",                    null: false
+    t.text     "content",          limit: 2147483647
+    t.integer  "author_id",                                       null: false
     t.integer  "project_id"
     t.datetime "created_at"
     t.datetime "updated_at"
     t.string   "file_name"
     t.datetime "expires_at"
     t.string   "type"
-    t.integer  "visibility_level", default: 0, null: false
+    t.integer  "visibility_level",                    default: 0, null: false
   end

   add_index "snippets", ["author_id"], name: "index_snippets_on_author_id", using: :btree

We should not have the limits. cc @DouweM @dosire

@DouweM
Copy link

DouweM commented May 12, 2015

Note that there are more differences than just those visible in schema.rb when you compare the outputs of \d snippets on original PG (snippets.table.psql) and converted PG (snippets.table.mysql):

diff --git a/Users/douwemaan/Desktop/snippets.table.psql b/Users/douwemaan/Desktop/snippets.table.mysql
index 0ca7fd6..8d77d01 100644
--- a/Users/douwemaan/Desktop/snippets.table.psql
+++ b/Users/douwemaan/Desktop/snippets.table.mysql
@@ -2,15 +2,15 @@
       Column      |            Type             |                       Modifiers                       
 ------------------+-----------------------------+-------------------------------------------------------
  id               | integer                     | not null default nextval('snippets_id_seq'::regclass)
- title            | character varying(255)      | 
+ title            | character varying(510)      | default NULL::character varying
  content          | text                        | 
  author_id        | integer                     | not null
  project_id       | integer                     | 
- created_at       | timestamp without time zone | 
- updated_at       | timestamp without time zone | 
- file_name        | character varying(255)      | 
- expires_at       | timestamp without time zone | 
- type             | character varying(255)      | 
+ created_at       | timestamp with time zone    | 
+ updated_at       | timestamp with time zone    | 
+ file_name        | character varying(510)      | default NULL::character varying
+ expires_at       | timestamp with time zone    | 
+ type             | character varying(510)      | default NULL::character varying
  visibility_level | integer                     | not null default 0
 Indexes:
     "snippets_pkey" PRIMARY KEY, btree (id)

@jacobvosmaer
Copy link
Author

Ughh... @DouweM could you create separate issues for the differences?

I think it would be really nice to stamp these out.

@DouweM
Copy link

DouweM commented May 12, 2015

mysql-postgresql-converter converts varchar(255) to character varying (510) on purpose, and I'm assuming it converts datetime to timestamp with time zone just to be on the safe side.

Oddly, the content type is set to text in both, but for some reason it ends up with limit: 2147483647 in the converted schema.rb. I don't see that in the table definition.

@DouweM
Copy link

DouweM commented May 12, 2015

@jacobvosmaer Yes, will do.

@jacobvosmaer
Copy link
Author

@DouweM the converter does all sorts of things on purpose; it is just that those things were put there by the lanyrd developers, not by us. :)

@DouweM
Copy link

DouweM commented May 12, 2015

@jacobvosmaer Yep. Unfortunately the reasoning behind these choices is completely undocumented. :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants