Re: SQL Property Graph Queries (SQL/PGQ)

On Tue, Nov 19, 2024 at 10:08 PM Vik Fearing <vik(at)postgresfriends(dot)org> wrote:
>
>
> On 05/11/2024 16:41, Ashutosh Bapat wrote:
>
> On Wed, Aug 28, 2024 at 3:48 PM Ashutosh Bapat
> <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>
> Patches 0001 - 0006 are same as the previous set.
> 0007 - fixes all the problems you reported till now and also the one I
> found. The commit message describes the fixes in detail.
>
> Here's an updated patchset based on the latest HEAD.
>
>
>
> I have been looking at this patchset from a user's and standards' perspective and I am quite pleased with what I am seeing for the most part. I have not been looking much at the code itself, although I do plan on reviewing some of the code in the future.

Thanks for looking at the patches.

>
>
> There are a few things that stick out to me.
>
>
> 1.
> I don't see any way to view the structure of of a property graph. For example:
>
>
> postgres=# CREATE TABLE objects (id INTEGER, color VARCHAR, shape VARCHAR, size INTEGER);
> CREATE TABLE
> postgres=# CREATE PROPERTY GRAPH propgraph VERTEX TABLES (objects KEY (id) PROPERTIES ALL COLUMNS);
> CREATE PROPERTY GRAPH
> postgres=# \dG propgraph
> List of relations
> Schema | Name | Type | Owner
> -------------------+-----------+----------------+-------------
> graph_table_tests | propgraph | property graph | vik.fearing
> (1 row)
>
> postgres=# \d propgraph
> Property graph "graph_table_tests.propgraph"
> Column | Type
> --------+------
>
> I don't really know what to do with that.

Yes. It is on my TODO list. IMO we should just print the first line
property graph "...". There are no predefined columns in this
relation. And somehow redirect them to use \dG instead.
>
>
> 2.
> There is a missing newline in the \? help of psql.
> HELP0(" \\dFt[+] [PATTERN] list text search templates\n");
> HELP0(" \\dg[S+] [PATTERN] list roles\n");
> HELP0(" \\dG[S+] [PATTERN] list property graphs"); <---
> HELP0(" \\di[S+] [PATTERN] list indexes\n");
> HELP0(" \\dl[+] list large objects, same as \\lo_list\n");
>

Will fix that in the next set. I notice that \dGS+ doesn't provide a
lot of information.
\dG+
List of relations
Schema | Name | Type | Owner | Persistence | Size |
Description
--------+------+----------------+------------+-------------+---------+-------------
public | g1 | property graph | ashutoshpg | permanent | 0 bytes |
(1 row)

Maybe it should print elements, their labels and properties.

>
>
> 3.
> The noise word "ARE" is missing from the <element table properties alternatives> clause.

Agreed. Possibly in v2 since it doesn't affect the functionality.

> There is also no support for the EXCEPT clause, but I imagine that can be added at a later time.
>

There's a patch proposed at [1], but there are some things missing
there. I guess it will be added at a later time.

>
> 4.
> I notice that tables in pg_catalog are not allowed in a property graph. What are the reasons for this? It is true that this might cause some problems with upgrades if a column is removed, but it shouldn't cause trouble for columns being added. That case works with user tables.
>

I don't know the reason. We don't allow tables belonging to other
users to be part of the property graph as explained in [3]. Those two
might have the same context. Maybe Peter can explain.

> 5.
>
> The ascii art characters (I am loathe to call them operators) allow junk in between them. For example:
>
>
> MATCH (c) -[:lbl]-> (d)
>
> can be written as
>
>
> MATCH (c) -
> [:lbl] -
> /* a comment here */
> > (d)
>
>
> Is that intentional?

This looks similar to [2]. I think it has to do with the way we parse
operators. But I haven't investigated it.

>
> I will continue to review this feature from the user's perspective. Thank you for working on it, I am very excited to get this in.

Thanks again.

Here's the latest version of patches rebased on
aac831cafa6f3106dfcbd3298757801c299351fc. I have fixed merge
conflicts, compilation issues and also fixed the failure seen on CI
from the previous patch set.

0001 - 0005 are the same as the previous set.
0007 - has RLS tests. It is the same as 0006 from the previous patch set.

0006 - is new addressing collation and edge-vertex link qual creation.
This patch adds code to store collation and typmod to
pg_propgraph_property catalog and propagate it to property graph
references in GRAPH_TABLE. Collation is used by
assign_query_collations and assign_expr_collations to propagate
collation up the query and expression tree respectively. typmod is
used to report typmod of property reference from exprTypemod().

While finishing code to create vertex-edge link quals, I found that we
need to find and store the operator to be used for key matching in
those quals. I think we have to do something similar to what primary
key handling code does - find the equality operator when creating edge
descriptor and store it in pg_propgraph_element. I am not sure whether
we should add a dependency on the operator. I will look into this
next.

[1] https://www.postgresql.org/message-id/CA+UBfakRHrTqTkx8W0cmxGD7zfnZkZ4nczUvp0FGF68LvMSG-A@mail.gmail.com
[2] https://www.postgresql.org/message-id/0b862b93-dc70-4ba1-b27f-e09104bc4c2c@proxel.se
[3] https://www.postgresql.org/message-id/CAExHW5serNyfY4v9oy6us=tM7ZyAd3_gYnMjOmRPk8eWEskSog@mail.gmail.com

--
Best Wishes,
Ashutosh Bapat