gah, of course, the new wikis are effected too for b/c. It can't just insert NULL since that would fail if it's not NULLable on some wiki.
The \0* -> NULL part may as well be run as a script later...since there will be intermediate bad values added after the patch is applied anyway.
Tim Starling-2 wrote:
On 02/03/11 16:57, Aaron Schulz wrote:
It would be nice if FlaggedRevs/archives/patch-fi_img_timestamp.sql was run on the wikis created before the patch.
The schema is broken before and after that patch. FlaggedRevision::insertOn() inserts an empty string into fi_img_timestamp instead of null, for "backwards compatibility". If you insert an empty string into a char(14), the effect is the same as a "char(14) not null": you get 14 spaces.
With a UTF-8 character set, this is not a problem, because trailing spaces are automatically trimmed from char(14) fields. But the default character set on Wikimedia is binary, which means that char(14) gets silently converted to binary(14). When you insert an empty string into a binary(14), you get 14 null characters. And null characters are not trimmed automatically.
It's the same problem as in patch-user_last_timestamp.sql, which I complained about on CR r33520 recently. You should have followed my lead from r23239 and used varbinary(14) for timestamp fields in cases where there is the slightest chance of an empty string being needed. A single byte of overhead for a length field is hardly a large price to pay for sensible application-level behaviour. I'm pretty sure making the field nullable has the same overhead.
If you fix both patches in the next 24 hours, I can include them both in the schema change batch.
-- Tim Starling
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l