Automatically delete duplicate keys in oc_appconfig table

[PVince81]

In some migration cases, the oc_appconfig table contains duplicate keys.
These need to be deleted/resolved on upgrade.

@icewind1991

[icewind1991]

Looking at the code it seems that latest row (in whatever order the sql server feels like) will be used while reading the values.

Do you know a clean (preferably pure SQL) way to remove the tables that are ignored while reading?
Only method I can think of at the moment is deleting all rows with duplicated keys and re-adding the values.

[PVince81]

Not really, and the order will probably vary depending on the DB engine. I remember that SQLite doesn't auto sort anything. (summonning @bantu for extra support)

Maybe doing a DELETE with a "LIMIT" parameter ?

Also, possibly related: #8252 adding primary keys. Hopefully the end of duplicate stuff!

[bantu]

Looking at the code it seems that latest row (in whatever order the sql server feels like) will be used while reading the values.

I would rather not rely on that actually being the case. I am pretty sure this won't hold for all DBMSes. If there is no consent it is basically a) Pick a random value or b) ask the user.

[karlitschek]

Ask the user is not an option because there is no way the user can do the right decision.

[icewind1991]

There is no need to ask the user. The code that reads the appconfig values from the database already implicitly chooses one of the values which is the same one as the over who we should keep during migration. That way the configuration stays the same and from the users perspective nothing had changed.

[bantu]

The code that reads the appconfig values from the database already implicitly chooses one of the values which is the same one as the over who we should keep during migration.

There is no guarantee that this selection is deterministic. Different queries may return different results. Internal database reorganisation may change returned values for the same query. So this strategy is basically the same as "pick a random value" but possibly with a higher chance of returning the correct value. I am fine with doing it like that.

[bantu]

Here is some pseudocode that might be helpful. I probably wouldn't invest more time to be more clever when deleting rows.

SELECT appid, configkey, COUNT(configvalue)
FROM oc_appconfig
GROUP BY appid, configkey
HAVING COUNT(configvalue) > 1

while ($row = fetchRow()) {
    // Use Appconfig to fetch value
    // Delete all rows
    // Insert single new row
}

[PVince81]

From what I saw while testing #7015, it seems that we already have an index on oc_appconfig in OC 6.
This means that this issue with duplicate keys can only happen when upgrading from OC 5.
I think we probably ask users to never upgrade directly from OC 5 to OC 7, so if we do want to implement a fix for this, it would have to go in OC 6.

Both OC 6 and OC 7 have an index in oc_appconfig, but not OC 5.

[karlitschek]

O.K. Closing for now.