Re: Add XMLNamespaces to XMLElement

From: Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>
To: Umar Hayat <postgresql(dot)wizard(at)gmail(dot)com>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add XMLNamespaces to XMLElement
Date: 2025-01-20 19:04:44
Message-ID: bdce670a-85ff-4ac5-84fb-fcd2c8bab09c@uni-muenster.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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:

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

Attachment Content-Type Size
v5-0001-Add-XMLNamespaces-option-to-XMLElement.patch text/x-patch 62.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Chapman Flack 2025-01-20 19:09:27 Re: XMLDocument (SQL/XML X030)
Previous Message Jacob Champion 2025-01-20 18:39:50 Re: [PATCH] Improve code coverage of network address functions