Page MenuHomePhabricator

maintenance/tables.sql contains wrong indeces (compared to patch-pl-tl-il-unique-index.sql)
Closed, ResolvedPublic


For upgraders, patch-pl-tl-il-unique-index.sql has added these three UNIQUE indeces to the scheme:

  • il_to
  • pl_namespace
  • tl_namespace

For new installations, maintenance/tables.sql in contrast only contains normal (non-unique) indeces here.

Adjust either the one or the other!

I guess it would be correct to fix tables.sql to have the indeces UNIQUE also for new installations.

Event Timeline

Joergi123 raised the priority of this task from to Needs Triage.
Joergi123 updated the task description. (Show Details)
Joergi123 added a project: MediaWiki-General.
Joergi123 changed Security from none to None.
Joergi123 added a subscriber: Joergi123.
Aklapper renamed this task from tables.sql contains wrong indeces to maintenance/tables.sql contains wrong indeces (compared to patch-pl-tl-il-unique-index.sql).Dec 15 2014, 10:58 AM
Aklapper triaged this task as Medium priority.

What is the actual outcome of this problem and how severe is it?

Yes, for upgraders to 1.24.

As far as I see, the indeces can without problem be made unique: For each of these three indeces, there already are other indeces, which use exactly the same columns (in a different order, but the same columns). And these other indeces already are unique:

  • il_to => same columns as the unique index il_from
  • pl_namespace => same columns as the unique index pl_from
  • tl_namespace => same columns as the unique index tl_from

These indeces already are unique, so basically what we have in il_to, pl_namespace and tl_namespace will be unique anyway.

(Or with other words: Even if the three new indeces were not unique indeces, then what they contain would be unique anyway, as the other, already existing indeces take care that only unique stuff will be there. So making them unique, as done in the patch file, is correct. This is why I proposed not to fix this in the patch file, but in tables.sql.)

Thanks for digging out the history of this bug! This is more a cosmetical issue - I don't care, which of the two solutions you take. Just pick one. But if you ask me, I would make the index unique again. The content is unique so let's also call it unique. As pointed out this can be done without causing technical issues for the user. The overhead of the (unique) constraint compared to the non unique index is insignificant.

Aklapper lowered the priority of this task from Medium to Low.Feb 9 2015, 11:35 AM

Change 306895 had a related patch set uploaded (by PleaseStand):
Remove patch-pl-tl-il-unique.sql

Change 306895 merged by jenkins-bot:
Remove patch-pl-tl-il-unique.sql

PleaseStand claimed this task.