Re: Validating problem in the isn contrib module

On Fri, 06 Mar 2009 00:32:40 +0100 Bernd Helmle wrote:

--On Freitag, März 06, 2009 02:26:12 +0100 Andreas 'ads' Scherbaum
<adsmail@wars-nicht.de> wrote:

test=# select is_valid('978-3-937514-69-6'::isbn13);
ERROR: invalid check digit for ISBN number: "978-3-937514-69-6",
should be 7 ROW 1: select is_valid('978-3-937514-69-6'::isbn13);

According to the docs, this is intended behavior. If you want to validate
those values later or yourself, you have to use the weak mode:

No.
Straight from the source:

-- isn_weak(boolean) - Sets the weak input mode.
-- This function is intended for testing use only!

The validator function should use the weak mode for itself to return
'f' in case of invalid input. It cannot be the users job to make sure a
validator function is working as expected.

Bye

--
Andreas 'ads' Scherbaum
German PostgreSQL User Group
European PostgreSQL User Group - Board of Directors

Andreas 'ads' Scherbaum <adsmail@wars-nicht.de> writes:

The validator function should use the weak mode for itself to return
'f' in case of invalid input. It cannot be the users job to make sure a
validator function is working as expected.

Well the input function is being called before the validator function ever
gets a chance to act. The validator function receives an already-formed isbn13
datum.

Is there an is_valid() which takes text input? Perhaps there should be --
though I think it would have to be isbn_is_valid() or something like that.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's RemoteDBA services!

On Fri, 06 Mar 2009 10:50:41 +0000 Gregory Stark wrote:

Andreas 'ads' Scherbaum <adsmail@wars-nicht.de> writes:

The validator function should use the weak mode for itself to return
'f' in case of invalid input. It cannot be the users job to make sure a
validator function is working as expected.

Well the input function is being called before the validator function ever
gets a chance to act. The validator function receives an already-formed isbn13
datum.

That's the problem.

Is there an is_valid() which takes text input? Perhaps there should be --
though I think it would have to be isbn_is_valid() or something like that.

I think that's the intention of the is_valid() function. However it's
not working. And yes, a moe descriptive name would be fine.

Bye

--
Andreas 'ads' Scherbaum
German PostgreSQL User Group
European PostgreSQL User Group - Board of Directors

--On Freitag, März 06, 2009 11:32:14 +0100 Andreas 'ads' Scherbaum
<adsmail@wars-nicht.de> wrote:

No.
Straight from the source:

-- isn_weak(boolean) - Sets the weak input mode.
-- This function is intended for testing use only!

The validator function should use the weak mode for itself to return
'f' in case of invalid input. It cannot be the users job to make sure a
validator function is working as expected.

I don't see anything that's not already documented. is_valid() checks the
presence of the invalid (!) marker only. It looks like the author never
intended is_valid() to be a "check wether this ISBN is semantically
correct", this is done by the input routines before.

I agree that the naming is a little bit misleading.

--
Thanks

Bernd

On Fri, 06 Mar 2009 12:44:31 +0100 Bernd Helmle wrote:

--On Freitag, März 06, 2009 11:32:14 +0100 Andreas 'ads' Scherbaum
<adsmail@wars-nicht.de> wrote:

I don't see anything that's not already documented. is_valid() checks the
presence of the invalid (!) marker only. It looks like the author never
intended is_valid() to be a "check wether this ISBN is semantically
correct", this is done by the input routines before.

So this function is useless. If the syntax is valid, the check is
already done and is_valid() returns true. If the syntax is invalid, PG
will raise an error even before this function returns. The invalid
marker is not checked at all.

This leads back to my initial question: intended behaviour or just a
bug? And how can one validate an ISBN without raising an error?

Bye

--
Andreas 'ads' Scherbaum
German PostgreSQL User Group
European PostgreSQL User Group - Board of Directors

Andreas 'ads' Scherbaum <adsmail@wars-nicht.de> writes:

So this function is useless. If the syntax is valid, the check is
already done and is_valid() returns true. If the syntax is invalid, PG
will raise an error even before this function returns. The invalid
marker is not checked at all.

This seems pretty clearly explained in the documentation.

When you insert invalid numbers in a table using the weak mode, the number
will be inserted with the corrected check digit, but it will be displayed with
an exclamation mark (!) at the end, for example 0-11-000322-5!. This invalid
marker can be checked with the is_valid function and cleared with the
make_valid function.

You can also force the insertion of invalid numbers even when not in the weak
mode, by appending the ! character at the end of the number.

This leads back to my initial question: intended behaviour or just a
bug? And how can one validate an ISBN without raising an error?

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's 24x7 Postgres support!

"Andreas 'ads' Scherbaum" <adsmail@wars-nicht.de> writes:

This leads back to my initial question: intended behaviour or just a
bug? And how can one validate an ISBN without raising an error?

Judging from the comments, is_valid (and the internal validity bit)
were a bad design decision that the author later regretted, but felt
he couldn't change for compatibility reasons. I'm not sure why not
... we make bigger incompatible changes than that all the time.

The way to validate an ISBN is exactly the same as it is for every
other data type: feed the string to the input function and see if
it throws an error.

regards, tom lane

On Fri, 06 Mar 2009 10:27:52 -0500 Tom Lane wrote:

Judging from the comments, is_valid (and the internal validity bit)
were a bad design decision that the author later regretted, but felt
he couldn't change for compatibility reasons. I'm not sure why not
... we make bigger incompatible changes than that all the time.

Looks a bit ugly and the way this module handles the input is unusual.

The way to validate an ISBN is exactly the same as it is for every
other data type: feed the string to the input function and see if
it throws an error.

For the record here's a function which validates a text if it contains
an ISBN-13, similar functions are possible for the other datatypes
defined by isn:

CREATE OR REPLACE FUNCTION validate_isbn13(TEXT)
RETURNS BOOLEAN
AS $$
DECLARE
isbn_nr ALIAS FOR $1;
weak_status BOOLEAN;
isbn_status BOOLEAN;
BEGIN

-- make sure weak mode is off
weak_status := isn_weak(FALSE);
-- this will either return 'true' or throw an exception
isbn_status := is_valid(isbn_nr::isbn13);
weak_status := isn_weak(weak_status);

RETURN isbn_status;

EXCEPTION
-- handle (only) the exception which is thrown by is_valid()
WHEN invalid_text_representation THEN
RETURN false;
END;
$$
LANGUAGE 'plpgsql';

Bye

--
Andreas 'ads' Scherbaum
German PostgreSQL User Group
European PostgreSQL User Group - Board of Directors