| From: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> |
|---|---|
| To: | Andres Freund <andres(at)anarazel(dot)de>, Peter Eisentraut <peter(at)eisentraut(dot)org> |
| Cc: | pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: PGQ catalog representation and pg_dump support |
| Date: | 2026-08-27 17:08:07 |
| Message-ID: | CAExHW5vFugkNuoLZ4Y4H1K0DmQR1ZpRzfBoAdhEG_3X4q2M8hQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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(at)anarazel(dot)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_element
>
> There 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
| Attachment | Content-Type | Size |
|---|---|---|
| v20260827-0002-WIP-DROP-CASCADE-and-orphaned-property-gra.patch | text/x-patch | 21.1 KB |
| v20260827-0001-pg_dump-Transfer-dependencies-of-propertie.patch | text/x-patch | 63.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Matheus Alcantara | 2026-08-27 17:08:45 | Re: Enable partitionwise join for partition keys wrapped by RelabelType |
| Previous Message | Bharath Rupireddy | 2026-08-27 17:04:26 | Online enable/disable data checksums functions return success even when the launcher fails to start |