updateItemWithReturning ignores previous modifications to querybuilder

[jellelicht]

Issue type:

[ ] Question [x] Bug report [ ] Feature request [ ] Documentation issue

Database system/driver:

[x] Postgres [ ] MSSQL [ ] MySQL [ ] MariaDB [ ] SQLite3 [ ] Oracle [ ] Amazon Redshift

typed-knex version:

[x] latest [ ] @next [ ] 0.x.x (or put your version here)

Knex.js version: 1.0.4

const payload = { name: 'hello-unique', value: 12 };

handle
            .query(MyTable)
            .where("name", payload.name)
            .updateItemWithReturning({value: payload.value});

Generates the psql query: update "mytable" set "value" = 12 returning *

Looking at the code, it seems that (at least) updateItemsWithReturning and possibly updateItemsByPrimaryKey don't make use of the this.querybuilder, thereby losing any pre-existing configurations such as the where('name', payload.name) in the previous snippet of code.

[wwwouter]

Thanks for reporting. This should be fixed in 4.8.3, can you check?

[jellelicht]

It sadly seems to not work yet. Anything I can do to help address this?

[jellelicht]

The distributed (js) sources don't match the updated (ts) code (which does seem to work, modulo a beforeInsertTransform instead of beforeUpdateTransform). Could you please check if this makes sense on your end? Thanks a bunch!

[wwwouter]

Ah good catch. This happened before, but only now I realise it's because of npm v8. I was switching quite a lot between versions, so I didn't catch this.

Should be fixed in v4.9.0

[jellelicht]

It works, thanks! FYI, the updateItemWithReturning still seems to call out to this.beforeInsertTransform, instead of this.beforeUpdateTransform as the normal updateItem does. I currently don't use these features as far as I know, but this does seem like a small oversight as well!

[wwwouter]

Thanks! I updated the code and added some tests for good measure.