PostgreSQL Weekly News - November 28, 2021

From: PWN via PostgreSQL Announce <announce-noreply(at)postgresql(dot)org>
To: PostgreSQL Announce <pgsql-announce(at)lists(dot)postgresql(dot)org>
Subject: PostgreSQL Weekly News - November 28, 2021
Date: 2021-11-29 09:37:51
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-announce

# PostgreSQL Weekly News - November 28, 2021

[Person of the week](

# PostgreSQL Jobs for November


# PostgreSQL Local

Nordic PGDay 2022 will be held in Helsinki, Finland at the Hilton Helsinki
Strand Hotel on March 22, 2022. The CfP is open through December 31, 2021

# PostgreSQL in the News

Planet PostgreSQL: [](

PostgreSQL Weekly News is brought to you this week by David Fetter

Submit news and announcements by Sunday at 3:00pm PST8PDT to david(at)fetter(dot)org(dot)

# Applied Patches

Peter Geoghegan pushed:

- Remove lazy_scan_heap parallel VACUUM comment block. This doesn't belong next
to very high level discussion of the tasks that lazy_scan_heap performs.
There is already a similar, longer comment block at the top of vacuumlazy.c
that mentions lazy_scan_heap directly.

- Go back to considering HOT on pages marked full. Commit 2fd8685e7f simplified
the checking of modified attributes that takes place within heap_update().
This included a micro-optimization affecting pages marked PD_PAGE_FULL: don't
even try to use HOT to save a few cycles on determining HOT safety. The
assumption was that it won't work out this time around, since it can't have
worked out last time around. Remove the micro-optimization. It could only
ever save cycles that are consumed by the vast majority of heap_update()
calls, which hardly seems worth the added complexity. It also seems quite
possible that there are workloads that will do worse over time by repeated
application of the micro-optimization, despite saving some cycles on average,
in the short term. Author: Peter Geoghegan <pg(at)bowt(dot)ie> Reviewed-By: Álvaro
Herrera <alvherre(at)alvh(dot)no-ip(dot)org> Discussion:

- Update high level vacuumlazy.c comments. Update vacuumlazy.c file header
comments (as well as comments above the lazy_scan_heap function) that were
largely written before the introduction of the HOT optimization, when
lazy_scan_heap did far less, and didn't actually prune during its initial heap
pass. Since lazy_scan_heap now outsources far more work to lower level
functions, it makes sense to introduce the function by talking about the high
level invariant that dictates the order in which each phase takes place. Also
deemphasize the case where we run out of memory for TIDs, since delaying that
discussion makes it easier to talk about issues of central importance.
Finally, remove discussion of parallel VACUUM from header comments. These
don't add much, and are in the wrong place.

- vacuumlazy.c: prefer the term "cleanup lock". The term "super-exclusive lock"
is an acceptable synonym of "cleanup lock". Even still, switching from one
term to the other in the same file is confusing. Standardize on "cleanup
lock" within vacuumlazy.c. Per a complaint from Andres Freund.

Fujii Masao pushed:

- Report wait events for local shell commands like archive_command. This commit
introduces new wait events for archive_command, archive_cleanup_command,
restore_command and recovery_end_command. Author: Fujii Masao Reviewed-by:
Bharath Rupireddy, Michael Paquier Discussion:

Peter Eisentraut pushed:

- Add ABI extra field to fmgr magic block. This allows derived products to
intentionally make their fmgr ABI incompatible, with a clean error message.

- Fix incorrect format placeholders. Also choose better types for the underlying
variables to make this more consistent.

- Remove unneeded Python includes. Inluding <compile.h> and <eval.h> has not
been necessary since Python 2.4, since they are included via <Python.h>.
Morever, <eval.h> is being removed in Python 3.11. So remove these includes.
Reviewed-by: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> Discussion:

- Update comments. Various places wanted to point out that tuple descriptors
don't contain the variable-length fields of pg_attribute. This started when
attacl was added, but more fields have been added since, and these comments
haven't been kept up to date consistently. Reword so that the purpose is
clearer and we don't have to keep updating them.

Álvaro Herrera pushed:

- Add missing words in comment. Reported by Zhihong Yu. Discussion:

- autovacuum: Improve wording in a couple places. A few strings (one WARNING and
some memory context names) in the autovacuum code were written in a world
where "worker" had no other possible meaning than "autovacuum worker", but
that's long time gone. Be more specific about it. Also, change the WARNING
from elog() to ereport(), to add translability. Author: Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> Reviewed-by: Nathan Bossart
<bossartn(at)amazon(dot)com> Reviewed-by: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Reviewed-by: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> Reviewed-by: Dilip
Kumar <dilipbalaut(at)gmail(dot)com> Reviewed-by: Masahiko Sawada
<sawada(dot)mshk(at)gmail(dot)com> Discussion:

- Be more specific about OOM in XLogReaderAllocate. A couple of spots can
benefit from an added errdetail(), which matches what we were already doing in
other places; and those that cannot withstand errdetail() can get a more
descriptive primary message. Author: Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> Reviewed-by: Daniel Gustafsson
<daniel(at)yesql(dot)se> Reviewed-by: Julien Rouhaud <rjuju123(at)gmail(dot)com> Discussion:

- Fix determination of broken LSN in OVERWRITTEN_CONTRECORD. In commit
ff9f111bce24 I mixed up inconsistent definitions of the LSN of the first
record in a page, when the previous record ends exactly at the page boundary.
The correct LSN is adjusted to skip the WAL page header; I failed to use that
when setting XLogReaderState->overwrittenRecPtr, so at WAL replay time
VerifyOverwriteContrecord would refuse to let replay continue past that
record. Backpatch to 10. 9.6 also contains this bug, but it's no longer
being maintained. Discussion:

- Document units for max_slot_wal_keep_size. The doc blurb failed to mention
units, as well as lacking the point about changeability. Backpatch to 13.
Reviewed-by: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> Reported by:
b1000101(at)pm(dot)me Discussion:

- Copy-edit vacuuumdb --analyze-in-stages doc blurb. I had made a few typos, and
Nikolai Berkoff made a wording change suggestion. Discussion:

- Harden be-gssapi-common.h for headerscheck. Surround the contents with a test
that the feature is enabled by configure, to silence header checking tools on
systems without GSSAPI installed. Backpatch to 12, where the file appeared.

Tom Lane pushed:

- Probe $PROVE not $PERL while checking for modules needed by TAP tests.
Normally "prove" and "perl" come from the same Perl installation, but we
support the case where they don't (mainly because the MSys buildfarm animals
need this). In that case, AX_PROG_PERL_MODULES is completely the wrong thing
to use, because it's checking what "perl" has. Instead, make a little TAP
test script including the required modules, and run that under "prove". We
don't need ax_prog_perl_modules.m4 at all after this change, so remove it.
Back-patch to all supported branches, for the buildfarm's benefit. (In v10,
this also back-patches the effects of commit 264eb03aa.) Andrew Dunstan and
Tom Lane, per an observation by Noah Misch Discussion:

