Re: Add XMLNamespaces to XMLElement

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Umar Hayat <postgresql(dot)wizard(at)gmail(dot)com>
Cc: Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add XMLNamespaces to XMLElement
Date: 2025-01-21 13:14:59
Message-ID: CAFj8pRAwHqO3HFMHVH3CmqavYZ-ckaD2+jQ9h3MqRstCbQcPoA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi

út 21. 1. 2025 v 11:45 odesílatel Umar Hayat <postgresql(dot)wizard(at)gmail(dot)com>
napsal:

> On Tue, 21 Jan 2025 at 04:04, Jim Jones <jim(dot)jones(at)uni-muenster(dot)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/)
>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Álvaro Herrera 2025-01-21 13:55:47 Re: Bug in detaching a partition with a foreign key.
Previous Message Marcos Pegoraro 2025-01-21 12:20:39 Year of first commit