Re: SQL Property Graph Queries (SQL/PGQ)

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Cc: Amit Langote <amitlangote09(at)gmail(dot)com>, Junwang Zhao <zhjwpku(at)gmail(dot)com>, 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>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: SQL Property Graph Queries (SQL/PGQ)
Date: 2025-11-25 13:52:21
Message-ID: 3365298a-6a49-4d13-a343-69b1b0c545fc@eisentraut.org
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I have done a rough review of the patch set version 20251120.

- There are "No newline at end of file" warnings from git diff for a
couple of files:

contrib/pg_overexplain/sql/pg_overexplain.sql
src/test/regress/sql/graph_table_rls.sql

Please fix those in the next patch set, and maybe check your editor
settings.

- Attached are two small patches with some small fixes I found in
passing. (I think some of them were also reported by Junwang Zhao in
the meantime.)

- In the test files, especially src/test/regress/sql/graph_table.sql,
there are wildly different SQL styles (capitalization, formatting) used,
depending on who added the test case (I guess). Let's keep that more
consistent.

- In src/backend/parser/analyze.c, the extracted function
constructSetOpTargetlist() needs a detailed comment.

- I'm not so sure about the semantics chosen in the patch "Access
permissions on property graph". I think according to the SQL standard,
once you have access to the property graph, you don't need access to the
underlying tables as well. I guess you did this to align with how views
work? We might need to think about this a bit more, and document
whatever the conclusion is. But for now it's just small amount of code
affected.

I think you could collapse all the patches into one patch now. I have
reviewed all the incremental patches and they all look ok to me. I have
made some notes about which things I want to review in more detail, such
as the access control issue, but that doesn't need to be kept as a
separate patch.

When you create future patches, consider using the git format-patch -v
option.

And then you can also just gzip the patch. That should make it even
smaller than the .zip file.

Attachment Content-Type Size
0001-Typos-and-minor-style-fixes.patch.nocfbot text/plain 5.2 KB
0002-Remove-added-tokens-from-plpgsql.patch.nocfbot text/plain 932 bytes

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2025-11-25 13:58:31 Re: SQL Property Graph Queries (SQL/PGQ)
Previous Message 河田達也 2025-11-25 13:48:06 Re: [Proposal] Adding TRIM_SPACE option to COPY