Re: [PATCH] Add pretty-printed XML output option

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>
Cc: Peter Smith <smithpb2250(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Nikolay Samokhvalov <samokhvalov(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andrey Borodin <amborodin86(at)gmail(dot)com>
Subject: Re: [PATCH] Add pretty-printed XML output option
Date: 2025-05-22 15:00:42
Message-ID: 1516164.1747926042@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> writes:
> On 22.05.25 01:48, Tom Lane wrote:
>> ... I considered adding an assertion that that call returns
>> NULL, but concluded that it wasn't worth the notational hassle.
>> I'm not strongly set on that conclusion, though, if you think
>> differently.

> I see. In that case I believe that at least a different comment
> explaining this decision would avoid confusion. Something like

Yeah, after sleeping on it I fear that leaving xml_parse entirely
alone will just be a recipe for future copy-and-paste errors.
The Assert solution seems like the way to go, approximately

xmlNodePtr root;
+ xmlNodePtr oldroot PG_USED_FOR_ASSERTS_ONLY;

...
/* This attaches root to doc, so we need not free it separately. */
- xmlDocSetRootElement(doc, root);
+ oldroot = xmlDocSetRootElement(doc, root);
+ /* ... and there can't yet be any old root to clean up. */
+ Assert(oldroot == NULL);

I'll make it so.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Sami Imseih 2025-05-22 15:04:02 Re: Relstats after VACUUM FULL and CLUSTER
Previous Message Tom Lane 2025-05-22 14:53:12 Re: Statistics Import and Export