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
pgq-1.sql
Description: application/sql
pgq-2.sql
Description: application/sql
pgq-3.sql
Description: application/sql
pgq-4.sql
Description: application/sql
pgq-5.sql
Description: application/sql
