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. -- Amit Langote EDB: http://www.enterprisedb.com
