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

Update items fail on remapped column - instance from find method #509

Closed
hkbenchan opened this issue May 26, 2014 · 7 comments
Closed

Update items fail on remapped column - instance from find method #509

hkbenchan opened this issue May 26, 2014 · 7 comments
Labels

Comments

@hkbenchan
Copy link

Node version: 0.10.25
orm version: 2.1.14
mysql version: 2.3.0

Reproduce of the bug:

Create a model that has a key with mapsTo enable, e.g.

var Model = db.define('model', {
    name: String,
    remapKey: { type: "integer", mapsTo: "remap_key", key: true }
});;

Later, when save an item that is gotten from Model.find(),

modelObject.name = "another value";
modelObject.save(function (err) {
    if (err) throw err;
    cb();
}

it will throw an error on mysql level, error message as follow

ER_BAD_FIELD_ERROR: Unknown column 'remapKey' in 'where clause'

Could you please help on fixing this?


Updated on 11:06 am Monday, 26 May, 2014 (GMT)

P.S. The error "ER_BAD_FIELD_ERROR" is gone when I define my model like this

var Model = db.define('model', {
    name: String,
    remapKey: { type: "integer", mapsTo: "remap_key" }
}, {
    id: "remap_key"
});;

But the update is not success as I lookup the query, it says

UPDATE `model` SET `name` = \'another value\', `remap_key` = 1 WHERE `remap_key` IS NULL
@hkbenchan
Copy link
Author

Similar to #506

@dxg
Copy link
Collaborator

dxg commented May 26, 2014

I cannot recreate this issue.
Please see the tests.
.one() calls .find() internally.
Even if I change the one() to find() the test still passes.
Can you provide a more complete example, or tweak the test to make it fail as it does for you? Thanks.

@hkbenchan
Copy link
Author

Please try this.

var _        = require('lodash');
var should   = require('should');
var helper   = require('../support/spec_helper');
var common   = require('../common');
var ORM      = require('../../');

if (common.protocol() == "mongodb") return;

describe("Property.mapsTo", function() {
    var db = null;
    var id1 = null, id2 = null;
    var Following = null;

    before(function (done) {
        helper.connect(function (connection) {
            db = connection;
            db.settings.set('instance.cache', false);

            return done();
        });
    });

    after(function () {
        return db.close();
    });

    var setup = function () {
        return function (done) {
            Following = db.define('following', {
                follower : { type: "integer" , mapsTo: "user_id1"},
                following : { type: "integer" , mapsTo: "user_id2"},
                status: [ 'requested', 'accepted' ]
            }, {
                id: [ "user_id1", "user_id2" ]
            });

            return helper.dropSync(Following, done);
        };
    };

    describe("", function (done) {
        before(setup());

        it("should create", function (done) {
            Following.create({
                follower: 1,
                following: 2,
                status: "requested"
            },function (err, followingPair) {
                should.equal(err, null);
                should.exist(followingPair);
                should.equal(1, followingPair.follower);
                done();
            })
        });

        it("should find", function (done) {
            Following.one({ follower: 1, following: 2}, function(err, followingPair) {
                should.not.exist(err);
                should.exist(followingPair);
                should.equal(1, followingPair.follower);
                should.equal(2, followingPair.following);
                should.equal("requested", followingPair.status);
                done();
            });
        });

        it("should update", function (done) {
            Following.one({ follower: 1, following: 2}, function(err, followingPair) {
                should.not.exist(err);
                should.exist(followingPair);
                followingPair.status = "accepted";

                followingPair.save(function (err) {
                    should.not.exist(err);
                    should.equal("accepted", followingPair.status);

                    Following.one({ follower: 1, following: 2}, function (err, newFollowingPair) {
                        should.not.exist(err);
                        should.exist(newFollowingPair);
                        should.equal("accepted", newFollowingPair.status);
                        done();
                    });
                });
            });
        });

    });
});

I guess the reason that your tests are passed is the remap key is not the primary key, so that's why it will pass the condition.

@dxg
Copy link
Collaborator

dxg commented May 27, 2014

Haha, yeh.. I didn't think setting mapsTo on keys would work. Will look into it.
Edit: setting id: [...] and key: true should be equivalent so I'll look at fixing that also.

@dxg dxg added the bug label May 27, 2014
@dxg
Copy link
Collaborator

dxg commented Jun 2, 2014

I believe I fixed it.
Please try the latest master version.

@hkbenchan
Copy link
Author

The bug is not fixed, how should I say, becoming worse
original the sql generated is:

UPDATE `following` SET `status` = \'accepted\' , `user_id1` = 1 ,  `user_id2` = 2 where `user_id1` is NULL AND `user_id2` is NULL

new the sql generated is:

UPDATE `following` SET `status` = \'accepted\'

which update all records

for now I will fall back not to use the mapsTo feature on primary key, will look at it later when I finish development on the part

@dxg
Copy link
Collaborator

dxg commented Jun 3, 2014

The update does fail as you show, but that's because the keys are defined incorrectly:

Following = db.define('following', {
    follower : { type: "integer" , mapsTo: "user_id1"},
    following : { type: "integer" , mapsTo: "user_id2"},
    status: [ 'requested', 'accepted' ]
}, {
    id: [ "user_id1", "user_id2" ] // these don't exist
});

Since neither of those properties exists, it acts as if the model has no keys.
The following works:

Following = db.define('following', {
    follower : { type: "integer" , mapsTo: "user_id1"},
    following : { type: "integer" , mapsTo: "user_id2"},
    status: [ 'requested', 'accepted' ]
}, {
    id: [ "follower", "following" ] // these do exist
});

however I recommend the key: true syntax which works too:

Following = db.define('following', {
    follower  : { type: "integer" , mapsTo: "user_id1", key: true},
    following : { type: "integer" , mapsTo: "user_id2", key: true},
    status: [ 'requested', 'accepted' ]
});

I'll make the model definition throw some sort of error when invalid keys are specified.

@dxg dxg closed this as completed Jun 5, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants