From: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
---|---|
To: | Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Fix Brin Private Spool Initialization (src/backend/access/brin/brin.c) |
Date: | 2023-12-26 22:07:09 |
Message-ID: | 9719540f-1204-aa6f-93ae-63b78d1a617a@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 12/26/23 19:10, Ranier Vilela wrote:
> Hi,
>
> The commit b437571 <http://b437571714707bc6466abde1a0af5e69aaade09c> I
> think has an oversight.
> When allocate memory and initialize private spool in function:
> _brin_leader_participate_as_worker
>
> The behavior is the bs_spool (heap and index fields)
> are left empty.
>
> The code affected is:
> buildstate->bs_spool = (BrinSpool *) palloc0(sizeof(BrinSpool));
> - buildstate->bs_spool->heap = buildstate->bs_spool->heap;
> - buildstate->bs_spool->index = buildstate->bs_spool->index;
> + buildstate->bs_spool->heap = heap;
> + buildstate->bs_spool->index = index;
>
> Is the fix correct?
>
Thanks for noticing this. Yes, I believe this is a bug - the assignments
are certainly wrong, it leaves the fields set to NULL.
I wonder how come this didn't fail during testing. Surely, if the leader
participates as a worker, the tuplesort_begin_index_brin shall be called
with heap/index being NULL, leading to some failure during the sort. But
maybe this means we don't actually need the heap/index fields, it's just
a copy of TuplesortIndexArg, but BRIN does not need that because we sort
the tuples by blkno, and we don't need the descriptors for that.
In any case, the _brin_parallel_scan_and_build does not actually need
the separate heap/index arguments, those are already in the spool.
I'll try to figure out if we want to simplify the tuplesort or remove
the arguments from _brin_parallel_scan_and_build.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Jelte Fennema-Nio | 2023-12-26 23:33:38 | Re: A tiny improvement of psql |
Previous Message | Tomas Vondra | 2023-12-26 21:52:06 | Re: Add the ability to limit the amount of memory that can be allocated to backends. |