Report roles in pg_upgrade pg_ prefix check
Looking at a recent pg_upgrade thread I happened to notice that the check for
roles with a pg_ prefix only reports the error, not the roles it found. Other
similar checks where the user is expected to alter the old cluster typically
reports the found objects in a textfile. The attached adds reporting to make
that class of checks consistent (the check for prepared transactions which also
isn't reporting is different IMO as it doesn't expect ALTER commands).
As this check is only executed against the old cluster the patch removes the
check when printing the error.
--
Daniel Gustafsson https://vmware.com/
Attachments:
v1-0001-Report-incompatible-roles-in-pg_upgrade-checking.patchapplication/octet-stream; name=v1-0001-Report-incompatible-roles-in-pg_upgrade-checking.patch; x-unix-mode=0644Download+28-8
On Thu, Nov 24, 2022 at 12:31:09PM +0100, Daniel Gustafsson wrote:
Looking at a recent pg_upgrade thread I happened to notice that the check for
roles with a pg_ prefix only reports the error, not the roles it found. Other
similar checks where the user is expected to alter the old cluster typically
reports the found objects in a textfile. The attached adds reporting to make
that class of checks consistent (the check for prepared transactions which also
isn't reporting is different IMO as it doesn't expect ALTER commands).As this check is only executed against the old cluster the patch removes the
check when printing the error.
+1
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Embrace your flaws. They make you human, rather than perfect,
which you will never be.
On Thu, Nov 24, 2022 at 12:31:09PM +0100, Daniel Gustafsson wrote:
Looking at a recent pg_upgrade thread I happened to notice that the check for
roles with a pg_ prefix only reports the error, not the roles it found. Other
similar checks where the user is expected to alter the old cluster typically
reports the found objects in a textfile. The attached adds reporting to make
that class of checks consistent (the check for prepared transactions which also
isn't reporting is different IMO as it doesn't expect ALTER commands).As this check is only executed against the old cluster the patch removes the
check when printing the error.
+1. A backpatch would be nice, though not strictly mandatory as
that's not a bug fix.
+ ntups = PQntuples(res);
+ i_rolname = PQfnumber(res, "rolname");Would it be worth adding the OID on top of the role name in the
generated report? That would be a free meal.
--
Michael
On 28 Nov 2022, at 02:18, Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Nov 24, 2022 at 12:31:09PM +0100, Daniel Gustafsson wrote:
Looking at a recent pg_upgrade thread I happened to notice that the check for
roles with a pg_ prefix only reports the error, not the roles it found. Other
similar checks where the user is expected to alter the old cluster typically
reports the found objects in a textfile. The attached adds reporting to make
that class of checks consistent (the check for prepared transactions which also
isn't reporting is different IMO as it doesn't expect ALTER commands).As this check is only executed against the old cluster the patch removes the
check when printing the error.+1. A backpatch would be nice, though not strictly mandatory as
that's not a bug fix.
Yeah, it doesn't really qualify since this not a bugfix.
+ ntups = PQntuples(res); + i_rolname = PQfnumber(res, "rolname");Would it be worth adding the OID on top of the role name in the
generated report? That would be a free meal.
We are a bit inconsistent in how much details we include in the report
textfiles, so could do that without breaking any consistency in reporting.
Looking at other checks, the below format would match what we already do fairly
well:
<rolname> (oid=<oid>)
Done in the attached.
--
Daniel Gustafsson https://vmware.com/
Attachments:
v2-0001-Report-incompatible-roles-in-pg_upgrade-checking.patchapplication/octet-stream; name=v2-0001-Report-incompatible-roles-in-pg_upgrade-checking.patch; x-unix-mode=0644Download+32-8
On Mon, Nov 28, 2022 at 09:58:46AM +0100, Daniel Gustafsson wrote:
We are a bit inconsistent in how much details we include in the report
textfiles, so could do that without breaking any consistency in reporting.
Looking at other checks, the below format would match what we already do fairly
well:<rolname> (oid=<oid>)
Done in the attached.
WFM. Thanks!
--
Michael
On 29 Nov 2022, at 00:24, Michael Paquier <michael@paquier.xyz> wrote:
On Mon, Nov 28, 2022 at 09:58:46AM +0100, Daniel Gustafsson wrote:
We are a bit inconsistent in how much details we include in the report
textfiles, so could do that without breaking any consistency in reporting.
Looking at other checks, the below format would match what we already do fairly
well:<rolname> (oid=<oid>)
Done in the attached.
WFM. Thanks!
Took another look at it, and applied it. Thanks!
--
Daniel Gustafsson https://vmware.com/