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