- Fix pg_dump --inserts mode for generated columns with dropped columns. If a
table contains a generated column that's preceded by a dropped column,
dumpTableData_insert failed to account for the dropped column, and would emit
DEFAULT placeholder(s) in the wrong column(s). This resulted in failures at
restore time. The default COPY code path did not have this bug, likely
explaining why it wasn't noticed sooner. While we're fixing this, we can be a
little smarter about the situation: (1) avoid unnecessarily fetching the
values of generated columns, (2) omit generated columns from the output, too,
if we're using --column-inserts. While these modes aren't expected to be as
high-performance as the COPY path, we might as well be as efficient as we can;
it doesn't add much complexity. Per report from Дмитрий Иванов. Back-patch to
v12 where generated columns came in. Discussion:

- Pacify perlcritic. Per buildfarm.

- Adjust pg_dump's priority ordering for casts. When a stored expression depends
on a user-defined cast, the backend records the dependency as being on the
cast's implementation function --- or indeed, if there's no cast function
involved but just RelabelType or CoerceViaIO, no dependency is recorded at
all. This is problematic for pg_dump, which is at risk of dumping things in
the wrong order leading to restore failures. Given the lack of previous
reports, the risk isn't that high, but it can be demonstrated if the cast is
used in some view whose rowtype is then used as an input or result type for
some other function. (That results in the view getting hoisted into the
functions portion of the dump, ahead of the cast.) A logically bulletproof
fix for this would require including the cast's OID in the parsed form of the
expression, whence it could be extracted by dependency.c, and then the stored
dependency would force pg_dump to do the right thing. Such a change would be
fairly invasive, and certainly not back-patchable. Moreover, since we'd
prefer that an expression using cast syntax be equal() to one doing the same
thing by explicit function call, the cast OID field would have to have special
ignored-by-comparisons semantics, making things messy. So, let's instead fix
this by a very simple hack in pg_dump: change the object-type priority order
so that casts are initially sorted before functions, immediately after types.
This fixes the problem in a fairly direct way for casts that have no
implementation function. For those that do, the implementation function will
be hoisted to just before the cast by the dependency sorting step, so that we
still have a valid dump order. (I'm not sure that this provides a full
guarantee of no problems; but since it's been like this for many years without
any previous reports, this is probably enough to fix it in practice.) Per
report from Дмитрий Иванов. Back-patch to all supported branches. Discussion:

- Doc: improve documentation about nextval()/setval(). Clarify that the results
of nextval and setval are not guaranteed persistent until the calling
transaction commits. Some people seem to have drawn the opposite conclusion
from the statement that these functions are never rolled back, so re-word to
avoid saying it quite that way. Discussion:

