Update operations do not support joins

[kibertoad]

Environment

Knex version: 0.15.2

Feature request

  await knex('table')
    .innerJoin('table2', function() {
      this.on(
        `table.id`,
        `table2.id`
      ).andOn(`table.field`, 'value')
    })
    .update({
      table.field2: 'newValue,
    })

This is going to execute without an error, but entire join part will be ignored and all entries will be silently updated.

#1534 suggests either using raw or subquery, but neither of these solutions is particularly elegant, and API-wise there is no reason why this shouldn't work, so I would consider this a bug.

[kibertoad]

I could try giving it a shot, but I have no idea how complicated this is going to be. @elhigu any pointers where to start?

[elhigu]

Not bug, but a missing feature. This has been discussed a lot during history and is one of the most wanted features in my list. I don't know if it would be hard to implement or not. I would suggest reading builder / compilation code of update operation to see if join parameters are even collected to builder for update operations.

Also to start with one needs to figure out how those joins with update work with different databases (postgresql, mysql, mssql, oracledb, sqlite) and see which databases even can be supported.

Pretty old issue about it #557
This is how it should not be implemented #2578
Somewhat related for delerte #873

[tgriesser]

IIRC joins are only supported on update with mysql and sqlite.

I've spent the last week or so working on a new API design that will make this way clearer by only allowing supported features on the builder each dialect, rather than just using the same giant base class to build the query and then having undefined runtime behavior depending on the dialect. Will open a PR with all of this once I get a little further.

[elhigu]

IIRC postgresql also supports kind of joins in updates, but it uses from keyword to implement them. I'm not sure how much different they are from joins. Mssql also seems to support it. For oracle there doesn't seem to be anything else but hacks for it https://stackoverflow.com/a/2446859/360060

[JQuezada0]

May I take a stab at this?

[kibertoad]

@JQuezada0 Yes, that would be great! You might want to take a look at how DELETE with joins was implemented:
#4568
#4800
#5016

[OlivierCavadenti]

@JQuezada0 you can ping me here for any questions if you want.

[jpike88]

+1 for this, I've switched from mysql to Postgres, and this was a shock... considering that Postgres actually doing everything SQL compliant and others aren't.

[jpike88]

The worst part is that the query fails silently, if using Postgres it should at least throw an error.

[jpike88]

I think it's worth implementing a stopgap solution. Perhaps if it was possible to add some sort of validation method on update that can inspect the AST of the query before it's executed.

Then rules such as banning joins on the update query can be enforced, with some suggestions on how to handle it

[jpike88]

Another one that is a gotcha for PostgresSQL users... this knex command will silently create an invalid query:

await sql
			.knexMaster('user')
			.join(sql.Table.Business, 'user.businessID', 'business.id')
			.where('user.id', creatorID)
			.increment('bookingCounter', 1);

The need for dialect-specific checks is definitely there.

[kibertoad]

Pull requests are very welcome.

[jpike88]

Which would be more ideal:

• The ability to import a Typescript definition set that overrides existing types for a given dialect (e.g. a type that excludes join when update or increment is present), I'm not sure that's even possible
• Throw errors at runtime

[joelanman]

Could this at least throw an error or warning for now - that this isn't supported? I'm using Postgres and the error was hard to track down to this issue: missing FROM-clause entry for table

[kibertoad]

@joelanman PR would be appreciated!