public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ashutosh Bapat <[email protected]>
To: Junwang Zhao <[email protected]>
Cc: Vik Fearing <[email protected]>
Cc: Ajay Pal <[email protected]>
Cc: Imran Zaheer <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: SQL Property Graph Queries (SQL/PGQ)
Date: Fri, 24 Jan 2025 19:06:11 +0530
Message-ID: <CAExHW5tADz+8kCXAZeEAbNoHwQ3RO22a9E7vKtd5mroHN098mg@mail.gmail.com> (raw)
In-Reply-To: <CAEG8a3KLQ4GLKA7cLNPfMVNMy7-QEGCW4jVz+pa+ErYbgmFh9w@mail.gmail.com>
References: <[email protected]>
<CAExHW5s3BL_qUwQ=ye8RX7GnVcGC+FeR7h6jFocBZC5JFXATEQ@mail.gmail.com>
<[email protected]>
<CAExHW5vhiawMAEtRwV=X-zQmJeUVY2iyuskWFFKobm_ZvgMf9Q@mail.gmail.com>
<CAExHW5tf9UDzhNWXHyDNBG4DRoq81-Ty4XqcY8wiixt844B0eA@mail.gmail.com>
<CAExHW5uF7KMcABuNVB1de6JRY3ne-jYEP3+8ycQZGkFA=1OTLw@mail.gmail.com>
<CA+UBfakRHrTqTkx8W0cmxGD7zfnZkZ4nczUvp0FGF68LvMSG-A@mail.gmail.com>
<CAExHW5uykgWS=4fqL8eJqCKOQcHgtK89=Nq7eS_av-28ge=LKQ@mail.gmail.com>
<CA+UBfamThQm9FQ7Kv=HQZPDy5CfULpn6yuU0bzPy-kX-=-jV4Q@mail.gmail.com>
<CABRHmytyWxGMaQejiZ28OubtqnOUfzB9nZQSctyvsmxvS_cKCw@mail.gmail.com>
<CABRHmyuvgFEXNgUsxXiSHh01iKcLrqyvcDWeOP+wuXjbMRYkEw@mail.gmail.com>
<CABRHmytkwZNiXEqLKx7r6w3xVL8pCiFLaG8_a=BkqfcuumM1vA@mail.gmail.com>
<CAExHW5vqo7iTcVznspb3HHD87Ps3Q=JF6_gg+h5mhyuwhd3Q4Q@mail.gmail.com>
<CAExHW5serNyfY4v9oy6us=tM7ZyAd3_gYnMjOmRPk8eWEskSog@mail.gmail.com>
<[email protected]>
<CAExHW5tYchoePjt3Z69zSmgMW50_jxNfpUiodRcQABUY6V8oxw@mail.gmail.com>
<CAExHW5vG8sgT7H=Q6V_+aMF3f3vs+F7Mu_3AFrufhD=BPkBP0A@mail.gmail.com>
<CAExHW5tDDwUK0dVqFRyUag8UsABmFNWhz8E6AVPDV2BXJLsY5w@mail.gmail.com>
<CAEG8a3KFWD-a-jRtA__opQQaC9Z=_+rw=Gtydk1zStR-roMwWw@mail.gmail.com>
<CAExHW5sM+ZGVzR1428FsDHuWc-FU2-6zQr5j91KLh6vZaWY0ow@mail.gmail.com>
<CAExHW5t9OkLPv5g68C9=kaduyaz2HZQ2bHzOT3d421Wm4_teuA@mail.gmail.com>
<CAEG8a3KLQ4GLKA7cLNPfMVNMy7-QEGCW4jVz+pa+ErYbgmFh9w@mail.gmail.com>
On Sun, Jan 19, 2025 at 6:45 PM Junwang Zhao <[email protected]> wrote:
>
> Hi Ashutosh,
>
> On Wed, Jan 1, 2025 at 5:02 PM Ashutosh Bapat
> <[email protected]> wrote:
> >
> > On Wed, Jan 1, 2025 at 2:22 PM Ashutosh Bapat
> > <[email protected]> wrote:
> > >
> > > On Sat, Dec 21, 2024 at 6:21 PM Junwang Zhao <[email protected]> wrote:
> > > Thanks Junwang for your review.
> > >
> > > > Here are some review opinions for 0001, I haven't seen the other
> > > > patches, forgive me if some of the opinions have already been
> > > > addressed.
> > > >
> > > > 1. In propgraph_edge_get_ref_keys, when finding a matching foreign key,
> > > > the fk pointer will always point to last ForeignKeyCacheInfo of
> > > > the edge relation, which is wrong. We should add another pointer
> > > > that remembers the matched ForeignKeyCacheInfo to ref_rel. See
> > > > attached 0001.
> > >
> > > Nice catch. I fixed it in a slightly different way reducing overall
> > > code by using foreach_node(). I have merged this as part of 0004 which
> > > has fixes for several other issues. Interestingly there was a SQL that
> > > had revealed the problem in create_property_graph.sql. But we had
> > > accepted a wrong output. Corrected that as well.
> > >
> > > >
> > > > 2. Some of the TODOs seem easy to address, attached 0002 does this.
> > >
> > > From a cursory glance those changes look useful and mostly correct. It
> > > will be good if you can provide a SQL test for those, covering that
> > > part of the code. Please post the whole patch-set with your changes as
> > > a separate commit/patch.
> > >
> > > >
> > > > 3. Since property name and label name are unique in property graph
> > > > scope, some of the wording are not accurate. See attached 0003.
> > >
> > > <para>
> > > - For each property graph element, all properties with the same name must
> > > - have the same expression for each label. For example, this would be
> > > + For each property graph, all properties with the same name must
> > > + have the same expression. For example, this would be
> > >
> > > Each property graph element may have a property with the same name
> > > associated with multiple labels. But each of those properties should
> > > have the same expression. You may see that as the same property being
> > > defined multiple times once in each label it is associated with OR
> > > multiple properties with the same name. Current wording uses the
> > > latter notion, so it looks fine to me. The changes made to error
> > > messages are not needed with this notion.
> >
> > My last email is held for moderation. It will arrive once moderators
> > release it. In the meantime trying to send the patches as a zip file
> > in a hope that it won't require moderation.
> >
> > --
> > Best Wishes,
> > Ashutosh Bapat
>
> 0007-RLS-tests-20250101.patch introduces regression test failure:
>
> +WARNING: outfuncs/readfuncs failed to produce an equal rewritten parse tree
>
> P.S. It seems the patch set doesn't apply to master.
Thanks for noticing it.
Here's rebased patch set on the current head with some minor conflicts
in various files fixed. I did not see the said failure on my laptop.
0001-0009 patches are the same as the previous set sans mergeconflict fixes.
0010 - is test for \dGx - to make sure that the extended format output
works with \dG. We may decide not to have this patch in the final
commit. But no harm in being there til that point.
--
Best Wishes,
Ashutosh Bapat
Attachments:
[application/zip] sql_pgq_patch_set_20250124.zip (163.5K, ../CAExHW5tADz+8kCXAZeEAbNoHwQ3RO22a9E7vKtd5mroHN098mg@mail.gmail.com/2-sql_pgq_patch_set_20250124.zip)
download
view thread (37+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: SQL Property Graph Queries (SQL/PGQ)
In-Reply-To: <CAExHW5tADz+8kCXAZeEAbNoHwQ3RO22a9E7vKtd5mroHN098mg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox