public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tomas Vondra <[email protected]>
To: Melanie Plageman <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Pg Hackers <[email protected]>
Cc: Nazir Bilal Yavuz <[email protected]>
Subject: Re: BitmapHeapScan streaming read user and prelim refactoring
Date: Wed, 24 Apr 2024 00:43:38 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAAKRu_YVQdGVYFAu7ng84GvHWXoSvpBtz+xktm1nUB7H48Pj-Q@mail.gmail.com>
References: <20240406134011.hjo3e2336sgcghfp@liskov>
<[email protected]>
<20240406160423.r2zcjex3dngnztx4@liskov>
<20240406213450.fatgalewqkbez4tg@liskov>
<[email protected]>
<20240407041734.74jwo5wjmxs6p5du@liskov>
<[email protected]>
<CAAKRu_Zg8Bj66OZD46Jd-ksh02OGrPR8z0JLPQqEZNEHASi6uw@mail.gmail.com>
<[email protected]>
<CAAKRu_bTXNe16j=mFASOBrkzD=uvfUUqqOwE1FDsT7YvA1P=Pg@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAAKRu_YA2YBQNr+qE_hq-a4ohqDx+dD2wWekzOhmMUmYcJ0Knw@mail.gmail.com>
<CAAKRu_YVQdGVYFAu7ng84GvHWXoSvpBtz+xktm1nUB7H48Pj-Q@mail.gmail.com>
On 4/23/24 18:05, Melanie Plageman wrote:
> On Mon, Apr 22, 2024 at 1:01 PM Melanie Plageman
> <[email protected]> wrote:
>>
>> On Thu, Apr 18, 2024 at 5:39 AM Tomas Vondra
>> <[email protected]> wrote:
>>>
>>> On 4/18/24 09:10, Michael Paquier wrote:
>>>> On Sun, Apr 07, 2024 at 10:54:56AM -0400, Melanie Plageman wrote:
>>>>> I've added an open item [1], because what's one open item when you can
>>>>> have two? (me)
>>>>
>>>> And this is still an open item as of today. What's the plan to move
>>>> forward here?
>>>
>>> AFAIK the plan is to replace the asserts with actually resetting the
>>> rs_empty_tuples_pending field to 0, as suggested by Melanie a week ago.
>>> I assume she was busy with the post-freeze AM reworks last week, so this
>>> was on a back burner.
>>
>> yep, sorry. Also I took a few days off. I'm just catching up today. I
>> want to pop in one of Richard or Tomas' examples as a test, since it
>> seems like it would add some coverage. I will have a patch soon.
>
> The patch with a fix is attached. I put the test in
> src/test/regress/sql/join.sql. It isn't the perfect location because
> it is testing something exercisable with a join but not directly
> related to the fact that it is a join. I also considered
> src/test/regress/sql/select.sql, but it also isn't directly related to
> the query being a SELECT query. If there is a better place for a test
> of a bitmap heap scan edge case, let me know.
>
I don't see a problem with adding this to join.sql - why wouldn't this
count as something related to a join? Sure, it's not like this code
matters only for joins, but if you look at join.sql that applies to a
number of other tests (e.g. there are a couple btree tests).
That being said, it'd be good to explain in the comment why we're
testing this particular plan, not just what the plan looks like.
> One other note: there is some concurrency effect in the parallel
> schedule group containing "join" where you won't trip the assert if
> all the tests in that group in the parallel schedule are run. But, if
> you would like to verify that the test exercises the correct code,
> just reduce the group containing "join".
>
That is ... interesting. Doesn't that mean that most test runs won't
actually detect the problem? That would make the test a bit useless.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
view thread (27+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: BitmapHeapScan streaming read user and prelim refactoring
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox