Re: Incorrect result of bitmap heap scan.
Matthias van de Meent <boekewurm+postgres@gmail.com>
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
To: vignesh C <vignesh21@gmail.com>
Cc: Peter Geoghegan <pg@bowt.ie>, Tom Lane <tgl@sss.pgh.pa.us>,
Andres Freund <andres@anarazel.de>, Andrew Dunstan <andrew@dunslane.net>,
Melanie Plageman <melanieplageman@gmail.com>, Thomas Munro <thomas.munro@gmail.com>,
Konstantin Knizhnik <knizhnik@garret.ru>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Alexander Kuzmenkov <akuzmenkov@timescale.com>
Date: 2025-03-17T15:17:03Z
Lists: pgsql-hackers
On Sun, 16 Mar 2025 at 13:55, vignesh C <vignesh21@gmail.com> wrote:
>
> On Wed, 5 Mar 2025 at 16:43, Matthias van de Meent
> <boekewurm+postgres@gmail.com> wrote:
> >
> > On Sun, 2 Mar 2025 at 01:35, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > >
> > > Peter Geoghegan <pg@bowt.ie> writes:
> > > > Is everybody in agreement about committing and back patching this fix,
> > > > which simply disables the optimization altogether?
> > > > I myself don't see a better way, but thought I'd ask before proceeding
> > > > with review and commit.
> > >
> > > If you don't see a clear path forward, then "disable" is the only
> > > reasonable choice for the back branches. Maybe we'll find a fix
> > > in future, but it seems unlikely that it'd be back-patchable.
> >
> > Agreed.
> >
> > Here's patch v5 for the master branch (now up to f4694e0f), with no
> > interesting changes other than fixing apply conflicts caused by
> > bfe56cdf.
>
> I noticed that Andres's comment from [1] is not yet addressed,
> changing the commitfest entry status to Waiting on Author, please
> address the comment and change it back to Needs review.
> [1] - https://www.postgresql.org/message-id/tf5pp2o2a5x5qjcseq354bd26ya4o7p2vjzm5z4w57ca3vy6bc@ep7enrljvdkr
As I understand it, Andres agrees that the feature is unsalvageable in
backbranches ("don't think there's a realistic way to fix it"). Andres
then continues to elaborate that even if the feature were salvageable,
it wouldn't contain much of the current code.
My patch removes the feature altogether in master and disables the
feature in the backbranches in the patches that were built against the
backbranches, which seems to match Andres' comments.
@vignesh, could you elaborate on what about Andres' comments I need to
address in my patch?
Kind regards,
Matthias van de Meent
Neon (https://neon.tech)
Commits
-
Add test for HeapBitmapScan's broken skip_fetch optimization
- 24da5b239a4b 18.0 landed
-
Remove HeapBitmapScan's skip_fetch optimization
- 459e7bf8e2f8 18.0 landed
- b9ec8125d167 13.21 landed
- 4934d38759cd 14.18 landed
- 77d90d6d6334 15.13 landed
- 980727b84107 16.9 landed
- 78cb2466f752 17.5 landed
-
Allow bitmap scans to operate as index-only scans when possible.
- 7c70996ebf09 11.0 cited