| From: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | Amit Langote <amitlangote09(at)gmail(dot)com>, "tsunakawa(dot)takay(at)fujitsu(dot)com" <tsunakawa(dot)takay(at)fujitsu(dot)com>, "Andrey V(dot) Lepikhov" <a(dot)lepikhov(at)postgrespro(dot)ru>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: POC: postgres_fdw insert batching |
| Date: | 2021-01-21 01:45:12 |
| Message-ID: | 442cdcdc-5501-50f7-6efd-5df1668e11f9@enterprisedb.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 1/21/21 2:22 AM, Tom Lane wrote:
> Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> writes:
>> I may be wrong, but the most likely explanation seems to be this is due
>> to the junk filter initialization, which simply moves past the end of
>> the mtstate->resultRelInfo array.
>
> resultRelInfo is certainly pointing at garbage at that point.
>
Yup. It's pretty amazing the x86 machines seem to be mostly OK with it.
>> It kinda seems the GetForeignModifyBatchSize call should happen before
>> that block. The attached patch fixes this for me (i.e. regression tests
>> pass with no valgrind reports.
>
>> Or did I get that wrong?
>
> Don't we need to initialize ri_BatchSize for *each* resultrelinfo,
> not merely the first one? That is, this new code needs to be
> somewhere inside a loop over the result rels.
>
Yeah, I think you're right. That's an embarrassing oversight :-(
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Hou, Zhijie | 2021-01-21 01:46:57 | RE: Parallel INSERT (INTO ... SELECT ...) |
| Previous Message | Nikita Glukhov | 2021-01-21 01:42:39 | Re: SQL/JSON: JSON_TABLE |