public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andrey Borodin <[email protected]>
To: Marcos Magueta <[email protected]>
Cc: Kirill Reshke <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: WIP - xmlvalidate implementation from TODO list
Date: Sun, 4 Jan 2026 14:46:06 +0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CAN3aFCdx8AapWSVpJ1kaC7OC_v7QwbjgbGw9WfPBBY2GMyOadQ@mail.gmail.com>
<CALdSSPjxLU+zhWx+CgwN+VHoHTso33trY6mse1A6Jks7hWAdrA@mail.gmail.com>
<CAN3aFCesNDiL-iZg4imC0n+NgT3JywqZYkuGH83u8ssLjJ-p5Q@mail.gmail.com>
<CAN3aFCfvVgXr77o=dB_E2kSCY+EgckSQbSBdd_N9n-LauWuQLw@mail.gmail.com>
<CAN3aFCcx_w5Ldb+SYurwd31es9hOJqLuKARQHHDOk7+5iOqBWQ@mail.gmail.com>
<CALdSSPhFzYCp=Aa8AAboz6TQaTmjWciQGfrEJQeOOO+0pD1GGw@mail.gmail.com>
<[email protected]>
> On 2 Jan 2026, at 23:07, Marcos Magueta <[email protected]> wrote:
>
> About the patch, let me know if you find the time to review!
I was looking to review something on commitfest and decided to look into this patch.
Unfortunately, I cannot verify adherence to SQL standard. But I'll take it as granted, grammar changes are minimal.
I'm not a big XML user, but definitely there are a lot of use cases. E.g. If someone want to check whole database against new schema - this feature would be invaluable. I can think of many different use cases. But I heard some complaints about libxml. I'm not sure, but maybe at some point we would like to get rid of it? [0]
The patch fails regression tests on Windows. See [1]. Regression taken from [2]
Meaningfull part is:
diff --strip-trailing-cr -U3 C:/cirrus/src/test/regress/expected/xml_1.out C:/cirrus/build/testrun/regress/regress/results/xml.out
--- C:/cirrus/src/test/regress/expected/xml_1.out 2026-01-03 19:13:07.092850000 +0000
+++ C:/cirrus/build/testrun/regress/regress/results/xml.out 2026-01-03 19:17:23.497562500 +0000
@@ -1496,3 +1496,278 @@
LINE 1: SELECT xmltext('x'|| '<P>73</P>'::xml || .42 || true || 'j':...
^
DETAIL: This functionality requires the server to be built with libxml support.
+SELECT xmlvalidate(DOCUMENT '<person><name>John</name><age>30</age></person>'
+ ACCORDING TO XMLSCHEMA '<?xml version="1.0"?>
....
So you need to update src/test/regress/expected/xml_1.out for systems without libxml.
You use PG_TRY(); and return from that block. I found no other cases of returning without PG_END_TRY(), looks like it is not supported.
xmloption is document_or_content. But xmlvalidate_text_schema() always validates as a document. IDK, maybe it's correct, or maybe it works by accident.
+#else
+ NO_XML_SUPPORT();
+ return NULL;
+#endif
This NULL is returned from bool function xmlvalidate_text_schema(). I know it's unreachable, but let's return false or true.
Also, single-line comments worth converting to our usual C comments. The patch could benefit from pgindent.
That's all what I could find for now. Thanks for working on this!
Best regards, Andrey Borodin.
[0] https://www.postgresql.org/message-id/flat/aUK8aBluNzMZTatU%40momjian.us
[1] https://api.cirrus-ci.com/v1/artifact/task/5580601438240768/testrun/build/testrun/regress/regress/re...
[2] https://cirrus-ci.com/task/5580601438240768
view thread (10+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: WIP - xmlvalidate implementation from TODO list
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox