Re: SQL Property Graph Queries (SQL/PGQ)

On Tue, Mar 17, 2026 at 12:57 PM Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>
> On Tue, Mar 17, 2026 at 10:08 AM Junwang Zhao <zhjwpku(at)gmail(dot)com> wrote:
> >
> > Hi Ashutosh,
> >
> > On Mon, Mar 16, 2026 at 11:55 PM Ashutosh Bapat
> > <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> > >
> > > Hi Peter,
> > >
> > > > On Mon, 16 Mar 2026 at 17:43, Peter Eisentraut <peter(at)eisentraut(dot)org> 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