public inbox for [email protected]
help / color / mirror / Atom feedFrom: Junwang Zhao <[email protected]>
To: Ashutosh Bapat <[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: Sun, 19 Jan 2025 21:15:45 +0800
Message-ID: <CAEG8a3KLQ4GLKA7cLNPfMVNMy7-QEGCW4jVz+pa+ErYbgmFh9w@mail.gmail.com> (raw)
In-Reply-To: <CAExHW5t9OkLPv5g68C9=kaduyaz2HZQ2bHzOT3d421Wm4_teuA@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>
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.
--
Regards
Junwang Zhao
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: <CAEG8a3KLQ4GLKA7cLNPfMVNMy7-QEGCW4jVz+pa+ErYbgmFh9w@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