Re: SQL Property Graph Queries (SQL/PGQ)

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.

--
Best Wishes,
Ashutosh Bapat