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

Fixed a bug that wrong sql generated when destroying in pg. #4027

Merged
merged 2 commits into from Aug 4, 2015

Conversation

3 participants
@smihica
Contributor

smihica commented Jun 29, 2015

Hi.

When I tried like following code, sequelize generated wrong SQL.

var Sequelize = require('sequelize');

var sequelize = new Sequelize('db', 'user', 'pass', { host: 'localhost', dialect: 'postgres' });

var User = sequelize.define('test_user', {
  id: {
    type:       Sequelize.INTEGER,
    primaryKey: true,
    field:      "test_user_id",
  }
}, { freezeTableName: true, timestamps:false });

User.create({id: 1}).then(function(u) {
  console.log(u.id);
  u.destroy().then(function(){
    console.log('delete success !!');
  }); // error
});

/* Log ---
Executing (default): INSERT INTO "test_user" ("test_user_id") VALUES (1) RETURNING *;
1
Executing (default): DELETE FROM "test_user" WHERE "id" IN (SELECT "id" FROM "test_user" WHERE "test_user_id" = 1 LIMIT 1) // *** WRONG SQL HERE
Unhandled rejection SequelizeDatabaseError: column "id" does not exist
*/

WIth these changes, sequelize generated collect SQL.

/* Log ---
Executing (default): INSERT INTO "test_user" ("test_user_id") VALUES (1) RETURNING *;
1
Executing (default): DELETE FROM "test_user" WHERE "test_user_id" IN (SELECT "test_user_id" FROM "test_user" WHERE "test_user_id" = 1 LIMIT 1)
delete success !!
*/
@janmeier

This comment has been minimized.

Member

janmeier commented Jun 29, 2015

Looks good - could you add a couple of sql unit tests, along the lines of https://github.com/sequelize/sequelize/blob/master/test/unit/sql/select.test.js

@smihica

This comment has been minimized.

Contributor

smihica commented Jun 29, 2015

I see

@BridgeAR

This comment has been minimized.

Contributor

BridgeAR commented Jul 6, 2015

Ping @smihica do you need any help? :)

@smihica

This comment has been minimized.

Contributor

smihica commented Jul 7, 2015

No. Thanks! @BridgeAR ;)

I'm really busy here recently...😇...
I'll write some tests, but may be late.
If you want to fix this bug immediately, I'm afraid but please write some tests by your self.

Fixed a bug that wrong sql generated when deleting object that has a …
…specific field as a primary key in postgres.

@smihica smihica force-pushed the smihica:bugfix/pg-delete-failed branch from 1f83686 to cbd13ea Aug 4, 2015

@smihica

This comment has been minimized.

Contributor

smihica commented Aug 4, 2015

@janmeier
@BridgeAR

Hi.
I added a test for this commit and rebased.
And all unit and integration tests were passed in my environment.

janmeier added a commit that referenced this pull request Aug 4, 2015

Merge pull request #4027 from smihica/bugfix/pg-delete-failed
Fixed a bug where wrong sql was generated when destroying in pg when primary key has an alias.

@janmeier janmeier merged commit 3603863 into sequelize:master Aug 4, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

janmeier added a commit that referenced this pull request Aug 4, 2015

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