Skip site navigation (1) Skip section navigation (2)

review: xml_is_well_formed

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: mike(at)mlfowler(dot)com
Subject: review: xml_is_well_formed
Date: 2010-07-29 13:21:55
Message-ID: AANLkTikTru1-KBjW3xQ9OSG2h76ZOVMB-qfms__atFTC@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
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(at)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)

Responses

pgsql-hackers by date

Next:From: Tom LaneDate: 2010-07-29 14:03:07
Subject: Re: [GENERAL] Incorrect FTS result with GIN index
Previous:From: Robert HaasDate: 2010-07-29 13:10:27
Subject: Re: page corruption on 8.3+ that makes it to standby

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group