Objection.js: upsertGraph + m2m relation with extra columns + modify on the join table issue

Created on 26 Jan 2018  路  11Comments  路  Source: Vincit/objection.js

Hi,
I ran into an issue when trying to save graph with upsertGraph with relations and values for "extra" columns at the same time. I've tested 0.9.4 and 1.0-rc6.

I will try to provide a stripped down variations of models I am using, hopefully enough to identify the problem.

class Group extends Model {
  static tableName = 'groups';

  static jsonSchema = {
    type: 'object',
    required: ['name'],

    properties: {
      id: { type: 'integer' },
      sort: { type: 'integer' },
      name: { type: 'string', minLength: 1, maxLength: 200 },
    },
  };

  static relationMappings = {
    decks: {
      relation: Model.ManyToManyRelation,
      modelClass: Deck,
      modify: (builder) => {
        builder.orderBy('sort');
      },
      join: {
        from: 'groups.id',
        through: {
          from: 'groups_decks.group_id`,
          to: 'groups_decks.deck_id`,
          extra: ['sort'],
        },
        to: 'decks.id',
      },
    },
  };
}

class Deck extends Model {
  static tableName = 'decks';

  static jsonSchema = {
    type: 'object',
    required: ['name'],

    properties: {
      id: { type: 'integer' },
      name: { type: 'string', minLength: 1, maxLength: 200 },
    },
  };

  static relationMappings = {
    group: {
      relation: Model.HasOneThroughRelation,
      modelClass: Group,
      join: {
        from: 'decks.id',
        through: {
          modelClass: GroupDeck,
          from: 'groups_decks.deck_id',
          to: 'groups_decks.group_id',
          extra: ['sort'],
        },
        to: 'groups.id',
      },
    },
  };
}

class GroupDeck extends Model {
  static tableName = 'groups_decks';

  static jsonSchema = {
    type: 'object',
    required: ['group_id', 'deck_id'],

    properties: {
      id: { type: 'integer' },
      group_id: { type: 'integer' },
      deck_id: { type: 'integer' },
      sort: { type: 'integer' },
    },
  };

  static relationMappings = {
    group: {
      relation: Model.HasOneRelation,
      modelClass: Group,
      join: {
        from: 'groups_decks.group_id',
        to: 'groups.id',
      },
    },
    deck: {
      relation: Model.HasOneRelation,
      modelClass: Deck,
      join: {
        from: 'groups_decks.deck_id',
        to: 'decks.id',
      },
    },
  };
}

Now assuming group id 1 and deck of id 1 and id 2 already exist in db, i want to create relation between them with additional 'sort' value. Note that i used 'modify' to automatically sort related records when using eager (and it works, but maybe there is a better option to do this?)

running:

Group.query().upsertGraphAndFetch({
    id:1,
    decks: [{id: 1,sort:0}, {id: 2, sort:1}]
  }, {relate: true, unrelate: true})

causes an error with a malformed sql query tried to be executed:

update `decks` set  where `decks`.`id` = 1 and `decks`.`id` in (select `groups_decks`.`deck_id` from `groups_decks` where `groups_decks`.`group_id` = 1) order by `sort` asc - ER_PARSE_ERROR: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'where `decks`.`id` = 1 and `decks`.`id` in (select `groups_decks' at line 1

firstly it attaches orderBy('sort') to update query, secondly not there where it should (instead it tries to use it in the target table and there is no sort column)
secondly, as a side note, i noticed that it tries to update id column. is such operation expected to be allowed?

bug

Most helpful comment

@unhawkable There was a bug (or missing feature) where extra columns could not be used in modify filters in all cases. I'm now working on a fix. After that, the isFindQuery check is not needed anymore. I'll close this when I'm done and release a new 1.0 RC version.

All 11 comments

upsertGraph is such a complex beast that unfortunately I need a full reproduction before I can start to investigate this. There are 1000 things you may be doing different than what I'd expect. There's a reproduction-template.js file in the repo root if you prefer to use that.

I just noticed that these both should be BelongsToOneRelations because the foreign key is in the source model:

  static relationMappings = {
    group: {
      relation: Model.HasOneRelation,
      modelClass: Group,
      join: {
        from: 'groups_decks.group_id',
        to: 'groups.id',
      },
    },
    deck: {
      relation: Model.HasOneRelation,
      modelClass: Deck,
      join: {
        from: 'groups_decks.deck_id',
        to: 'decks.id',
      },
    },
  };

@unhawkable Did you solve this? Can I close this issue? If not, please answer something so that I know that you are working on a reproduction.

Thanks for the tip about Model.BelongsToOneRelation. It did not have any impact though, because I found out that I also missed modelClass: GroupDeck from "through" config in relationMappings :D So the model for the join table was not used at all, but objection auto-generated it in-memory I suppose.

Anyway, attached you may find the reproduction file: (there is reproduction-template.js in that zip)
reproduction-template.zip

