Horton

Re: SQL Property Graph Queries (SQL/PGQ)

Peter Eisentraut <peter@eisentraut.org>

View thread
From: Peter Eisentraut <peter@eisentraut.org>
To: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, Amit Langote <amitlangote09@gmail.com>
Cc: Junwang Zhao <zhjwpku@gmail.com>, Vik Fearing <vik@postgresfriends.org>, Ajay Pal <ajay.pal.k@gmail.com>, Imran Zaheer <imran.zhir@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2025-08-11T13:42:55Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix some typos and make small stylistic improvements

  2. Cleanup users and roles in graph_table_rls test

  3. Dump labels in reproducible order

  4. SQL Property Graph Queries (SQL/PGQ)

  5. Factor out constructSetOpTargetlist() from transformSetOperationTree()

  6. Sort out table_open vs. relation_open in rewriter

  7. Rename grammar nonterminal to simplify reuse

  8. Make ecpg parse.pl more robust with braces

  9. Don't lock partitions pruned by initial pruning

  10. Remove pg_regex_collation

  11. Use auxv to check for CRC32 instructions on ARM.

  12. Fix inappropriate uses of atol()

  13. Remove unnecessary array object_classes[] in dependency.c

I have browsed this patch set a bit to get myself up to date.

General thoughts:

This is a large patch set, and it's not going to be perfect at the first 
try.  Especially, some of the parsing rules, query semantics, that kind 
of thing.  So I'm thinking that we should aim at integrating this with 
an understanding that it would be somewhat experimental, and then 
iterate on some of the details in the tree.  Obviously, that would still 
require that the overall architecture is ok, that it doesn't crash, that 
it satisfies security requirements.  Also, it should as much as possible 
follow the "if you don't use it, it doesn't affect you" rule.  So I'm 
specifically looking at where the patch intersects with existing code in 
critical ways, especially in parse analysis.  I want to spend more time 
reviewing that in particular.  Much of the rest of patch is 
almost-boilerplate: New DDL commands, new catalogs, associated tests and 
documentation.  It looks a lot, but most of it is not very surprising.

I found two areas where a bit more work could be done to separate the 
new code from existing code:

src/backend/utils/cache/inval.c: This looks suspicious.  The existing 
code only deals with very fundamental catalogs such as pg_class and 
pg_index.  It doesn't feel right to hardcode additional arguably very 
high-level PGQ-related catalogs there.  We should think of a different 
approach here.

src/test/regress/sql/rowsecurity.sql: I think it would be better to 
separate the new test cases into a separate file.  This test file is 
already large and complicated, and sprinkling a bunch of more stuff all 
over the place is going to make review and maintenance more complicated.

We also need to make sure that property graphs have security features 
equivalent to views.  For example, I suspect we need to integrate them 
with restrict_nonsystem_relation_kind.  There might be other similar 
things, to be checked.