review: xml_is_well_formed

Hello

I looked on patch
https://commitfest.postgresql.org/action/patch_view?id=334 .This patch
moves function xml_is_well_formed from contrib xm2 to core.

*  Is the patch in context diff format?
yes

* Does it apply cleanly to the current CVS HEAD?
yes

* Does it include reasonable tests, necessary doc patches, etc?
yes

*  Does the patch actually implement that?
yes

* Do we want that?
yes

* Do we already have it?
yes - simplified version in core

* Does it follow SQL spec, or the community-agreed behavior?
no - I didn't find any resources about conformance with SQL spec, but
it has same behave like original contrib function

* Does it include pg_dump support (if applicable)?
not related

* Are there dangers?
no

*Are there any assertion failures or crashes?

not found

I have a few issues:
* broken regress test (fedora 13 - xmllint: using libxml version 20707)

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@pavel-stehule ~]$ xmllint xxx
xxx:1: parser error : error parsing attribute name
<pg:foo xmlns:pg="http://postgresql.org/stuff";>bar</pg:foo>

* 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??

* 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.


Regards

Pavel Stehule

postgres=# select version();                                                      version

--------------------------------------------------------------------------------------------------------------
--------PostgreSQL 9.1devel on x86_64-unknown-linux-gnu, compiled by GCC gcc
(GCC) 4.4.4 20100630 (Red Hat 4.4.4-10),64-bit
(1 row)