Commitfest 2021-11 Patch Triage - Part 1

We have amassed quite a lot of patches in the CF app, and while Jaime did a
very good job closing patches in the last CF there is still a lot to go
through. I've attempted a brief per-patch triage here to see where we are with
these. Reading every email in the threads for these patches is a herculean
task, so I may very well have misunderstood things. If so I apologize and
please do jump and correct me. If not, then please also jump in and comment
below (or on the relevant threads) to try and get closure.

While I don't have the expertise to replicate Andres' famous patch triage [0]
from a few years back, below is at least a try. These are the patches with 7
or more commitfests under their belts (the record holder having 18), the next
installment will be 6 CF's and down limited by when the Shiraz runs out.

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?

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?

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?

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.

2138: Incremental Materialized View Maintenance
===============================================
There seems to be concensus on the thread that this is a feature that we want,
and after initial design discussions there seems to be no disagreements with
the approach taken. The patch was marked ready for committer almost a year
ago, but have since been needs review (which seems correct). 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 review can summarize the current state and outline
what is left for it to be committable.

2215: pg_upgrade fails with non-standard ACL
============================================
This thread has been stalled since March with open questions from Noah, but the
patch seems to be in pretty good shape. Do you have any thoughts on the state
of this patch Noah? Seems like a pretty good fix to get in.

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?

2048: SQL:2011 application time
===============================
The patch no longer applies, but there is a semi-active review discussion
ongoing. It doesn't seem like the patch is nearing completion, but the feature
would no doubt be welcome by many. Is this realistic for 15?

2316: WITH SYSTEM VERSIONING Temporal Tables
============================================
This patch no longer applies, and judging by the thread there is a pretty major
re-design ongoing. I think we should close this entry and open a new one when
there is a new patch rather than push it along. What are your thoughts Simon?

2096: psql - add SHOW_ALL_RESULTS option
========================================
Peter posted an updated version of Fabiens patch about a month ago (which at
this point no longer applies) fixing a few issues, but also point at old review
comments still unadressed. Since this was pushed, but had to be reverted, I
assume there is a desire for the feature but it seems to need more work still.

1651: GROUP BY optimization
===========================
This is IMO a desired optimization, and after all the heavy lifting by Tomas
the patch seems to be in pretty good shape.

2377: pg_ls_* functions for showing metadata and recurse (pg_ls_tmpdir to show
shared filesets)
==============================================================================
The question of what to do with lstat() on Windows is still left unanswered,
but the patchset has been split to up to be able to avoid it. Stephen and Tom,
having done prior reviews do you have any thoughts on this?

2349: global temporary table
============================
GTT has been up for discussion numerous times in tbe past, and I can't judge
whether this proposal has a better chance than previous ones. I do note the
patch has a number of crashes reported lately, and no reviews from senior
contributors in a while, making it seem unlikely to be committed in this CF.
Since the patch is very big, can it be teased apart and committed separately
for easier review?

2433: Erase the distinctClause if the result is unique by definition
====================================================================
(parts of) The approach taken in this patch has been objected against in favor
of work that Tom has proposed. Until that work materialize this patch is
blocked, and thus I think we are better of closing it and re-opening it when it
gets unstuck. Unless Tom has plans to hack on this shortly?

2482: jsonpath syntax extensions
================================
This thread has stalled, with no reviews so far a good year and half after
being. Rather than continue to push this along, should we instead close it due
to lack of interest? It's pretty big, can it be broken up into smaller
features for easier review?

2490: Make message at end-of-recovery less scary
================================================
This thread stalled, but has had recent interest. The patch no longer applies
so while the patch has support it will be marked as returned with feedback
during this CF unless revived.

2573: pg_dump - read data for some options from external file
=============================================================
This patch has been heavily debated, but I think we've landed in a compromise
that has been agreed upon. The gist of the debate has been around the file
format and it's extensibility, the final version proposing an extensible yet
simple format which can be referenced from a larger configfile (rather than
making the configfile now). Having done a lot of review myself I think the
patch is ready with one exception: IMO the parsing code isn't maintainable
enough to be committed (if another committer disagree with that, feel free to
jump in). I have on my TODO to implement it as a Bison parser to see what that
would look like.

2553: should INSERT SELECT use a BulkInsertState?
==================================================
The conclusion on the thread seems to be that there is little support for it
due to the risk of regressions, and that closing the patch might be the best
course of action. If anyone is in favor of going ahead with it, now is the
time to speak up.

2518: Corruption during WAL replay
==================================
Both Robert and Tom have given positive remarks about this, and recent review
concerns raised turned out to have been handled. Robert, Tom: any thoughts on
the latest posted version?

2601: Fast COPY FROM command for the foreign tables
===================================================
This approach taken in this patch has stabilized and the benchmarks posted are
very promising. It seems pretty uncontroversial to add COPY to the FDW API SQL
spec wise. Amit, Justin and Takayuki-san has done a lot of review and the
patch is marked Ready for Committer. Any committer interested in taking this
on? Tomas?

2602: ALTER SYSTEM READ { ONLY | WRITE }
========================================
AFAIK Robert is actively working on this and have committed parts of it
already, so there isn't much to add. I'm personally quite excited about this
feature so I'm looking forward to (hopefully) seeing it go in. Is the full
feature likely to make 15?

2627: More scalable multixacts buffers and locking
==================================================
Skimming the thread makes it seem like this patch is still undergoing some
fairly big changes. Thomas and Andrey, how far along is this from RFC?

2716: fix spinlock contention in LogwrtResult
=============================================
This addresses a bottleneck which definitely seems like one we want to fix, I
don't have a hard time imagining it impacting other production usecases then
the reported one. The goalposts moved early in the thread, and while there has
been work on it the patch has stalled on Andres' review which raised some
concerns. Are you still pursuing this Alvaro?

2684: enhancing plpgsql API for debugging and tracing
======================================================
This hasn't seen much review, but what has been performed has been positive.
It does add some weight to plpgsql by passing the NS around, but maybe the
effect is negligable. Given the size and complexity of the test module to use
this, it's perhaps not obvious to see who the target audience is. Maybe a
better option would be to polish the test module and propose it for contrib/
along with the patch, and write the tests on using said module?

2710: Fix behavior of geo_ops when NaN is involved
==================================================
Two thirds of this bugfix patchset have been committed already, with the thread
stalling on the remaining bit. Tom, Georgios: any thoughts on the last patch?

2694: Automatic HASH and LIST partition creation
================================================
The subject of automatic partition creation is pretty hot, so getting something
done in this area would be good. This proposal has gotten criticism over the
syntax chosen, and the thread has effectively stalled after Robert's review in
July. It seems like this patch should be returned with feedback, and any new
attempt at this should start by just discussing the syntax and only once agreed
upon hack on the code.

Thanks for reading all this way, I hope it did something to help progress.

--
Daniel Gustafsson https://vmware.com/

[0] https://www.postgresql.org/message-id/flat/20180302075242.yfqkcgzbrmjysboa%40alap3.anarazel.de#b5015aab4ba3a95b67636faef74a1d51