MIME-Version: 1.0
References: <19382-4c2060ffee72759b@postgresql.org> <CAOVWO5rVBKsjG4YwO_PJQu2OBGp8qUdF1jineYY6Lm3zc6-KWQ@mail.gmail.com>
 <CAOVWO5rbwKgHWLYJMvKuvGxW9eFSk7LADk=ZxDEvwA1uTefvAg@mail.gmail.com>
 <CAOVWO5qN8UXdwMnDa+a7aVq=irGSfm2aeYvEc-uV6j4QHZiyrA@mail.gmail.com>
 <CAOVWO5py4zLPYsnGK1EEzHnC4feTSmbsEZn-BiDgY_=J1P6Wmw@mail.gmail.com>
 <CAOVWO5pbgCVx0zgTr1mxZug2hoGwxZOk+-Owvwg0jaQv9JE3Fw@mail.gmail.com>
 <CAOVWO5r19cctAFKbW24jfMKsD-pkyV21w+z7L3pCPxM1CArtjQ@mail.gmail.com>
 <CAOVWO5oSeBouPv0ueVByh+_6EgRCjWh0spSmnF6Cv-TF1twqKg@mail.gmail.com> <tencent_04DAF0C76A6A76A8C7B48212F2362C818A09@qq.com>
In-Reply-To: <tencent_04DAF0C76A6A76A8C7B48212F2362C818A09@qq.com>
From: surya poondla <suryapoondla4@gmail.com>
Date: Fri, 20 Mar 2026 11:16:16 -0700
Message-ID: <CAOVWO5oH37CETZuxxXw3dhCMOHPMA0xFoJBWTpfJ06OV7sGzTQ@mail.gmail.com>
Subject: Re: BUG #19382: Server crash at __nss_database_lookup
To: songjinzhou <tsinghualucky912@foxmail.com>
Cc: dllggyx <dllggyx@outlook.com>, pgsql-bugs <pgsql-bugs@lists.postgresql.org>
Content-Type: multipart/alternative; boundary="000000000000124e65064d78af37"
Archived-At: <https://www.postgresql.org/message-id/CAOVWO5oH37CETZuxxXw3dhCMOHPMA0xFoJBWTpfJ06OV7sGzTQ%40mail.gmail.com>
Precedence: bulk

--000000000000124e65064d78af37
Content-Type: text/plain; charset="UTF-8"

Hi Songjinzhou,

Thank you for reviewing the patch.

You're correct that there is a minor inefficiency, once a
tupDesc_identifier version mismatch is detected, the patch allocates
new_values/new_nulls arrays and calls deconstruct_expanded_record() before
knowing whether any field types actually differ.
If the version changed but no field types changed (e.g., a constraint-only
ALTER), we do unnecessary work and hit the if (!need_conversion) return
late.

That said, your suggestion is clean and correct. I can restructure the code
to do a first pass over the TupleDesc attributes (which is pure metadata,
no deconstruction needed) to set need_conversion,
and only proceed with deconstruct_expanded_record() and array allocation if
that returns true. This avoids any unnecessary memory allocation in that
intermediate case.

I'll post an updated patch with this improvement.

Thanks again for the careful review!

Regards,
Surya Poondla

--000000000000124e65064d78af37
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi Songjinzhou,<br><br>Thank you for revi=
ewing the patch.<br><br>You&#39;re correct that there is a minor inefficien=
cy, once a tupDesc_identifier version mismatch is detected, the patch alloc=
ates=C2=A0<div>new_values/new_nulls arrays and calls deconstruct_expanded_r=
ecord() before knowing whether any field types actually differ.=C2=A0</div>=
<div>If the version changed but no field types changed (e.g., a constraint-=
only ALTER), we do unnecessary work and hit the if (!need_conversion) retur=
n late.<br><br>That said, your suggestion is clean and correct. I can restr=
ucture the code to do a first pass over the TupleDesc attributes (which is =
pure metadata, no deconstruction needed) to set need_conversion,=C2=A0</div=
><div>and only proceed with deconstruct_expanded_record() and array allocat=
ion if that returns true. This avoids any unnecessary memory allocation in =
that intermediate case.<br><br>I&#39;ll post an updated patch with this imp=
rovement.<br><br>Thanks again for the careful review!<br><br>Regards,=C2=A0=
</div><div>Surya Poondla</div></div></div>

--000000000000124e65064d78af37--