Re: SQL Property Graph Queries (SQL/PGQ)

On Tue, Mar 17, 2026 at 11:25 AM Junwang Zhao <zhjwpku(at)gmail(dot)com> wrote:
>
> 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

Can you please share the link to the message? This is an attachment
without message context.

--
Best Wishes,
Ashutosh Bapat