Re: Slight refactoring of state check in pg_upgrade check_ function

On Sun, Aug 28, 2022 at 03:06:09PM -0700, Nathan Bossart wrote:
> On Sun, Aug 28, 2022 at 10:42:24PM +0200, Daniel Gustafsson wrote:
> > I noticed that the pg_upgrade check_ functions were determining failures found
> > in a few different ways.  Some keep a boolen flag variable, and some (like
> > check_for_incompatible_polymorphics) check the state of the script filehandle
> > which is guaranteed to be set (with the error message referring to the path of
> > said file).  Others like check_loadable_libraries only check the flag variable
> > and fclose the handle assuming it was opened.
> > 
> > The attached diff changes the functions to do it consistently in one way, by
> > checking the state of the filehandle.  Since we are referring to the file by
> > path in the printed error message it seemed the cleanest approach, and it saves
> > a few lines of code without IMO reducing readability.
> > 
> > There is no change in functionality, just code consistency.
> 
> The patch looks reasonable to me.

+1.  Those checks have accumulated over time with different authors,
hence the stylistic differences.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Indecision is a decision.  Inaction is an action.  Mark Batterson