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
