| 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-09-01 07:01:27 |
| Message-ID: | CAExHW5tBqekx0WDdm8pt5at6+Zxm3sNRP1J5SqnxsBMwWGPtAQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Aug 27, 2026 at 10:38 PM Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)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(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.
>
> - 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
| Attachment | Content-Type | Size |
|---|---|---|
| v20260901-0002-Duplicate-code-in-performDeletion-and-perf.patch | text/x-patch | 3.6 KB |
| v20260901-0001-Clarify-the-testcase-intent-in-create_prop.patch | text/x-patch | 2.1 KB |
| v20260901-0003-pg_dump-Transfer-dependencies-of-propertie.patch | text/x-patch | 63.1 KB |
| v20260901-0004-DROP-CASCADE-to-property-graph-components.patch | text/x-patch | 36.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | 韩建桥 | 2026-09-01 07:04:29 | Re: [PATCH] Add row estimate tests for unnest() and integer generate_series() |
| Previous Message | vignesh C | 2026-09-01 06:57:32 | Re: Logical replication row filter loses unchanged toasted columns |