Hi, Tomas:
In my opinion, my patch is a little better.
Suppose one of the conditions in the if block changes in between the start of loop and the end of the loop:
* Determine if the FDW supports batch insert and determine the batch
* size (a FDW may support batching, but it may be disabled for the
* server/table).
My patch would reflect that change. I guess this was the reason the if / else block was placed there in the first place.
Cheers
On 1/21/21 1:17 AM, Zhihong Yu wrote:
> Hi,
> The assignment to resultRelInfo is done when junk_filter_needed is true:
>
> if (junk_filter_needed)
> {
> resultRelInfo = mtstate->resultRelInfo;
>
> Should the code for determining batch size access mtstate->resultRelInfo
> directly ?
>
IMO the issue is that code iterates over all plans and moves to the next
for each one:
resultRelInfo++;
so it ends up pointing past the last element, hence the failures. So
yeah, either the code needs to move before the loop (per my patch), or
we need to access mtstate->resultRelInfo directly.
I'm pretty amazed this did not crash during any of the many regression
runs I did recently.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company