Обсуждение: CF entries for 17 to be reviewed

Hi hackers!

In this thread, I want to promote entries from CommitFest that require review. I have scanned through the bugs,
clients,and documentation sections, and here is my take on the current situation. All of these threads are currently in
the"Needs review" state and were marked by the patch author as targeting version 17. 

Bugs:
* LockAcquireExtended improvement
    Not really a bug, but rather a limitation. Thread might be of interest for a reviewer who wants to dive into heavy
locks.Some input from Robert. I doubt this improvement will land into 17. 
* Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY column(s)
    3-liner fix, with input from Alvaro.
* Avoid deadlock and concurrency during orphan temp table removal
    A real deadlock from production. 0 prior external review, hurry up to be first reviewer :) The patch is small, yet
needa deep understanding of lock protocols of different combination of objects. 
* Explicitly show dependent types as extension members
    Some issues on the tip of FDW and types subsystems. Fix by Tom, not backpatchable. If you want better understanding
ofextendebility and type dependencies - take this for review. IMO most probably will be in 17, just need some extra
eyes.
* initdb's -c option behaves wrong way
    Fundamental debate, might seems much like tabs vs spaces (strncmp vs strncasecmp). Patch by Tom, perfect as usual,
needsagreement from someone who's already involved. 

Clients:
* vacuumdb/clusterdb/reindexdb: allow specifying objects to process in all databases
    Nice feature, some review from Kyotaro Horiguchi, but need more.
* Support for named parsed statement in psql
    Some review by Jelte was done, but seem to require more attention.
* Extend pgbench partitioning to pgbench_history
    Tomas Vondra explressed some backward comparability concerns, Melanie questioned implementation details. I doubt
thiswill land into 17, but eventually might be part of a famous "sort of TPC-B". 
* psql: Allow editing query results with \gedit
    There's a possible new nice psql feature, but as of January architectural discussion was still going on. It does
notseem like it will be in 17, unless some agreement emerge. 

Documentation:
* SET ROLE documentation improvement
    Thin matters of superuser documentation. Nathan signed up as a committer, but the thread has no updates for some
months.
* Add detail regarding resource consumption wrt max_connections
    Cary Huang reviewd the patch, but the result is inconclusive.
* Quick Start Guide to PL/pgSQL and PL/Python Documentation
    Some comments from Pavel and Li seem like were not addressed.
* Simplify documentation related to Windows builds
    Andres had a couple of notes that were addressed by the author.
* PG DOCS - protocol specifying boolean parameters without values.
    Small leftover in a relatevely old documentation thread. Needs a bump from someone in the thread.
* Documentation: warn about two_phase when altering a subscription
    Amit LGTMed, I've added him to cc of the followup.

Stay tuned for other CF sections.


Best regards, Andrey Borodin, learning how to be a CFM.

On Sat, Mar 2, 2024 at 1:32 PM Andrey M. Borodin <x4mmm@yandex-team.ru> wrote:
>
> Hi hackers!
>
> In this thread, I want to promote entries from CommitFest that require review. I have scanned through the bugs,
clients,and documentation sections, and here is my take on the current situation. All of these threads are currently in
the"Needs review" state and were marked by the patch author as targeting version 17. 

Hi Andrey, thanks for volunteering. I at least had forgotten to update
the target version for all of my registered patches. I suspect others
may be in the same situation. Anyway, I've done that now. But, I'm not
sure if the ones that do have a target version = 17 are actually all
the patches targeting 17.

- Melanie

> On 3 Mar 2024, at 01:19, Melanie Plageman <melanieplageman@gmail.com> wrote:
>
>  I'm not
> sure if the ones that do have a target version = 17 are actually all
> the patches targeting 17.

Yes, for me it’s only a hint where to bump things up. I will extend scope on other versions when I fill finish a pass
thoughentries with version 17. 


Here’s my take on “Miscellaneous” section.


* Permute underscore separated components of columns before fuzzy matching
    The patch received some review, but not for latest version. I pinged the reviewer for an update.
* Add pg_wait_for_lockers() function
    Citus-related patch, but may be of use to other distributed systems. This patch worth attention at least because
authorreplied to himself 10+ times. Interesting addition, some feedback from Andres and Laurenz. 
* Improve the log message output of basic_archive when basic_archive.archive_directory parameter is not set
    Relatively simple change to improve user-friendliness. Daniel Gustafsson expressed interest recently.
* Fix log_line_prefix to display the transaction id (%x) for statements not in a transaction block
    Reasonable log improvement, code is simple but in a tricky place. There was some feedback, I've asked if respondent
canbe a reviewer. 
* Add Index-level REINDEX with multiple jobs
    An addition to DBA toolset. Some unaddressed feedback, pinged authors.
* Add LSN <-> time conversion facility
    There's ongoing discussion between Melanie and Tomas. Relatively heavyweight patchset, but given current solid
