Objection.js: Graph Upsert syntax issue with SQL Server

[tankers746]

Hi,

I'm having an issue with a query produced during a graph upsert. The query:

delete from [fixture_sport] 
where ([fixture_sport].[fixture_id], [fixture_sport].[sport_slug]) 
in ((@p0, @p1),(@p2, @p3),(@p4, @p5)) and [fixture_sport].[fixture_id] in (@p6);
select @@rowcount

The issue is that SQL server doesn't support composite WHERE IN statements.
The issue is described here

I was wondering if it was possible to use an alternative to WHERE IN for tables with composite keys.

Cheers

[koskimas]

This should be fixed when #677 is merged

[koskimas]

677 is now merged. It will be a part of the next release. Until that you can use the master directly if you want.

[koskimas]

Let me know if that works. There's no automated tests for SQL server because Microsoft

[tankers746]

Hi,

It's fixed the WHERE IN issue however its still failing because upsertGraph is trying to update the id field which is an identity column.

update [fixture] 
set [opponent_name] = @p0, [home] = @p1, [id] = @p2 
where [fixture].[id] = @p3;
select @@rowcount - Cannot update identity column 'id'.

I believe there was already an issue on this:
https://github.com/Vincit/objection.js/issues/480

Cheers

[koskimas]

Yeah, that should be fixed now too. Could you test the 1.0.0 branch to see if it works for you?

[tankers746]

Still getting the same issue with 1.0.0.
Here is my upsert code:

Fixture.query().upsertGraph(
      {
        id: req.params.id,
        updated_by: req.user.displayName,
      },
    );

and here is the model I'm trying to update:

class Fixture extends Model {
  static get tableName() {
    return 'fixture';
  }

  static get idColumn() {
    return 'id';
  }

  static get defaultEagerAlgorithm() {
    return Model.JoinEagerAlgorithm;
  }

  static get namedFilters() {
    return {
      upcoming: (builder) => {
        builder.where('date', '>=', new Date());
      },
      past: (builder) => {
        builder.where('date', '<', new Date());
      },
    };
  }

  $beforeInsert() {
    this.updated_at = new Date().toISOString();
  }

  $beforeUpdate() {
    this.updated_at = new Date().toISOString();
  }

  static get relationMappings() {
    const Opponent = require('./Opponent');
    const Sport = require('./Sport');
    const Subfixture = require('./Subfixture');

    return {
      opponent: {
        relation: Model.HasOneRelation,
        modelClass: Opponent,
        join: {
          from: 'fixture.opponent_name',
          to: 'opponent.name',
        },
      },

      subfixtures: {
        relation: Model.HasManyRelation,
        modelClass: Subfixture,
        join: {
          from: 'fixture.id',
          to: 'subfixture.fixture_id',
        },
      },

      sports: {
        relation: Model.ManyToManyRelation,
        modelClass: Sport,
        join: {
          from: 'fixture.id',
          through: {
            from: 'fixture_sport.fixture_id',
            to: 'fixture_sport.sport_slug',
          },
          to: 'sport.slug',
        },
      },
    };
  }
}

[koskimas]

@tankers746 Sorry I haven't answered. I've disabled notifications from closed issues. Otherwise answering the would be all I have time to do 😄 It's a good idea to open a new issue in the future.

Weird, that should now work. Are your ids strings in the database? If not, could you try this:

Fixture.query().upsertGraph(
      {
        id: parseInt(req.params.id, 10),
        updated_by: req.user.displayName,
      },
    );

[danigb]

I would like to test the 1.0.0 branch but I can't find it (inside branches). How should I proceed? Install the objection library from source?: yarn add https://github.com/Vincit/objection.js.git ?

[koskimas]

@danigb npm install objection@next

The 1.0.0 branch has been merged to master.

[danigb]

It works for me using 1.0.0-rc2 👍

[tankers746]

@koskimas cheers, the parseInt did it :) congrats on reaching 1.0.0

[koskimas]

@tankers746 Thanks! We should probably use a non-strict comparison in there and parseInt shouldn't be necessary. I'll create a separate issue about that.