Re: PATCH: Using BRIN indexes for sorted output

On 2/16/23 17:10, Justin Pryzby wrote:
> On Thu, Feb 16, 2023 at 03:07:59PM +0100, Tomas Vondra wrote:
>> Rebased version of the patches, fixing only minor conflicts.
>
> Per cfbot, the patch fails on 32 bit builds.
> +ERROR: count mismatch: 0 != 1000
>
> And causes warnings in mingw cross-compile.
>

There was a silly mistake in trying to store block numbers as bigint
when sorting the ranges, instead of uint32. That happens to work on
64-bit systems, but on 32-bit systems it produces bogus block.

The attached should fix that - it passes on 32-bit arm, even with
valgrind and all that.

> On Sun, Oct 23, 2022 at 11:32:37PM -0500, Justin Pryzby wrote:
>> I think new GUCs should be enabled during patch development.
>> Maybe in a separate 0002 patch "for CI only not for commit".
>> That way "make check" at least has a chance to hit that new code
>> paths.
>>
>> Also, note that indxpath.c had the var initialized to true.
>
> In your patch, the amstats guc is still being set to false during
> startup by the guc machinery. And the tests crash everywhere if it's
> set to on:
>
> TRAP: failed Assert("(nmatches_unique >= 1) && (nmatches_unique <= unique[nvalues-1])"), File: "../src/backend/access/brin/brin_minmax.c", Line: 644, PID: 25519
>

Right, that was a silly thinko in building the stats, and I found a
couple more issues nearby. Should be fixed in the attached version.

>> . Some typos in your other patches: "heuristics heuristics". ste.
>> lest (least).
>
> These are still present.

Thanks for reminding me, those should be fixed too now.

regards

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