technicallevel of the discussion this might land into 17. Take your chance to chime-in with review! :) 
* date_trunc function in interval version
    Some time tricks. There are review notes by Tomas. I pinged authors.
* Adding comments to help understand psql hidden queries
    A couple of patches for psql --echo-hidden. Seems useful and simple. No reviews at all though. I moved the patch to
"Clients"to reflect actual patch purpose and lighten generic “Miscellaneous". 
* Improving EXPLAIN's display of SubPlan nodes
    Some EXPLAIN changes, Alexander Alekseev was looking into this. I've asked him if he would be the reviewer.
* Should we remove -Wdeclaration-after-statement?
    Not really a patch, kind of an opinion poll. The result is now kind of -BigNumber, I see no chances for this to get
into17, but maybe in future. 
* Add to_regtypemod() SQL function
    Cool nice function, some reviewers approved the patch. I've took a glance on the patch, seems nice, switched to
"Readyfor Committer". Some unrelated changes to genbki.pl, but according to thread it was needed for something. 
* un-revert MAINTAIN privilege and pg_maintain predefined role
    The work seems to be going on.
* Checkpoint extension hook
    The patch is not provided yet. I've pinged the thread.

Stay tuned, I hope everyone interested in reviewing will find themself a cool interesting patch or two.


Best regards, Andrey Borodin.

> On 4 Mar 2024, at 13:42, Andrey M. Borodin <x4mmm@yandex-team.ru> wrote:
>
> Here’s my take on “Miscellaneous” section.

I’ve read other small sections.

Monitoring & Control
* Logging parallel worker draught
    The patchset on improving loggin os resource starvation when parallel workers are spawned. Some major refactoring
proposed.I've switched to WoA. 
* System username in pg_stat_activity
    There's active discussion on extending or not pg_stat_activity.

Testing
* Add basic tests for the low-level backup method
    Michael Paquier provided feedback, so I switched to WoA.

System Administration
* recovery modules
    There was a very active discussion, but after April 2023 it stalled, and only some rebases are there. Maybe a fresh
lookcould revive the thread. 
* Possibility to disable `ALTER SYSTEM`
    The discussion seems active, but inconclusive.
* Add checkpoint/redo LSNs to recovery errors.
    Michael Paquier provided feedback, so I switched to WoA.

Security
* add not_before and not_after timestamps to sslinfo extension and pg_stat_ssl
    Most recent version provided by Daniel Gustafsson, but the thread stalled in September 2023. Maybe Jacob or some
otherreviewer could refresh it, IMO this might land in 17. 

Thanks!


Best regards, Andrey Borodin.

> On 4 Mar 2024, at 14:51, Andrey M. Borodin <x4mmm@yandex-team.ru> wrote:
>
> I’ve read other small sections.

Here are statuses for "Refactoring" section:
* New [relation] options engine
    Relatively heavy refactoring. Author keeps interest to the patch for some years now. As I understood the main
problemis that big refactoring cannot be split into incremental steps. Definitely worth reviewing, but I think not for
17already... 
* Confine vacuum skip logic to lazy_scan_skip
    There was a discussion at the end of 2023, but no recent review activity. Author actively improves the patchset.
* Change prefetch and read strategies to use range in pg_prewarm
    Some discussion is happening. Changed to WoA to reflect actual status.
* Potential issue in ecpg-informix decimal converting functions
    On Daniel's TODO list.
* BitmapHeapScan table AM violation removal (and use streaming read API)
    Active discussion with reviewers is going on.
* Streaming read sequential and TID range scan
    Seems like discussion on this patch is going on in nearby threads. In this thread I observe only improved patch
versionsposted. 

All in all "Refactoring" section seemed to me more complex and demanding in-depth knowledge. It's difficult to judge
whynew approaches are an improvement. So for newcomer reviewers I'd recommend to look to other sections. 

Thanks.


Best regards, Andrey Borodin.

> On 6 Mar 2024, at 18:49, Andrey M. Borodin <x4mmm@yandex-team.ru> wrote:
>
> Here are statuses for "Refactoring" section:

I've made a pass through "Replication and Recovery" and "SQL commands" sections.
"SQL commands" section seems to me most interesting stuff on the commitfest (but I'm far from inspecting every thing
threadyet). But reviewing patches from this section is not a work for one CF definitely. Be prepared that this is a
longrun, with many iterations and occasional architectural pivots. Typically reviewers work on these items for much
morethan one month. 

Replication and Recovery
* Synchronizing slots from primary to standby
    Titanic work. A lot of stuff already pushed, v108 is now in the discussion. ISTM that entry barrier of jumping into
discussionwith something useful is really high. 
* CREATE SUBSCRIPTION ... SERVER
    The discussion stopped in January. Authors posted new version recently.
* speed up a logical replication setup (pg_createsubscriber)
    Newest version posted recently, but it fails CFbot's tests. Pinged authors.
* Have pg_basebackup write "dbname" in "primary_conninfo"?
    Some feedback and descussin provided. Switched to WoA.

