Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 05 Nov, 2021 1 commit
  3. 03 Nov, 2021 3 commits
  5. 02 Nov, 2021 3 commits
    • Peter Geoghegan's avatar
      Don't overlook indexes during parallel VACUUM. · 61a86ed5
      Peter Geoghegan authored 
      Commit b4af70cb, which simplified state managed by VACUUM, performed
refactoring of parallel VACUUM in passing.  Confusion about the exact
details of the tasks that the leader process is responsible for led to
code that made it possible for parallel VACUUM to miss a subset of the
table's indexes entirely.  Specifically, indexes that fell under the
min_parallel_index_scan_size size cutoff were missed.  These indexes are
supposed to be vacuumed by the leader (alongside any parallel unsafe
indexes), but weren't vacuumed at all.  Affected indexes could easily
end up with duplicate heap TIDs, once heap TIDs were recycled for new
heap tuples.  This had generic symptoms that might be seen with almost
any index corruption involving structural inconsistencies between an
index and its table.

To fix, make sure that the parallel VACUUM leader process performs any
required index vacuuming for indexes that happen to be below the size
cutoff.  Also document the design of parallel VACUUM with these
below-size-cutoff indexes.

It's unclear how many users might be affected by this bug.  There had to
be at least three indexes on the table to hit the bug: a smaller index,
plus at least two additional indexes that themselves exceed the size
cutoff.  Cases with just one additional index would not run into
trouble, since the parallel VACUUM cost model requires two
larger-than-cutoff indexes on the table to apply any parallel
processing.  Note also that autovacuum was not affected, since it never
uses parallel processing.

Test case based on tests from a larger patch to test parallel VACUUM by
Masahiko Sawada.

Many thanks to Kamigishi Rei for her invaluable help with tracking this
problem down.

Author: Peter Geoghegan <pg@bowt.ie>
Author: Masahiko Sawada <sawada.mshk@gmail.com>
Reported-By: default avatarKamigishi Rei <iijima.yun@koumakan.jp>
Reported-By: default avatarAndrew Gierth <andrew@tao11.riddles.org.uk>
Diagnosed-By: default avatarAndres Freund <andres@anarazel.de>
Bug: #17245
Discussion: https://postgr.es/m/17245-ddf06aaf85735f36@postgresql.org
Discussion: https://postgr.es/m/20211030023740.qbnsl2xaoh2grq3d@alap3.anarazel.de
Backpatch: 14-, where the refactoring commit appears.
      61a86ed5
    • Tom Lane's avatar
      Fix variable lifespan in ExecInitCoerceToDomain(). · 16a56774
      Tom Lane authored 
      This undoes a mistake in 1ec7679f: domainval and domainnull were
meant to live across loop iterations, but they were incorrectly
moved inside the loop.  The effect was only to emit useless extra
EEOP_MAKE_READONLY steps, so it's not a big deal; nonetheless,
back-patch to v13 where the mistake was introduced.

Ranier Vilela

Discussion: https://postgr.es/m/CAEudQAqXuhbkaAp-sGH6dR6Nsq7v28_0TPexHOm6FiDYqwQD-w@mail.gmail.com
      16a56774
    • Tom Lane's avatar
      Avoid O(N^2) behavior in SyncPostCheckpoint(). · 08cfa598
      Tom Lane authored 
      As in commits 6301c3ada and e9d9ba2a4, avoid doing repetitive
list_delete_first() operations, since that would be expensive when
there are many files waiting to be unlinked.  This is a slightly
larger change than in those cases.  We have to keep the list state
valid for calls to AbsorbSyncRequests(), so it's necessary to invent a
"canceled" field instead of immediately deleting PendingUnlinkEntry
entries.  Also, because we might not be able to process all the
entries, we need a new list primitive list_delete_first_n().

list_delete_first_n() is almost list_copy_tail(), but it modifies the
input List instead of making a new copy.  I found a couple of existing
uses of the latter that could profitably use the new function.  (There
might be more, but the other callers look like they probably shouldn't
overwrite the input List.)

As before, back-patch to v13.

Discussion: https://postgr.es/m/CD2F0E7F-9822-45EC-A411-AE56F14DEA9F@amazon.com
      08cfa598
  7. 01 Nov, 2021 3 commits
  9. 31 Oct, 2021 2 commits
    • Tom Lane's avatar
      Don't try to read a multi-GB pg_stat_statements file in one call. · 7104e0b2
      Tom Lane authored 
      Windows fails on a request to read() more than INT_MAX bytes,
