"Jeremy Kronuz" <kronuz@hotmail.com> writes:
> Tom, I've checked the version in the cvs and I had made significant changes
> from that version.
Hm, it sounds like I guessed wrong about which version was newer ... is
there something flaky about your machine's system clock? The file
timestamps in the two tarballs definitely pointed the other way.
> I fixed some major bugs that prevented some ISBN numbers
> from working. I've merged the changes made for the cvs with my own, hoping I
> didn't miss anything that was already fixed in the cvs... I noticed you
> changed some log messages and that you added GET_STR and GET_TEXT. indeed
> there was that thing that took me some time to figure in both
> ean13_cast_to_text() and isn_cast_to_text() functions, not changing to a
> valid TEXT the return value; but I did not know about the other
> *_cast_from_text() functions having the problem of the missing GET_STR() (I
> was using VARDATA()).
Yeah, your to/from text functions simply did not work --- the VARDATA
hack for example presumes that the data part of a text value is
null-terminated, which it isn't. In any case it seems pretty likely
that the content of TEXT datums will change someday to allow better
charset/collation support, so you really are better off calling
textin/textout if possible rather than assuming you know the
representation of type TEXT. I copied the GET_STR/GET_TEXT macros
from some other contrib module ... you're free to do it differently
if you want, but in any case you have to deal with the fact that TEXT
is *not* a C-string.
As for the log message changes, there's room for debate about that, but
the way you had it struck me as far too chatty. The use-case for weak
checking is that you have a large pile of data you don't want to
validate right now, correct? So why would you want a warning for every
single bad datum during the import? Also, all the fancy formatting in
that one big warning was directly against our message style guidelines.
regards, tom lane
