From: | Junwang Zhao <zhjwpku(at)gmail(dot)com> |
---|---|
To: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> |
Cc: | Vik Fearing <vik(at)postgresfriends(dot)org>, Ajay Pal <ajay(dot)pal(dot)k(at)gmail(dot)com>, Imran Zaheer <imran(dot)zhir(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: SQL Property Graph Queries (SQL/PGQ) |
Date: | 2025-01-28 11:01:41 |
Message-ID: | CAEG8a3J8xBH1ujWwES6V38M43ENmfb=UDX4ytJo-hVVjgF0NOQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Jan 28, 2025 at 6:17 PM Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>
> On Fri, Jan 24, 2025 at 9:16 PM Junwang Zhao <zhjwpku(at)gmail(dot)com> wrote:
> >
> > I figured out it's because I have `-DWRITE_READ_PARSE_PLAN_TREES` this option,
> > removing this option clears the warning, but I'm not sure if this is a
> > hidden issue.
> >
>
> Thanks for the hint. I found the issue and the fix appears in 0004.
> Some members of RangeTblEntry were not being handled in read/out
> functions.
Thanks for the fix, I will apply and test again.
>
> > >
> > > 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.
> >
> > I have some changes based on the latest patch set. I attached two patches with
> > the following summary.
> >
> > 0001:
> > - doc: some of the query in ddl.sgml can not be executed
>
> The standard seems to require a property reference to be qualified by
> element pattern variable, even if the property is referenced within
> the same element pattern bound to the variable. Hence I accepted your
> document fixes which qualify property reference with element pattern
> variable.
>
> However, I didn't understand why you changed a LABEL name from order to order_.
Ah, that's because `order` is a keyword of SQL(order by), so the alias
is a conflict.
I'm not saying `order_` is a good name though.
>
> > - after path factor was introduced, some comments doesn't apply
>
> Thanks pointing those out. Accepted after rephrasing those and also
> correcting some related comments.
>
> >
> > 0002:
> > - add a get_propgraph_element_alias_name function and do some trivial refactor
> >
>
> I see you have added some negative tests - object not found tests, but
> I didn't see positive tests. Hence I haven't added those changes in
> the attached patchset. But we certainly need those changes. You may
> want to submit a patch with positive tests. That code needs to be
> fixed before committing anyway.
Ok, I'll add positive tests.
>
> --
> Best Wishes,
> Ashutosh Bapat
--
Regards
Junwang Zhao
From | Date | Subject | |
---|---|---|---|
Next Message | Ashutosh Bapat | 2025-01-28 11:03:19 | Re: SQL Property Graph Queries (SQL/PGQ) |
Previous Message | Amul Sul | 2025-01-28 10:58:00 | Re: NOT ENFORCED constraint feature |