Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1tDRFW-000hVn-MO for pgsql-hackers@arkaria.postgresql.org; Tue, 19 Nov 2024 16:38:46 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1tDRFV-00Egbu-44 for pgsql-hackers@arkaria.postgresql.org; Tue, 19 Nov 2024 16:38:45 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1tDRFU-00Egbm-63 for pgsql-hackers@lists.postgresql.org; Tue, 19 Nov 2024 16:38:44 +0000 Received: from relay6-d.mail.gandi.net ([2001:4b98:dc4:8::226]) by magus.postgresql.org with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1tDRFQ-002mLm-RX for pgsql-hackers@postgresql.org; Tue, 19 Nov 2024 16:38:43 +0000 Received: by mail.gandi.net (Postfix) with ESMTPSA id 79B56C0003; Tue, 19 Nov 2024 16:38:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=postgresfriends.org; s=gm1; t=1732034317; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=uZOlx6H7p786FKvH1BQMZB/ziKTwWBvJXp67fItirzU=; b=QiVWKvUVLnqmzwz23luWs9EfLm0h7bWYET1sh7bwqJ15qNWf5/r5wERJ6JMicqlI+Rm2RC qUBfE8+6PC9Mc5JJ2uyI8kP/y/KRDwMpaAv/NENsBfYhyp9fzOUhT/LPKIBiECH80ANWGz L9kSz1VgYBl1ukd44wMD8nT7uEh7kj34072Omd1DA6pFIZ9uv5S7xTT/fx/RWFMO+J47ON sdBHPM4vmrxEtd6OEDM8HT8VF63IUhny69Hhg4NeL5dOC5JD/jS5BoD0CVaWZzloprv95I r7p0+Np88foA36sBEUQq+uWCMuj8/T+/LLZVgOsxosMcadmn2j252CMCUNCbeg== Content-Type: multipart/alternative; boundary="------------bZv9yukA7AmVQ5MDV9S4xuyc" Message-ID: <3b190de8-910a-4091-82f9-e98cf562d832@postgresfriends.org> Date: Tue, 19 Nov 2024 17:38:36 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: SQL Property Graph Queries (SQL/PGQ) To: Ashutosh Bapat , Ajay Pal Cc: Imran Zaheer , Peter Eisentraut , pgsql-hackers References: <04c0abe2-e9c5-4ffc-97b2-bedc1e3cd178@eisentraut.org> Content-Language: en-US From: Vik Fearing In-Reply-To: X-GND-Sasl: vik@postgresfriends.org List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk This is a multi-part message in MIME format. --------------bZv9yukA7AmVQ5MDV9S4xuyc Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 05/11/2024 16:41, Ashutosh Bapat wrote: > On Wed, Aug 28, 2024 at 3:48 PM Ashutosh Bapat > 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. 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. 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"); 3. The noise word "ARE" is missing from the clause. There is also no support for the EXCEPT clause, but I imagine that can 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. 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? ---- 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. -- Vik Fearing --------------bZv9yukA7AmVQ5MDV9S4xuyc Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit


On 05/11/2024 16:41, Ashutosh Bapat wrote:
On Wed, Aug 28, 2024 at 3:48 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.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.


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.


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");



3.
The noise word "ARE" is missing from the <element table properties alternatives> clause.
There is also no support for the EXCEPT clause, but I imagine that can 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.

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?


----


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.

--

Vik Fearing

--------------bZv9yukA7AmVQ5MDV9S4xuyc--