
Dual primary key declaration in table statement for Postgresql migrations
Reported by Liseki | September 1st, 2010 @ 04:54 AM
In dm-do-adapter.rb:133 create_table_statement generates the
table statement by joining the property
schema statements for each property and adding a primary key
declaration.
However in postgres.rb:22 property_schema_statement for a serial property has 2 problems:
-
I'm not sure where :quote_column_name comes from, legacy? This results in a schema statement
with no property name, just "SERIAL PRIMARY KEY" -
If the :quote_column_name is replaced with quote_name(schema[:name]) we now have a correct
schema statement like "id SERIAL PRIMARY KEY". However this now results in a dual primary key
declaration when this is called from dm-do-adapter.rb:133 create_table_statement.
I fixed this by changing postgres.rb:24 from:
statement = "#{schema[:quote_column_name]} SERIAL PRIMARY KEY"
to:
statement = "#{schema[:quote_column_name] ||
quote_name(schema[:name])} SERIAL"
The primary key declaration is now left to create_table_statement in dm-do-adapter.
Comments and changes to this ticket
-
-
Martin Gamsjaeger (snusnu) October 29th, 2010 @ 03:53 PM
- State changed from new to unconfirmed
- Assigned user set to Martin Gamsjaeger (snusnu)
-
Martin Gamsjaeger (snusnu) November 4th, 2010 @ 05:16 AM
- State changed from unconfirmed to not-applicable
I had a look at this and came to the following conclusion:
lib/sql/postgres.rb
is never mixed into the adapter and thus actually doesn't overwritecreate_table_statement
. Instead it uses it's ownTableCreator#to_sql
to produce acreate table
statement. The adapters only get extended with automigration specific functionality (if you have a look at thecreate_table_statement
signature for example, you'll see that it expects a Model and properties). The functionality provided bylib/sql/postgres.rb
is only used for classic migrations, where theSQL
is to be built from migration objects and not readymade DataMapper objects.This means that patching
lib/sql/postgres.rb
like you suggested, wouldn't have the desired effect, since it actually wouldn't leave establishing thePRIMARY KEY
up to the adapter and thus leave us withcreate table
statements without any primary keys.I totally agree that this code should (and will) be unified, but this probably won't happen before any major rewrite of
dm-migrations
. Such an effort is in fact underway, but will probably take quite some more time. (If you're interested, have a look at do-schema)Did you actually run into any problems that your suggested patch solved? I'd be curious to hear about them!
Marking this not applicable for now. We can always reopen it.
-
Liseki November 8th, 2010 @ 10:32 AM
Thanks Martin. Let me try and recreate the scenario of the 'issue':
I had a couple of models and I was auto-migrating and auto-upgrading just fine. Then I added
some 'classical' migrations. In a particular instance it so happened that I had required migration_runner,
then my schema files, then I did DataMapper.auto_migrate! This was in a spec helper file for RSpec.
That is where the 'issue' showed up. Below is a sample stack trace and as you can see the error shows
dual primary key declaration. The patch solved the 'issue' but I later decided I didn't really need the migrations
and solved my problem without them. I am not deeply versed, yet, in the workings of DM but I guess part
of the problem was my then assumption that auto_migrate! would pick up on my schema files as well as
my models.~ ERROR: syntax error at or near "PRIMARY" LINE 1: CREATE TABLE "objectives" ( SERIAL PRIMARY KEY, "description... ^ (code: 16801924, sql state: 42601, query: CREATE TABLE "objectives" ( SERIAL PRIMARY KEY, "description" TEXT NOT NULL, "created_at" TIMESTAMP NOT NULL, "updated_at" TIMESTAMP NOT NULL, "plan_id" INTEGER NOT NULL, PRIMARY KEY("id")), uri: postgres://moneval_old_test)
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-migrations-1.0.0/lib/dm-migrations/adapters/dm-do-adapter.rb:92 :in
execute_non_query'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-migrations-1.0.0/lib/dm-migrations/adapters/dm-do-adapter.rb:92 :in
block (2 levels) in create_model_storage'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-migrations-1.0.0/lib/dm-migrations/adapters/dm-do-adapter.rb:90 :in
each'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-migrations-1.0.0/lib/dm-migrations/adapters/dm-do-adapter.rb:90 :in
block in create_model_storage'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-do-adapter-1.0.0/lib/dm-do-adapter/adapter.rb:260 :in
with_connection'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-migrations-1.0.0/lib/dm-migrations/adapters/dm-do-adapter.rb:85 :in
create_model_storage'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-migrations-1.0.0/lib/dm-migrations/adapters/dm-postgres-adapter.rb:23 :in
block in create_model_storage'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-migrations-1.0.0/lib/dm-migrations/adapters/dm-postgres-adapter.rb:58 :in
without_notices'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-migrations-1.0.0/lib/dm-migrations/adapters/dm-postgres-adapter.rb:23 :in
create_model_storage'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-migrations-1.0.0/lib/dm-migrations/auto_migration.rb:79 :in
create_model_storage'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-migrations-1.0.0/lib/dm-migrations/auto_migration.rb:175 :in
auto_migrate_up!'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-migrations-1.0.0/lib/dm-migrations/auto_migration.rb:130 :in
auto_migrate!'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-migrations-1.0.0/lib/dm-migrations/auto_migration.rb:45 :in
block in repository_execute'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-core-1.0.0/lib/dm-core/model/descendant_set.rb:33 :in
block in each'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-core-1.0.0/lib/dm-core/model/descendant_set.rb:33 :in
each'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-core-1.0.0/lib/dm-core/model/descendant_set.rb:33 :in
each'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-migrations-1.0.0/lib/dm-migrations/auto_migration.rb:44 :in
repository_execute'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-migrations-1.0.0/lib/dm-migrations/auto_migration.rb:22 :in
auto_migrate!'
/opt/local/lib/ruby1.9/gems/1.9.1/gems/dm-constraints-1.0.0/lib/dm-constraints/migrations.rb:11 :in
auto_migrate!'
Please Sign in or create a free account to add a new ticket.
With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »