Re: Add XMLNamespaces to XMLElement

čt 26. 12. 2024 v 14:46 odesílatel Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>
napsal:

> 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.
>

You can check Oracle too.

>
> > On Sat, 21 Dec 2024 at 14:57, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> >> +1
> >>
> >> Pavel
>
> rebase in v2 attached - due to changes in gram.y
>
>
I checked this patch

The parser part looks a little bit dirty - it multiplies numbers of
XMLELEMENT rules. Maybe xmlattributes and xml_namespaces can be processed
elsewhere like list of xml_element_options?

Regards

Pavel

> Thanks a lot
>
> Best, Jim
>
> 1 - https://dbfiddle.uk/0QsWlfZR
> 2 - https://dbfiddle.uk/SyiDfXod