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
Views: Raw Message | Whole Thread | Download mbox | Resend email
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

Browse pgsql-hackers by date

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