Re: WIP - xmlvalidate implementation from TODO list

On 14/01/2026 02:23, Marcos Magueta wrote:
> Please follow the updated version attached.

A few comments:

== grammar ==

In gram.y you restricted CREATE XMLSCHEMA to Sconst:

DO $$
DECLARE xsd text := '<foo></foo>';
BEGIN
CREATE XMLSCHEMA person_schema AS xsd;
END $$;
ERROR: syntax error at or near "xsd"
LINE 4: CREATE XMLSCHEMA person_schema AS xsd;

Any particular reason for that? If not, take a look at other options,
e.g. a_expr

== pg_xmlschema ==

Why did you choose text over xml for schemadata?

postgres=# \d pg_xmlschema
Table "pg_catalog.pg_xmlschema"
Column | Type | Collation | Nullable | Default
-----------------+-----------+-----------+----------+---------
oid | oid | | not null |
schemaname | name | | not null |
schemanamespace | oid | | not null |
schemaowner | oid | | not null |
schemadata | text | C | not null |
schemaacl | aclitem[] | | |
Indexes:
"pg_xmlschema_oid_index" PRIMARY KEY, btree (oid)
"pg_xmlschema_name_nsp_index" UNIQUE CONSTRAINT, btree (schemaname,
schemanamespace)

== psql command to display and list xml schemas ==

Not a requirement for this patch (specially not at the current stage),
but you should add it to your TODO list.

\dz
\dz foo
\dz+ foo

* z here is just an example

== tab completion ==

CREATE <TAB> should suggest XMLSCHEMA and CREATE XML<TAB> should
autocomplete CREATE XMLSCHEMA. The same applies for DROP XMLSCHEMA [IF
EXISTS], where it should additionally list the available schemas after
DROP XMLSCHEMA <TAB>.

== white-space warnings ==

The patch does not apply cleanly:

/home/jim/patches/xmlvalidate/0002-xmlschema-catalog-and-xmlvalidate.patch:594:
indent with spaces.
Oid schemanamespace,
/home/jim/patches/xmlvalidate/0002-xmlschema-catalog-and-xmlvalidate.patch:595:
indent with spaces.
Oid schemaowner,
/home/jim/patches/xmlvalidate/0002-xmlschema-catalog-and-xmlvalidate.patch:596:
indent with spaces.
const char *schemadata,
/home/jim/patches/xmlvalidate/0002-xmlschema-catalog-and-xmlvalidate.patch:597:
indent with spaces.
bool if_not_exists,
/home/jim/patches/xmlvalidate/0002-xmlschema-catalog-and-xmlvalidate.patch:598:
indent with spaces.
bool quiet)
warning: squelched 139 whitespace errors
warning: 144 lines add whitespace errors.

== file naming ==

Your patch suggests that it is part of a patch set, from which 0001 is
missing. In case you meant a version 2 of the previous patch, a better
format would be

v2-0001-xmlschema-catalog-and-xmlvalidate.patch

which can be generated with

$ git format-patch -1 -v2

== xml_1.out not updated ==

After every change in xml.sql you must create an equivalent file for a
postgres compiled without --with-libxml, and put the changes in
xml_1.out.[1]

== corrupt pg_dump ==

I understand we agreed to work on XMLVALIDATE only after CREATE
XMLSCHEMA is settled, but since the code is partially already there, you
might wanna take a look at pg_dump. It is not serialising the CREATE
XMLSCHEMA statements:

$ /usr/local/postgres-dev/bin/psql postgres
psql (19devel)
Type "help" for help.

postgres=# CREATE XMLSCHEMA person_schema AS '<?xml version="1.0"?>
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema">
<xs:element name="person">
<xs:complexType>
<xs:sequence>
<xs:element name="name" type="xs:string"/>
<xs:element name="age" type="xs:integer"/>
</xs:sequence>
</xs:complexType>
</xs:element>
</xs:schema>';
CREATE XMLSCHEMA
postgres=# CREATE VIEW v AS
SELECT XMLVALIDATE(DOCUMENT '<bar></bar>'::xml
ACCORDING TO XMLSCHEMA person_schema);
CREATE VIEW
postgres=# \q

$ /usr/local/postgres-dev/bin/pg_dump postgres
--
-- PostgreSQL database dump
--

\restrict WLaIQWmNJVW2yc4Jv5W81qh2TZGHnupJlpl4Urm4Pp6Ku3VPyH5dO3ReFc4LMmd

-- Dumped from database version 19devel
-- Dumped by pg_dump version 19devel

SET statement_timeout = 0;
SET lock_timeout = 0;
SET idle_in_transaction_session_timeout = 0;
SET transaction_timeout = 0;
SET client_encoding = 'UTF8';
SET standard_conforming_strings = on;
SELECT pg_catalog.set_config('search_path', '', false);
SET check_function_bodies = false;
SET xmloption = content;
SET client_min_messages = warning;
SET row_security = off;

--
-- Name: test_xmlschema_ns; Type: SCHEMA; Schema: -; Owner: jim
--

CREATE SCHEMA test_xmlschema_ns;

ALTER SCHEMA test_xmlschema_ns OWNER TO jim;

--
-- Name: v; Type: VIEW; Schema: public; Owner: jim
--

CREATE VIEW public.v AS
SELECT XMLVALIDATE(DOCUMENT '<bar></bar>'::xml ACCORDING TO XMLSCHEMA
person_schema) AS "xmlvalidate";

ALTER VIEW public.v OWNER TO jim;

--
-- PostgreSQL database dump complete
--

\unrestrict WLaIQWmNJVW2yc4Jv5W81qh2TZGHnupJlpl4Urm4Pp6Ku3VPyH5dO3ReFc4LMmd

Take a look at pg_dump.c. You might need a new function, e.g.
dumpXmlSchemas(Archive *fout, const SchemaInfo *schemaInfo)

== patch structure ==

To make the review a bit easier, I suggest to split this patch into a
patch set with **at least 4** smaller patches - the more seasoned
hackers here might correct me if I am wrong. For instance:

0001 - CREATE XMLSCHEMA (code + tests + documentation)
0002 - pg_dump changes to output CREATE XMLSCHEMA
0003 - psql tab completion + new command to display and list xml schemas
0004 - XMLVALIDATE (code + tests + documentation)

Thanks for working on this!

Best, Jim

[1] https://cirrus-ci.com/task/4872456290172928?logs=check_world#L126