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

object.$relatedQuery('relation') builds incorrect query when object's 'join: { from: 'something.id' } has value 0 #228

Closed
minusreality opened this issue Oct 24, 2016 · 11 comments
Labels

Comments

@minusreality
Copy link

minusreality commented Oct 24, 2016

With a fresh table, user.id is starting with value 0. I ran into this issue with MySQL 5 when using the statement:

user.$relatedQuery('locations').then( ... )

knex debugging yields:
...sql: 'select * from GPSLocation where 1 = ?'

Here is my relationshipMappings:

User.relationMappings = {
  locations: {
    relation: Model.HasManyRelation,
    modelClass: GPSLocation,
    join: {
      from: 'User.id',
      to: 'GPSLocation.user_id'
    }
  }
};
@koskimas
Copy link
Collaborator

koskimas commented Oct 24, 2016

Could you paste the code that produces the error? There must be something else wrong with the code. There is no way objection has a bug in such basic level. The test suite has tens of tests that run that exact code.

$relatedQuery creates a WHERE IN (?) clause. The where X = Y must come from somewhere else.

ID of a row should never be zero. Why do your sequences start with zero?

@Xatenev
Copy link

Xatenev commented Oct 24, 2016

The value 0 is no valid value for an auto increment value (if not specified otherwise), thus this is no bug in my opinion.

@minusreality
Copy link
Author

minusreality commented Oct 24, 2016

It's totally possible that something's wrong with my table, but I might suggest ensuring that the value 0 does not break $relatedQuery

@koskimas
Copy link
Collaborator

As I said $relatedQuery creates a WHERE IN (?) clause. The where X = Y must come from somewhere else. I'm sorry, but testing this would take alot of my time, and I'm pretty sure the problem is in your code. I will fix this immediately if you provide a test case that proves there is a bug.

@koskimas
Copy link
Collaborator

koskimas commented Oct 24, 2016

And by test case I mean any code I can easily run that reproduces the bug.

@koskimas
Copy link
Collaborator

koskimas commented Oct 24, 2016

And even if a zero id doesn't work, I'm not sure it's a bug as identifier should never be 0.

@minusreality
Copy link
Author

Indeed, an index should never be 0. I believe some other issue with an earlier version of my code inserted a 0 index user. That said, there may be situations where a user needs to join on a value 0. I'm very new to node.js so please forgive the very rough nature of this test. I'm still seeing repeatable results:

CREATE TABLE GPSLocation(id INT NOT NULL PRIMARY KEY AUTO_INCREMENT, testzero INT NOT NULL);
CREATE TABLE User (id INT NOT NULL PRIMARY KEY AUTO_INCREMENT, username VARCHAR(255), testzero INT NOT NULL);
INSERT INTO User(username, testzero) VALUES('testuser', 0);
INSERT INTO GPSLocation(testzero) VALUES(0);

/******** index.js *********/

var Knex = require('knex');
var Model = require('objection').Model;
var User = require('./models/User');
var GPSLocation = require('./models/GPSLocation');
var express = require('express');
var app = express();
var port = 3000;

var knex = require('knex')({
        debug: true,
        client: 'mysql',
        connection: {
                host : '127.0.0.1',
                user : '',
                password : '',
                database : ''
        }
});
Model.knex(knex);

app.get('/test', function (req, res) {
                User.query().where('username', req.query.username).first().then(function(user) {
                        if (user instanceof User) {
                                user.$relatedQuery('locations').first().then(function(loc) {
                                        console.log('test done');
                                });
                        }
                });
});

app.listen(3000, function () {
  console.log('Example app listening on port 3000!');
});

/******** models/User.js *********/

var Model = require('objection').Model;
var GPSLocation = require('./GPSLocation');

function User() {
        Model.apply(this, arguments);
}

Model.extend(User);

module.exports = User;

User.tableName = 'User';

User.jsonSchema = {
  type: 'object',
  required: ['username'],

  properties: {
    id: {type: 'integer'},
    testzero: {type: 'integer'},
    username: {type: 'string', minLength: 1, maxLength: 255},
  }
};

User.relationMappings = {
  locations: {
    relation: Model.HasManyRelation,
    modelClass: GPSLocation,
    join: {
      from: 'User.testzero',
      to: 'GPSLocation.testzero'
    }
  }
};

/******** models/GPSLocations.js *********/

var Model = require('objection').Model;

function GPSLocation() {
        Model.apply(this, arguments);
}

Model.extend(GPSLocation);

module.exports = GPSLocation;

GPSLocation.tableName = 'GPSLocation';

GPSLocation.jsonSchema = {
  type: 'object',
  required: ['user_id'],

  properties: {
    id: {type: 'integer'},
    testzero: {type: 'integer'},
  }
};

GPSLocation.relationMappings = {
  owner: {
    relation: Model.BelongsToOneRelation,
    modelClass: __dirname + '/User',
    join: {
      from: 'GPSLocation.testzero',
      to: 'User.testzero'
    }
  }
};

Output:

method: 'select',
  options: {},
  timeout: false,
  cancelOnTimeout: false,
  bindings: [ 0 ],
  __knexQueryUid: 'f84b7120-e52f-4a8c-bd3b-0cf837b50580',
  **sql: 'select * from `GPSLocation` where 1 = ?'**
test done

http://myserver:3000/test?username=testuser

@koskimas
Copy link
Collaborator

koskimas commented Oct 24, 2016

Thank you for the test case! It indeed appears to fail as you said. I have absolutely no idea where the where 1 = 0 comes from :D. I think I found the cause though. Could you test if changing the chunk:

        if (id) {
          hasIds = true;
          break;
        }

into

        if (id !== null && id !== undefined) {
          hasIds = true;
          break;
        }

in node_modules/objection/lib/relations/Relation.js fixes your problem?

@minusreality
Copy link
Author

Unfortunately, there is the same output.

@koskimas
Copy link
Collaborator

This is truly weird.. I'll write some tests and fix this.

@koskimas
Copy link
Collaborator

Fixed in 0.7.0-rc.1.

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

3 participants