budu / lobos

A library to create and manipulate SQL database schemas with migrations support.
http://budu.github.com/lobos/
267 stars 56 forks source link

Alter table error with Postgres stable 9.3.5 #74

Open ricardojmendez opened 10 years ago

ricardojmendez commented 10 years ago

Running a test on posgres 9.3.5 using:

:dependencies [[org.clojure/clojure "1.6.0"]
                 [postgresql "9.3-1102.jdbc41"]
                 [lobos "1.0.0-beta3"] 
                ])

Created a table with:

user=> (create
  #_=>  (table :customers
  #_=> (integer :id :primary-key)
  #_=> (varchar :firstname 75)))

Then tried to modify the column:

user=> (alter :modify (table :customers (varchar :firstname 50)))

But that raises a Postgres error. It seems lobos is sending:

ALTER TABLE "customers" ALTER COLUMN "firstname" VARCHAR(50)

when it should be sending

ALTER TABLE "customers" ALTER COLUMN "firstname" TYPE VARCHAR(50)
jonabbey commented 10 years ago

ricardo, I've hacked together what may be a working fix for this. Could you try https://github.com/budu/lobos/pull/75 to see if it fixes your case?

ricardojmendez commented 10 years ago

Hi Jonathan,

It doesn't. From an initial inspection it seems that what's passed to the compile function you modified on postgresql.clj is the DataTypeClause contained in the :data-type section of the map created on compiler.clj.

(Just getting familiar with the lobos codebase)

ricardojmendez commented 10 years ago

For what it's worth, there are tests for alter table in postgresql and they are currently failing:

ERROR in (test-alter-table-postgresql) (QueryExecutorImpl.java:2198) Uncaught exception, not in assertion. expected: nil actual: org.postgresql.util.PSQLException: ERROR: syntax error at or near "VARCHAR"

jonabbey commented 10 years ago

I know my initial pull request was faulty. Did you test the revised version?

Both versions of the pull request were passing the Travis CI build and test, but the CI server isn't testing the postgres driver..

jonabbey commented 10 years ago

"It doesn't. From an initial inspection it seems that what's passed to the compile function you modified on postgresql.clj is the DataTypeClause contained in the :data-type section of the map created on compiler.clj."

Yeah, that was my initial pull request. The fixed version modifies the method on [:postgreslql DataTypeClause] to add the "TYPE" keyword after the identifier and before the data type.

ricardojmendez commented 10 years ago

I don't believe I saw the second one... I'll try it later today. Cheers!

jonabbey commented 9 years ago

Any luck?

ricardojmendez commented 9 years ago

Hi Jonathan,

No time to test yet - busy on an unrelated client project - but I'll give it a shot in a few days and report back.

ricardojmendez commented 9 years ago

Confirmed, #75 fixes it.

jonabbey commented 9 years ago

Good to hear. It really is an ad-hoc patch, though. I think lobos needs to be modified so that it can pass a more general context map down to the lowest level emitters in the compilation blocks.

Really, it looks like lobos could use quite a bit of elaboration to support lots more kinds of table and column alterations.