Thread: [Commitfest 2022-07] Patch Triage: Needs Review, Part 1
Hi, Next up is the large list of Needs Review. This part 1 should include entries as old or older than seven commitfests running. My heuristics for classifying these continue to evolve as I go, and there's a lot to read, so please let me know if I've made any mistakes. = Stalled Patches, Recommend Return = These are stalled and I recommend outright that we return them. We don't have a separate status for "needs more interest" (working on a patch) so I'd just RwF, with a note explaining that what is actually needed to continue isn't more code work but more coalition building. - Implement INSERT SET syntax https://commitfest.postgresql.org/38/2218/ A recent author rebase this CF, but unfortunately I think the the real issue is just a lack of review interest. It's been suggested for return for a few CFs now. - Fix up partitionwise join on how equi-join conditions between the partition keys are identified https://commitfest.postgresql.org/38/2266/ It looks like this one was Returned with Feedback but did not actually have feedback, which may have caused confusion. (Solid motivation for a new close status.) I don't think there's been any review since 2020. - New default role allowing to change per-role/database settings https://commitfest.postgresql.org/38/2918/ Stalled on review in January, and needs a rebase. = Stalled Patches, Need Help = These are stalled but seem to have interest. They need help to either get them out of the rut, or else be Returned so that the author can try a different approach instead of perma-rebasing. I plan to move them to the next CF unless someone speaks up to say otherwise. - Show shared filesets in pg_ls_tmpdir (pg_ls_* functions for showing metadata and recurse) https://commitfest.postgresql.org/38/2377/ From a quick skim it looks like there was a flurry of initial positive feedback followed by a stall and then some design whiplash. This thread needs help to avoid burnout, I think. - Make message at end-of-recovery less scary https://commitfest.postgresql.org/38/2490/ This got marked RfC twice, fell back out, and has been stuck in a rebase loop. - Fix behavior of geo_ops when NaN is involved https://commitfest.postgresql.org/38/2710/ Stuck in a half-committed state, which is tricky. Could maybe use a reframing or recap (or a new thread?). - Add extra statistics to explain for Nested Loop https://commitfest.postgresql.org/38/2765/ I think the author is hoping for help with testing and performance characterization. - CREATE INDEX CONCURRENTLY on partitioned table https://commitfest.postgresql.org/38/2815/ This had an author switch since last CF, so I think it'd be inappropriate to close it out this time around, but it needs assistance. - New Table Access Methods for Multi and Single Inserts https://commitfest.postgresql.org/38/2871/ Although there was a brief flicker in March, I think this one has stalled out and is just about ready to be returned. - Fix pg_rewind race condition just after promotion https://commitfest.postgresql.org/38/2864/ Seems like an important fix, but it's silent? Does it need to be promoted to an Open Issue? - pg_stat_statements and "IN" conditions https://commitfest.postgresql.org/38/2837/ Some good, recent interest. Last review in March. - Function to log backtrace of postgres processes https://commitfest.postgresql.org/38/2863/ This is just starting to stall; I think it needs some assistance. - Allow batched insert during cross-partition updates https://commitfest.postgresql.org/38/2992/ Was RfC (twice), then dropped out, now it's stuck rebasing. Last substantial review in 2021. = Active Patches = The following are actively being worked and I expect to move them to next CF: - session variables, LET command - Remove self join on a unique column - Incremental Materialized View Maintenance - More scalable multixacts buffers and locking - Fast COPY FROM command for the foreign tables - Extended statistics / estimate Var op Var clauses Thanks, --Jacob
Jacob Champion <jchampion@timescale.com> writes: > Next up is the large list of Needs Review. This part 1 should include > entries as old or older than seven commitfests running. I'm just commenting on a couple that I've been involved with. > = Stalled Patches, Recommend Return = > - Fix up partitionwise join on how equi-join conditions between the > partition keys are identified > https://commitfest.postgresql.org/38/2266/ > It looks like this one was Returned with Feedback but did not actually > have feedback, which may have caused confusion. (Solid motivation for a > new close status.) I don't think there's been any review since 2020. Yeah, there was an earlier discussion of this same patch in some previous CF-closing thread, IIRC, but I can't find that right now. I think it basically is stuck behind the outer-join-variables work I'm pursuing at https://commitfest.postgresql.org/39/3755/ ... and when/if that lands, the present patch probably won't be anywhere near what we want anyway. +1 for RWF. > = Stalled Patches, Need Help = > - Fix behavior of geo_ops when NaN is involved > https://commitfest.postgresql.org/38/2710/ > Stuck in a half-committed state, which is tricky. Could maybe use a > reframing or recap (or a new thread?). We fixed a couple of easy cases but then realized that the hard cases are hard. I don't have much faith that the current patch is going to lead to anything committable, and it doesn't look like anyone has the appetite to put in a lot of work on the topic. I'd vote for RWF. regards, tom lane
Hi, On Thu, Jul 28, 2022 at 02:28:23PM -0700, Jacob Champion wrote: > > = Stalled Patches, Need Help = > [...] > - Add extra statistics to explain for Nested Loop > https://commitfest.postgresql.org/38/2765/ > > I think the author is hoping for help with testing and performance > characterization. As I mentioned in [1], this patch breaks the current assumption that INSTRUMENT_ALL will lead to statement-level metrics that are generally useful. According to the benchmark, the proposed patch would add a 1.5% overhead for pg_stat_statements or any other similar extension that relies on INSTRUMENT_ALL for no additional information, and I don't think it's acceptable. I'm still not sure of what is the best way to fix that, but clearly something has to be done, ideally without requiring every single pg_stat_statements-like extension to be modified. > The following are actively being worked and I expect to move them to > next CF: > > - session variables, LET command Yes please.
On Thu, Jul 28, 2022 at 2:51 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > - Fix behavior of geo_ops when NaN is involved > > https://commitfest.postgresql.org/38/2710/ > > > Stuck in a half-committed state, which is tricky. Could maybe use a > > reframing or recap (or a new thread?). > > We fixed a couple of easy cases but then realized that the hard cases > are hard. I don't have much faith that the current patch is going to > lead to anything committable, and it doesn't look like anyone has the > appetite to put in a lot of work on the topic. I'd vote for RWF. Barring any competing votes, that's what I'll do, then. Thanks! --Jacob
Hi Julien, On Thu, Jul 28, 2022 at 11:38 PM Julien Rouhaud <rjuju123@gmail.com> wrote: > > - Add extra statistics to explain for Nested Loop > > https://commitfest.postgresql.org/38/2765/ > > > > [...] > > As I mentioned in [1], this patch breaks the current assumption that > INSTRUMENT_ALL will lead to statement-level metrics that are generally useful. > According to the benchmark, the proposed patch would add a 1.5% overhead for > pg_stat_statements or any other similar extension that relies on INSTRUMENT_ALL > for no additional information, and I don't think it's acceptable. (I'm missing the [1] link.) From skimming the end of the thread, it looks like Ekaterina responded to that concern and was hoping for feedback. If you still think it doesn't go far enough, would you mind dropping a note in the thread? Then we can mark WoA and go from there. Thanks! --Jacob
Hi Jacob, On Fri, Jul 29, 2022 at 10:08:08AM -0700, Jacob Champion wrote: > > On Thu, Jul 28, 2022 at 11:38 PM Julien Rouhaud <rjuju123@gmail.com> wrote: > > > - Add extra statistics to explain for Nested Loop > > > https://commitfest.postgresql.org/38/2765/ > > > > > > [...] > > > > As I mentioned in [1], this patch breaks the current assumption that > > INSTRUMENT_ALL will lead to statement-level metrics that are generally useful. > > According to the benchmark, the proposed patch would add a 1.5% overhead for > > pg_stat_statements or any other similar extension that relies on INSTRUMENT_ALL > > for no additional information, and I don't think it's acceptable. > > (I'm missing the [1] link.) Ah sorry I forgot to include it, here it's: https://www.postgresql.org/message-id/20220307050830.zahd57wbvezu2d6r%40jrouhaud. >From skimming the end of the thread, it > looks like Ekaterina responded to that concern and was hoping for > feedback. If you still think it doesn't go far enough, would you mind > dropping a note in the thread? Then we can mark WoA and go from there. I think that the problem still exist, unfortunately the benchmark done only tests various EXPLAIN commands and not normal query execution with pgss enabled. I will double check and reply on the thread tomorrow!