Re: review: xml_is_well_formed

Hello

2010/7/30 Mike Fowler <mike(at)mlfowler(dot)com>:
> Hi Pavel,
>
> Thanks for taking the time to review my patch. Attached is a new version
> addressing your concerns.
>
> On 29/07/10 14:21, Pavel Stehule wrote:
>>
>> I have a few issues:
>> * broken regress test (fedora 13 - xmllint: using libxml version 20707)

ok - main regress test is ok now, next I checked a contrib test for xml2

inside contrib/xml2 make installcheck, and there is a problem

SET client_min_messages = warning;
\set ECHO none
+ psql:pgxml.sql:10: ERROR: could not find function
"xml_is_well_formed" in file "/usr/local/pgsql.xwf/lib/pgxml.so"
+ psql:pgxml.sql:15: ERROR: could not find function
"xml_is_well_formed" in file "/usr/local/pgsql.xwf/lib/pgxml.so"
RESET client_min_messages;
select query_to_xml('select 1 as x',true,false,'');

you have to remove declaration from pgxml.sql.in and
uninstall_pgxml.sql, and other related files in contrib/xml2/ regress
test

>>
>> postgres=# SELECT xml_is_well_formed('<pg:foo
>> xmlns:pg="http://postgresql.org/stuff";>bar</pg:foo>');
>>  xml_is_well_formed
>> --------------------
>>  f
>> (1 row)
>>
>> this xml is broken - but in regress tests is ok
>>
>> [pavel(at)pavel-stehule ~]$ xmllint xxx
>> xxx:1: parser error : error parsing attribute name
>> <pg:foo xmlns:pg="http://postgresql.org/stuff";>bar</pg:foo>
>>
>
> This is a problem that was observered recently by Thom Brown - the commit
> fest webapp adds the semicolon after the quote. If you look at the
> attachment I sent in a email client you'll find there is no semicolon. The
> patch attached here will also not have the semicolon.
>

ok - attached patch is correct, ... please, can you remove a broken patch?

>> * xml_is_well_formed returns true for simple text
>>
>> postgres=# SELECT xml_is_well_formed('ssss');
>>  xml_is_well_formed
>> --------------------
>>  t
>> (1 row)
>>
>> it is probably wrong result - is it ok??
>>
>
> Yes this is OK, pure text is valid XML content.

It interesting for me - is somewhere some documentation about it?

My colleagues speak some else :)

http://zvon.org/comp/r/tut-XML.html#Pages~MinimalQ20XnumberQ20XofQ20Xelements
http://www.w3.org/TR/REC-xml/#sec-prolog-dtd

I am not a specialist on XML - so just don't know

>
>> * I don't understand to this fragment
>>
>>        PG_TRY();
>> +       {
>> +               size_t          count;
>> +               xmlChar    *version = NULL;
>> +               int                     standalone = -1;
>> +.
>> +               res_code = parse_xml_decl(string,&count,&version,
>> NULL,&standalone);
>> +               if (res_code != 0)
>> +                       xml_ereport_by_code(ERROR,
>> ERRCODE_INVALID_XML_CONTENT,
>> +                                                 "invalid XML
>> content: invalid XML declaration",
>> +                                                       res_code);
>> +.
>> +               doc = xmlNewDoc(version);
>> +               doc->encoding = xmlStrdup((const xmlChar *) "UTF-8");
>> +               doc->standalone = 1;
>>
>> why? This function can raise exception when declaration is wrong. It
>> is wrong - I think, this function should returns false instead
>> exception.
>>
>>
>
> You're quite right,  I should be returning false here and not causing an
> exception. I have corrected this in the attached patch.
>

ok

> Regards,
>
> --
> Mike Fowler
> Registered Linux user: 379787
>
>