Probably not much of a case on live sites, but still can happen.
In simplenews_update_840002 the NOT IN condition assumes the result of the result of querying simplenews_subscriber__subscriptions is non empty array.
This can result in
> [error] Query condition 'id NOT IN ()' cannot be empty.
> [error] Update failed: simplenews_update_840002
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | simplenews-update_fail-3405456-9.patch | 921 bytes | berdir |
| #5 | simplenews-update_fail-3405456-5.patch | 1.13 KB | primsi |
| #2 | simplenews-update_fail-3405456-2.patch | 991 bytes | primsi |
Comments
Comment #2
primsi commentedComment #3
primsi commentedOh snap, this was already fixed in #3396668: Update hook 840002 fails with SQL error. Was working on alpha version so I noticed just now when I created the patch :) closing.
Comment #4
berdirReopening this.
@AdamPS: FYI, the fix in the other issue is not correct, the issue is also not correctly explained in the issue summary there.
You select subscribers *with* a subscription and then update all others. Now the query is skipped, but it's a NOT IN condition, that could mean that you have no subscribers with a subscription but still have some without and now they are not updated.
The query also has scalability issues, if you have tens of thousands of subscribers, it loads all of their ids and then creates a _huge_ IN condition with tens of thousands of separate placeholders.
One option is to change the query so it can be an IN condition, with a query on subscriber, left join on subscriptions with a condition on subscription being NULL. But if you have a lot those that's still a problem.
So we could try to same with a nested condition, I think ->condition() supports passing in another select query object, or we could try with an expression. doing a subquery on an UPDATE condition on the same table afaik has some issues, we'll investigate a bit.
Comment #5
primsi commentedSomething like this then?
Comment #6
adamps commentedThanks @Berdir for explaining and @Primsi for the patch.
I believe #5 is a bit mixed up and $has_subscriptions will always be empty. Are you aiming to do this?
In which case you would need to
However as Berdir says this solution anyway still has a problem and he suggests some alternative.
Thanks! Let's see if Berdir can work some magic. I don't know much myself about optimising SQL queries 😃.
Comment #7
adamps commentedComment #8
berdirYes, the patch is not correct, it is indeed a mix.
As you said, the variable should be renamed and the query should remain condition with an IN condition inside, just like before we only want to run the query if we have any matches.
Comment #9
berdirLike this.
inline conditions might be possible but are differently more complicated to handle and I'm not sure about different database backends. This should be performant enough for our case and most others too.
Comment #11
adamps commentedGreat thanks