Re: Add XMLNamespaces to XMLElement

Hi Umar, hi Pavel,

Thanks for taking a look at this patch!

On 26.12.24 05:15, Umar Hayat wrote:
> Hi,
> +1 for the enhancement.
>
> I haven't compiled and reviewed the full patch yet, please see a few
> comments from my side based on static analysis.
>
> 1. Though changes are targeted for XMLNAMESPACES for XMLElement but in
> my opinion it will affect XMLTABLE as well because the
> 'xml_namespace_list' rule is shared now.
> Adding 'NO DEFAULT' in xml_namespace_list will allow users to use it
> with XMLTABLE XMLNAMESPACES as well.PostgreSQL grammar allow to
> specify DEFAULT in NAMESPACE but resulting in following error:
> "ERROR:  DEFAULT namespace is not supported"

I also considered creating a new rule to avoid any conflict with
XMLTable, but as it didn't break any regression test and the result
would be pretty much the same as with "DEFAULT 'str'", I thought that
extending the existing rule would be the way to go.

SELECT * FROM XMLTABLE(XMLNAMESPACES(NO DEFAULT),
                      '/rows/row'
                      PASSING '<rows
xmlns="http://x.y"><row><a>10</a></row></rows>'
                      COLUMNS a int PATH 'a');
ERROR:  DEFAULT namespace is not supported

What do you think?


> What would be behavior with this change for XMLTABLE, should this be
> allowed and the error messages need to be updated (may be this will
> not be an error at all) or we need to restrict users to not use 'NO
> DEFAULT' with XMLTable.


Perhaps updating the error message would suffice?


>
> 2. Should we reuse the 'xml_namespaces' rule for XMLTable, as the
> definition is the same.


That would be good. I'm just afraid it would deviate a bit from the
scope of this patch - here I mean touching other function. Would you
suggest to add it to a patch series?

> 3. In this patch 'NO DEFAULT' behavior is like DEFAULT '<blank>'
> (empty uri) , should not it be more like 'DEFAULT NULL' to result in
> the following ?
> SELECT xmlelement(NAME "root", xmlnamespaces(NO DEFAULT));
>     xmlelement
> ------------------
> <root/>
>
> instead of
>
> SELECT xmlelement(NAME "root", xmlnamespaces(NO DEFAULT));
>     xmlelement
> ------------------
> <root xmlns=""/>
>
The idea of NO DEFAULT is pretty much to free an element (and its
children) from a previous DEFAULT in the same scope.

SELECT
  xmlserialize(DOCUMENT
    xmlelement(NAME "root",
      xmlnamespaces(DEFAULT 'http:/x.y/ns1'),
      xmlelement(NAME "foo",
        xmlnamespaces(NO DEFAULT))
  ) AS text INDENT);

         xmlserialize         
------------------------------
 <root xmlns="http:/x.y/ns1">+
   <foo xmlns=""/>           +
 </root>
(1 row)


I believe this behaviour might be confusing if NO DEFAULT is used in the
root element, as it there is no previously declared namespace. Perhaps
making NO DEFAULT behave like DEFAULT NULL only in the root element
would make things clearer? The SQL/XML spec doesn't say anything
specific about it, but DB2 had the same thought[1]. For reference, here
are the regress tests[2] of this patch tested with the DB2 implementation.

> On Sat, 21 Dec 2024 at 14:57, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> +1
>>
>> Pavel

rebase in v2 attached - due to changes in gram.y

Thanks a lot

Best, Jim

1 - https://dbfiddle.uk/0QsWlfZR
2 - https://dbfiddle.uk/SyiDfXod