čt 26. 12. 2024 v 14:46 odesílatel Jim Jones <jim.jo...@uni-muenster.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.steh...@gmail.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
