Re: ISN extension bug? (with patch)

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: ISN extension bug? (with patch)
Date: 2013-12-22 07:36:05
Message-ID: alpine.DEB.2.10.1312220806210.7034@sto
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


> sh> psql
> psql (9.3.2)
> Type "help" for help.
> # CREATE EXTENTION isn;
> # SELECT ISMN 'M123456782';
> M-1234-5678-5
> # SELECT ISMN 'M123456785';
> ERROR: invalid check digit for ISMN number: "M123456785", should be 2
> LINE 1: SELECT ISMN 'M123456785';

With the attached one liner patch compiled with pgxs:

# SELECT ISMN 'M123456785';
M-1234-5678-5

I'm not sure whether the policy is to update the version number of the
extension for such a change. As the library is always "isn.so", two
versions cannot live in parallel anyway. If it is useful, the second patch
attached also upgrade the version number.

Also, I notice that there is no test for this module, so I do not know
whether I've broken anything, or whether there are other simular bugs.
ISTM that it is not the case because I could test other types.

Because of the complexity of the code (there are embedded automata with
plenty of gotos and pointer arithmetic), I find this astoundingly unwise.
If the code was cleaner/simpler, it would just find it very unwise:-) Thus
I would suggest to add to some todo list: "check that all extensions have
regression tests, and add some if not."

--
Fabien.

Attachment Content-Type Size
ismn-checksum.patch text/x-diff 538 bytes
isn-1.1.patch text/x-diff 191.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2013-12-22 09:31:43 Re: ALTER SYSTEM SET command to change postgresql.conf parameters
Previous Message Tom Lane 2013-12-22 03:27:36 Re: SQL objects UNITs