Re: SQL Property Graph Queries (SQL/PGQ)

On Wed, Jan 1, 2025 at 2:22 PM Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>
> On Sat, Dec 21, 2024 at 6:21 PM Junwang Zhao <zhjwpku(at)gmail(dot)com> 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