and perhaps other platforms could have similar issues.  Let's
adjust this code to read at most 1GB per call.

(One would not have thought the file could get that big, but now
we have a field report of trouble, so it can.  We likely ought to
add some mechanism to limit the size of the query-texts file
separately from the size of the hash table.  That is not this
patch, though.)

Per bug #17254 from Yusuke Egashira.  It's been like this for
awhile, so back-patch to all supported branches.

Discussion: https://postgr.es/m/17254-a926c89dc03375c2@postgresql.org
      7104e0b2
    • Tom Lane's avatar
      Avoid O(N^2) behavior when the standby process releases many locks. · 8424dfce
      Tom Lane authored 
      When replaying a transaction that held many exclusive locks on the
primary, a standby server's startup process would expend O(N^2)
effort on manipulating the list of locks.  This code was fine when
written, but commit 1cff1b95 made repetitive list_delete_first()
calls inefficient, as explained in its commit message.  Fix by just
iterating the list normally, and releasing storage only when done.
(This'd be inadequate if we needed to recover from an error occurring
partway through; but we don't.)

Back-patch to v13 where 1cff1b95 came in.

Nathan Bossart

Discussion: https://postgr.es/m/CD2F0E7F-9822-45EC-A411-AE56F14DEA9F@amazon.com
      8424dfce
  11. 29 Oct, 2021 2 commits
    • Peter Geoghegan's avatar
      Demote pg_unreachable() in heapam to an assertion. · bd9f4cf0
      Peter Geoghegan authored 
      Commit d168b666, which overhauled index deletion, added a
pg_unreachable() to the end of a sort comparator used when sorting heap
TIDs from an index page.  This allows the compiler to apply
optimizations that assume that the heap TIDs from the index AM must
always be unique.

That doesn't seem like a good idea now, given recent reports of
corruption involving duplicate TIDs in indexes on Postgres 14.  Demote
to an assertion, just in case.

Backpatch: 14-, where index deletion was overhauled.
      bd9f4cf0
    • Tom Lane's avatar
      Update time zone data files to tzdata release 2021e. · 0c8a40b3
      Tom Lane authored 
      DST law changes in Fiji, Jordan, Palestine, and Samoa.  Historical
corrections for Barbados, Cook Islands, Guyana, Niue, Portugal, and
Tonga.

Also, the Pacific/Enderbury zone has been renamed to Pacific/Kanton.
The following zones have been merged into nearby, more-populous zones
whose clocks have agreed since 1970: Africa/Accra, America/Atikokan,
America/Blanc-Sablon, America/Creston, America/Curacao,
America/Nassau, America/Port_of_Spain, Antarctica/DumontDUrville,
and Antarctica/Syowa.
      0c8a40b3
  13. 28 Oct, 2021 2 commits
    • Tom Lane's avatar
      Improve contrib/amcheck's tests for CREATE INDEX CONCURRENTLY. · b1f943d2
      Tom Lane authored 
      Commits fdd965d07 and 3cd9c3b92 tested CREATE INDEX CONCURRENTLY by
launching two separate pgbench runs concurrently.  This was needed so
that only a single client thread would run CREATE INDEX CONCURRENTLY,
avoiding deadlock between two CICs.  However, there's a better way,
which is to use an advisory lock to prevent concurrent CICs.  That's
better in part because the test code is shorter and more readable, but
mostly because it automatically scales things to launch an appropriate
number of CICs relative to the number of INSERT transactions.
As committed, typically half to three-quarters of the CIC transactions
were pointless because the INSERT transactions had already stopped.

In passing, remove background_pgbench, which was added to support
these tests and isn't needed anymore.  We can always put it back
if we find a use for it later.

Back-patch to v12; older pgbench versions lack the
conditional-execution features needed for this method.

Tom Lane and Andrey Borodin

Discussion: https://postgr.es/m/139687.1635277318@sss.pgh.pa.us
      b1f943d2
    • Michael Paquier's avatar
      doc: Fix link to SELinux user guide in sepgsql page · da7d0fb1
      Michael Paquier authored 
      Reported-by: Anton Voloshin
Discussion: https://postgr.es/m/15a86d4e-a237-1acd-18a2-fd69730f1ab9@postgrespro.ru
Backpatch-through: 10
      da7d0fb1
  15. 27 Oct, 2021 3 commits
  17. 26 Oct, 2021 3 commits
  19. 24 Oct, 2021 2 commits
    • Noah Misch's avatar
      Fix CREATE INDEX CONCURRENTLY for the newest prepared transactions. · a5b9a000
      Noah Misch authored 
      The purpose of commit 8a54e12a was to
fix this, and it sufficed when the PREPARE TRANSACTION completed before
the CIC looked for lock conflicts.  Otherwise, things still broke.  As
before, in a cluster having used CIC while having enabled prepared
transactions, queries that use the resulting index can silently fail to
find rows.  It may be necessary to reindex to recover from past
occurrences; REINDEX CONCURRENTLY suffices.  Fix this for future index
builds by making CIC wait for arbitrarily-recent prepared transactions
and for ordinary transactions that may yet PREPARE TRANSACTION.  As part
of that, have PREPARE TRANSACTION transfer locks to its dummy PGPROC
before it calls ProcArrayClearTransaction().  Back-patch to 9.6 (all
supported versions).

Andrey Borodin, reviewed (in earlier versions) by Andres Freund.

Discussion: https://postgr.es/m/01824242-AA92-4FE9-9BA7-AEBAFFEA3D0C@yandex-team.ru
      a5b9a000
    • Noah Misch's avatar
      Avoid race in RelationBuildDesc() affecting CREATE INDEX CONCURRENTLY. · dde966ef
      Noah Misch authored 
      CIC and REINDEX CONCURRENTLY assume backends see their catalog changes
no later than each backend's next transaction start.  That failed to
hold when a backend absorbed a relevant invalidation in the middle of
running RelationBuildDesc() on the CIC index.  Queries that use the
resulting index can silently fail to find rows.  Fix this for future
index builds by making RelationBuildDesc() loop until it finishes
without accepting a relevant invalidation.  It may be necessary to
reindex to recover from past occurrences; REINDEX CONCURRENTLY suffices.
Back-patch to 9.6 (all supported versions).

Noah Misch and Andrey Borodin, reviewed (in earlier versions) by Andres
Freund.

Discussion: https://postgr.es/m/20210730022548.GA1940096@gust.leadboat.com
      dde966ef
  21. 23 Oct, 2021 1 commit
  23. 22 Oct, 2021 3 commits
  25. 21 Oct, 2021 3 commits
  27. 20 Oct, 2021 2 commits
  29. 19 Oct, 2021 7 commits
    • Alvaro Herrera's avatar
      Ensure correct lock level is used in ALTER ... RENAME · 3ce3fb2f
      Alvaro Herrera authored 
      Commit 1b5d797c intended to relax the lock level used to rename
indexes, but inadvertently allowed *any* relation to be renamed with a
lowered lock level, as long as the command is spelled ALTER INDEX.
That's undesirable for other relation types, so retry the operation with
the higher lock if the relation turns out not to be an index.

After this fix, ALTER INDEX <sometable> RENAME will require access
exclusive lock, which it didn't before.

Author: Nathan Bossart <bossartn@amazon.com>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reported-by: default avatarOnder Kalaci <onderk@microsoft.com>
Discussion: https://postgr.es/m/PH0PR21MB1328189E2821CDEC646F8178D8AE9@PH0PR21MB1328.namprd21.prod.outlook.com
      3ce3fb2f
    • Andres Freund's avatar
      Adapt src/test/ldap/t/001_auth.pl to work with openldap 2.5. · 533315b6
      Andres Freund authored 
      ldapsearch's deprecated -h/-p arguments were removed, need to use -H now -
which has been around for over 20 years.

As perltidy insists on reflowing the parameters anyway, change order and
"phrasing" to yield a less confusing layout (per suggestion from Tom Lane).

Discussion: https://postgr.es/m/20211009233850.wvr6apcrw2ai6cnj@alap3.anarazel.de
Backpatch: 11-, where the tests were added.
      533315b6
    • Tom Lane's avatar
      Fix assignment to array of domain over composite. · 04dae19f
      Tom Lane authored 
      An update such as "UPDATE ... SET fld[n].subfld = whatever"
failed if the array elements were domains rather than plain
composites.  That's because isAssignmentIndirectionExpr()
failed to cope with the CoerceToDomain node that would appear
in the expression tree in this case.  The result would typically
be a crash, and even if we accidentally didn't crash, we'd not
correctly preserve other fields of the same array element.

Per report from Onder Kalaci.  Back-patch to v11 where arrays of
domains came in.

Discussion: https://postgr.es/m/PH0PR21MB132823A46AA36F0685B7A29AD8BD9@PH0PR21MB1328.namprd21.prod.outlook.com
      04dae19f
    • Tom Lane's avatar
      Remove bogus assertion in transformExpressionList(). · f627fd54
      Tom Lane authored 
      I think when I added this assertion (in commit 8f889b10), I was only
thinking of the use of transformExpressionList at top level of INSERT
and VALUES.  But it's also called by transformRowExpr(), which can
certainly occur in an UPDATE targetlist, so it's inappropriate to
suppose that p_multiassign_exprs must be empty.  Besides, since the
input is not expected to contain ResTargets, there's no reason it
should contain MultiAssignRefs either.  Hence this code need not
be concerned about the state of p_multiassign_exprs, and we should
just drop the assertion.

Per bug #17236 from ocean_li_996.  It's been wrong for years,
so back-patch to all supported branches.

Discussion: https://postgr.es/m/17236-3210de9bcba1d7ca@postgresql.org
      f627fd54
    • Daniel Gustafsson's avatar
      Fix bug in TOC file error message printing · 3e2f32b0
      Daniel Gustafsson authored 
      If the blob TOC file cannot be parsed, the error message was failing
to print the filename as the variable holding it was shadowed by the
destination buffer for parsing.  When the filename fails to parse,
the error will print an empty string:

 ./pg_restore -d foo -F d dump
 pg_restore: error: invalid line in large object TOC file "": ..

..instead of the intended error message:

 ./pg_restore -d foo -F d dump
 pg_restore: error: invalid line in large object TOC file "dump/blobs.toc": ..

Fix by renaming both variables as the shared name was too generic to
store either and still convey what the variable held.

Backpatch all the way down to 9.6.

Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/A2B151F5-B32B-4F2C-BA4A-6870856D9BDE@yesql.se
Backpatch-through: 9.6
      3e2f32b0
    • Daniel Gustafsson's avatar
      Fix sscanf limits in pg_basebackup and pg_dump · 121be6a6
      Daniel Gustafsson authored 
      Make sure that the string parsing is limited by the size of the
destination buffer.

In pg_basebackup the available values sent from the server
is limited to two characters so there was no risk of overflow.

In pg_dump the buffer is bounded by MAXPGPATH, and thus the limit
must be inserted via preprocessor expansion and the buffer increased
by one to account for the terminator. There is no risk of overflow
here, since in this case, the buffer scanned is smaller than the
destination buffer.

Backpatch the pg_basebackup fix to 11 where it was introduced, and
the pg_dump fix all the way down to 9.6.

Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/B14D3D7B-F98C-4E20-9459-C122C67647FB@yesql.se
Backpatch-through: 11 and 9.6
      121be6a6
    • Michael Paquier's avatar
      Block ALTER INDEX/TABLE index_name ALTER COLUMN colname SET (options) · b1b797ec
      Michael Paquier authored 
      The grammar of this command run on indexes with column names has always
been authorized by the parser, and it has never been documented.

Since 911e7020, it is possible to define opclass parameters as of CREATE
INDEX, which actually broke the old case of ALTER INDEX/TABLE where
relation-level parameters n_distinct and n_distinct_inherited could be
defined for an index (see 76a47c0e and its thread where this point has
been touched, still remained unused).  Attempting to do that in v13~
would cause the index to become unusable, as there is a new dedicated
code path to load opclass parameters instead of the relation-level ones
previously available.  Note that it is possible to fix things with a
manual catalog update to bring the relation back online.

This commit disables this command for now as the use of column names for
indexes does not make sense anyway, particularly when it comes to index
expressions where names are automatically computed.  One way to properly
support this case properly in the future would be to use column numbers
when it comes to indexes, in the same way as ALTER INDEX .. ALTER COLUMN
.. SET STATISTICS.

Partitioned indexes were already blocked, but not indexes.  Some tests
are added for both cases.

There was some code in ANALYZE to enforce n_distinct to be used for an
index expression if the parameter was defined, but just remove it for
now until/if there is support for this (note that index-level parameters
never had support in pg_dump either, previously), so this was just dead
code.

Reported-by: Matthijs van der Vleuten
Author: Nathan Bossart, Michael Paquier
Reviewed-by: Vik Fearing, Dilip Kumar
Discussion: https://postgr.es/m/17220-15d684c6c2171a83@postgresql.org
Backpatch-through: 13
      b1b797ec