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

Hi,
Sending the email again since my earlier email was held for moderation
because of patchset size. Attached as a zip here.

On Thu, Nov 20, 2025 at 8:31 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
> On Mon, Aug 18, 2025 at 5:01 PM Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
>
> >
> > >
> > > 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.
> >
> > I implemented it that way considering similarity between properties
> > and attributes. But I agree with you. I think it's a matter of calling
> > CacheInvalidateRelcacheByRelid with OID of property graph at
> > appropriate places in AlterPropGraph(), which is the only place where
> > components of the property graph are changed without changing property
> > graph's pg_class tuple. Creating and dropping a property graph will
> > touch the pg_class tuple which will trigger the current invalidation
> > mechanism.
> >
>
> Done.
>
> > >
> > > 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.
> > >
> >
> > Thanks for pointing me to restrict_nonsystem_relation_kind. I can take
> > care of that.
>
> Reading commit message of 66e94448abec3aad04faf0a79cab4881ae08e08a
> which introduced this GUC and CVE
> https://www.postgresql.org/support/security/CVE-2024-7348/, it does
> not seem likely that property graphs can be used that way. SELECT *
> FROM <property graph> will end up in an error. A property graph
> reference is allowed only in GRAPH_TABLE(), which pg_dump does not
> use. Do you have something else in mind?
>
> As part of this investigation, I found that a property graph reference
> outside GRAPH_TABLE does throw error, but from unexpected places.
> Added error messages in addRangeTableEntry and
> addRangeTableEntryForRelation instead.
>
> > Apart from that views have two security related settings
> > - security_invoker and security_barrier.
> >
> > As mentioned in the commit message of 0003, property graphs behave
> > like views with security_invoker set to true. This will avoid any
> > privilege escalations through property graphs. This should be the
> > safest MVP. But it's different from the default security_invoker
> > setting for views. I was thinking that we will integrate the patches
> > with this behaviour and add security_invoker semantics in the next
> > iteration. Please let me know if you think that security_invoker and
> > security_definer semantics should both be supported before the first
> > commit itself.
> >
> > Property graphs do not have security_barrier setting right now.
> > Property graphs do not have any conditions of their own. But
> > GRAPH_TABLE() may have. WHERE conditions in the GRAPH_TABLE are
> > executed like other conditions on the underlying table (if our
> > optimizer can push the conditions below UNION). I guess we need to
> > support security_barrier semantics on property graph so that WHERE
> > conditions in GRAPH_TABLE are executed before other conditions, but it
> > doesn't seem to be critical to me for the first cut as users can
> > leverage security_barrier views for the same, if required. But please
> > let me know if you think that security_barrier semantics should also
> > be supported before the first commit.
> >
> > RLS is already covered.
> >
> > Is there anything that is missing?
>
> Waiting for your response 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.
> >
> > I sprinkled the queries in that file since the tables, RLS rules,
> > scenarios to test were readily available there. But I agree with you
> > that a file would be better. In fact, it's better to create a separate
> > file graph_table_security.sql for tests related to security aspects of
> > property graph covering RLS, restrict_nonsystem_relation_kind,
> > security_invoker and security_barrier. What do you think? I will work
> > on that.
>
> Done. Added a new test file graph_table_rls.sql. in 0012.
>
> --
> Best Wishes,
> Ashutosh Bapat



--
Best Wishes,
Ashutosh Bapat