On Tue, Mar 17, 2026 at 12:57 PM Ashutosh Bapat <[email protected]> wrote: > > 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.
Fair enough. Andrew mentioned in the SQL/PGQ commit thread that we should fence the dependency check in pg_dump.c, maybe collecting it in your patch set, see: https://www.postgresql.org/message-id/attachment/192197/fix-propgraph-def.patch > > -- > Best Wishes, > Ashutosh Bapat -- Regards Junwang Zhao
