On 23/10/2020 05:56, Amit Langote wrote:
On Thu, Oct 22, 2020 at 11:25 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote:
On 2020-Oct-22, Amit Langote wrote:
0001 fixes a thinko of the recent commit 1375422c782 that I discovered
when debugging a problem with 0003.
Hmm, how hard is it to produce a test case that fails because of this
problem?
I checked and don't think there's any live bug here. You will notice
if you take a look at 1375422c7 that we've made es_result_relations an
array of pointers while the individual ModifyTableState nodes own the
actual ResultRelInfos. So, EvalPlanQualStart() setting the parent
EState's es_result_relations array to NULL implies that those pointers
become inaccessible to the parent query's execution after
EvalPlanQual() returns. However, nothing in the tree today accesses
ResulRelInfos through es_result_relations array, except during
ExecInit* stage (see ExecInitForeignScan()) but it would still be
intact at that stage.
With the lazy-initialization patch though, we do check
es_result_relations when trying to open a result relation to see if it
has already been initialized (a non-NULL pointer in that array means
yes), so resetting it in the middle of the execution can't be safe.
For one example, we will end up initializing the same relation many
times after not finding it in es_result_relations and also add it
*duplicatively* to es_opened_result_relations list, breaking the
invariant that that list contains distinct relations.
Pushed that thinko-fix, thanks!
- Heikki
