eeshwarg commented on PR #2497: URL: https://github.com/apache/age/pull/2497#issuecomment-5222833952
> Verified the argument-order fix directly: > > * `object_ownercheck(Oid classid, Oid objectid, Oid roleid)` — confirmed the signature in `utils/acl.h`. > * `RelationRelationId` is already in scope in this file (used two lines earlier via `ObjectAddressSet` at line 1007) and `catalog/pg_class_d.h` is already included, so no new dependency. > * The old call passed `rel_oid` as `classid` and a namespace OID as `objectid` — neither is a valid catalog OID for the classid slot, which explains the `unrecognized class ID` error hitting the default case in `get_object_property`. Superusers skip this path entirely via the `superuser_arg()` short-circuit, which is why only non-superusers hit it. > * Regression test reproduces the failure end-to-end as a non-superuser role that owns the graph/label, and cleans up after itself (drops the role, graph). Good coverage. > > Note: CI hasn't run yet on this branch (workflow runs sitting unapproved, same as #2469) — needs a maintainer to approve the run before this can merge. > > LGTM. Thanks for the review and approval Greg! Would you able to merge the PR for me? Or do I need another approval before that? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
