From: Tom Lane <tgl@sss.pgh.pa.us>
To: Jim Jones <jim.jones@uni-muenster.de>
cc: Peter Smith <smithpb2250@gmail.com>,
        Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
        Nikolay Samokhvalov <samokhvalov@gmail.com>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
        Andrey Borodin <amborodin86@gmail.com>
Subject: Re: [PATCH] Add pretty-printed XML output option
In-reply-to: <29ec1b21-1113-403c-bd00-b5009d9c4fdb@uni-muenster.de>
References: <2f5df461-dad8-6d7d-4568-08e10608a69b@uni-muenster.de>
 <fb2a543b-e615-d9ba-8402-909631f9b212@uni-muenster.de>
 <CAHut+Pvryt_+wA7TwKVaaF0e6Pm5LaPZQLKatp=78oFd8vMh3A@mail.gmail.com>
 <6cb296fe-4b49-3691-633e-2e2366997b04@uni-muenster.de>
 <3baf8f25-3bd7-7db5-0b31-e4c92b6269ba@enterprisedb.com>
 <1cbe1087-6acb-2b0a-dc9b-0a2d5b99e466@uni-muenster.de>
 <CAHut+PvBUTf9p+Y6+mpWpz59=3eL_EKQ+7Ysxpa9hYGdncDMdg@mail.gmail.com>
 <532259.1678393296@sss.pgh.pa.us> <906cfd1d-63a0-3cf5-7291-2dadb2e!
 149ff@uni-muenster.de> <803304.1678458776@sss.pgh.pa.us> <efe0b19b-!
 d41c-f8ab-f3b8-afb0108f3706@uni-muenster.de>
 <2752578.1678815625@sss.pgh.pa.us>
 <abd25443-ef6d-7b8a-c593-a2a991d3e5ce@uni-! ! muenster.de>
 <3057762.1678836321@sss.pgh.pa.us> <3622266.1678914816@sss.pgh.pa.us>
 <1c7cf9f0-14c3-0341-6894! -8ca577bf325e@uni-muenster.de>
 <1358967.1747858817@sss.pgh.pa.us> <!
 552cf981-643e-417b-88f4-1476c1320c36@uni-muenster.de>
 <1418827.1747871285@sss.pgh.pa.us> <29ec1b21-1113-403c-bd00-b5009d9c!
 4fdb@uni-muenster.de>
Comments: In-reply-to Jim Jones <jim.jones@uni-muenster.de>
	message dated "Thu, 22 May 2025 10:42:46 +0200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1516163.1747926042.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Thu, 22 May 2025 11:00:42 -0400
Message-ID: <1516164.1747926042@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1516164.1747926042%40sss.pgh.pa.us>
Precedence: bulk

Jim Jones <jim.jones@uni-muenster.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 separatel=
y. */
-           xmlDocSetRootElement(doc, root);
+           oldroot =3D xmlDocSetRootElement(doc, root);
+           /* ... and there can't yet be any old root to clean up. */
+           Assert(oldroot =3D=3D NULL);

I'll make it so.

			regards, tom lane