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

Hello Heikki,

>> 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, and put a
> warning in the release notes that you should not be using the 10-digit isbn
> datatype as a column type anymore.
>
> As long as you don't use the isbn datatype in any tables or views, the
> upgrade script could just DROP the datatype, and recreate it. It's
> unfortunate that it won't work if you've used it in a view, but it seems
> acceptable. Could you write an upgrade script like that, please?

Probably I could. I'll have a look. It has to drop the type *and* some
casts I think. Should it cascade? Probably not, because people will no
like loosing their library data:-)

> In the long term, I think we should deprecate all the legacy "short"
> datatypes, isbn, issn, ismn and upc. As a replacement for display purposes,
> provide functions to print an ean13 in those legacy forms. I'm not even sure
> we need the isbn13, issn13 etc. datatypes, to be honest. A single ean13
> datatype and a function to check if an EAN is an ISBN, ISSN etc. should be
> enough. You could then use that in a CHECK constraint or create a domain if
> you really need to. Oh, and get rid of the "weak input mode" while we're at
> it - that's just horrible.

Hmmm. I like the principle of using a type to embed the constraint check,
and I*N13 types are fine, only the short types are indeed legacy and have
an issue.

> Perhaps we should add those functions now, so that you can rewrite your
> application to use them now, and we could remove the legacy datatypes in a
> future release.

I do not have an application, I just wrote the patch because I used these
types some time ago. Also, I'm not sure how to deprecate a type.

--
Fabien.