Re: BUG #13442: ISBN doesn't always roundtrip with text

Mon, 27 Jul 2015 11:15:17 +0200 (CEST)
Hello Heikki,

>> Attached patch makes isn to distinguish when needed between ISBN and
>> ISBN13. ISBN is the legacy ISBN10 number which is part of ISBN13, but not
>> all ISBN13 numbers are ISBN.
>>
>> The initial version was confusing both types, leading to accept valid
>> ISBN13 number as valid ISBN(10) number although they use the 979 prefix,
>> hence the reported issues.
>>
>> The patch also adds some tests.
>>
>> There is no clear upgrade path from 1.0 to 2.0, as the ISBN "in" function
>> is changed, as well as some casts.  I'm not sure of a safe way to upgrade,
>> because the initial version was accepting invalid ISBN which would now be
>> rejected, thus some store data may be considered corrupted in a database.
>> On the other hand, probably few people would use a legacy ISBN type and
>> put ISBN13 number in them, hopefully? Hmmm...
>
> 10-digit ISBNs are indeed legacy, I don't have much sympathy for an
> application that still uses the 10-digit isbn datatype in a table. The cast
> might well be used for display purposes, though. You might do something like
> "SELECT isbn_column as bisbn13, isbn_column::isbn as isbn FROM ...". So I
> think your patch is OK, and we should provide an upgrade script,

After some tests, it seems that an upgrade script cannot be provided,
or I missed something.

Basically version 1.0 was confusing ISBN & ISBN13, and for upgrading the
ISBN type definition must be modified. However, there is no such thing as
ALTER TYPE, and trying DROP TYPE/ALTER TYPE is rejected because the type
belongs to an extension (hey I know that, this is what is being done!).

Probably this is some kind of feature/bug of the extension mecanism, that
I'm not planing to fix for isn...

So the v2 attached provides the two versions (1.0 more or less bug
compatible, 2.0 without the confusion bug), without path for an upgrade
between the two, can't help it. Maybe providing a 1.0 buggy version is a
bad idea, though.

> and put a warning in the release notes that you should not be using the
> 10-digit isbn datatype as a column type anymore.

A warning would be a good think.

--
Fabien.