Allow update/delete with joined tables

[csirkeee]

The missing feature

Right now Diesel doesn't allow for update/delete with joined tables. This seems like an explicit decision based on having

pub trait IntoUpdateTarget: HasTable

Which means the update target must be a query that only touches one table.

But, using multiple tables in update/delete is a feature that both MySQL and Postgres support. (I didn't research the other databases.) So, it would be nice if Diesel could also support it. In this feature request I'll describe how that could look like.

Example code

Update

For example, working with the tables in the test suite, this could be a valid query:

update(
    comments::table
        .inner_join(posts::table.on(posts::id.eq(comments::post_id)))
        .inner_join(users::table.on(users::id.eq(posts::user_id)))
        .filter(users::id.eq(banned_id)))
    .set(comments::text.eq("Banned user"))
    .execute(&connection)
    .unwrap();

This would translate to the following query in MySQL:

UPDATE `comments`
	INNER JOIN `posts` ON `posts`.`id` = `comments`.`post_id`
	INNER JOIN `users` ON `users`.`id` = `posts`.`user_id` 
SET 
    `text` = ?
WHERE
    `users`.`id` = ?;

And into this query on Postgres:

UPDATE "comments"
SET 
    "text" = $1
FROM "posts"
    INNER JOIN "users" ON "users"."id" = "posts"."user_id"
WHERE
    "posts"."id" = "comments"."post_id"
    AND "users"."id" = $2;

Delete

Delete is a similar case:

delete(
    comments::table
        .inner_join(posts::table.on(posts::id.eq(comments::post_id)))
        .inner_join(users::table.on(users::id.eq(posts::user_id)))
        .filter(users::id.eq(banned_id)))
    .execute(&connection)
    .unwrap();

could translate, in MySQL, to:

DELETE FROM `comments`
USING `comments`
	INNER JOIN `posts` ON `posts`.`id` = `comments`.`post_id`
	INNER JOIN `users` ON `users`.`id` = `posts`.`user_id` 
WHERE
    `users`.`id` = ?;

and in Postgres:

DELETE FROM "comments"
USING "posts"
    INNER JOIN "users" ON "users"."id" = "posts"."user_id"
WHERE
    "posts"."id" = "comments"."post_id"
    AND "users"."id" = $1;

(I've checked all SQL to be valid on SQLFiddle.)

Implementation

The main problem with the implementation is that the query builder would need to handle join clauses differently from other filters. Also, the syntax is different for the databases, I don't know how much that complicates things. (It's possible that there exists a syntax that both MySQL and Postgres accept, but I haven't found one.)

Maybe the first part could be handled by using a different syntax in rust, like calling the joins on the update query and not on the table, like this:

update(comments::table.filter(users::id.eq(banned_id)))
    .inner_join(posts::table.on(posts::id.eq(comments::post_id)))
    .inner_join(users::table.on(users::id.eq(posts::user_id)))
    .set(comments::text.eq("Banned user"))
    .execute(&connection)
    .unwrap();

[Ten0]

Currently we've been using an insert_into on_conflict do_update to workaround this not being implemented.

[Bajix]

Will this be part of Diesel 2.0?

[weiznich]

@Bajix If someone submits a PR till then. I personally do not plan to work on this at least till the 2.0 release.

[Bajix]

Is there a timetable or write up regarding 2.0? I was confused as I had seen previously the source code already had 2.0 and yet was never released, but at the time there wasn’t much written

Sent from my iPhone

On Oct 7, 2020, at 11:28 PM, Georg Semmler [email protected] wrote:

 @Bajix If someone submits a PR till then. I personally do not plan to work on this at least till the 2.0 release.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[weiznich]

@Bajix There is no complete timetable or write up for diesel 2.0. As this is a free time project at least I do not give any ETA's other than: When it's done. Before releasing 2.0 I want to fix or at least have a look at least those issues collected in the 2.0 milestone

[lermana]

Hi @weiznich , thanks for all your work on diesel! It's a really great library. I thought I'd ask if there were any updates here? Trying to do a delete (just like what's in the original comment) where I want to join in another table to filter what gets deleted, but it appears I can't currently write a query like that.

I am new to Rust but at some point later in the year I'd love to help you out on this project -- really important for the Rust ecosystem.

[Ten0]

Hello, There haven't been updates on this topic, it's still pending workforce. As far as I'm concerned we still use the same workaround : https://github.com/diesel-rs/diesel/issues/1478#issuecomment-562113367

I think we make a reasonably good job at tagging the issues we work on so unless proven otherwise you may generally assume that if an issue has seen no comment or mention or closing it hasn't moved.

[lermana]

thanks @Ten0, makes sense. I saw that comment for update -- is there a complement for delete?

[Ten0]

Not that I know of - you'd have to make a select first then delete with eq_any.

[lermana]

yeah that's what I ended up doing

[thomasmost]

👀 @weiznich I'd be interested in working on this PR if you have any guidance/thoughts

For context, this is the kind of query I'm hoping to execute:

  pub fn update_from_assessments(
    connection: &mut MysqlConnection,
    emissions: &Vec<super::emission::Emission>,
  ) -> Result<(), ()> {
    let transaction_ids = emissions.iter().filter_map(|e| e.transaction_id.clone()).collect();
    // Joining on the assessed emission records, update the corresponding transactions with the assessed values
    // based on each corresponding emission record; also set assessment_pending to false
    diesel::update(
        transaction::table
          .filter(transaction::id.eq_any(transaction_ids))
      )
      .inner_join(emission::table.on(transaction::id.eq(emission::transaction_id
        .assume_not_null())))
      .set((
        transaction::represented_offsets_bill_id.eq(emission::represented_offsets_bill_id),
        transaction::merchant_name.eq(emission::name),
        transaction::merchant_org_id.eq(emission::merchant_org_id),
        transaction::category_id.eq(emission::category_id),
        transaction::emission_id.eq(emission::id.nullable()),
        transaction::assessment_pending.eq(false),
      ))
      .execute(connection)
      .is_ok()
  }

[weiznich]

@thomasmost We don't have a design for that yet. Without having checked anything yet I would expect that the following steps need to be done:

• Implement InternalJoinDsl for UpdateStatement
• Make that return a custom query dsl node that allows to provide different QueryFragment implementations for mysql and postgres
• Provide these query dsl node implementations (Might require additional
• Write tests
• Write compile-fail tests showing that certain invariants are upheld (which ones?)
• Research what to do with sqlite (it's fine to say it's not supported there)

I would expect that this is not impossible to do, but likely it's also nothing that can be done in one or two hours.