Michaël Paquier pushed:

- Add SQL functions to monitor the directory contents of replication slots. This
commit adds a set of functions able to look at the contents of various paths
related to replication slots: - pg_ls_logicalsnapdir, for
pg_logical/snapshots/ - pg_ls_logicalmapdir, for pg_logical/mappings/ -
pg_ls_replslotdir, for pg_replslot/<slot_name>/ These are intended to be used
by monitoring tools. Unlike pg_ls_dir(), execution permission can be granted
to non-superusers. Roles members of pg_monitor gain have access to those
functions. Bump catalog version. Author: Bharath Rupireddy Reviewed-by:
Nathan Bossart, Justin Pryzby Discussion:

- Add support for Visual Studio 2022 in build scripts. Documentation and any
code paths related to VS are updated to keep the whole consistent. Similarly
to 2017 and 2019, the version of VS and the version of nmake that we use to
determine which code paths to use for the build are still inconsistent in
their own way. Backpatch down to 10, so as buildfarm members are able to use
this new version of Visual Studio on all the stable branches supported.
Author: Hans Buschmann Discussion:
Backpatch-through: 10

- Remove useless LZ4 system call on failure when writing file header. If an
error occurs when writing the LZ4 file header, LZ4F_compressEnd() was called
in the error code path of write(), followed by LZ4F_freeCompressionContext()
to finish the cleanup. The code as-is was not broken, but the
LZ4F_compressEnd() proves to not be necessary as there are no contents to
flush at this stage, so remove it. Per gripe from Jeevan Ladhe and Robert
Haas. Discussion:

- Fix fstat() emulation on Windows with standard streams. The emulation of
fstat() in win32stat.c caused two issues with the existing in-core callers,
failing on EINVAL when using a stream as argument: - psql's \copy would crash
when using a stream. - pg_recvlogical would fail with -f -. The tests in
copyselect.sql from the main test suite covers the first case, and there is a
TAP test for the second case. However, in both cases, as the standard streams
are always redirected, automated tests did not notice those issues, requiring
a terminal on Windows to be reproducible. This issue has been introduced in
bed9075, and the origin of the problem is that GetFileInformationByHandle()
does not work directly on streams, so this commit adds an extra code path to
emulate and return a set of stats that match best with the reality. Note that
redirected streams rely on handles that can be queried with
GetFileInformationByHandle(), but we can rely on GetFinalPathNameByHandleA()
to detect this case. Author: Dmitry Koval, Juan José Santamaría Flecha
Backpatch-through: 14

- Block ALTER TABLE .. DROP NOT NULL on columns in replica identity index.
Replica identities that depend directly on an index rely on a set of
properties, one of them being that all the columns defined in this index have
to be marked as NOT NULL. There was a hole in the logic with ALTER TABLE DROP
NOT NULL, where it was possible to remove the NOT NULL property of a column
part of an index used as replica identity, so block it to avoid problems with
logical decoding down the road. The same check was already done columns part
of a primary key, so the fix is straight-forward. Author: Haiying Tang, Hou
Zhijie Reviewed-by: Dilip Kumar, Michael Paquier Discussion:
Backpatch-through: 10

David Rowley pushed:

- Allow Memoize to operate in binary comparison mode. Memoize would always use
the hash equality operator for the cache key types to determine if the current
set of parameters were the same as some previously cached set. Certain types
such as floating points where -0.0 and +0.0 differ in their binary
representation but are classed as equal by the hash equality operator may
cause problems as unless the join uses the same operator it's possible that
whichever join operator is being used would be able to distinguish the two
values. In which case we may accidentally return in the incorrect rows out of
the cache. To fix this here we add a binary mode to Memoize to allow it to
the current set of parameters to previously cached values by comparing
bit-by-bit rather than logically using the hash equality operator. This
binary mode is always used for LATERAL joins and it's used for normal joins
when any of the join operators are not hashable. Reported-by: Tom Lane
Author: David Rowley Discussion:
Backpatch-through: 14, where Memoize was added

- Flush Memoize cache when non-key parameters change. It's possible that a
subplan below a Memoize node contains a parameter from above the Memoize node.
If this parameter changes then cache entries may become out-dated due to the
new parameter value. Previously Memoize was mistakenly not aware of this. We
fix this here by flushing the cache whenever a parameter that's not part of
the cache key changes. Bug: #17213 Reported by: Elvis Pranskevichus Author:
David Rowley Discussion:
Backpatch-through: 14, where Memoize was added

- Revert "Flush Memoize cache when non-key parameters change". This reverts
commit 1050048a315790a505465bfcceb26eaf8dbc7e2e.

