Em qui., 15 de jul. de 2021 às 09:45, David Rowley <dgrowleyml@gmail.com> escreveu:
On Thu, 15 Jul 2021 at 23:38, Aleksander Alekseev <aleksander@timescale.com> wrote: > I'm updating the status to "Ready for Committer".
I think that might be a bit premature. I can't quite see how changing the pids List to a const List makes any sense, especially when the code goes and calls lappend_int() on it to assign it some different value.
There are also problems in BackendPidGetProcWithLock around consts.
Much of this patch kinda feels like another one of those "I've got a fancy new static analyzer" patches. Unfortunately, it just introduces a bunch of compiler warnings as a result of the changes it makes.
I'd suggest splitting each portion of the patch out into parts related to what it aims to achieve. For example, it looks like there's some renaming going on to remove a local variable from shadowing a function parameter. Yet the patch is claiming performance improvements. I don't see how that part relates to performance. The changes to ProcArrayClearTransaction() seem also unrelated to performance.
I'm not sure what the point of changing things like for (int i =0... to move the variable declaration somewhere else is about. That just seems like needless stylistic changes that achieve nothing but more headaches for committers doing backpatching work.
I'd say if this patch wants to be taken seriously it better decide what it's purpose is, because to me it looks just like a jumble of random changes that have no clear purpose.
I'm going to set this back to waiting on author.
I understood.
I will try to address all concerns in the new version.
