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