- Flush Memoize cache when non-key parameters change, take 2. It's possible that
a subplan below a Memoize node contains a parameter from above the Memoize
node. If this parameter changes then cache entries may become out-dated due
to the new parameter value. Previously Memoize was mistakenly not aware of
this. We fix this here by flushing the cache whenever a parameter that's not
part of the cache key changes. Bug: #17213 Reported by: Elvis Pranskevichus
Author: David Rowley Discussion:
Backpatch-through: 14, where Memoize was added

Amit Kapila pushed:

- Rename `SnapBuild*` macros in slot.c. Same macro names for
SnapBuildOnDiskNotChecksummedSize and SnapBuildOnDiskChecksummedSize are being
used in slot.c and snapbuild.c. This patch renames them, in slot.c, to
ReplicationSlotOnDiskNotChecksummedSize and
ReplicationSlotOnDiskChecksummedSize similar to the other macros. This makes
all macro names look consistent in slot.c. Author: Bharath Rupireddy

Robert Haas pushed:

- Fix corner-case failure to detect improper timeline switch.
rescanLatestTimeLine() contains a guard against switching to a timeline that
forked off from the current one prior to the current recovery point, but that
guard does not work if the timeline switch occurs before the first WAL recod
(which must be the checkpoint record) is read. Without this patch, an improper
timeline switch is therefore possible in such cases. This happens because
rescanLatestTimeLine() relies on the global variable EndRecPtr to understand
the current position of WAL replay. However, EndRecPtr at this point in the
code contains the endpoint of the last-replayed record, not the startpoint or
endpoint of the record being replayed now. Thus, before any records have been
replayed, it's zero, which causes the sanity check to always pass. To fix,
pass down the correct timeline explicitly. The EndRecPtr value we want is the
one from the xlogreader, which will be the starting position of the record
we're about to try to read, rather than the global variable, which is the
ending position of the last record we successfully read. They're usually the
same, but not in the corner case described here. No back-patch, because in
v14 and earlier branhes, we were using the wrong TLI here as well as the wrong
LSN. In master, that was fixed by commit
4a92a1c3d1c361ffb031ed05bf65b801241d7cdd, but that and it's prerequisite
patches are too invasive to back-patch for such a minor issue. Patch by me,
reviewed by Amul Sul. Discussion:

- xlog.c: Remove global variables ReadRecPtr and EndRecPtr. In most places, the
variables necessarily store the same value as the eponymous members of the
XLogReaderState that we use during WAL replay, because ReadRecord() assigns
the values from the structure members to the global variables just after
XLogReadRecord() returns. However, XLogBeginRead() adjusts the structure
members but not the global variables, so after XLogBeginRead() and before the
completion of XLogReadRecord() the values can differ. Otherwise, they must be
identical. According to my analysis, the only place where either variable is
referenced at a point where it might not have the same value as the structure
member is the refrence to EndRecPtr within XLogPageRead. Therefore, at every
other place where we are using the global variable, we can just switch to
using the structure member instead, and remove the global variable. However,
we can, and in fact should, do this in XLogPageRead() as well, because at that
point in the code, the global variable will actually store the start of the
record we want to read - either because it's where the last WAL record ended,
or because the read position has been changed using XLogBeginRead since the
last record was read. The structure member, on the other hand, will already
have been updated to point to the end of the record we just read. Elsewhere,
the latter is what we use as an argument to emode_for_corrupt_record(), so we
should do the same here. This part of the patch is perhaps a bug fix, but I
don't think it has any important consequences, so no back-patch. The point
here is just to continue to whittle down the entirely excessive use of global
variables in xlog.c. Discussion:

Heikki Linnakangas pushed:

- Fix missing space in docs. Author: Japin Li Discussion:

Andres Freund pushed:

- Replace straggling uses of ReadRecPtr/EndRecPtr. d2ddfa681db removed
ReadRecPtr/EndRecPtr, but two uses within an #ifdef WAL_DEBUG escaped.

Daniel Gustafsson pushed:

- Fix GRANTED BY support in REVOKE ROLE statements. Commit 6aaaa76bb added
support for the GRANTED BY clause in GRANT and REVOKE statements, but missed
adding support for checking the role in the REVOKE ROLE case. Fix by checking
that the parsed role matches the CURRENT_ROLE/CURRENT_USER requirement, and
also add some tests for it. Backpatch to v14 where GRANTED BY support was
introduced. Discussion:
Backpatch-through: 14

syntax didn't have ample test coverage. Fix by adding coverage in the
privileges test suite. Author: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>

Browse pgsql-announce by date

  From Date Subject
Next Message United States PostgreSQL Association via PostgreSQL Announce 2021-11-30 18:09:58 PGConf NYC 2021 starts Thursday!
Previous Message PWN via PostgreSQL Announce 2021-11-23 08:49:07 PostgreSQL Weekly News - November 21, 2021