Hi út 21. 1. 2025 v 11:45 odesílatel Umar Hayat <postgresql.wiz...@gmail.com> napsal:
> On Tue, 21 Jan 2025 at 04:04, Jim Jones <jim.jo...@uni-muenster.de> wrote: > > > > Hi Umar > > > > On 20.01.25 17:19, Umar Hayat wrote: > > > Hi Jim & Pavel, > > > Sorry for getting back a bit late on this. Few more case you might > > > need consider: > > > > > > As I mentioned in my first static review about XMLTable existing > > > behaviour might change, I give it a run time review and here are few > > > findings: > > > > > > 1. Because of this patch XMLTable namespace will accept NO DEFAULT > > > value, I was expecting an error message based on prior behavior '' but > > > If I run following query it show something different: > > > " > > >> 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: cache lookup failed for type 0 > > > " > > > Can you please re-check this behaviour (there might be something wrong > > > with my build environment) > > > > There is nothing wrong with your environment :) I forgot to test > > XMLTable for NO DEFAULT namespaces. To be consistent with the existing > > code, I added the following condition to transformRangeTableFunc: > > > > > > if (!r->name && !r->val) > > ns_uri = transformExpr(pstate, makeStringConst("", r->location), > > EXPR_KIND_FROM_FUNCTION); > > else > > ns_uri = transformExpr(pstate, r->val, EXPR_KIND_FROM_FUNCTION); > > > > > > It adds an empty string to the uri, which is the value expected for NO > > DEFAULT. > > > > I also added a NO DEFAULT test for XMLTable in xml.sql > > > > v5 attached. > > > > > 2. Seems like namespace as ColumnRef was already allowed (I doubt if > > > that's a correct implementation, I see other DB allows only strings > > > AFAIK), for non-default namespaces may be its fair, should this be > > > allowed for default namespace, any opinion. > > > create table tbl1 ( col1 text); > > > insert into tbl1 values ('abc'); > > > insert into tbl1 values ('def'); > > > select xmlelement(NAME "root", xmlnamespaces(default col1)) from tbl1; > > > xmlelement > > > --------------------- > > > <root xmlns="abc"/> > > > <root xmlns="def"/> > > > (2 rows) > > > > What are your concerns about supporting ColumnRef for the URI's? > > > > It is currently supported by XMLAttributes: > > > For XMLAttributes attribute it should have ColumnRef/Expr because > that's the data/content we want to generate. But namespaces and xml > tags, IMO they should be considered as part of the structure/schema of > XML. Allowing namespaces (default or otherwise) to be generated > arbitrarily for each record does not seem correct to me, it's like > generating arbitrary XML using print string which does not require XML > functions. > > - DB2 allows XMLElements namespace but it does not allow Expr/ColumnRef. > - Oracle Allow namespace in only XMLTable, and it does not allow > Expr/ColumnRef. > > - Having SConst/String or numeric can limit the error handling at > parsing stage which can validate the schema instead of expression > evaluation per record, which leads to following problem at runtime: > > CREATE TABLE xmltab (uri TEXT); > INSERT INTO xmltab VALUES ('good'), (''); > SELECT XMLElement(NAME "root", XMLNamespaces(uri AS zz)) from xmltab; > ERROR: invalid XML namespace URI for "zz" > DETAIL: a regular XML namespace cannot be a zero-length string > > Imagine there millions of records and in the middle it fails. > > - Also expression evaluation per record and doing validation (valid > URI etc) I believe will have some performance impact ( I haven't > tested it, so can't say for sure , how much ) > > - It's possible that there might be some other angle that I am missing > right now, maybe Pavel/Àlvaro can shed light on this. I briefly went > through the first implementation thread. There was a bit of discussion > to use b_expr instead of c_expr, but I have yet to explore the > reasoning of not using SConst. > > Postgres usually doesn't force string constants in functions or pseudofunctions arguments. I remember a discussion related to string_agg, where the field separator is not constant too, although it makes sense there. I think there are more cases - probably aggregate (maybe window) functions where postgres allows any expression and other databases allow just constants there. Personally, using only string constants can be sometimes too messy (when I use other db). Regards Pavel > > CREATE TABLE t AS SELECT 'http://x.y' AS uri; > > SELECT xmlelement(NAME el, xmlattributes("uri" AS att)) FROM t; > > > > xmlelement > > ------------------------ > > <el att="http://x.y"/> > > (1 row) > > > > > > Many thanks for the review! > > > > Best, Jim > > > > > > > > > > -- > > Jim > > > > -- > Umar Hayat > Bitnine (https://bitnine.net/) >
