Re: SQL Property Graph Queries (SQL/PGQ) - Mailing list pgsql-hackers

On Wed, Mar 12, 2025 at 10:01 AM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
> >
>
> Thanks Junwang. I have added your patches to my local repository. Next
> time I post an updated set, I will post it along with your patches.
> Will merge them into the original patches as time permits.

I have reviewed your patches. adjusted them as described below.

0001 - is original patch by Peter but it has now accumulated a lot of
conflict resolution fixes, compilation fixes and bug fixes. The commit
message has a list of all the fixes.

0002 - WHERE clause support in graph pattern

0003 - supports cyclic path patterns

0004 - contains a variety of fixes, described in the commit message

0005 - Fixes access permission checks on property graph

0006 - support collation specification for property and handles
collation of vertex - edge quals, this includes Junwang's 0012 and
some part of his 0015. The edit related to reducing number of call
sites didn't look like a net improvement. Having two callsites each
surrounded by the caller's context is better than one.

0007 - pg_overexplain support for property graph (description of
GRAPH_TABLE RTE)

0008 - documentation fixes

0009 - support \d variants for property graph, includes Junwang's 0013

0010 - handles RTE_GRAPH_TABLE at some more places. This includes
Amit's fix for crash in ExecCheckPermissions.

0011 - deals with functions related to object addresses. It's clear
what should be the output of pg_describe_object(),
pg_identify_object() and pg_identify_object_as_address() for property
graph elements, property graph labels and property graph properties.
Those changes are included in the patch. But in order to support those
objects in pg_get_object_address() we need to add ObjectTypes
corresponding to those objects. Every object that is supported by
pg_describe_object(), pg_identify_object() and
pg_identify_object_as_address() is not supported by
pg_get_object_address(). The objects which are not supported do not
have their object types defined in the ObjectTypes enum. E.g. "index
column" or "view column" etc. So I have not implemented that support
for now. If required, we will need to add the corresponding types in
ObjectTypes and then handle them in all the places where ObjectType
enum is used. This contains Junwang's 0014.

0012 - A change to in the properties of labels or expression
associated with an element property requires the cached plans for
queries referring to the corresponding property graph. For that
CacheInvalidateHeapTupleCommon() needs to lookup OID of property graph
given a Form_pg_propgraph_label_property or
Form_pg_propgraph_element_label tuple. These catalogs do not store the
property graph OID directly. Instead they store label or property or
element oid. From those OIDs it is possible to get the property graph
ID from the corresponding label or property or element tuples.
However, when tuples in those catalogs are added along with the label,
element or property tuples, the latter are not visible. So either we
have to add property graph id to pg_propgraph_label_property and
pg_propgraph_element_label catalogs or add a CommandCounterIncrement()
after inserting element, property or label tuples. The latter may
cause some unwanted changes to be visible other than the desired ones,
so seems risky. Hence going with the first option. However, that
option consumes more space in the catalog. Once the catalog changes
are visible, the property graph id in the tuple won't be useful while
consuming space.

0013 and 0014 - minimum RLS related tests. In previous patchsets this
patch was too large and probably not all tests in that patch were
necessary. Now it's broken into two patches  .0013 which has extra
tests for the combinations. 0014 which I think are tested indirectly
in our regression suite. We may include some of those tests in the
main patchset.

I think two more changes are necessary but not included in the patchset
1. graph_table.sql uses mixed cases in SQL queries (e.g. it uses both
SELECT and select). I think we should use a uniform.
2. We have enhanced 002_pg_upgrade.pl to test dump and restore using
regression database. I think we should use that facility instead of
property graph specific dump/restore tests.

Here's the patchset rebased on the latest master.
--
Best Wishes,
Ashutosh Bapat