On Tue, Mar 17, 2026 at 10:08 AM Junwang Zhao <[email protected]> wrote: > > Hi Ashutosh, > > On Mon, Mar 16, 2026 at 11:55 PM Ashutosh Bapat > <[email protected]> wrote: > > > > Hi Peter, > > > > > On Mon, 16 Mar 2026 at 17:43, Peter Eisentraut <[email protected]> > > > wrote: > > > > > > > > On 11.03.26 08:34, Ashutosh Bapat wrote: > > > > > There are two new patches 0004 and 0005 in the attached patchset. > > > > > > > > I have committed this, including the 0004 patch. > > > > Thanks a lot. > > > > > > Let's consider the > > > > 0005 patch separately. > > > > Will share the rebased patch soon. This thread may see discussion > > about the commit itself. Should I start a new thread for 0005 or use > > this one? New one seems better to me with a new CF entry. > > > > > > > > > > The buildfarm shows some instability in the pg_upgrade test, because > > > > labels are printed by pg_get_propgraphdef() in implementation-dependent > > > > order. Attached is a quick patch to sort the labels before printing. > > > > Check please. > > > > The patch looks fine to me. While reviewing it, I noticed that the > > function has an extra loop to count the number of variables. I don't > > think it's needed. The count can be obtained from the list length. In > > the attached patch, I have removed that loop. Am I missing something? > > > > 0001 is your patch > > 0002 removes the loop + some cosmetic changes > > > > Hi Kirill, > > > > > Do we need to keep relation lock until end of function > > > (table_close(pglrel, AccessShareLock);)? > > > > I think you are right. Fixed in the attached. > > > > > I'm not sure if list_sort is > > > interruptible. > > > > I don't think it matters here. It will be very rare, if not > > impossible, to have so many labels as to let the sorting run for > > milliseconds together. The foreach loop afterwards is also not > > interruptible. Any reason you think it should be interruptible? > > > > -- > > Best Wishes, > > Ashutosh Bapat > > Do you think it's necessary to add a list_free_deep(label_list) at > the end? make_propgraphdef_labels itself it not on critical path > so explicit free non-used memory doesn't impact performance. >
It's not that the list or the labels will be too long to consume a lot of memory. If that would have been the case, calling list_free_deep() makes sense. But in this case, the memory will get freed when the immediate memory context gets freed; in worst case that will be when the statement finishes. I don't think it's worth the distraction and the possible minimal risk it carries. -- Best Wishes, Ashutosh Bapat
