Re: [PATCH] Add XMLText function (SQL/XML X038) - Mailing list pgsql-hackers

From Vik Fearing
Subject Re: [PATCH] Add XMLText function (SQL/XML X038)
Date
Msg-id 00ec9799-aaa6-9acf-ccad-77bb237a8f8f@postgresfriends.org
Whole thread Raw
In response to Re: [PATCH] Add XMLText function (SQL/XML X038)  (Jim Jones <jim.jones@uni-muenster.de>)
Responses Re: [PATCH] Add XMLText function (SQL/XML X038)
Re: [PATCH] Add XMLText function (SQL/XML X038)
List pgsql-hackers
On 8/25/23 14:42, Jim Jones wrote:
> Hi Vik
> 
> Thanks for reviewing my patch!

Thank you for writing it!

> On 25.08.23 12:05, Vik Fearing wrote:
>> I am replying to this email, but my comments are based on the v2 patch.
>>
>> Thank you for working on this, and I think this is a valuable 
>> addition. However, I have two issues with it.
>>
>> 1) There seems to be several spurious blank lines added that I do not 
>> think are warranted.
> 
> I tried to copy the aesthetics of other functions, but it seems I failed 
> :) I removed a few blank lines. I hope it's fine now.

I am talking specifically about this:

@@ -505,6 +506,10 @@ xmlcomment(PG_FUNCTION_ARGS)
      appendStringInfoText(&buf, arg);
      appendStringInfoString(&buf, "-->");

+
+
+
+
      PG_RETURN_XML_P(stringinfo_to_xmltype(&buf));
  #else
      NO_XML_SUPPORT();


>> 2) This patch does nothing to address the <XML returning clause> so we 
>> can't claim to implement X038 without a disclaimer.  Upon further 
>> review, the same is true of XMLCOMMENT() so maybe that is okay for 
>> this patch, and a more comprehensive patch for our xml features is 
>> necessary.
> 
> If we decide to not address this point here, I can take a look at it and 
> work in a separated patch.

I do not think this should be addressed in this patch because there are 
quite a lot of functions that need to handle this.
-- 
Vik Fearing




pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: initdb caching during tests
Next
From: "Imseih (AWS), Sami"
Date:
Subject: Re: pg_stat_get_backend_subxact() and backend IDs?