SQL Commands
* Add SPLIT PARTITION/MERGE PARTITIONS commands
    Cool new commands, very useful for sharding. CF item was RfC recently, need review update after rebase.
* Add CANONICAL option to xmlserialize
    Vignesh C and Chapman Flack provided some feedback back in October 2023, but the patch still needs review.
* Incremental View Maintenance
    This is a super cool feature. IMO at this point is too late for 17, but you should consider reviewing this not
becauseit's easy, but because it's hard. It's real rocket science. Fine-grained 11-step patchset, which can change a
lotof workloads if committed. CFbot finds some failures, but this should not stop anyone frome reviewing in this case. 
* Implement row pattern recognition feature
    SQL:2016 feature, carefully split into 8 steps. Needs review, probably review in a long run. The feature seems big.
3reviewers are working on this, but no recent input for some months. 
* COPY TO json
    Active thread with many different authors proposing different patches. I could not summarize it, asked CF entry
authorfor help. 
* Add new error_action COPY ON_ERROR "log"
    There's an active discussion in the thread.
* add COPY option RECECT_LIMIT
    While the patch seems much similar to previous, there's no discussion in this thread...

Thanks!


Best regards, Andrey Borodin.

Hi,

> > On 6 Mar 2024, at 18:49, Andrey M. Borodin <x4mmm@yandex-team.ru> wrote:
> >
> > Here are statuses for "Refactoring" section:
>
> [...]

> Aleksander, I would greatly appreciate if you join me in managing CF. Together we can move more stuff :)
> Currently, I'm going through "SQL Commands". And so far I had not come to "Performance" and "Server Features" at
all...So if you can handle updating statuses of that sections - that would be great.
 

Server Features:

* Fix partitionwise join with partially-redundant join clauses
    I see there is a good discussion in the progress here. Doesn't
seem to be needing more reviewers at the moment.
* ALTER TABLE SET ACCESS METHOD on partitioned tables
    Ditto.
* Patch to implement missing join selectivity estimation for range types
    The patch doesn't apply and has been "Waiting on Author" for a few
months now. Could be a candidate for closing with RwF.
* logical decoding and replication of sequences, take 2
   According to Tomas Vondra the patch is not ready for PG17. The
patch is marked as "Waiting on Author". Although it could be withrowed
for now, personally I see no problem with keeping it WoA until the
PG18 cycle begins.
* Add the ability to limit the amount of memory that can be allocated
to backends
   v20231226 doesn't apply. The patch needs love from somebody interested in it.
* Multi-version ICU
   By a quick look the patch doesn't apply and was moved between
several commitfests, last time in "Waiting on Author" state. Could be
a candidate for closing with RwF.
* Post-special Page Storage TDE support
    A large patch divided into 28 (!) parts. Currently needs a rebase.
Which shouldn't necessarily stop a reviewer looking for a challenging
task.
* Built-in collation provider for "C" and "C.UTF-8"
    Peter E left some feedback today, so I changed the status to
"Waiting on Author"
* ltree hash functions
    Marked as RfC and cfbot seems to be happy with the patch. Could
use some attention from a committer?
* UUID v7
    The patch is in good shape. Michael argued that the patch should
be merged when RFC is approved. No action seems to be needed until
then.
* (to be continued)


--
Best regards,
Aleksander Alekseev

Hi,

> > Aleksander, I would greatly appreciate if you join me in managing CF. Together we can move more stuff :)
> > Currently, I'm going through "SQL Commands". And so far I had not come to "Performance" and "Server Features" at
all...So if you can handle updating statuses of that sections - that would be great.
 
>
> Server Features:
>
> [...]

I noticed that "Avoid mixing custom and OpenSSL BIO functions" had two
entries [1][2]. I closed [1] and marked it as "Withdrawn" due to lack
of a better status. Maybe we need an additional "Duplicate" status.

[1]: https://commitfest.postgresql.org/47/4834/
[2]: https://commitfest.postgresql.org/47/4835/

-- 
Best regards,
Aleksander Alekseev

Hi,

> > Aleksander, I would greatly appreciate if you join me in managing CF. Together we can move more stuff :)
> > Currently, I'm going through "SQL Commands". And so far I had not come to "Performance" and "Server Features" at
all...So if you can handle updating statuses of that sections - that would be great.
 
>
> Server Features:
>
> [...]
>
> * (to be continued)

Server Features:

* Custom storage managers (SMGR), redux
    The patch needs a rebase. I notified the authors.
* pg_tracing
   Ditto. The patch IMO is promising. I encourage anyone interested in
the topic to take a look.
* Support run-time partition pruning for hash join
  The patch needs (more) review. It doesn't look extremely complex.
* Support prepared statement invalidation when result or argument types change
  I changed the status to "Waiting on Author". The patch needs a
rebase since January.
* Allow INSTEAD OF DELETE triggers to modify the tuple for RETURNING
  Needs rebase.

--
Best regards,
Aleksander Alekseev