Hello @live627 please make sure this gets included in the next SMF 2.1 if one ever gets released. I think the developers need to add another upgrade which fixes all past upgrades to 2.1 for attachments. This bug looks like it's been there since the beginning of 2.1. I think you could easily add a function to recheck for attachments/avatars which failed to update.

First it's not smart two combine to fixes in one pr
secondly why you extend the query about attachment_type and later on you skip attachment_type 1 ?

1. you will fine pr from me how more than one year old...

2. you second fix is none fix, i guess you run this so many time that it at some point got done or you database is too small for the amount of data,
   would be also a good explaination why you got Upgrade Script Error #6176 since your setup didn't match the size of your forum.

i'm not part of the smf team, should be easy to since the collaborator text beside my name.

The order by hint from arantor looks more in right direction to possible fix the issue,
also you forget postgres upgrade here to fix too.

Instead of doing this, you should be able to fix the problem by way of instead forcing attachments to be handled in a consistent order (i.e. applying a ORDER BY to the processing query that steps through in groups). I don't believe this fixes whatever problem you actually have.

I guess if you would like to develop a different fix that works the same way then that's fine. I have confirmed for myself at least that this fixes the problem. This bug affects every prior version of smf when upgrading attachments/avatars to 2.1. You can test this if you have a 1.1.21 or 2.0.17 forum with a few hundred attachments/avatars.

This bug does not happen when you do not have any avatars and just a bunch of attachments. It has to be a mix of attachments/avatars in the attachment folder/database.

In other words it works by accident for you because you have MyISAM tables and lowering the block size is why it works. You could, of course, just change the query to include ORDER BY id_attach and get all of the fix to work without any other changes.

Pretty sure that will not work as you are later setting attachment_type = 1 which changes the total size of the rows being selected each time the updated avatar is changed to attachment_type = 1. Either way I don't want to spend more time retesting something that works just fine in this pr.

fyi my tables are InnoDB

So the upgrade logic here is to rename all non avatars (avatars are attachment_type = 1)

First the attachment_type logic seems to be
0 == attachment
1 == avatar
3 == thumbnail

I can't find a usage of 2.

A way to fix this would be to the search where file name NOT LIKE '%.dat' as well, and then walk over that until we get no results. Only issue is with a large forum there would be no index on the file name and this could be slow.

... you are later setting attachment_type = 1 which changes the total size of the rows being selected each time the updated avatar is changed to attachment_type = 1.

Butterflysaway is correct here. This explains why the check for type makes more sense within the processing loop than in the query.

But others are also correct here, in that using a LIMIT without an ORDER BY may yield questionable navigation thru the set as well. Lots of articles out there on that (& in fact I think that has bitten us before).

So... I think we need a companion PR to this that adds an ORDER BY here, and we should test the two PRs together. I'll label this as final & work on doing so.