From: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
---|---|
To: | Justin Pryzby <pryzby(at)telsasoft(dot)com> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, Greg Stark <stark(at)mit(dot)edu>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, Zhihong Yu <zyu(at)yugabyte(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: PATCH: Using BRIN indexes for sorted output |
Date: | 2023-02-18 12:19:49 |
Message-ID: | 9f73f798-1dac-c75d-d834-aa012ba13b65@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
0001-Allow-index-AMs-to-build-and-use-custom-sta-20230218.patch | text/x-patch | 63.2 KB |
0002-wip-introduce-debug_brin_stats-20230218.patch | text/x-patch | 5.7 KB |
0003-wip-introduce-debug_brin_cross_check-20230218.patch | text/x-patch | 19.7 KB |
0004-Allow-BRIN-indexes-to-produce-sorted-output-20230218.patch | text/x-patch | 132.8 KB |
0005-wip-brinsort-explain-stats-20230218.patch | text/x-patch | 12.8 KB |
0006-wip-multiple-watermark-steps-20230218.patch | text/x-patch | 6.3 KB |
0007-wip-adjust-watermark-step-20230218.patch | text/x-patch | 9.1 KB |
0008-wip-adaptive-watermark-step-20230218.patch | text/x-patch | 13.1 KB |
0009-wip-add-brin_sort.sql-test-20230218.patch | text/x-patch | 179.8 KB |
0010-wip-test-generator-script-20230218.patch | text/x-patch | 11.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2023-02-18 12:56:38 | occasional valgrind reports for handle_sig_alarm on 32-bit ARM |
Previous Message | Amit Kapila | 2023-02-18 10:42:52 | Re: pg_upgrade and logical replication |