Note that it requires mysql, because it turned out it works fine with sqlite!
I also skipped a model for the join table, as it does not matter for the case. I've only added extra attributes, 'modify' functions and the column in the migration and modified the test case.

install: npm install objection knex mysql chai

The queries which are being called by upsertGraph are as follows:

select `Person`.`id` from `Person` where `Person`.`id` in (?)'
select `Person_Movie`.`personId` as `objectiontmpjoin0`, `Movie`.`id` from `Movie` inner join `Person_Movie` on `Movie`.`id` = `Person_Movie`.`movieId` where `Person_Movie`.`personId` in (?) order by `awesomeness` desc
update `Movie` set `id` = ? where `Movie`.`id` = ? and `Movie`.`id` in (select `Person_Movie`.`movieId` from `Person_Movie` where `Person_Movie`.`personId` = ?) order by `awesomeness` desc

and then it fails with Error: Unknown column 'awesomeness' in 'order clause'.

fyi - Sqlite queries for the upsertGraph operation: (which run without an error)

select `Person`.`id` from `Person` where `Person`.`id` in (?)
select `Person_Movie`.`personId` as `objectiontmpjoin0`, `Movie`.`id` from `Movie` inner join `Person_Movie` on `Movie`.`id` = `Person_Movie`.`movieId` where `Person_Movie`.`personId` in (?) order by `awesomeness` desc
update `Movie` set `id` = ? where `Movie`.`id` = ? and `Movie`.`_rowid_` in (select `Movie_rel_movies`.`_rowid_` from `Person_Movie` inner join `Movie` as `Movie_rel_movies` on `Person_Movie`.`movieId` = `Movie`.`id` where `Person_Movie`.`personId` = ?)
update `Person_Movie` set `awesomeness` = ? where `Person_Movie`.`personId` = ? and `Person_Movie`.`_rowid_` in (select `Person_Movie`.`_rowid_` from `Movie` inner join `Person_Movie` as `Person_Movie` on `Movie`.`id` = `Person_Movie`.`movieId` where `Movie`.`id` = ? order by `awesomeness` desc)
update `Movie` set `id` = ? where `Movie`.`id` = ? and `Movie`.`_rowid_` in (select `Movie_rel_movies`.`_rowid_` from `Person_Movie` inner join `Movie` as `Movie_rel_movies` on `Person_Movie`.`movieId` = `Movie`.`id` where `Person_Movie`.`personId` = ?)
update `Person_Movie` set `awesomeness` = ? where `Person_Movie`.`personId` = ? and `Person_Movie`.`_rowid_` in (select `Person_Movie`.`_rowid_` from `Movie` inner join `Person_Movie` as `Person_Movie` on `Movie`.`id` = `Person_Movie`.`movieId` where `Movie`.`id` = ? order by `awesomeness` desc)

You shouldn't add order by statement to an update. For some reason knex omits it for sqlite, but not for mysql. You can use isFindQuery method to check if the query is a select query and only add the order by in that case.

IT WORKS! I can't thank you enough for helping me out with this.

But something fishy is still going on. As you said, the order by statement is in the wrong place. I'll see what's going on there. Let's leave this open.

@unhawkable There was a bug (or missing feature) where extra columns could not be used in modify filters in all cases. I'm now working on a fix. After that, the isFindQuery check is not needed anymore. I'll close this when I'm done and release a new 1.0 RC version.

@koskimas i'm sorry to bother you again, but when I run the reproduction-template I attached above with 1.0.0-rc9 i am getting the following error:

update `Movie` set  where `Movie`.`id` in (select `Movie`.`id` from (select `Movie`.* from `Movie` inner join `Person_Movie` on `Movie`.`id` = `Person_Movie`.`movieId` where `Person_Movie`.`personId` in (?) and `Movie`.`id` = ? order by `awesomeness` de
sc) as `Movie`)

Error: ER_PARSE_ERROR: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'where `Movie`.`id` in (select `Movie`.`id` from (select `Movie`.* from `Movie` i' at line 1

@unhawkable Oh yeah, that's another bug 馃槃 I've already created an issue for that #768. Your example works if you add name for the movies in upsert

  await Person.query().upsertGraph({
    id: 1,
    movies: [
      {
        id: 1,
        name: 'The Room',
        awesomeness: 100,
      },
      {
        id: 2,
        name: 'East of Eden',
        awesomeness: 2000,
      }
    ]
  }, {relate: true, unrelate: true});

oh I see. Thanks for the info. I will wait patiently and keep the ifs with isFindQuery / isFind for now :)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mycahjay-nms picture mycahjay-nms  路  4Comments

Gustav0ar picture Gustav0ar  路  4Comments

AhmadRaza786 picture AhmadRaza786  路  3Comments

njleonzhang picture njleonzhang  路  4Comments

rickmed picture rickmed  路  4Comments