Date: Sat, 2 Nov 2024 20:21:04 -0700
From: Noah Misch <noah@leadboat.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Move I/O before the index_update_stats() buffer lock
 region.
Message-ID: <20241103032104.bf.nmisch@google.com>
References: <E1t7GgB-003cVa-Th@gemulon.postgresql.org>
 <2317336.1730601176@sss.pgh.pa.us>
 <20241103031027.09.nmisch@google.com>
 <2322263.1730603637@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <2322263.1730603637@sss.pgh.pa.us>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20241103032104.bf.nmisch%40google.com>
Precedence: bulk

On Sat, Nov 02, 2024 at 11:13:57PM -0400, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > On Sat, Nov 02, 2024 at 10:32:56PM -0400, Tom Lane wrote:
> >> In branches before 17, this commit is causing some compilers
> >> to complain:
> 
> > Hopefully now the code won't fool any compiler.
> 
> I saw your commit, and I'm content with it, but did you figure
> out why the warning only appears pre-17?  (At least, that's
> what I got locally.)

Nope.  The principles behind the variable's lifecycle didn't change.  Here's
the function's diff between v16 and v17.  (Same diff all the way from v12 to
v17.)  Perhaps the reference to non-local variable IsBinaryUpgrade changed the
compiler's deductive capability, somehow.

@@ -2836,7 +2806,11 @@ index_update_stats(Relation rel,
 	if (reltuples == 0 && rel->rd_rel->reltuples < 0)
 		reltuples = -1;
 
-	update_stats = reltuples >= 0;
+	/*
+	 * Don't update statistics during binary upgrade, because the indexes are
+	 * created before the data is moved into place.
+	 */
+	update_stats = reltuples >= 0 && !IsBinaryUpgrade;
 
 	/*
 	 * Finish I/O and visibility map buffer locks before