Feedback-ID: ia2694551:Fastmail
Date: Wed, 15 May 2024 17:14:15 +0200
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>, 
	Michael Paquier <michael@paquier.xyz>, Thomas Munro <thomas.munro@gmail.com>, 
	Heikki Linnakangas <hlinnaka@iki.fi>, Andres Freund <andres@anarazel.de>, 
	Pg Hackers <pgsql-hackers@postgresql.org>, Nazir Bilal Yavuz <byavuz81@gmail.com>
Subject: Re: BitmapHeapScan streaming read user and prelim refactoring
Message-ID: <202405151514.owoztajhysog@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAAKRu_bBcVFnrd4_K8JO_8DUsTbUHM1n+ArqkyJ5G8rYcSDHJw@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/202405151514.owoztajhysog%40alvherre.pgsql>
Precedence: bulk

On 2024-May-14, Melanie Plageman wrote:

> On Tue, May 14, 2024 at 4:05 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

> > I do wonder how do we _know_ that the test is testing what it wants to
> > test:

> We include the explain output (the plan) to ensure it is still
> using a bitmap heap scan. The test exercises the skip fetch
> optimization in bitmap heap scan when not all of the inner tuples are
> emitted.

I meant -- the query returns an empty resultset, so how do we know it's
the empty resultset that we want and not a different empty resultset
that happens to be identical?  (This is probably not a critical point
anyhow.)

> Without the patch, the test fails, so it is protection against someone
> adding back that assert in the future. It is not protection against
> someone deleting the line
> scan->rs_empty_tuples_pending = 0
> That is, it doesn't verify that the empty unused tuples count is
> discarded. Do you think that is necessary?

I don't think that's absolutely necessary.  I suspect there are
thousands of lines that you could delete that would break things, with
no test directly raising red flags.

At this point I would go with the RMT's recommendation of pushing this
now to make sure the bug is fixed for beta1, even if the test is
imperfect.  You can always polish the test afterwards if you feel like
it ...

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"No necesitamos banderas
 No reconocemos fronteras"                  (Jorge González)