PGQ catalog representation and pg_dump support
Hi,
I don't know much about PGQ. So maybe I'm way off base with my questions
below.
I noticed some things that struck me as odd, when, while working on another
patch, I had occasion to look at dumpTableSchema(), which started me looking
into dump / dependency handling of PGQ:
- Why do property graphs have pg_attribute entries?
As far as I can tell, the system attributes don't make any sense for a
property graph as system attributes aren't ever referenced? Other objects
for which system attributes, like composite types, don't have pg_attribute
rows for system attributes?
And, IIUC, there aren't any other kind of attributes for property graph
relations?
- Pretty sure pg_dump's dependency handling for property graphs is
insufficient?
I see there's code to handle dependencies via pg_propgraph_element, but
there's also dependencies like pg_propgraph_property.pgtypid? I don't
immediately see such a dependency would be visible to pg_dump, as
getDependencies() only additionally queries dependencies via
pg_propgraph_element
There probably are unhandled dependencies other than
pg_propgraph_property.pgtypid.
- The prior UNION arms in getDependencies() prevent dependencies on itself -
but I don't think the pg_propgraph_element query does?
Compare with e.g. the amproc case which has
"AND NOT (refclassid = 'pg_opfamily'::regclass AND amprocfamily = refobjid)\n");
- Why are property graphs dumped as part of dumpTableSchema()?
I think it's already pretty weird that views are created as part of
dumpTableSchema(), but they at least share some infrastructure with
tables. I don't see any reason for propgraphs to not have been redirected in
dumpTable(), just like it's done for dumpSequence()?
- I am a bit worried that having something as complicated as
pg_get_propgraphdef() done purely server side will prevent some challenges
when we end up having to evolve any aspect of the property graph grammar
over time. But I guess the alternatives are also decidedly unattractive.
- More curiosity: Why do property graphs have pg_class entries at all? As far
as I can tell it doesn't use anything from it?
- Harmless, but it's a bit odd for the propgraph portion of getDependencies()
to filter deptype = 'p' away, given how long that has not existed.
Perhaps getDependencies() code should just have a comment about why the
queries include 'p', despite that being an unknown kind of dependency these
days.
Greetings,
Andres Freund
Hi,
On 2026-08-22 15:33:01 -0400, Andres Freund wrote:
- Pretty sure pg_dump's dependency handling for property graphs is
insufficient?I see there's code to handle dependencies via pg_propgraph_element, but
there's also dependencies like pg_propgraph_property.pgtypid? I don't
immediately see such a dependency would be visible to pg_dump, as
getDependencies() only additionally queries dependencies via
pg_propgraph_elementThere probably are unhandled dependencies other than
pg_propgraph_property.pgtypid.
Indeed:
DROP PROPERTY GRAPH IF EXISTS g_dump;
DROP TABLE IF EXISTS zz_rowtab CASCADE;
DROP TABLE IF EXISTS t_dump CASCADE;
CREATE TABLE zz_rowtab (a int, b int);
CREATE TABLE t_dump (id int PRIMARY KEY, txt text);
CREATE PROPERTY GRAPH g_dump
VERTEX TABLES (
t_dump KEY (id) LABEL l PROPERTIES (id, row(1,2)::zz_rowtab AS p)
);
dumps as
CREATE PROPERTY GRAPH public.g_dump
VERTEX TABLES (
public.t_dump KEY (id) LABEL l PROPERTIES (ROW(1, 2)::public.zz_rowtab AS p)
);
...
CREATE TABLE public.zz_rowtab (
a integer,
b integer
);
Which obviously can't work.
This example is the easy case, because they both g_dump and zz_rowtab have the
same priority. But I'm pretty sure that dependencies on other object types,
even if they are in a lower priority class, can still trigger problems when
reordered due to the topo sort.
Greetings,
Andres
Hi,
Given the things I already found manually, I did a short AI review (Opus 5 and
GPT 5.6 Sol) and they found quite a few more things, just looking at the
dependency stuff. I did briefly look over the reported issues, and they all
looked valid to me:
- External cascades leave orphan graph metadata
ALTER PROPERTY GRAPH explicitly removes unused labels/properties, but generic
dependency deletion bypasses that cleanup:
CREATE TABLE v (id int PRIMARY KEY);
CREATE PROPERTY GRAPH g
VERTEX TABLES (v LABEL l PROPERTIES (id AS p));
DROP TABLE v CASCADE;
g retains global label l and integer property p. Adding a new text property named p then incorrectly
reports a type mismatch.
SELECT count(*) AS elements
FROM pg_propgraph_element
WHERE pgepgid = 'g'::regclass;
SELECT count(*) AS labels
FROM pg_propgraph_label
WHERE pglpgid = 'g'::regclass;
SELECT count(*) AS properties
FROM pg_propgraph_property
WHERE pgppgid = 'g'::regclass;
CREATE TABLE v2 (id text PRIMARY KEY);
ALTER PROPERTY GRAPH g
ADD VERTEX TABLES (
v2 LABEL l PROPERTIES (id AS p)
);
ERROR: 42601: property "p" data type mismatch: integer vs. text
DETAIL: In a property graph, a property of the same name has to have the same data type in each label.
- Similarly, query plan caching is not handled correctly after a CASCADE style
dropping.
AlterPropGraph() calls CacheInvalidateRelcacheByRelid(), but that's not
invoked when done via performDeletion() -> DropObjectById().
- Views depend only on global pg_propgraph_label and pg_propgraph_property rows
not the specific label/property association.
CREATE TABLE v1 (id integer PRIMARY KEY, n integer);
CREATE TABLE v2 (id integer PRIMARY KEY, n integer);
CREATE PROPERTY GRAPH g
VERTEX TABLES (
v1 LABEL l1 PROPERTIES (n AS p) LABEL keep NO PROPERTIES,
v2 LABEL l2 PROPERTIES (n AS p)
);
CREATE VIEW gv AS
SELECT *
FROM GRAPH_TABLE (
g MATCH (x IS l1)
COLUMNS (x.p)
);
ALTER PROPERTY GRAPH g
ALTER VERTEX TABLE v1
ALTER LABEL l1 DROP PROPERTIES (p);
SELECT to_regclass('gv') AS view_still_exists;
SELECT * FROM gv;
results in:
ERROR: 42704: property "p" for element variable "x" not found
- Edge links omit implicit-cast dependencies
Edge creation may accept an implicit cast but records only the equality
operator. Rewrite reconstructs the cast later. DROP CAST (...) therefore
succeeds, after which GRAPH_TABLE fails.
- Graph/materialized-view cycles are unrestorable
A matview can query graph g, then be added as an element of g. pg_dump
reports an unresolved dependency loop and restore fails because either the
graph or matview must exist first. Property graphs lack the
staged/dummy-definition repair used for ordinary views.
- opclass/opfamily for edge key equality
propgraph_edge_get_ref_keys() uses get_opfamily_member() but only the
dependency on the resulting pg_operator is recorded, not the opfamily.
I suggest doing a broader review of the propgraph code yourselves, if there's
this much to find just around propgraph dependencies, there's probably more.
Greetings,
Andres
Hi,
I also encountered an issue with pg_get_propgraphdef() and pg_dump.
CREATE TABLE wr (
id int PRIMARY KEY,
x text
);
CREATE PROPERTY GRAPH gwr
VERTEX TABLES (
wr PROPERTIES (wr AS whole)
);
SELECT pg_get_propgraphdef('gwr'::regclass);
ERROR: cache lookup failed for attribute 0 of relation ...
The graph itself works, and the whole row property can be queried through
GRAPH_TABLE. However, pg_dump also fails because it calls
pg_get_propgraphdef().
Reproduced this on PG 19beta3 as well as current master.
Regards,
Taha
On 2026-08-23 Su 4:41 AM, Taha Naveed wrote:
Hi,
I also encountered an issue with pg_get_propgraphdef() and pg_dump.
CREATE TABLE wr (
id int PRIMARY KEY,
x text
);CREATE PROPERTY GRAPH gwr
VERTEX TABLES (
wr PROPERTIES (wr AS whole)
);SELECT pg_get_propgraphdef('gwr'::regclass);
ERROR: cache lookup failed for attribute 0 of relation ...The graph itself works, and the whole row property can be queried
through GRAPH_TABLE. However, pg_dump also fails because it calls
pg_get_propgraphdef().
Reproduced this on PG 19beta3 as well as current master.
Hi Andres, Taha,
I ran a broader audit using Opus 5 off the back of this thread and can
confirm essentially everything reported here, including Taha's whole-row
crash (that one's a one-line guard — get_attname() needs varattno > 0 before
the shortcut, ruleutils.c:1951).
Two further issues, both more urgent than the dump problems since
neither needs any privilege on the underlying tables:
- AlterPropGraph() never checks its target is actually a property graph.
ALTER PROPERTY GRAPH <any table/view/index/sequence you own> ADD VERTEX
TABLES (...) succeeds and writes catalog rows nobody can read
back.
- GRAPH_TABLE's rewriter enumerates the full cartesian product of
candidate paths with no bound on breadth. A short EXPLAIN over a
middling multi-element pattern — SELECT on someone else's graph is
enough, or
just the default TEMP privilege — can OOM the backend bringing down
the postmaster. That one probably belongs first in the queue.
Repro for that one:
CREATE TEMP TABLE z (id int primary key, s int, t int);
ALTER TABLE z ADD CONSTRAINT zfk FOREIGN KEY (s) REFERENCES z(id);
CREATE PROPERTY GRAPH gz
VERTEX TABLES (z AS v1, z AS v2, z AS v3, z AS v4)
EDGE TABLES (z AS e1 KEY (id) SOURCE KEY (s) REFERENCES v1 (id)
DESTINATION KEY (t) REFERENCES v1 (id),
z AS e2 KEY (id) SOURCE KEY (s) REFERENCES v2 (id)
DESTINATION KEY (t) REFERENCES v2 (id),
z AS e3 KEY (id) SOURCE KEY (s) REFERENCES v3 (id)
DESTINATION KEY (t) REFERENCES v3 (id),
z AS e4 KEY (id) SOURCE KEY (s) REFERENCES v4 (id)
DESTINATION KEY (t) REFERENCES v4 (id));
EXPLAIN (COSTS OFF) SELECT count(*) FROM GRAPH_TABLE (gz MATCH
(a1)-[b1]->(a2)-[b2]->(a3)-[b3]->(a4)-[b4]->(a5)-[b5]->(a6) COLUMNS
(a1.id AS c1));
On the AI-assisted items in Andres' third message: the correctness bugs
behind all four hold up, but the privilege-escalation framing attached
to each doesn't survive inspection. No surviving orphan row
carries a relation OID; ExecCheckPermissions() reruns on every
execution, so the relcache-invalidation gap is a plan-staleness issue,
not an auth one; the cross-label lookup is keyed by (elemoid, propid) and
can't cross tables; and replacing a cast or operator already requires
owning the type. Real bugs, not exploitable ones.
On Andres' original question — the pg_attribute rows do look vestigial.
heap.c already carves the rowtype out for propgraphs (and toast tables,
sequences); nobody made the matching carve-out for attributes.
Keeping the pg_class representation overall seems right (the ACL and
relcache-invalidation reuse pays for itself), but three targeted fixes —
the attribute carve-out, a propgraph_open() along the lines
Ashutosh floated and dropped upthread, and giving pg_dump its own
dump-object type instead of riding on DO_TABLE — would close most of
what's turned up here, ordering bug included.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Hi Andres,
Thanks for all the reports. I am going through them one by one. I will
post patches as I create them. Also responding to your questions that
I can respond to right now.
On Sun, Aug 23, 2026 at 1:03 AM Andres Freund <andres@anarazel.de> wrote:
- Why do property graphs have pg_attribute entries?
As far as I can tell, the system attributes don't make any sense for a
property graph as system attributes aren't ever referenced? Other objects
for which system attributes, like composite types, don't have pg_attribute
rows for system attributes?And, IIUC, there aren't any other kind of attributes for property graph
relations?
Those attributes are not needed and I think we can avoid creating them
safely. If we do that in PG 19, it may need a catversion bump - since
a property graph created before the change will have those attributes
and that after the change won't. The change itself won't remove the
attributes. AFAIU, even if we don't fix this in PG 19 to avoid
catversion bump those attributes are useless and harmless. We will fix
the code in PG 20 for sure. Peter, what do you think?
- Pretty sure pg_dump's dependency handling for property graphs is
insufficient?I see there's code to handle dependencies via pg_propgraph_element, but
there's also dependencies like pg_propgraph_property.pgtypid? I don't
immediately see such a dependency would be visible to pg_dump, as
getDependencies() only additionally queries dependencies via
pg_propgraph_elementThere probably are unhandled dependencies other than
pg_propgraph_property.pgtypid.
A property graph has three types of components: elements, labels and
properties. Query on pg_propgraph_element transfers the dependency of
elements to the property graph. Query on pg_propgraph_property
transfers the dependency of properties to the property graph. Labels
are dependent on the property graph and its elements. They are not
dependent on any object outside the property graph. We don't need a
separate query for pg_propgraph_labels. Same is the case with
pg_propgraph_element_label objects, which depend upon
pg_propgraph_label and pg_propgraph_element, nothing outside the
property graph. pg_propgraph_label_properties depends upon the
expressions (types, functions, columns etc.) which are outside a
property graph. So, we need a query to transfer dependencies from
pg_propgraph_label_properties to property graph. Ddependecies of
pg_propgraph_label_property also cover the dependencies of
pg_propgraph_property. But it's better to process each catalog that
can have dependencies outside the property graph. With that, we have
covered all the dependency transfers for property graphs in
getDependencies(). I have added one test each for dependency transfer
from pg_propgraph_property and pg_propgraph_label_properties to
property graph.
On a related note, we may avoid any dependency transfers if we handle
property graph components as dumpable objects and dump them separately
using ALTER PROPERTY GRAPH commands. But that will slow down the dump
and restore process.
- The prior UNION arms in getDependencies() prevent dependencies on itself -
but I don't think the pg_propgraph_element query does?Compare with e.g. the amproc case which has
"AND NOT (refclassid = 'pg_opfamily'::regclass AND amprocfamily = refobjid)\n");
Fixed. Also eliminated the dependency of edges on vertexes.
- Why are property graphs dumped as part of dumpTableSchema()?
I think it's already pretty weird that views are created as part of
dumpTableSchema(), but they at least share some infrastructure with
tables. I don't see any reason for propgraphs to not have been redirected in
dumpTable(), just like it's done for dumpSequence()?
Need to look into this one. Will do that soon.
- More curiosity: Why do property graphs have pg_class entries at all? As far
as I can tell it doesn't use anything from it?
Andrew has answered it already. There's one more. SQL/PGQ standard
section 11.19, syntax rule 4, note 157 mentions that property graphs
share the same namespace as the tables. Adding them to pg_class makes
it easy.
- Harmless, but it's a bit odd for the propgraph portion of getDependencies()
to filter deptype = 'p' away, given how long that has not existed.Perhaps getDependencies() code should just have a comment about why the
queries include 'p', despite that being an unknown kind of dependency these
days.
I think it's just to be consistent with the other queries. A comment
on getDependencies() would be good.
- External cascades leave orphan graph metadata
ALTER PROPERTY GRAPH explicitly removes unused labels/properties, but generic
dependency deletion bypasses that cleanup:CREATE TABLE v (id int PRIMARY KEY);
CREATE PROPERTY GRAPH g
VERTEX TABLES (v LABEL l PROPERTIES (id AS p));
DROP TABLE v CASCADE;g retains global label l and integer property p. Adding a new text property named p then incorrectly
reports a type mismatch.SELECT count(*) AS elements
FROM pg_propgraph_element
WHERE pgepgid = 'g'::regclass;SELECT count(*) AS labels
FROM pg_propgraph_label
WHERE pglpgid = 'g'::regclass;SELECT count(*) AS properties
FROM pg_propgraph_property
WHERE pgppgid = 'g'::regclass;CREATE TABLE v2 (id text PRIMARY KEY);
ALTER PROPERTY GRAPH g
ADD VERTEX TABLES (
v2 LABEL l PROPERTIES (id AS p)
);ERROR: 42601: property "p" data type mismatch: integer vs. text
DETAIL: In a property graph, a property of the same name has to have the same data type in each label.
Thanks for reporting it. I did not find any existing code which deals
with delete-when-reference-drops-to-zero behaviour. Ideally, we should
invent a new kind of dependency that will delete the dependent objects
when the number of references to the object drops to zero. But it's
possibly too late for that kind of change for PG 19 and the semantics
of such a dependency need to be carefully thought through to be
applicable beyond property graphs. We can rework this in a future
release and introduce such a dependency cleanup mechanism. For now, I
am adding a special handling in performDeletion() and
performMultipleDeletions() to collect and delete all the orphaned
property graph objects. To make it easy to convert it entirely driven
by dependency mechanism later, the code in the patch makes use of
pg_depend to find the orphaned property graph objects instead of using
the property graph catalog as much as possible.
To avoid code duplication, AlterPropGraph() also uses the same
routines to delete orphaned property graph objects. Current
AlterPropGraph() cleans up all the orphaned property graph objects
once per command but it examines every object irrespective of whether
its parent object was deleted or not. With this change we examine only
objects downstream to the objects that are being deleted but it means
we might be doing it multiple times for the same object. Given that
ALTER PROPERTY GRAPH can drop only one parent object at a time, the
probability of that leading to multiple deletions cascading to a
single orphaned object is low. So, I think it's a net win.
This fix is in 0002 patch which is WIP. I will be working more on it
tomorrow. Early comments are welcome.
I will go through rest of the issues and update the patchset in this thread.
--
Best Wishes,
Ashutosh Bapat
Attachments:
v20260827-0002-WIP-DROP-CASCADE-and-orphaned-property-gra.patchtext/x-patch; charset=US-ASCII; name=v20260827-0002-WIP-DROP-CASCADE-and-orphaned-property-gra.patchDownload+312-81
v20260827-0001-pg_dump-Transfer-dependencies-of-propertie.patchtext/x-patch; charset=US-ASCII; name=v20260827-0001-pg_dump-Transfer-dependencies-of-propertie.patchDownload+131-62
On Thu, Aug 27, 2026 at 10:38 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
Hi Andres,
Thanks for all the reports. I am going through them one by one. I will
post patches as I create them. Also responding to your questions that
I can respond to right now.On Sun, Aug 23, 2026 at 1:03 AM Andres Freund <andres@anarazel.de> wrote:
- Why do property graphs have pg_attribute entries?
As far as I can tell, the system attributes don't make any sense for a
property graph as system attributes aren't ever referenced? Other objects
for which system attributes, like composite types, don't have pg_attribute
rows for system attributes?And, IIUC, there aren't any other kind of attributes for property graph
relations?Those attributes are not needed and I think we can avoid creating them
safely. If we do that in PG 19, it may need a catversion bump - since
a property graph created before the change will have those attributes
and that after the change won't. The change itself won't remove the
attributes. AFAIU, even if we don't fix this in PG 19 to avoid
catversion bump those attributes are useless and harmless. We will fix
the code in PG 20 for sure. Peter, what do you think?- Pretty sure pg_dump's dependency handling for property graphs is
insufficient?I see there's code to handle dependencies via pg_propgraph_element, but
there's also dependencies like pg_propgraph_property.pgtypid? I don't
immediately see such a dependency would be visible to pg_dump, as
getDependencies() only additionally queries dependencies via
pg_propgraph_elementThere probably are unhandled dependencies other than
pg_propgraph_property.pgtypid.A property graph has three types of components: elements, labels and
properties. Query on pg_propgraph_element transfers the dependency of
elements to the property graph. Query on pg_propgraph_property
transfers the dependency of properties to the property graph. Labels
are dependent on the property graph and its elements. They are not
dependent on any object outside the property graph. We don't need a
separate query for pg_propgraph_labels. Same is the case with
pg_propgraph_element_label objects, which depend upon
pg_propgraph_label and pg_propgraph_element, nothing outside the
property graph. pg_propgraph_label_properties depends upon the
expressions (types, functions, columns etc.) which are outside a
property graph. So, we need a query to transfer dependencies from
pg_propgraph_label_properties to property graph. Ddependecies of
pg_propgraph_label_property also cover the dependencies of
pg_propgraph_property. But it's better to process each catalog that
can have dependencies outside the property graph. With that, we have
covered all the dependency transfers for property graphs in
getDependencies(). I have added one test each for dependency transfer
from pg_propgraph_property and pg_propgraph_label_properties to
property graph.On a related note, we may avoid any dependency transfers if we handle
property graph components as dumpable objects and dump them separately
using ALTER PROPERTY GRAPH commands. But that will slow down the dump
and restore process.- The prior UNION arms in getDependencies() prevent dependencies on itself -
but I don't think the pg_propgraph_element query does?Compare with e.g. the amproc case which has
"AND NOT (refclassid = 'pg_opfamily'::regclass AND amprocfamily = refobjid)\n");Fixed. Also eliminated the dependency of edges on vertexes.
- Why are property graphs dumped as part of dumpTableSchema()?
I think it's already pretty weird that views are created as part of
dumpTableSchema(), but they at least share some infrastructure with
tables. I don't see any reason for propgraphs to not have been redirected in
dumpTable(), just like it's done for dumpSequence()?Need to look into this one. Will do that soon.
- More curiosity: Why do property graphs have pg_class entries at all? As far
as I can tell it doesn't use anything from it?Andrew has answered it already. There's one more. SQL/PGQ standard
section 11.19, syntax rule 4, note 157 mentions that property graphs
share the same namespace as the tables. Adding them to pg_class makes
it easy.- Harmless, but it's a bit odd for the propgraph portion of getDependencies()
to filter deptype = 'p' away, given how long that has not existed.Perhaps getDependencies() code should just have a comment about why the
queries include 'p', despite that being an unknown kind of dependency these
days.I think it's just to be consistent with the other queries. A comment
on getDependencies() would be good.- External cascades leave orphan graph metadata
ALTER PROPERTY GRAPH explicitly removes unused labels/properties, but generic
dependency deletion bypasses that cleanup:CREATE TABLE v (id int PRIMARY KEY);
CREATE PROPERTY GRAPH g
VERTEX TABLES (v LABEL l PROPERTIES (id AS p));
DROP TABLE v CASCADE;g retains global label l and integer property p. Adding a new text property named p then incorrectly
reports a type mismatch.SELECT count(*) AS elements
FROM pg_propgraph_element
WHERE pgepgid = 'g'::regclass;SELECT count(*) AS labels
FROM pg_propgraph_label
WHERE pglpgid = 'g'::regclass;SELECT count(*) AS properties
FROM pg_propgraph_property
WHERE pgppgid = 'g'::regclass;CREATE TABLE v2 (id text PRIMARY KEY);
ALTER PROPERTY GRAPH g
ADD VERTEX TABLES (
v2 LABEL l PROPERTIES (id AS p)
);ERROR: 42601: property "p" data type mismatch: integer vs. text
DETAIL: In a property graph, a property of the same name has to have the same data type in each label.Thanks for reporting it. I did not find any existing code which deals
with delete-when-reference-drops-to-zero behaviour. Ideally, we should
invent a new kind of dependency that will delete the dependent objects
when the number of references to the object drops to zero. But it's
possibly too late for that kind of change for PG 19 and the semantics
of such a dependency need to be carefully thought through to be
applicable beyond property graphs. We can rework this in a future
release and introduce such a dependency cleanup mechanism. For now, I
am adding a special handling in performDeletion() and
performMultipleDeletions() to collect and delete all the orphaned
property graph objects. To make it easy to convert it entirely driven
by dependency mechanism later, the code in the patch makes use of
pg_depend to find the orphaned property graph objects instead of using
the property graph catalog as much as possible.To avoid code duplication, AlterPropGraph() also uses the same
routines to delete orphaned property graph objects. Current
AlterPropGraph() cleans up all the orphaned property graph objects
once per command but it examines every object irrespective of whether
its parent object was deleted or not. With this change we examine only
objects downstream to the objects that are being deleted but it means
we might be doing it multiple times for the same object. Given that
ALTER PROPERTY GRAPH can drop only one parent object at a time, the
probability of that leading to multiple deletions cascading to a
single orphaned object is low. So, I think it's a net win.This fix is in 0002 patch which is WIP. I will be working more on it
tomorrow. Early comments are welcome.- Similarly, query plan caching is not handled correctly after a CASCADE style
dropping.AlterPropGraph() calls CacheInvalidateRelcacheByRelid(), but that's not
invoked when done via performDeletion() -> DropObjectById().
I extended the earlier fix to drop the orphaned property and label
entries also to invalidate the caches.
Attached patchset has
0001 - a minor code refactoring to help 0004
0002 - a minor test case comment clarifying intention of the test case
0003 - pg_dump dependency transfer fix
0004 - fixes dropping orphaned property and label entries and also
invalidate caches because of cascaded drops.
- Views depend only on global pg_propgraph_label and pg_propgraph_property rows
not the specific label/property association.
Working on this next.
--
Best Wishes,
Ashutosh Bapat
Attachments:
v20260901-0002-Duplicate-code-in-performDeletion-and-perf.patchtext/x-patch; charset=US-ASCII; name=v20260901-0002-Duplicate-code-in-performDeletion-and-perf.patchDownload+32-23
v20260901-0001-Clarify-the-testcase-intent-in-create_prop.patchtext/x-patch; charset=US-ASCII; name=v20260901-0001-Clarify-the-testcase-intent-in-create_prop.patchDownload+2-1
v20260901-0003-pg_dump-Transfer-dependencies-of-propertie.patchtext/x-patch; charset=US-ASCII; name=v20260901-0003-pg_dump-Transfer-dependencies-of-propertie.patchDownload+131-62
v20260901-0004-DROP-CASCADE-to-property-graph-components.patchtext/x-patch; charset=US-ASCII; name=v20260901-0004-DROP-CASCADE-to-property-graph-components.patchDownload+611-95
On Tue, Sep 1, 2026 at 12:31 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
- External cascades leave orphan graph metadata
ALTER PROPERTY GRAPH explicitly removes unused labels/properties, but generic
dependency deletion bypasses that cleanup:CREATE TABLE v (id int PRIMARY KEY);
CREATE PROPERTY GRAPH g
VERTEX TABLES (v LABEL l PROPERTIES (id AS p));
DROP TABLE v CASCADE;g retains global label l and integer property p. Adding a new text property named p then incorrectly
reports a type mismatch.SELECT count(*) AS elements
FROM pg_propgraph_element
WHERE pgepgid = 'g'::regclass;SELECT count(*) AS labels
FROM pg_propgraph_label
WHERE pglpgid = 'g'::regclass;SELECT count(*) AS properties
FROM pg_propgraph_property
WHERE pgppgid = 'g'::regclass;CREATE TABLE v2 (id text PRIMARY KEY);
ALTER PROPERTY GRAPH g
ADD VERTEX TABLES (
v2 LABEL l PROPERTIES (id AS p)
);ERROR: 42601: property "p" data type mismatch: integer vs. text
DETAIL: In a property graph, a property of the same name has to have the same data type in each label.Thanks for reporting it. I did not find any existing code which deals
with delete-when-reference-drops-to-zero behaviour. Ideally, we should
invent a new kind of dependency that will delete the dependent objects
when the number of references to the object drops to zero. But it's
possibly too late for that kind of change for PG 19 and the semantics
of such a dependency need to be carefully thought through to be
applicable beyond property graphs. We can rework this in a future
release and introduce such a dependency cleanup mechanism. For now, I
am adding a special handling in performDeletion() and
performMultipleDeletions() to collect and delete all the orphaned
property graph objects. To make it easy to convert it entirely driven
by dependency mechanism later, the code in the patch makes use of
pg_depend to find the orphaned property graph objects instead of using
the property graph catalog as much as possible.To avoid code duplication, AlterPropGraph() also uses the same
routines to delete orphaned property graph objects. Current
AlterPropGraph() cleans up all the orphaned property graph objects
once per command but it examines every object irrespective of whether
its parent object was deleted or not. With this change we examine only
objects downstream to the objects that are being deleted but it means
we might be doing it multiple times for the same object. Given that
ALTER PROPERTY GRAPH can drop only one parent object at a time, the
probability of that leading to multiple deletions cascading to a
single orphaned object is low. So, I think it's a net win.This fix is in 0002 patch which is WIP. I will be working more on it
tomorrow. Early comments are welcome.- Similarly, query plan caching is not handled correctly after a CASCADE style
dropping.AlterPropGraph() calls CacheInvalidateRelcacheByRelid(), but that's not
invoked when done via performDeletion() -> DropObjectById().I extended the earlier fix to drop the orphaned property and label
entries also to invalidate the caches.Attached patchset has
0001 - a minor code refactoring to help 0004
0002 - a minor test case comment clarifying intention of the test case
0003 - pg_dump dependency transfer fix
0004 - fixes dropping orphaned property and label entries and also
invalidate caches because of cascaded drops.- Views depend only on global pg_propgraph_label and pg_propgraph_property rows
not the specific label/property association.CREATE TABLE v1 (id integer PRIMARY KEY, n integer);
CREATE TABLE v2 (id integer PRIMARY KEY, n integer);CREATE PROPERTY GRAPH g
VERTEX TABLES (
v1 LABEL l1 PROPERTIES (n AS p) LABEL keep NO PROPERTIES,
v2 LABEL l2 PROPERTIES (n AS p)
);CREATE VIEW gv AS
SELECT *
FROM GRAPH_TABLE (
g MATCH (x IS l1)
COLUMNS (x.p)
);ALTER PROPERTY GRAPH g
ALTER VERTEX TABLE v1
ALTER LABEL l1 DROP PROPERTIES (p);SELECT to_regclass('gv') AS view_still_exists;
SELECT * FROM gv;results in:
ERROR: 42704: property "p" for element variable "x" not found
This behaviour is governed by section 11.26, syntax rule 8.a. It says
If ALTER LABEL action immediately contains DROP PROPERTY then:
a. If RESTRICT is specified, then SPG (the property graph specified in
the ALTER PROPERTY GRAPH statement) shall not be referenced in any of
the following: the list here includes, views, functions, constraints,
value expressions in other property graph etc.
b. Note 164 says "If CASCADE is specified, then any such dependent
object will be dropped by the execution of the revoke statement
specified in the general rules of this subclause." I did not find any
revoke statement in section 11.26. Something to fix , but there's a
revoke statement in DROP PROPERTY GRAPH section.
Interpreting a and b together, in the above case dropping the property
should not be allowed in RESTRICT mode. In CASCADE mode however, the
view should be dropped. That's consistent with the other DROP CASCADE
behaviours. However, consider the case below:
CREATE FUNCTION shared_f() RETURNS int LANGUAGE SQL RETURN 1;
ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t1 ADD LABEL shared_l
PROPERTIES (shared_f() AS shared_p);
ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t2 ADD LABEL shared_l
PROPERTIES (i AS shared_p);
DROP FUNCTION shared_f() CASCADE;
DROP FUNCTION would lead to dropping the property shared_p in label
shared_l of element t1. But the shared_p in label shared_l of element
t2 remains unaffected, but now the two labels are inconsistent with
each other. The section 11.26 does not specify directly the outcome of
this situation. Should DROP FUNCTION be rejected because property
graph consistency rules fail even if CASCADE is specified? That looks
inconsistent with how CASCADE mode is applied in other cases. Can you
please clarify?
--
Best Wishes,
Ashutosh Bapat
On Tue, Sep 1, 2026 at 5:20 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
On Tue, Sep 1, 2026 at 12:31 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:- External cascades leave orphan graph metadata
ALTER PROPERTY GRAPH explicitly removes unused labels/properties, but generic
dependency deletion bypasses that cleanup:CREATE TABLE v (id int PRIMARY KEY);
CREATE PROPERTY GRAPH g
VERTEX TABLES (v LABEL l PROPERTIES (id AS p));
DROP TABLE v CASCADE;g retains global label l and integer property p. Adding a new text property named p then incorrectly
reports a type mismatch.SELECT count(*) AS elements
FROM pg_propgraph_element
WHERE pgepgid = 'g'::regclass;SELECT count(*) AS labels
FROM pg_propgraph_label
WHERE pglpgid = 'g'::regclass;SELECT count(*) AS properties
FROM pg_propgraph_property
WHERE pgppgid = 'g'::regclass;CREATE TABLE v2 (id text PRIMARY KEY);
ALTER PROPERTY GRAPH g
ADD VERTEX TABLES (
v2 LABEL l PROPERTIES (id AS p)
);ERROR: 42601: property "p" data type mismatch: integer vs. text
DETAIL: In a property graph, a property of the same name has to have the same data type in each label.Thanks for reporting it. I did not find any existing code which deals
with delete-when-reference-drops-to-zero behaviour. Ideally, we should
invent a new kind of dependency that will delete the dependent objects
when the number of references to the object drops to zero. But it's
possibly too late for that kind of change for PG 19 and the semantics
of such a dependency need to be carefully thought through to be
applicable beyond property graphs. We can rework this in a future
release and introduce such a dependency cleanup mechanism. For now, I
am adding a special handling in performDeletion() and
performMultipleDeletions() to collect and delete all the orphaned
property graph objects. To make it easy to convert it entirely driven
by dependency mechanism later, the code in the patch makes use of
pg_depend to find the orphaned property graph objects instead of using
the property graph catalog as much as possible.To avoid code duplication, AlterPropGraph() also uses the same
routines to delete orphaned property graph objects. Current
AlterPropGraph() cleans up all the orphaned property graph objects
once per command but it examines every object irrespective of whether
its parent object was deleted or not. With this change we examine only
objects downstream to the objects that are being deleted but it means
we might be doing it multiple times for the same object. Given that
ALTER PROPERTY GRAPH can drop only one parent object at a time, the
probability of that leading to multiple deletions cascading to a
single orphaned object is low. So, I think it's a net win.This fix is in 0002 patch which is WIP. I will be working more on it
tomorrow. Early comments are welcome.- Similarly, query plan caching is not handled correctly after a CASCADE style
dropping.AlterPropGraph() calls CacheInvalidateRelcacheByRelid(), but that's not
invoked when done via performDeletion() -> DropObjectById().I extended the earlier fix to drop the orphaned property and label
entries also to invalidate the caches.Attached patchset has
0001 - a minor code refactoring to help 0004
0002 - a minor test case comment clarifying intention of the test case
0003 - pg_dump dependency transfer fix
0004 - fixes dropping orphaned property and label entries and also
invalidate caches because of cascaded drops.- Views depend only on global pg_propgraph_label and pg_propgraph_property rows
not the specific label/property association.CREATE TABLE v1 (id integer PRIMARY KEY, n integer);
CREATE TABLE v2 (id integer PRIMARY KEY, n integer);CREATE PROPERTY GRAPH g
VERTEX TABLES (
v1 LABEL l1 PROPERTIES (n AS p) LABEL keep NO PROPERTIES,
v2 LABEL l2 PROPERTIES (n AS p)
);CREATE VIEW gv AS
SELECT *
FROM GRAPH_TABLE (
g MATCH (x IS l1)
COLUMNS (x.p)
);ALTER PROPERTY GRAPH g
ALTER VERTEX TABLE v1
ALTER LABEL l1 DROP PROPERTIES (p);SELECT to_regclass('gv') AS view_still_exists;
SELECT * FROM gv;results in:
ERROR: 42704: property "p" for element variable "x" not found
This behaviour is governed by section 11.26, syntax rule 8.a. It says
If ALTER LABEL action immediately contains DROP PROPERTY then:
a. If RESTRICT is specified, then SPG (the property graph specified in
the ALTER PROPERTY GRAPH statement) shall not be referenced in any of
the following: the list here includes, views, functions, constraints,
value expressions in other property graph etc.
b. Note 164 says "If CASCADE is specified, then any such dependent
object will be dropped by the execution of the revoke statement
specified in the general rules of this subclause." I did not find any
revoke statement in section 11.26. Something to fix , but there's a
revoke statement in DROP PROPERTY GRAPH section.Interpreting a and b together, in the above case dropping the property
should not be allowed in RESTRICT mode. In CASCADE mode however, the
view should be dropped. That's consistent with the other DROP CASCADE
behaviours.
I forgot to mention something earlier.
Above section says that SPG (the property graph whose property is
being deleted) should not be referenced in any of the objects in
RESTRICT mode. It does not say "property" shouldn't be referenced in
any of the objects in RESTRICT mode. So dropping a property from any
element would be prohibited if there exists another object which
references the property graph not just that property. For example,
dropping property from label l2 of v2 won't be allowed because view gv
depends upon property graph g even if the view does not refer label l2
(and property p there in). Am I interpreting it right?
Together with note 164, this means that if any of the components of a
property graph is dropped, it will lead to dropping all the objects
that depend upon the property graph whether or not they reference the
component being dropped. Is that interpretation correct?
However, consider the case below:
CREATE FUNCTION shared_f() RETURNS int LANGUAGE SQL RETURN 1;
ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t1 ADD LABEL shared_l
PROPERTIES (shared_f() AS shared_p);
ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t2 ADD LABEL shared_l
PROPERTIES (i AS shared_p);
DROP FUNCTION shared_f() CASCADE;DROP FUNCTION would lead to dropping the property shared_p in label
shared_l of element t1. But the shared_p in label shared_l of element
t2 remains unaffected, but now the two labels are inconsistent with
each other. The section 11.26 does not specify directly the outcome of
this situation. Should DROP FUNCTION be rejected because property
graph consistency rules fail even if CASCADE is specified? That looks
inconsistent with how CASCADE mode is applied in other cases. Can you
please clarify?
--
Best Wishes,
Ashutosh Bapat
On Tue, Sep 1, 2026 at 11:20 AM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
However, consider the case below:
CREATE FUNCTION shared_f() RETURNS int LANGUAGE SQL RETURN 1;
ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t1 ADD LABEL shared_l
PROPERTIES (shared_f() AS shared_p);
ALTER PROPERTY GRAPH g4 ALTER VERTEX TABLE t2 ADD LABEL shared_l
PROPERTIES (i AS shared_p);
DROP FUNCTION shared_f() CASCADE;DROP FUNCTION would lead to dropping the property shared_p in label
shared_l of element t1. But the shared_p in label shared_l of element
t2 remains unaffected, but now the two labels are inconsistent with
each other. The section 11.26 does not specify directly the outcome of
this situation. Should DROP FUNCTION be rejected because property
graph consistency rules fail even if CASCADE is specified? That looks
inconsistent with how CASCADE mode is applied in other cases. Can you
please clarify?
[RMT hat]
The RMT has been spending time looking at the current state of PGQ
given the release time, and we have some serious concerns about the
number of design issues with either recently posted patches or without
consensus on behavior. The majority of those are in this thread, so
I'll summarize my understanding of a few of them that are currently
unresolved:
Orphaned label/property metadata (External generic CASCADEs (e.g. DROP
TABLE ... CASCADE) leave orphan graph metadata):
---
These show up as phantom entries in information_schema views, can
cause later legitimate alterations of the property graph to fail, and
can render the graph undumpable/unrestorable.
There is a patch that was posted yesterday (September 1st) that aims
to fix this but it is considered a stop-gap measure and not the ideal
fix. Others have not weighed in on how effective this stop-gap measure
is. And it is acknowledged that the ideal fix is out-of-scope for 19.
Global vs per-label scoping of labels and properties:
---
A view over a GRAPH_TABLE becomes silently unqueryable after a
property/label is dropped from one element when it still exists
elsewhere in the graph.
Ashutosh has analyzed the standard and has an interpretation of what
the correct behavior should be with RESTRICT vs CASCADE, but others
have not weighed in. Additionally, there is an unanswered question
around what should happen for a function/property shared across two
labels.
There is not a posted fix for this yet, the desired behavior in some
cases is unresolved pending an interpretation of the standard, and
there is acknowledgement that a more correct fix is
association-granularity dependencies which is out of scope for 19.
Whether there should be a pg_class entry for property graphs:
---
Andres who posted his initial concern about this has not indicated if
the answers resolve his concern. Separately, though having a pg_class
entry seems to buy you some functionality for free, the bug Andrew
reported where AlterPropGraph() doesn't verify its target is a
property graph feels like it may be indicative of more problems
related to code that is reading pg_class entries and not
distinguishing between graphs and tables.
Assorted other issues in this thread:
---
Elsewhere in this thread, there are several more dependency ordering
problems that don't have proposed fixes yet, for example, the
unresolved dependency loop in pg_dump when materialized view queries a
GRAPH_TABLE.
To be clear, our concern is not a result of having reviewed the
uncommitted patches in this thread or PGQ code more broadly. We are
most concerned that there are several reported issues that pose design
and behavior questions that the community and patch authors have yet
to resolve. We feel that if any desired behavior is still unsettled,
there is insufficient time to fix these in time for the release. This
work is in addition to other reported PGQ bugs that do not involve
design issues but that nonetheless need to be fixed before releasing
Postgres 19.
As such, unless we are misunderstanding something about the
discussion, we feel it would be better to revert this in 19. That
would take off the time pressure now and would make it easier to fix
these things properly in 20 without having to be burdened by backwards
compatibility and backpatching.
- Melanie
Hi,
There is a patch that was posted yesterday (September 1st) that aims
to fix this but it is considered a stop-gap measure and not the ideal
fix. Others have not weighed in on how effective this stop-gap measure
is. And it is acknowledged that the ideal fix is out-of-scope for 19.Global vs per-label scoping of labels and properties:
---
A view over a GRAPH_TABLE becomes silently unqueryable after a
property/label is dropped from one element when it still exists
elsewhere in the graph.Ashutosh has analyzed the standard and has an interpretation of what
the correct behavior should be with RESTRICT vs CASCADE, but others
have not weighed in. Additionally, there is an unanswered question
around what should happen for a function/property shared across two
labels.There is not a posted fix for this yet, the desired behavior in some
cases is unresolved pending an interpretation of the standard, and
there is acknowledgement that a more correct fix is
association-granularity dependencies which is out of scope for 19.
I was planning on replying about this patch set, since I started going
through it yesterday/this morning. The one in this series that gives me
the most pause is v20260901-0004, because it introduces
property-graph-specific deletion planning into dependency.c. To me,
that reads as compensation for the property-graph-related objects not
being represented cleanly enough in the first place. I understand this
is a stop-gap for v19, but I am not sure we should accept a stop-gap
that introduces this kind of design violation.
The situation here is a bit unusual, because what we really need to
know is whether deleting a given row removes the last remaining
reference to the shared property-graph object. The approach in
v20260901-0004 seems to work, but only by teaching dependency.c far too
much about property graphs. After findDependentObjects() has built the
initial deletion set, this otherwise generic code path now needs an
extra property-graph-specific orphan-cleanup pass.
--
Sami Imseih
Amazon Web Services (AWS)
Hi,
On 2026-09-02 15:29:06 -0400, Melanie Plageman wrote:
Whether there should be a pg_class entry for property graphs:
---
Andres who posted his initial concern about this has not indicated if
the answers resolve his concern.
It does. I was really just curious why that choice was made. And I think
Ashutosh's answer explains it and I think alternative ways of implementing the
single-namespace would be way worse. Other than perhaps immortalizing it in a
comment somewhere, not further action is needed.
Separately, though having a pg_class entry seems to buy you some
functionality for free, the bug Andrew reported where AlterPropGraph()
doesn't verify its target is a property graph feels like it may be
indicative of more problems related to code that is reading pg_class entries
and not distinguishing between graphs and tables.
I think that's a serious bug, but it's also pretty easy to fix, and it's
something that a lot of different places already have to have checks
for. Needing to check for a specific relkind is far from unique to propgraphs.
To me the concerning aspect about that omission isn't so much that it
indicates an architectural issue, but that it pretty clearly indicates
insufficiently reviewed code.
I was looking at AlterPropGraph() while writing the above paragraphs, and it
made me wonder why it'd be sufficient to use ShareRowExclusiveLock? Why is it
ok to alter a propgraph fairly fundamentally while there's queries using the
propgraph (with AccessShareLock, which does not conflict with
ShareRowExclusiveLock)? It's obviously sometimes possible to use
ShareRowExclusiveLock, but typically not for stuff that could completely
invalidate concurrently running queries, and changing the schema of a
propgraph by dropping vertex/edges certainly seems like it would?
Greetings,
Andres Freund
Hi,
On 2026-09-02 15:29:06 -0400, Melanie Plageman wrote:
[RMT hat]
The RMT has been spending time looking at the current state of PGQ
given the release time, and we have some serious concerns about the
number of design issues with either recently posted patches or without
consensus on behavior. The majority of those are in this thread, so
I'll summarize my understanding of a few of them that are currently
unresolved:
I concur, this isn't ready for v19. And I think it might not be ready to stay
in 20 either.
Observations from looking at the relevant code for about half the time since
my last email:
- Afaict the locking in rewriteGraphTable() is wrong. Unless I am missing
something, we're accessing the schema of the referenced tables without
holding any locks on the tables (c.f. get_atttypetypmodcoll() calls in
build_edge_vertex_link_quals())
This isn't just bad when the graph table query is in a view (where the
parser wouldn't have acquired locks), afaict parse-analysis doesn't acquire
any locks on the element tables?
- As mentioned in my last email, nothing protects the graph object against
being altered while rewriteGraphTable() is running, due to AlterPropGraph()
only holding an AccessShareLock. That's bad.
- Also bad: Afaict a table or column - involved in a property graph - being
dropped, does not seem to actually lock the property graph? Which means that
could happen concurrently with rewriteGraphTable() even if AlterPropGraph()
- The code doesn't seem to follow postgres' coding style much. E.g. there are
a lot of very long lines without there being any reason for that.
This is a partial manual review, for less than 40min, looking just at one
aspect. And I am pretty sure at least some of the above is valid. There's
bound to me more.
I think this feature is nowhere near ready.
Greetings,
Andres Freund
Hi,
On 2026-09-02 17:33:38 -0400, Andres Freund wrote:
- As mentioned in my last email, nothing protects the graph object against
being altered while rewriteGraphTable() is running, due to AlterPropGraph()
only holding an AccessShareLock. That's bad.
An particularly odd thing about that is that some things in
rewriteGraphTable.c actually use a more heavyweight lock level -
is_property_associated_with_label() and get_element_property_expr() use
RowShareLock. I don't really understand why? There's no comment about that
either?
There are also unexplained uses of RowShareLock in propgraphcmds.c - why is
e.g. pg_attribute opened with RowShareLock?
Separately (noticed when searching for RowShareLock uses): Isn't it somewhat
of a problem to do a seqscan of PropgraphLabelPropertyRelationId for
/* Remove any orphaned pg_propgraph_property entries */
I don't know why that code uses RowShareLock either. Not using AccessShareLock
makes sense, afaict the performDeletion() of PropgraphPropertyRelationId done
during the scan will recurse to PropgraphLabelPropertyRelationId - avoiding
deadlock hazards due to lock upgrades could make sense. But a deletion takes
RowExclusive, not RowShare? So what's the point of RowShare?
Greetings,
Andres Freund
Hi,
In addition to the manually found things in the last few emails, I also
started an AI review. Which found some not good stuff. I checked manually
that all of these actually are issues.
- out of bound reads due to broken unknown handling
The problem here is that transformRangeGraphTable() uses p_resolve_unknowns
to control what unknown is resolved to. Other places explicitly specify
resolve_uknowns for from items (c.f. transformRangeSubselect() passing
resolve_unknowns = true to parse_sub_analyze()).
The problem is that
generate_setop_from_pathqueries()->constructSetOpTargetlist() then resolves
those uknowns as text. But the upper query levels still think its an
unknown.
Depending on the path taken, this then either triggers errors via
CheckVarSlotCompatibility() or out-of-bound reads / crashes.
This whole thing requires resolve_unknown = false, which I think basically
means an INSERT.
Repro in pgq-1.sql.
- Whole-row reference to a GRAPH_TABLE expands to zero columns / triggers errors
`addRangeTableEntryForGraphTable()` computes `coltypes`/`coltypmods`/`colcollations` locally and
passes them only to `buildNSItemFromLists()` (`parse_relation.c:2219-2249`); it never assigns them
to the RTE. The VALUES, TABLEFUNC, CTE and ENR cases in the same file all do. `expandRTE()` handles
`RTE_GRAPH_TABLE` by iterating exactly those three lists (`parse_relation.c:3155`), reached from
`expandRecordVariable()` (`parse_target.c:1558`).
Repro in pgq-2.sql.
- No uniqueness check on element keys or edge reference keys
propgraph_element_get_key(), in the column-list != NIL case, doesn't check
that there's actually a unique key. But the docs seem to pretty clearly say
that the key should be unique?
Repro in pgq-3.sql.
- ALTER PROPERTY GRAPH ... ADD LABEL/PROPERTIES` doesn't check privs on the element table
While vertex and edge table paths do check permissions, the add_labels path
does not.
Repro in pgq-4.sql
- Whole row references inside a graph break pg_dump (and \d+)
make_propgraphdef_properties() calls get_attname() with missing_ok = false
for any Var property expression. A whole-row reference has varattno == 0 and
causes get_attname() to error out. The property does work at query time.
Repro in pgq-5.sql
There's also some minor stuff:
- Wrong SQLSTATE + missing error position on undefined property: parse_graphtable.c:132-135 uses
ERRCODE_SYNTAX_ERROR and omits parser_errposition, unlike the sibling label case at :189 (
ERRCODE_UNDEFINED_OBJECT). Undefined label at :187-191 also omits the position.
- Element tables in a rewritten GRAPH_TABLE are always checked as the current
user, while the property-graph object's own ACL is checked as the view owner
I think this is intentional and somewhat defensible? I don't think it can
realistically lead to more disclosure than intended? But it might be worth
for others to think about.
Greetings,
Andres Freund
- Wrong SQLSTATE + missing error position on undefined property: parse_graphtable.c:132-135 uses
ERRCODE_SYNTAX_ERROR and omits parser_errposition, unlike the sibling label case at :189 (
ERRCODE_UNDEFINED_OBJECT). Undefined label at :187-191 also omits the position.
This one has already been raised https://commitfest.postgresql.org/patch/7173/
--
Sami Imseih
Amazon Web Services (AWS)
As I was looking more into dependency related parts of GRAPH,
I encountered this, which I don't see mentioned in this thread already.
```
postgres=# CREATE TABLE t_alter_type (
id int PRIMARY KEY,
payload int
);
CREATE PROPERTY GRAPH g_alter_type
VERTEX TABLES (
t_alter_type PROPERTIES (payload AS payload_prop)
);
ALTER TABLE t_alter_type ALTER COLUMN payload TYPE bigint;
CREATE TABLE
CREATE PROPERTY GRAPH
ERROR: unexpected object depending on column: property payload_prop
of label t_alter_type of vertex t_alter_type of property graph
g_alter_type
postgres=#
```
ISTM that RememberAllDependentForRebuilding() does not handle all
property graph cases
properly.
--
Sami Imseih
Amazon Web Services (AWS)
I concur, this isn't ready for v19. And I think it might not be ready to stay
in 20 either.
and
In addition to the manually found things in the last few emails, I also
started an AI review. Which found some not good stuff. I checked manually
that all of these actually are issues.
My claude feature-cross check analysis also finished, and a significant amount of issues it reported on master are related to PGQ. I did not attempt a targeted detailed review of the PGQ commits independently, this was only the "Let's try to use PGQ together with <X> and see what happens" review.
I planned to open separate threads about the most critical not yet reported findings today and start working on fixes for some of them, but based on the recent messages in this thread I'll just drop a short summary here for now. I think all of these are unreported, but I might have missed something. Initially I asked claude to cross-check what was already reported, then I tried to verify again manually that the remaining are not really not yet mentioned.
All of these have claude generated reproducers I can share, but I want to point out that I did not properly verify them manually yet, I was planning on doing that later during the day before starting threads about them. But even if some of them are end up being false reports, the number of them itself is significant.
1. DROP OWNED BY fails as soon as the role holds any privilege on a property graph, and DROP ROLE then fails on the leftover pg_shdepend row.
2. pg_dump silently loses any graph whose edge keys were inferred from a foreign key.
3. Wrong results from keys that do not identify. The uniqueness check you mentioned is one half of it, there are two more ways to get there with a key that is enforced:
- the collation guard in propgraph_edge_get_ref_keys() only fires when both sides carry an explicit non-default collation, so a plain text vertex key against an edge column declared with a nondeterministic collation is accepted, and one edge row becomes one path per case-insensitive match. ATAddForeignKeyConstraint() refuses the same column pair.
- element RTEs are built with inh = true, but the implicit key comes from the parent's PK only. CREATE TABLE ... INHERITS (vertex_table) with duplicate ids in the child multiplies paths from the same edge row, and the child needs no privilege on the graph. ONLY is not spellable in the element table syntax. Partitioned tables are fine, the PK covers the partition key.
4. No relkind check on element tables either, not only on the ALTER target.
5. UPDATE / DELETE / MERGE with a GRAPH_TABLE in the source, and SELECT ... FOR UPDATE joining one, fail whenever the graph subquery is not pulled up (multi path patterns, volatile COLUMNS expressions):
'relation "g" does not have a composite type' from makeWholeRowVar(), and after fixing that 'cannot open relation "g"' from set_rel_width().
6. The unqualified FOR UPDATE / FOR SHARE over a query using GRAPH_TABLE is accepted and silently locks nothing.
The regression test even records this as "-- ignored". I want to point out that this is not only a missing feature: the element tables are opened with AccessShareLock, no UPDATE privilege is required (an equivalent security_invoker view refuses), and a two session test shows the other session's UPDATE going through while the "locked" reader sees the new value in the same transaction.
7. The privilege model has 3 surfaces that disagree, and your note about element tables being checked as the current user is one of them:
- CREATE / ALTER PROPERTY GRAPH requires ownership of every element table (RangeVarCallbackOwnsRelation), not SELECT, which contradicts the "acts like a security invoker view" description, and an owner without SELECT can create a graph it cannot query.
- at query time the graph ACL is checked as the view/rule owner and the element tables as the invoker, which makes property graphs a second, undocumented exception to rules.sgml's "checked against the privileges of the rule owner".
- the SQL/PGQ information_schema views gate on privilege on the graph, not on the element table whose columns and property expressions they publish, and pg_element_table_properties prints the user written expression with no ownership test, unlike views.view_definition or triggers.action_condition.
None of these leaks anything that pg_propgraph_* and pg_get_propgraphdef() don't already hand out ungated, but the boundary described is not the one enforced.
8. pg_get_propgraphdef() qualifies a temporary graph with get_namespace_name(), so it prints pg_temp_NN where every other deparse entry point prints pg_temp.
Additionally it flagged that the current patches for the whole-row property issue and the pg_dump dependency patch don't cover all cases it discovered. I'll try to verify that later today and post separate replies about it, if it's the case.
On Wed, Sep 2, 2026 at 5:33 PM Andres Freund <andres@anarazel.de> wrote:
- Afaict the locking in rewriteGraphTable() is wrong. Unless I am missing
something, we're accessing the schema of the referenced tables without
holding any locks on the tables (c.f. get_atttypetypmodcoll() calls in
build_edge_vertex_link_quals())
Ouch.
This isn't just bad when the graph table query is in a view (where the
parser wouldn't have acquired locks), afaict parse-analysis doesn't acquire
any locks on the element tables?
I'm having trouble parsing this sentence. Are there some extra
negatives in here someplace, or some missing ones?
- As mentioned in my last email, nothing protects the graph object against
being altered while rewriteGraphTable() is running, due to AlterPropGraph()
only holding an AccessShareLock. That's bad.
AlterPropGraph() takes ShareRowExclusiveLock on the property graph
itself. The comment justifies why the lock level isn't weaker, but not
why it isn't stronger; normally, DDL takes AccessExclusiveLock, and
that would be my baseline expectation here, too. This takes
AccessShareLock on the tables involved. As you also sort of say in
your followup email, there's no justification in here of any of the
lock levels taken, and some of them are non-standard for the types of
operations that they are.
- Also bad: Afaict a table or column - involved in a property graph - being
dropped, does not seem to actually lock the property graph? Which means that
could happen concurrently with rewriteGraphTable() even if AlterPropGraph()
I think the end of this sentence got cut off.
What generally bothers me here is that it's sort of systematically
unclear how the integrity model is supposed to work. If we look at
something like build_edge_vertex_link_quals(), I think it incidentally
depends on quite a bit of stuff. That function does three consecutive
syscache lookups on PROPGRAPHELOID, which gives it two arrays of
attribute numbers and an array of equality operators. But this
function doesn't document why things can't change between one of those
syscache lookups and the next. Maybe we're guaranteed to have a strong
enough lock on the property graph at this point to prevent that, but
there's no clear documentation of that. Then we call
get_atttypetypmodcoll(), as you said, but I don't think we're
guaranteed to have any locks on the tables at all at that point, as
you said. So what guarantees that the table properties are still in
sync with the property graph properties? In fact, even apart from
locking, it doesn't appear that everything relevant is checked.
For example, uniqueness is a requirement:
CREATE TABLE v (a int, b text);
CREATE TABLE e (id int PRIMARY KEY, va int);
CREATE PROPERTY GRAPH g
VERTEX TABLES (v)
EDGE TABLES (e SOURCE KEY (va) REFERENCES v (a)
DESTINATION KEY (va) REFERENCES v (a));
This fails, because v has no primary key. But if you do ALTER TABLE v
ADD PRIMARY KEY (a) first then it succeeds, and you can immediately
turn around and do ALTER TABLE v DROP CONSTRAINT v_pkey afterwards.
Foreign keys are are a requirement, so this fails:
CREATE TABLE v1 (a int PRIMARY KEY, name text);
CREATE TABLE v2 (m int PRIMARY KEY, name text);
CREATE TABLE e (id int PRIMARY KEY, k1 int, k2 int);
CREATE PROPERTY GRAPH g
VERTEX TABLES (v1, v2)
EDGE TABLES (e SOURCE v1 DESTINATION v2);
But if you temporarily add foreign keys with ALTER TABLE e ADD
CONSTRAINT e_k1_fkey FOREIGN KEY (k1) REFERENCES v1, ADD CONSTRAINT
e_k2_fkey FOREIGN KEY (k2) REFERENCES v2, then the above command
succeeds, and you can turn around and drop the foreign keys afterward
with ALTER TABLE e DROP CONSTRAINT e_k1_fkey.
Or consider this example:
CREATE TABLE t1 (a int PRIMARY KEY, b text);
CREATE TABLE t2 (a int PRIMARY KEY);
CREATE PROPERTY GRAPH g
VERTEX TABLES (t1 LABEL l PROPERTIES ALL COLUMNS,
t2 LABEL l PROPERTIES ALL COLUMNS);
This fails, because the number of properties doesn't match. If we do
ALTER TABLE t2 ADD COLUMN b text the this works, and a command like
SELECT * FROM GRAPH_TABLE (g MATCH (n IS l) COLUMNS (n.a, n.b)) ORDER
BY a; succeeds. However, afterward we can do ALTER TABLE t2 DROP
COLUMN b CASCADE (the CASCADE is required), and the same SELECT now
fails. So once again, we can put the object into a state after
creation that wouldn't have been valid at creation time.
Also consider this:
CREATE TABLE v1 (a int PRIMARY KEY, b text, unused text);
CREATE TABLE v2 (m int PRIMARY KEY);
CREATE TABLE e (id int PRIMARY KEY, k1 int, k2 int);
CREATE PROPERTY GRAPH g
VERTEX TABLES (v1 LABEL l PROPERTIES (a, upper(b) AS bu),
v2)
EDGE TABLES (e KEY (id)
SOURCE KEY (k1) REFERENCES v1 (a)
DESTINATION KEY (k2) REFERENCES v2 (m));
ALTER TABLE v1 ALTER COLUMN a TYPE bigint;
This produces "ERROR: unexpected object depending on column: vertex
v1 of property graph g" which is obviously not good enough. I see Sami
has reported this one, too.
To me, this class of problem seems completely unacceptable in a
committed feature. Generally, if creating the object requires certain
properties to hold, then later DDL that would cause those properties
to be violated should also be blocked. That's not an absolutely
inviolable rule, and I believe we do have other cases where certain
things are only checked at creation time. But I think those are
usually hard-to-avoid corner cases with careful justification and
careful testing, which doesn't seem to be the case here. And notice
that we're not reaching the question of locking, where we might fear
that certain kinds of invalid states can be created under concurrency
that wouldn't normally be allowed. In these cases, we don't even need
the concurrency; the sanity checks aren't even there for the
single-session case. And it also doesn't look like this is a case
where some design principle exists but the code fails to fully conform
to it. There's just a big design gap here, AFAICS, which is not the
sort of thing we should be trying to plug after the initial commit.
- The code doesn't seem to follow postgres' coding style much. E.g. there are
a lot of very long lines without there being any reason for that.
Yes, this code is really hard to read.
--
Robert Haas
EDB: http://www.enterprisedb.com
Hi,
On 2026-09-03 15:47:32 -0400, Robert Haas wrote:
On Wed, Sep 2, 2026 at 5:33 PM Andres Freund <andres@anarazel.de> wrote:
This isn't just bad when the graph table query is in a view (where the
parser wouldn't have acquired locks), afaict parse-analysis doesn't acquire
any locks on the element tables?I'm having trouble parsing this sentence. Are there some extra
negatives in here someplace, or some missing ones?
Something indeed isn't quite right, I think I was mainly just being too terse
(IIRC I had a longer explanation there that felt redundant, that I shortened
... too much).
What I wanted to draw attention to is that there are two different "locking
schemes" related to rewrite/ code. Rewrites on plain queries can rely on
locks being acquired during parse-analysis. Queries that originate in views
(and other rule related stuff) however, can't rely on that, since
parse-analysis is done during the view's creation. Therefore in the view case,
we rely on acquiring locks via AcquireRewriteLocks().
Here I think we acquire locks on the graph table elements in neither of the
two paths.
- As mentioned in my last email, nothing protects the graph object against
being altered while rewriteGraphTable() is running, due to AlterPropGraph()
only holding an AccessShareLock. That's bad.AlterPropGraph() takes ShareRowExclusiveLock on the property graph
itself. The comment justifies why the lock level isn't weaker, but not
why it isn't stronger; normally, DDL takes AccessExclusiveLock, and
that would be my baseline expectation here, too.
Yep. Seems pretty clear that AlterPropGraph() needs an AEL as it can affect
the "result shape". It's only ok to use a lower lock level if it's ok to
continue operating with an outdated definition of the object.
I guess we could make uses of the propgraph use a more heavyweight lock
instead, but ... :)
- Also bad: Afaict a table or column - involved in a property graph - being
dropped, does not seem to actually lock the property graph? Which means that
could happen concurrently with rewriteGraphTable() even if AlterPropGraph()I think the end of this sentence got cut off.
Ooops. I guess I was intending to say something like "even if
AlterPropGraph() were to take an AEL, as a table that is part of the property
graph could be dropped without going through AlterPropGraph(), and therefore
without locking the propgraph with an AEL".
[ more problems]
To me, this class of problem seems completely unacceptable in a
committed feature.
A small number of oversights of this class, in a feature of the size of PGQ,
doesn't necessarily seem entirely unacceptable to me. We are all human, and we
don't always have an existing pattern to model things after, making it harder
to get it right (e.g. I don't think we have a real precedent for the type of
dependencies that are needed here).
But the density of such problems in PGQ, at this stage of the release cycle
(and, TBH, probably at commit time), doesn't seem OK.
Greetings,
Andres Freund