Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: postgresql-cfbot/postgresql
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: cf/5582~1
Choose a base ref
...
head repository: postgresql-cfbot/postgresql
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: cf/5582
Choose a head ref
  • 5 commits
  • 4 files changed
  • 2 contributors

Commits on Feb 18, 2025

  1. Demonstrate crash in brininsertcleanup during logical replication. 

    A cross-partition update crashes if the subscriber's table has
a BRIN index.  This is easily demonstrated by adding such an
index to 013_partition.pl.  I'm not sure if we actually want
to include this in the final patchset, so it's a separate patch
for now.

Bug: #18815
Reported-by: Sergey Belyashov <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: TBD
    @tglsfdc
    tglsfdc authored and Commitfest Bot committed Feb 18, 2025
    Configuration menu
    Copy the full SHA
    9296ccf View commit details
    Browse the repository at this point in the history

  2. Harden brininsertcleanup against repeat calls. 

    Leaving an empty BrinInsertState struct linked in ii_AmCache
confuses both brininsertcleanup itself and brininsert, were that
to be called again.  We should fully remove it, instead.

This stops the crash reported in bug #18815.  I think it's fixing
the symptom rather than the root cause, but nonetheless there's
little argument for brininsertcleanup to be leaving an obviously
broken data structure behind.

Bug: #18815
Reported-by: Sergey Belyashov <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: TBD
    @tglsfdc
    tglsfdc authored and Commitfest Bot committed Feb 18, 2025
    Configuration menu
    Copy the full SHA
    66102d0 View commit details
    Browse the repository at this point in the history

  3. Assert that ExecOpenIndices and ExecCloseIndices are not repeated. 

    These functions should be called at most once per ResultRelInfo;
it's wasteful to do otherwise, and certainly the pattern of
opening twice and then closing twice is a bad idea.  Moreover,
index_insert_cleanup functions might not be prepared to be
called twice, as the just-hardened code in BRIN demonstrates.

Sadly, logical replication's apply_handle_tuple_routing() does
exactly that, meaning that either hunk of this patch is
sufficient to make it crash.

While other patches in this series are back-patch candidates,
this one should only be applied to HEAD, as perhaps there
are extensions doing the now-forbidden coding pattern.
We shouldn't break them in minor releases.

Bug: #18815
Reported-by: Sergey Belyashov <[email protected]>
Discussion: https://postgr.es/m/[email protected]
    @tglsfdc
    tglsfdc authored and Commitfest Bot committed Feb 18, 2025
    Configuration menu
    Copy the full SHA
    8d80010 View commit details
    Browse the repository at this point in the history

  4. Avoid duplicate ExecOpenIndices/ExecCloseIndices in logrep worker. 

    During a cross-partition update, apply_handle_tuple_routing calls
ExecFindPartition which does ExecOpenIndices (and expects that
ExecCleanupTupleRouting will close the indexes again).  Therefore,
we must avoid re-opening/closing the table's indexes when the
update code path calls apply_handle_insert_internal or
apply_handle_delete_internal.

That leaves us with only one caller of each function that needs
indexes to be opened/closed, so we can just move those calls
to the callers.

Also, remove the entirely-duplicative open/close calls within
apply_handle_tuple_routing itself.

Bug: #18815
Reported-by: Sergey Belyashov <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: TBD
    @tglsfdc
    tglsfdc authored and Commitfest Bot committed Feb 18, 2025
    Configuration menu
    Copy the full SHA
    26e7709 View commit details
    Browse the repository at this point in the history

  5. [CF 52/5582] v1 - fix for bug #18815 

    This commit was automatically generated by a robot at cfbot.cputube.org.
It is based on patches submitted to the PostgreSQL mailing lists and
registered in the PostgreSQL Commitfest application.

This branch will be overwritten each time a new patch version is posted to
the email thread, and also periodically to check for bitrot caused by changes
on the master branch.

Commitfest entry: https://commitfest.postgresql.org/52/5582
Patch(es): https://www.postgresql.org/message-id/[email protected]
Author(s):
    Commitfest Bot committed Feb 18, 2025
    Configuration menu
    Copy the full SHA
    bc02368 View commit details
    Browse the repository at this point in the history
Loading