On Fri, Apr 1, 2022 at 5:20 PM David Rowley <dgrowle...@gmail.com> wrote: > On Fri, 1 Apr 2022 at 19:58, Amit Langote <amitlangot...@gmail.com> wrote: > > Yes, the ExecLockRelsInfo node in the current patch, that first gets > > added to the QueryDesc and subsequently to the EState of the query, > > serves as that stashing place. Not sure if you've looked at > > ExecLockRelInfo in detail in your review of the patch so far, but it > > carries the initial pruning result in what are called > > PlanInitPruningOutput nodes, which are stored in a list in > > ExecLockRelsInfo and their offsets in the list are in turn stored in > > an adjacent array that contains an element for every plan node in the > > tree. If we go with a PlannedStmt.partpruneinfos list, then maybe we > > don't need to have that array, because the Append/MergeAppend nodes > > would be carrying those offsets by themselves. > > I saw it, just not in great detail. I saw that you had an array that > was indexed by the plan node's ID. I thought that wouldn't be so good > with large complex plans that we often get with partitioning > workloads. That's why I mentioned using another index that you store > in Append/MergeAppend that starts at 0 and increments by 1 for each > node that has a PartitionPruneInfo made for it during create_plan. > > > Maybe a different name for ExecLockRelsInfo would be better? > > > > Also, given Tom's apparent dislike for carrying that in PlannedStmt, > > maybe the way I have it now is fine? > > I think if you change how it's indexed and the other stuff then we can > have another look. I think the patch will be much easier to review > once the ParitionPruneInfos are moved into PlannedStmt.
Will do, thanks. -- Amit Langote EDB: http://www.enterprisedb.com