Re: PGQ catalog representation and pg_dump support

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