Message-ID: <3b7f45c8-5b95-7c74-13cf-a58cb44c1de1@enterprisedb.com>
Date: Sun, 16 Oct 2022 16:14:32 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.1
Subject: Re: PATCH: Using BRIN indexes for sorted output
Content-Language: en-US
To: Zhihong Yu <zyu@yugabyte.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <e70fa091-e338-1598-9de4-6d0ef6b693e2@enterprisedb.com>
 <CALNJ-vTA-QvA3bQYwTuv6euzt1kXb4kcTHuRnpwE8gfg4YTrFw@mail.gmail.com>
 <c83596b3-994e-a32b-598a-efcbd99ef58a@enterprisedb.com>
 <CALNJ-vS=mMT+Km3FAfbzYCn-vQKNk5inHCUfTYi4rD8DJfG1vg@mail.gmail.com>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
In-Reply-To: 
 <CALNJ-vS=mMT+Km3FAfbzYCn-vQKNk5inHCUfTYi4rD8DJfG1vg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/3b7f45c8-5b95-7c74-13cf-a58cb44c1de1%40enterprisedb.com>
Precedence: bulk


On 10/16/22 03:36, Zhihong Yu wrote:
> 
> 
> On Sat, Oct 15, 2022 at 8:23 AM Tomas Vondra
> <tomas.vondra@enterprisedb.com <mailto:tomas.vondra@enterprisedb.com>>
> wrote:
> 
>     On 10/15/22 15:46, Zhihong Yu wrote:
>     >...
>     >     8) Parallel version is not supported, but I think it shouldn't be
>     >     possible. Just make the leader build the range info, and then
>     let the
>     >     workers to acquire/sort ranges and merge them by Gather Merge.
>     > ...
>     > Hi,
>     > I am still going over the patch.
>     >
>     > Minor: for #8, I guess you meant `it should be possible` .
>     >
> 
>     Yes, I meant to say it should be possible. Sorry for the confusion.
> 
> 
> 
>     regards
> 
>     -- 
>     Tomas Vondra
>     EnterpriseDB: http://www.enterprisedb.com <http://www.enterprisedb.com>
>     The Enterprise PostgreSQL Company
> 
> Hi,
> 
> For brin_minmax_ranges, looking at the assignment to gottuple and
> reading gottuple, it seems variable gottuple can be omitted - we can
> check tup directly.
> 
> +   /* Maybe mark the range as processed. */
> +   range->processed |= mark_processed;
> 
> `Maybe` can be dropped.
> 

No, because the "mark_processed" may be false. So we may not mark it as
processed in some cases.

> For brinsort_load_tuples(), do we need to check for interrupts inside
> the loop ?
> Similar question for subsequent methods involving loops, such
> as brinsort_load_unsummarized_ranges.
> 

We could/should, although most of the loops should be very short.


regrds

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company