search

1602 / jugglingdb

Multi-database ORM for nodejs: redis, mongodb, mysql, sqlite3, postgresql, arango, in-memory...
http://1602.github.io/jugglingdb/
2.04k stars 241 forks source link

Primary key has to be called 'id' #343

Closed fire015 closed 3 years ago

fire015 commented 10 years ago

If I want to use find, destory, save or exists methods jugglingdb seems to create an SQL query with primary key field name id and there doesn't seem to be a way to override this.

My table primary key is called user_id not id so none of these methods will work as the field does not exist.

Using the MySQL adapter.

kolypto commented 10 years ago

+1. A good ORM must never depend on any specific database structure

frontend-3 commented 10 years ago

+2 its horrible this feature.

androidyo commented 10 years ago

+3 the same problem

unibasil commented 10 years ago

+4 Spatial enabled tables and views in PostGIS (PostgreSQL extention) traditionally use gid as primary key field instead of id, so this nice ORM is useless for me now.

Primary key field name should be specified in settings parameter, the same way as tableName now.

yanickrochon commented 10 years ago

Yes. As with the index model property option, I would add primaryKey: true to define the model's primary key. Along with PR #312 , this problem could be solved.

reneolivo commented 10 years ago

even though I always use "id" instead of "user_id" it would be nice if the ORM would be flexible about field names.

unibasil commented 10 years ago

yanickrochon, the problem is every adapter should be rewritten to take advantage of primaryKey attribute. Now id field is just hard coded in adapter's queries.

yanickrochon commented 10 years ago

@unibasil perhaps, but it's not because something is already implemented that it's necessarily a good thing, don't you think?

The plan with this is to create an integration and a regression test plans about it, then close the issue when things are done to an acceptable level.

  1. The first step is to discuss how the issue can be addressed. I proposed the primaryKey model field attribute, and integrate PR #312 .
  2. When, and if, everyone agrees with this, the next step is to work on a separate branch and make the changes. Then all adapters should also branch off and comply in accordance with the new specs.
  3. The third step is to write regression tests and make sure everything is ok and is ready to merge with master. For any problem, go back at step 1.
  4. Finally, coordinate a merge with all the concerned branches across the projects.

It seems big, but it's not so much as the changes are not that big. Even for the adapters.

diogobeda commented 10 years ago

@unibasil I agree with that. At the moment i need the primaryKey feature and can help implement or write the tests for it. I'm used to postgres, so I could make the changes on that adapter.

anatoliychakkaev commented 10 years ago

I merged https://github.com/1602/jugglingdb/pull/312

On Thu, Jan 23, 2014 at 5:45 PM, Diogo Beda notifications@github.comwrote:

@unibasil https://github.com/unibasil I agree with that. At the moment i need the primaryKey feature and can help implement or write the tests for it. I'm used to postgres, so I could make the changes on that adapter.

— Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/issues/343#issuecomment-33148708 .

4ndroid commented 10 years ago

Hi guys! Any update about this issue? As I'm connecting to an old db, I have the same limitation, which makes jugglingdb almost irrelevant. Maybe a fork implementing this feature?