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

Maintain column order from migration, even with 10+ columns #54

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hotwoofy
Copy link

Currently, up to 9 columns, the correct order is maintained in the table definition (first listing below). However, upon hitting 10 columns, the order is lost (second listing). I've forced the column map to now be ordered, and now any number of columns can be added without losing the order (third listing). I've done this by taking a dependency on ordered, I hope that's acceptable.

The driver for this was ClojureQL's implementation of JDBC's .getGeneratedKeys, which expects a primary key column to be the first that it finds when inserting.

user> (pprint (table :test
                     (integer :col1)
                     (integer :col2)
                     (integer :col3)
                     (integer :col4)
                     (integer :col5)
                     (integer :col6)
                     (integer :col7)
                     (integer :col7)
                     (integer :col8)
                     (integer :col9)))
{:name :test,
 :columns
 {:col1
  {:cname :col1,
   :data-type {:name :integer, :args [], :options {}},
   :default nil,
   :auto-inc nil,
   :not-null nil,
   :others ()},
  .
  .
  .
  {:cname :col9,
   :data-type {:name :integer, :args [], :options {}},
   :default nil,
   :auto-inc nil,
   :not-null nil,
   :others ()}},
 :constraints {},
 :indexes {}}
user> (pprint (table :test
                     (integer :col1)
                     (integer :col2)
                     (integer :col3)
                     (integer :col4)
                     (integer :col5)
                     (integer :col6)
                     (integer :col7)
                     (integer :col7)
                     (integer :col8)
                     (integer :col9)
                     (integer :col10)))
{:name :test,
 :columns
 {:col6
  {:cname :col6,
   :data-type {:name :integer, :args [], :options {}},
   :default nil,
   :auto-inc nil,
   :not-null nil,
   :others ()},
  :col7
  {:cname :col7,
   :data-type {:name :integer, :args [], :options {}},
   :default nil,
   :auto-inc nil,
   :not-null nil,
   :others ()},
  :col4
  {:cname :col4,
   :data-type {:name :integer, :args [], :options {}},
   :default nil,
   :auto-inc nil,
   :not-null nil,
   :others ()},
  :col5
  .
  .
  .
user> (pprint (table :test
                     (integer :col1)
                     (integer :col2)
                     (integer :col3)
                     (integer :col4)
                     (integer :col5)
                     (integer :col6)
                     (integer :col7)
                     (integer :col7)
                     (integer :col8)
                     (integer :col9)
                     (integer :col10)))
{:name :test,
 :columns
 {:col1
  {:cname :col1,
   :data-type {:name :integer, :args [], :options {}},
   :default nil,
   :auto-inc nil,
   :not-null nil,
   :others ()},
  :col2
  .
  .
  .
  {:cname :col10,
   :data-type {:name :integer, :args [], :options {}},
   :default nil,
   :auto-inc nil,
   :not-null nil,
   :others ()}},
 :constraints {},
 :indexes {}}

@budu
Copy link
Owner

budu commented Dec 17, 2012

Looks good, I'll probably work a bit on Lobos during the holidays and merge it then. Thanks for your patience.

@jplaza
Copy link

jplaza commented Jan 18, 2013

@budu any update on this issue? I really like Lobos but I'm considering other options because of this issue :(

@hotwoofy
Copy link
Author

@jplaza one workaround in the meantime is to use my fork, the other is to write your migrations for large tables in two steps - first create it with just the ID, the alter it to append the rest of the columns.

@@ -689,15 +690,15 @@
[name & [columns constraints indexes]]
(name-required name "table")
(Table. name
(or columns {})
(or columns (ordered-map))

Choose a reason for hiding this comment

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

I think it's better to order the whole expression :

(ordered-map (or columns {}))

Beause there is other places in code where columns should be ordered. See defmethod analyze [::standard Table] in analyzer.clj

@@ -5,7 +5,8 @@
:license {:name "Eclipse Public License"}
:dependencies [[org.clojure/clojure "1.4.0"]
[org.clojure/java.jdbc "0.1.1"]
[org.clojure/tools.macro "0.1.1"]]
[org.clojure/tools.macro "0.1.1"]
[org.flatland/ordered "1.4.0"]]

Choose a reason for hiding this comment

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

1.5.4 now :)

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.

4 participants