> On 2 Jan 2026, at 23:07, Marcos Magueta <maguetamarcos@gmail.com> 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
againstnew schema - this feature would be invaluable. I can think of many different use cases. But I heard some
complaintsabout 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
isnot 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/regression.diffs
[2] https://cirrus-ci.com/task/5580601438240768
