CF entries for 17 to be reviewed

Started by Andrey M. Borodinalmost 2 years ago9 messages
#1Andrey M. Borodin
x4mmm@yandex-team.ru

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 need a 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 of extendebility 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, needs agreement 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 this will 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 not seem 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.

#2Melanie Plageman
melanieplageman@gmail.com
In reply to: Andrey M. Borodin (#1)
Re: CF entries for 17 to be reviewed

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

#3Andrey M. Borodin
x4mmm@yandex-team.ru
In reply to: Melanie Plageman (#2)
Re: CF entries for 17 to be reviewed

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 though entries 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 author replied 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 can be 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 technical level 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 into 17, 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 "Ready for 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.

#4Andrey M. Borodin
x4mmm@yandex-team.ru
In reply to: Andrey M. Borodin (#3)
Re: CF entries for 17 to be reviewed

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 look could 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 other reviewer could refresh it, IMO this might land in 17.

Thanks!

Best regards, Andrey Borodin.

#5Andrey M. Borodin
x4mmm@yandex-team.ru
In reply to: Andrey M. Borodin (#4)
Re: CF entries for 17 to be reviewed

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 problem is that big refactoring cannot be split into incremental steps. Definitely worth reviewing, but I think not for 17 already...
* 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 versions posted.

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

Thanks.

Best regards, Andrey Borodin.

#6Andrey M. Borodin
x4mmm@yandex-team.ru
In reply to: Andrey M. Borodin (#5)
Re: CF entries for 17 to be reviewed

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 thread yet). But reviewing patches from this section is not a work for one CF definitely. Be prepared that this is a long run, with many iterations and occasional architectural pivots. Typically reviewers work on these items for much more than 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 discussion with 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 because it's easy, but because it's hard. It's real rocket science. Fine-grained 11-step patchset, which can change a lot of 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. 3 reviewers 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 author for 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.

#7Aleksander Alekseev
aleksander@timescale.com
In reply to: Andrey M. Borodin (#6)
Re: CF entries for 17 to be reviewed

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

#8Aleksander Alekseev
aleksander@timescale.com
In reply to: Aleksander Alekseev (#7)
Re: CF entries for 17 to be reviewed

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]https://commitfest.postgresql.org/47/4834/[2]https://commitfest.postgresql.org/47/4835/. I closed [1]https://commitfest.postgresql.org/47/4834/ 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

#9Aleksander Alekseev
aleksander@timescale.com
In reply to: Aleksander Alekseev (#7)
Re: CF entries for 17 to be reviewed

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