Re: Commitfest 2022-03 Patch Triage Part 1a.i

On Tue, 1 Mar 2022 13:39:45 -0500
Greg Stark <stark(at)mit(dot)edu> wrote:

> As Justin suggested I CC the authors from these patches I'm adding
> them here. Some of the patches have multiple "Authors" listed in the
> commitfest which may just be people who posted updated patches so I
> may have added more people than necessary.
>
> On Tue, 1 Mar 2022 at 11:16, Greg Stark <stark(at)mit(dot)edu> wrote:
> >
> > Last November Daniel Gustafsson did a patch triage. It took him three
> > emails to get through the patches in the commitfest back then. Since
> > then we've had the November and the January commitfests so I was
> > interested to see how many of these patches had advanced....
> >
> > I'm only part way through the first email but so far only two patches
> > have changed status -- and both to "Returned with feedback" :(
> >
> > So I'm going to post updates but I'm going to break it up into smaller
> > batches because otherwise it'll take me a month before I post
> > anything.
> >
> >
> >
> > > 1608: schema variables, LET command
> > > ===================================
> > > After 18 CF's and two very long threads it seems to be nearing completion
> > > judging by Tomas' review. There is an ask in that review for a second pass
> > > over the docs by a native speaker, any takers?
> >
> > Patch has a new name, "session variables, LET command"
> >
> > There's been a *lot* of work on this patch so I'm loath to bump it.
> > The last review was from Julien Rouhaud which had mostly code style
> > comments but it had one particular concern about using xact callback
> > in core and about EOX cleanup.
> >
> > Pavel, do you have a plan to improve this or are you looking for
> > suggestions from someone about how you should solve this problem?
> >
> > > 1741: Index Skip Scan
> > > =====================
> > > An often requested feature which has proven hard to reach consensus on an
> > > implementation for. The thread(s) have stalled since May, is there any hope of
> > > taking this further? Where do we go from here with this patch?
> >
> > "Often requested indeed! I would love to be able to stop explaining to
> > people that Postgres can't handle this case well.
> >
> > It seems there are multiple patch variants around and suggestions from
> > Heikki and Peter G about fundamental interface choices. It would be
> > nice to have a good summary from someone who is involved about what's
> > actually left unresolved.
> >
> >
> > > 1712: Remove self join on a unique column
> > > =========================================
> > > This has moved from "don't write bad queries" to "maybe we should do something
> > > about this". It seems like there is concensus that it can be worth paying the
> > > added planner cost for this, especially if gated by a GUC to keep it out of
> > > installations where it doesn't apply. The regression on large join queries
> > > hasn't been benchmarked it seems (or I missed it), but the patch seems to have
> > > matured and be quite ready. Any takers on getting it across the finish line?
> >
> > There hasn't been any review since the v29 patch was posted in July.
> > That said, to my eye it seemed like pretty basic functionality errors
> > were being corrected quite late. All the bugs got patch updates
> > quickly but does this have enough tests to be sure it's working right?
> >
> > The only real objection was about whether the planning time justified
> > the gains since the gains are small. But I think they were resolved by
> > making the optimization optional. Do we have consensus that that
> > resolved the issue or do we still need benchmarks showing the planning
> > time hit is reasonable?
> >
> > > 2161: standby recovery fails when re-replaying due to missing directory which
> > > was removed in previous replay.
> > > =============================================================================
> > > Tom and Robert seem to be in agreement that parts of this patchset are good,
> > > and that some parts are not. The thread has stalled and the patch no longer
> > > apply, so unless someone feels like picking up the good pieces this seems like
> > > a contender to close for now.
> >
> > Tom's feedback seems to have been acted on last November. And the last
> > update in January was that it was passing CI now. Is this ready to
> > commit now?
> >
> >
> > > 2138: Incremental Materialized View Maintenance
> > > ===============================================
> > > The size of the
> > > patchset and the length of the thread make it hard to gauge just far away it
> > > is, maybe the author or a reviewer can summarize the current state and outline
> > > what is left for it to be committable.
> >
> > There is an updated patch set as of February but I have the same
> > difficulty wrapping my head around the amount of info here.
> >
> > Is this one really likely to be commitable in 15? If not I think we
> > should move this to 16 now and concentrate on patches that will be
> > commitable in this release.

I think this patch set needs more reviews to be commitable in 15, so I
returned the target version to blank. I'll change it to 16 later.

> >
> > > 2218: Implement INSERT SET syntax
> > > =================================
> > > The author has kept this patch updated, and has seemingly updated according to
> > > the review comments. Tom: do you have any opinions on whether the updated
> > > version addresses your concerns wrt the SELECT rewrite?
> >
> > I don't see any discussion implying that Tom's concerns were met. I'm
> > not exactly clear why Tom's concerns are real problems though --
> > wouldn't it be a *good* thing if we have a more expressive syntax? But
> > that's definitely what needs to be resolved before it can move ahead.
> >
> > So unless there's objections I'm going to update this to "waiting on author".
> >
> > --
> > greg
>
>
>
> --
> greg

--
Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>