oguimbal / pg-mem

An in memory postgres DB instance for your unit tests
MIT License
1.98k stars 97 forks source link

knex onConflict merge with columns #186

Closed elad-sf closed 2 years ago

elad-sf commented 2 years ago

Describe the bug

When using createKnex adapter, we have an issue with queries that do upsert on selected columns. merge without specifying columns is working fine.

Column "0" not found

To Reproduce

const data = [
        { id: 1, other_id: 111, some_field: 'value', created_at: new Date(), updated_at: new Date() },
        { id: 2, other_id: 222, some_field: 'value2', created_at: new Date(), updated_at: new Date() }
    ];
    const query = knex('example').insert(data).onConflict(['id', 'other_id']).merge(['updated_at', 'some_field']);
    console.log(query.toSQL());

it will produce do update set "0" = ?,"1" = ?' should be do update set "some_field" = excluded."some_field", "updated_at" = excluded."updated_at"'

{
  method: 'insert',
  options: {},
  timeout: false,
  cancelOnTimeout: false,
  bindings: [
    2022-01-31T21:17:53.130Z,
    1,
    111,
    'value',
    2022-01-31T21:17:53.130Z,
    2022-01-31T21:17:53.130Z,
    2,
    222,
    'value2',
    2022-01-31T21:17:53.130Z,
    'updated_at',
    'some_field'
  ],
  __knexQueryUid: 'lwEt8xzcV2xKl87iZdJQl',
  sql: 'insert into "example" ("created_at", "id", "other_id", "some_field", "updated_at") values (?, ?, ?, ?, ?), (?, ?, ?, ?, ?) on conflict ("id", "other_id") do update set "0" = ?,"1" = ?',
  returning: undefined
}

pg-mem version

2.3.0

oguimbal commented 2 years ago

Hi !

This is the SQL generated by Knex:

insert into "example" ("created_at", "id", "other_id", "some_field", "updated_at") 
values ($1, $2, $3, $4, $5), ($6, $7, $8, $9, $10) 
on conflict ("id", "other_id") 
do update set "0" = $11,"1" = $12

The do update set "0" = $11,"1" = $12 part is really wrong.

Are you sure that your statement works against a real database ? Because it really does looks like a Knex issue, not a pg-mem issue 🤷‍♂️

elad-sf commented 2 years ago

Hi, yes it works. I'm using knex@1.0.1 with pg@8.7.1 and it works fine, I just run this on my local env with real DB:

const knex = require('knex');

const db = knex({
    client: 'pg',
    connection: {
        user: 'postgres',
        database: 'postgres',
        password: 'postgres',
        host: 'localhost',
        port: 5432
    }
});

const data = [
    { id: 1, other_id: 111, some_field: 'value', created_at: new Date(), updated_at: new Date() },
    { id: 2, other_id: 222, some_field: 'value2', created_at: new Date(), updated_at: new Date() }
];
const query = db('example').insert(data).onConflict(['id', 'other_id']).merge(['updated_at', 'some_field']);
console.log(query.toSQL());

and got this response:


{
  method: 'insert',
  options: {},
  timeout: false,
  cancelOnTimeout: false,
  bindings: [
    2022-02-01T10:54:38.869Z,
    1,
    111,
    'value',
    2022-02-01T10:54:38.869Z,
    2022-02-01T10:54:38.869Z,
    2,
    222,
    'value2',
    2022-02-01T10:54:38.869Z
  ],
  __knexQueryUid: 'p5dlZTvr0WhMiI915Q0Zj',
  sql: 'insert into "example" ("created_at", "id", "other_id", "some_field", "updated_at") values (?, ?, ?, ?, ?), (?, ?, ?, ?, ?) on conflict ("id", "other_id") do update set "updated_at" = excluded."updated_at", "some_field" = excluded."some_field"',
  returning: undefined
}```
oguimbal commented 2 years ago

That is not what I'm getting... 😅 image

oguimbal commented 2 years ago

Which version of Knex are you using ?

Is it the same version that generated the "working" example that you mentioned, compared to the one you're using the repo using pg-mem ?

elad-sf commented 2 years ago

haha that's weird! I'm using knex@1.0.1 with pg@8.7.1 - same for tests what are you using? maybe pg-mem need to update some deps?

I've used pg-mem for my UTs and it was awesome using pg-promise, moved my codebase to knex.js and all works great, but can't get the UTs to work because of this issue.

elad-sf commented 2 years ago

@oguimbal even works on this web playground: playground example

oguimbal commented 2 years ago

Yup, that seems to be a Knex versioning issue.

pg-mem declares a peer dependency "knex": "^0.21.15", which will not pick up >=1.0 as a valid peer dependency (see semver.npmjs.com to check that)

I think that if you run cat node_modules/knex/package.json | grep version you will not get 1.0.1 on the repo you have installed pg-mem into ? Or is there a node_modules sub-directory containing another version of knex in your node_modules/pg-mem directory ?

Anyway, I just published pg-mem@2.3.2 which is a bit more tolerant on knex version ("knex": ">=0.20"), and should allow you to use knex 1.01

Does it helps solving your issue ?

elad-sf commented 2 years ago

you are right.. node_modules/knex version is:

"type": "version",
"version": "0.21.19"

I still waiting for npm to update pg-mem to 2.3.2, it's still on 2.3.1 and will keep you updated. thanks!

oguimbal commented 2 years ago

I think you might have to upgrade knex after having upgraded pg-mem => Something like npm i pg-mem@latest -D && npm i knex@latest -S (if using npm) should solve your problem

elad-sf commented 2 years ago

I will try, how long does it take to publish 2.3.2 version to npm? :)

oguimbal commented 2 years ago

Wops, I'm sorry, I thought it was done. Something must have failed when I launched the deployment earlier.

It's now published.

elad-sf commented 2 years ago

awesome, it works! thanks a lot :)

oguimbal commented 2 years ago

No problem !