On Sat, Jun  6, 2026 at 03:08:05PM +0200, Álvaro Herrera wrote:
> On 2026-Jun-05, Bruce Momjian wrote:
> 
> > > I want to thank everyone for the fixes/improvements they have supplied
> > > for the PG 19 release notes.  I am now satisfied with them and I think
> > > they are close to what they will look link for PG 19 final. Here are the
> > > current contents:
> 
> >     https://momjian.us/pgsql_docs/release-19.html
> 
> Thanks for putting these together!  I have a few comments:
> 
> * In the incompatibilities section, we have
>   - Prevent carriage returns and line feeds in database, role, and
>   tablespace names
> 
>   This was changed to avoid security problems. pg_upgrade will also
>   disallow upgrading of clusters that use such names.
> 
>   The verb "prevent" here is a bit strange; I think "reject" would be
>   better.  Also, I think the phrase involving pg_upgrade should come
>   before the reason for the change.

Agreed.  I went with "Disallow carriage returns and line feeds", and I
moved the security text to the end as you suggested.

> 
>   We also have this item:
> 
>   Change the default index opclasses for inet and cidr data types from
>   btree_gist to GiST
> 
>   The btree_gist inet/cidr opclasses are broken because they can exclude
>   rows that should be returned. pg_upgrade will fail to upgrade if
>   btree_gist inet/cidr indexes exist in the old server.
> 
>   Why do we say "pg_upgrade will fail to upgrade" here instead of the
>   (IMO better) wording in the previous item?  That is, "pg_upgrade will
>   disallow upgrading if btree_gist inet/cidr indexes exist in the old
>   server".  The current wording of "fail to" suggest that this is a
>   pg_upgrade shortcoming, which it isn't.

I went with similar wording "pg_upgrade will disallow upgrading".

> * In section E.1.3.1.1 Optimizer, I think the item
>   "Allow extended statistics on virtual generated columns"
>   should come before all other items, because it's the only one that
>   requires user action in order for them to take advantage of it.  All
>   the other items refer to some optimization that occurs automatically.

While the first item in each section is the most important, I then list
later items in groups that have similar characteristics, which I think
simplifies reading.  While I could order all items in importance order,
I think it would be much more confusing to read.  Right now the item you
mention is in a group of items about optimizer statistics.

> * Section E.1.3.1.2 General Performance, the item
>   Allow autovacuum to use parallel vacuum workers
>   lacks a link to
>   
> https://momjian.us/pgsql_docs/sql-createtable.html#RELOPTION-AUTOVACUUM-PARALLEL-WORKERS

Added link.

> * Section E.1.3.1.3 System Views, the two items
>   Add vacuum initiation details to system view pg_stat_progress_vacuum
>   Add analyze initiation details to system view pg_stat_progress_analyze
>   Maybe they should be a single entry?
>   Add vacuum and analyze initiation details to system view
>   pg_stat_progress_vacuum and pg_stat_progress_analyze respectively
>   (Not really sure about this one)

I looked at merging these before, and the existence of the "mode" column
in vacuum but not analyze made the merged item too complicated to
understand.

> * E.1.3.1.4 Monitoring
>   Add server variable log_autoanalyze_min_duration to log long-running
>   autoanalyze operations (Shinya Kato) §
>   Server variable log_autovacuum_min_duration now only controls logging
>   of autovacuum operations.
> 
>   I think it's confusing to talk about "autoanalyze" as if it were
>   an action taken by an agent other than autovacuum.  I mean, we have
>   autovacuum which runs autovacuums and also autovacuum which runs
>   autoanalyzes?  To my mind that makes no sense -- I think we have one
>   autovacuum, which runs vacuum and analyze.
> 
>   I would explain this as
>   Add server variable log_autoanalyze_min_duration to log long-running
>   analyze operations run by autovacuum (Shinya Kato) §
>   Server variable log_autovacuum_min_duration now only controls logging
>   of vacuum operations run by autovacuum.

Well, the column calls it autoanalyze, but I agree when we are talking
about the tool, autovacuum is better, so changed.

>   * Add WAL full-page write bytes reporting to VACUUM and ANALYZE logging
>   (Shinya Kato) §
>   Maybe this should be "Report bytes of full-page WAL writes to VACUUM
>   and ANALYZE logging".
>   (This also appears in E.1.3.3.3 EXPLAIN and E.1.3.1.3 System Views)

Yes, saying "Add reporting of X" is better than "add X reporting";  fixed
in both cases.

>   * Add function pg_get_multixact_stats() to report multixact activity
>   (Naga Appani) §
>   I think this item should appear second in the section, right after the
>   one for log_min_messages, on importance grounds.

Again, already grouped in a section about monitoring functions, and I do
think the earlier items are more important.

>   * Two entries make the acronym LSN point to 
>   https://momjian.us/pgsql_docs/wal-internals.html
>   I think a better target is the glossary item
>   https://momjian.us/pgsql_docs/glossary.html#GLOSSARY-LOG-SEQUENCE-NUMBER
>   The shorter definition in the glossary is possibly more useful for a
>   release note reader; and if they want even more detail, the glossary
>   definition does point to the WAL internals.
>   A third entry appears in E.1.3.1.6

Well, the second paragraph does explain LSN so I think pointing them to
a glossary makes things worse.

> * E.1.3.1.5 Server Configuration
>   * Allow online enabling and disabling of data checksums
>   Previously the checksum status could only be set at initialization and
>   changed only while the cluster was offline using pg_checksums.
> 
>   The word "only" appears twice in the second phrase, which is awkward.
>   Maybe reword it as
>   Previously the checksum status would be fixed at initialization time and
>   only changed while the cluster was offline usiNG PG_checksums.

Yes, and the bigger problem is that there is no need to mention
initialization setting of checksums, so the first part of the sentence
is now gone.

>   * Add scoring system to control the order that tables are autovacuumed
>   I think using "autovacuumed" as a verb is terrible grammar.  I would
>   rather have "... are processed by autovacuum".

Yes, agreed, fixed.

>   * Allow background workers to be configured to terminate before
>   database-level operations (Aya Iwata) §
>   This sounds far too mysterious; it probably warrants more detail.
>   Also, move it a bit upwards: just below SNI perhaps?  (That isn't
>   much, but all the other items in this section also look valuable.)

Agreed.  I added a sentence to explain its purpose.

> * E.1.3.3.2 Copy
>   * Allow COPY TO to output partitioned tables (Jian He, Ajin Cherian) § §
>   "to output partitioned tables"?  This reads really awkward.
>   What about ... "Allow partitioned tables to be [processed / read] by
>   COPY TO directly" or something like that?

Yes, I used "process".  New output here:

        https://momjian.us/pgsql_docs/release-19.html

You certainly found some confusing items and I like the new output.

-- 
  Bruce Momjian  <[email protected]>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Do not let urgent matters crowd out time for investment in the future.


Reply via email to