Page MenuHomePhabricator

ipblocks_restrictions.ir_type is tinyint(1) in code but tinyint(4) in production
Closed, ResolvedPublic

Description

It wasn't a mistake during abstraction so I don't know what it is and why it's tinyint(4).

Affected wikis to be altered in production by @Marostegui :

alter table ipblocks_restrictions change column ir_type ir_type tinyint(4) NOT NULL;
  • s5
    • apiportalwiki
    • arbcom_ruwiki
    • avkwiki
    • jawikivoyage
    • lldwiki
    • thankyouwiki
    • smnwiki (T264900)
  • s3
    • arywiki
    • awawiki
    • banwiki
    • gcrwiki
    • gewikimedia
    • gomwiktionary
    • grwikimedia
    • hiwikisource
    • hywwiki
    • lijwikisource
    • liwikinews
    • minwiktionary
    • mnwwiki
    • napwikisource
    • ngwikimedia
    • nqowiki
    • punjabiwikimedia
    • shnwiki
    • shnwiktionary
    • shywiktionary
    • sysop_itwiki
    • szywiki
    • yuewiktionary
  • Patch code to make this tinyint(4) so new wikis are created with this, to be done by @Ladsgroup

Related Objects

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 13 2020, 5:33 AM
Marostegui triaged this task as Low priority.Oct 13 2020, 5:40 AM
Marostegui added a project: Data-Persistence.
Marostegui moved this task from Triage to Ready on the DBA board.

Internally tinyint(1) and tinyint(4) are the same for the values stored. It is just a visualization width indication, which doesn't affect anything else.
I will get it fixed to avoid the report being firing up!

So tinyint(4) happens on:

s1, s2, some wikis of s5 (the old ones, as s5 also holds some new ones), s6, s7, s8.

s3 is on going

s3 has tinyint(4) on all the wikis but the most recent ones, which is a total of 23 out of 905.
It is probably less time consuming (and way less risky) to change this on code, and only alter those affected 23 + a few on s5 wikis.

@Ladsgroup would this work for you?

Sure thing! I can apply it with the abstract schema change (since the schema of that table itself is abstracted now) to make the patch quick and easy ^_^

Marostegui moved this task from Ready to Blocked external/Not db team on the DBA board.
Marostegui added a project: Schema-change.

Excellent - thank you! Going to assign it to you for now then!

Restricted Application added a project: User-Ladsgroup. · View Herald TranscriptOct 13 2020, 8:26 AM

So tinyint(4) happens on:

s1, s2, some wikis of s5 (the old ones, as s5 also holds some new ones), s6, s7, s8.

s3 is on going

s2 was clean, my bad when checking.

Marostegui updated the task description. (Show Details)Oct 14 2020, 7:50 AM
Marostegui updated the task description. (Show Details)
Marostegui updated the task description. (Show Details)Oct 14 2020, 7:54 AM
Marostegui updated the task description. (Show Details)Oct 14 2020, 7:57 AM
Marostegui updated the task description. (Show Details)Oct 14 2020, 8:28 AM
Marostegui moved this task from Blocked to Done on the DBA board.

@Ladsgroup s3 and s5 are fixed.
The only pending thing is to push the patch to change this on code, so future wikis get the (4).

Marostegui moved this task from Next to Done on the Data-Persistence board.Oct 14 2020, 8:29 AM

Sure, I'm just waiting to finish the abstract schema change system merged so doing the patch would be several times easier. Hopefully soon.

LSobanski added a subscriber: LSobanski.

Unsubscribing DBA. Please add us back if you need any further assistance or are aware of new wikis created between the time of this comment and final code deployment.

Interesting trivia: MySQL is going to deprecate definition of length for numeric values: https://dev.mysql.com/worklog/task/?id=13127 We should stop defining them in the first place.

Mentioned in SAL (#wikimedia-operations) [2020-10-19T12:15:02Z] <marostegui> Deploy schema change on smnwiki T265321 T264900

Marostegui updated the task description. (Show Details)Oct 19 2020, 12:15 PM
Reedy added a comment.Oct 19 2020, 2:48 PM

Interesting trivia: MySQL is going to deprecate definition of length for numeric values: https://dev.mysql.com/worklog/task/?id=13127 We should stop defining them in the first place.

It's something we seemingly only do in a minority. At least in MW core, only on TINYINT, in two cases. 17 other tinyint in tables.sql don't do it

It's something we seemingly only do in a minority. At least in MW core, only on TINYINT, in two cases. 17 other tinyint in tables.sql don't do it

Not sure about this, but we could switch to BOOLEAN/BOOL in those cases where it would make sense. It is not a real type but maps to TINYINT(1). It could maybe useful for deprecation and semantically, but could cause other issues, so just mentioning as a possibility. Depends on how abstract schema handles it?

Change 641170 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/core@master] Increase size of ipblocks_restrictions.ir_type

https://gerrit.wikimedia.org/r/641170

Change 641170 merged by jenkins-bot:
[mediawiki/core@master] Increase size of ipblocks_restrictions.ir_type

https://gerrit.wikimedia.org/r/641170

Ladsgroup closed this task as Resolved.Mon, Nov 16, 5:45 PM
Maintenance_bot moved this task from Incoming to Done on the User-Ladsgroup board.Mon, Nov 16, 6:15 PM