On Wed, Mar 12, 2025 at 4:28 PM Alena Rybakina <a.rybak...@postgrespro.ru> wrote: > Thank you for the explanation! > > Now I see why these changes were made. > > After your additional explanations, everything really became clear and I > fully agree with the current code regarding this part.
Cool. > However I did not see an explanation to the commit regarding this place, > as well as a comment next to the assert and the parallel_aware check and > why BitmapIndexScanState was added in the ExecParallelReInitializeDSM. I added BitmapIndexScanState to the switch statement in ExecParallelReInitializeDSM because it is in the category of planstates that never need their shared memory reinitialized -- that's just how we represent such a plan state there. I think that this is supposed to serve as a kind of documentation, since it doesn't really affect how things behave. That is, it wouldn't actually affect anything if I had forgotten to add BitmapIndexScanState to the ExecParallelReInitializeDSM switch statement "case" that represents that it is in this "plan state category": the switch ends with catch-all "default: break;". > In my opinion, there is not enough additional explanation about this in > the form of comments, although I think that it has already been > explained here enough for someone who will look at this code. What can be done to improve the situation? For example, would adding a comment next to the new assertions recently added to ExecIndexScanReInitializeDSM and ExecIndexOnlyScanReInitializeDSM be an improvement? And if so, what would the comment say? -- Peter Geoghegan
