Horton

Thread

Commits

  1. Report incompatible roles in pg_upgrade checking

  1. Report roles in pg_upgrade pg_ prefix check

    Daniel Gustafsson <daniel@yesql.se> — 2022-11-24T11:31:09Z

    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/

  2. Re: Report roles in pg_upgrade pg_ prefix check

    Bruce Momjian <bruce@momjian.us> — 2022-11-25T20:20:02Z

    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.

  3. Re: Report roles in pg_upgrade pg_ prefix check

    Michael Paquier <michael@paquier.xyz> — 2022-11-28T01:18:53Z

    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

  4. Re: Report roles in pg_upgrade pg_ prefix check

    Daniel Gustafsson <daniel@yesql.se> — 2022-11-28T08:58:46Z

    > 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/

  5. Re: Report roles in pg_upgrade pg_ prefix check

    Michael Paquier <michael@paquier.xyz> — 2022-11-28T23:24:47Z

    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

  6. Re: Report roles in pg_upgrade pg_ prefix check

    Daniel Gustafsson <daniel@yesql.se> — 2022-12-02T12:27:14Z

    > 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/