Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 11 Oct, 2019 1 commit
  3. 10 Oct, 2019 3 commits
  5. 09 Oct, 2019 5 commits
  7. 08 Oct, 2019 4 commits
  9. 07 Oct, 2019 9 commits
  11. 06 Oct, 2019 2 commits
    • Tom Lane's avatar
      Doc: improve docs about pg_statistic_ext_data. · 732457b5
      Tom Lane authored 
      Commit aa087ec6 was a bit over-hasty about the doc changes needed
while splitting pg_statistic_ext_data off from pg_statistic_ext.
It duplicated one para and inserted another in what seems to me
to be the wrong section.  Fix that up, and in passing do some minor
copy-editing.

Per report from noborusai.

Discussion: https://postgr.es/m/CAAM3qnLXLUz4mOBkqa8jxigpKhKNxzSuvwpjvCRPvO5EqWjxSg@mail.gmail.com
      732457b5
    • Tom Lane's avatar
      Avoid trying to release a List's initial allocation via repalloc(). · ac12ab06
      Tom Lane authored 
      Commit 1cff1b95 included some code that supposed it could repalloc()
a memory chunk to a smaller size without risk of the chunk moving.
That was not a great idea, because it depended on undocumented behavior
of AllocSetRealloc, which commit c477f3e4 changed thereby breaking it.
(Not to mention that this code ought to work with other memory context
types, which might not work the same...)  So get rid of the repalloc
calls, and instead just wipe the now-unused ListCell array and/or tell
Valgrind it's NOACCESS, as if we'd freed it.

In cases where the initial list allocation had been quite large, this
could represent an annoying waste of space.  In principle we could
ameliorate that by allocating the initial cell array separately when
it exceeds some threshold.  But that would complicate new_list() which
is hot code, and the returns would materialize only in narrow cases.
On balance I don't think it'd be worth it.

Discussion: https://postgr.es/m/17059.1570208426@sss.pgh.pa.us
      ac12ab06
  13. 05 Oct, 2019 5 commits
  15. 04 Oct, 2019 10 commits
    • Andres Freund's avatar
      Add isolation tests for the combination of EPQ and triggers. · c8841199
      Andres Freund authored 
      As evidenced by bug #16036 this area is woefully under-tested. Add
fairly extensive tests for the combination.

Backpatch back to 9.6 - before that isolationtester was not capable
enough. While we don't backpatch tests all the time, future fixes to
trigger.c would potentially look different enough in 12+ from the
earlier branches that introducing bugs during backpatching is more
likely than normal. Also, it's just a crucial and undertested area of
the code.

Author: Andres Freund
Discussion: https://postgr.es/m/16036-28184c90d952fb7f@postgresql.org
Backpatch: 9.6-, the earliest these tests work
      c8841199
    • Andres Freund's avatar
      Fix crash caused by EPQ happening with a before update trigger present. · d986d4e8
      Andres Freund authored 
      When ExecBRUpdateTriggers()'s GetTupleForTrigger() follows an EPQ
chain the former needs to run the result tuple through the junkfilter
again, and update the slot containing the new version of the tuple to
contain that new version. The input tuple may already be in the
junkfilter's output slot, which used to be OK - we don't need the
previous version anymore. Unfortunately ff11e7f4 started to use
ExecCopySlot() to update newslot, and ExecCopySlot() doesn't support
copying a slot into itself, leading to a slot in a corrupt
state, which then can cause crashes or other symptoms.

Fix this by skipping the ExecCopySlot() when copying into itself.

While we could have easily made ExecCopySlot() handle that case, it
seems better to add an assert forbidding doing so instead. As the goal
of copying might be to make the contents of one slot independent from
another, it seems failure prone to handle doing so silently.

A follow-up commit will add tests for the obviously under-covered
combination of EPQ and triggers. Done as a separate commit as it might
make sense to backpatch them further than this bug.

Also remove confusion with confusing variable names for slots in
ExecBRDeleteTriggers() and ExecBRUpdateTriggers().

Bug: #16036
Reported-By: Антон Власов
Author: Andres Freund
Discussion: https://postgr.es/m/16036-28184c90d952fb7f@postgresql.org
Backpatch: 12-, where ff11e7f4 was merged
      d986d4e8
    • Andres Freund's avatar
      Use a fd opened for read/write when syncing slots during startup, take 2. · a586cc4b
      Andres Freund authored 
      Cribbing from dfbaed45:
    Some operating systems, including the reporter's windows, return EBADFD
    or similar when fsync() is invoked on a O_RDONLY file descriptor.
    Unfortunately RestoreSlotFromDisk() does exactly that; which causes
    failures after restarts in at least some scenarios.

    If you hit the bug the error message will be something like
    ERROR: could not fsync file "pg_replslot/$name/state": Bad file descriptor

    Simply use O_RDWR instead of O_RDONLY when opening the relevant file
    descriptor to fix the bug.

Unfortunately this fix was undone in 82a5649f. Re-apply, and add a
comment.

Bug: 16039
Reported-By: Hans Buschmann
Author: Andres Freund
Discussion: https://postgr.es/m/16039-196fc97cc05e141c@postgresql.org
Backpatch: 12-, as 82a5649f
      a586cc4b
    • Andrew Dunstan's avatar
      Handle spaces in OpenSSL install location for MSVC · ad7595b8
      Andrew Dunstan authored 
      First, make sure that the .exe name is quoted when trying to get the
version number. Also, don't quote the lib name for using in the project
files if it's already been quoted. This second change applies to all
libraries, not just OpenSSL.

This has clearly been broken forever, so backpatch to all live branches.
      ad7595b8
    • Robert Haas's avatar
      Rename some toasting functions based on whether they are heap-specific. · 2e8b6bfa
      Robert Haas authored 
      The old names for the attribute-detoasting functions names included
the word "heap," which seems outdated now that the heap is only one of
potentially many table access methods.

On the other hand, toast_insert_or_update and toast_delete are
heap-specific, so rename them by adding "heap_" as a prefix.

Not all of the work of making the TOAST system fully accessible to AMs
other than the heap is done yet, but there seems to be little harm in
getting this renaming out of the way now. Commit
8b94dab0 already divided up the
functions among various files partially according to whether it was
intended that they should be heap-specific or AM-agnostic, so this is
just clarifying the division contemplated by that commit.

Patch by me, reviewed and tested by Prabhat Sabu, Thomas Munro,
Andres Freund, and Álvaro Herrera.

Discussion: http://postgr.es/m/CA+TgmoZv-=2iWM4jcw5ZhJeL18HF96+W1yJeYrnGMYdkFFnEpQ@mail.gmail.com
      2e8b6bfa
    • Tom Lane's avatar
      Fix bitshiftright()'s zero-padding some more. · 61aa9f54
      Tom Lane authored 
      Commit 5ac0d936 failed to entirely fix bitshiftright's habit of
leaving one-bits in the pad space that should be all zeroes,
because in a moment of sheer brain fade I'd concluded that only
the code path used for not-a-multiple-of-8 shift distances needed
to be fixed.  Of course, a multiple-of-8 shift distance can also
cause the problem, so we need to forcibly zero the extra bits
in both cases.

Per bug #16037 from Alexander Lakhin.  As before, back-patch to all
supported branches.

Discussion: https://postgr.es/m/16037-1d1ebca564db54f4@postgresql.org
      61aa9f54
    • Tomas Vondra's avatar
      Use Size instead of int64 to track allocated memory · f2369bc6
      Tomas Vondra authored 
      Commit 5dd7fc15 added block-level memory accounting, but used int64 variable to
track the amount of allocated memory. That is incorrect, because we have Size for
exactly these purposes, but it was mostly harmless until c477f3e4 which changed
how we handle with repalloc() when downsizing the chunk. Previously we've ignored
these cases and just kept using the original chunk, but now we need to update the
accounting, and the code was doing this:

    context->mem_allocated += blksize - oldblksize;

Both blksize and oldblksize are Size (so unsigned) which means the subtraction
underflows, producing a very high positive value. On 64-bit platforms (where Size
has the same size as mem_alllocated) this happens to work because the result wraps
to the right value, but on (some) 32-bit platforms this fails.

This fixes two things - it changes mem_allocated (and related variables) to Size,
and it splits the update to two separate steps, to prevent any underflows.

Discussion: https://www.postgresql.org/message-id/15151.1570163761%40sss.pgh.pa.us
      f2369bc6
    • Robert Haas's avatar
      Remove AtSubStart_Notify. · 967e276e
      Robert Haas authored 
      Allocate notify-related state lazily instead. This makes trivial
subtransactions noticeably faster.

Patch by me, reviewed and tested by Dilip Kumar, Kyotaro Horiguchi,
and Jeevan Ladhe.

Discussion: https://postgr.es/m/CA+TgmobE1J22S1eC-6N-je9LgrcwZypkwp+zH6JXo9mc=4Nk3A@mail.gmail.com
      967e276e
    • Michael Paquier's avatar
      Fix issues in pg_rewind with --no-ensure-shutdown/--write-recovery-conf · 6837632b
      Michael Paquier authored 
      This fixes two issues with recent features added in pg_rewind:
- --dry-run should do nothing on the target directory, but 927474ce
forgot to consider that for --write-recovery-conf.
- --no-ensure-shutdown was not actually working.  There is no test
coverage for this option yet, but a subsequent patch will add that.

Author: Alexey Kondratov
Discussion: https://postgr.es/m/7ca88204-3e0b-2f4c-c8af-acadc4b266e5@postgrespro.ru
      6837632b
    • Michael Paquier's avatar
      Fix --dry-run mode of pg_rewind · 6f3823b0
      Michael Paquier authored 
      Even if --dry-run mode was specified, the control file was getting
updated, preventing follow-up runs of pg_rewind to work properly on the
target data folder.  The origin of the problem came from the refactoring
done by ce6afc68.

Author: Alexey Kondratov
Discussion: https://postgr.es/m/7ca88204-3e0b-2f4c-c8af-acadc4b266e5@postgrespro.ru
Backpatch-through: 12
      6f3823b0
  17. 03 Oct, 2019 1 commit
    • Tom Lane's avatar
      Avoid unnecessary out-of-memory errors during encoding conversion. · 8e10405c
      Tom Lane authored 
      Encoding conversion uses the very simplistic rule that the output
can't be more than 4X longer than the input, and palloc's a buffer
of that size.  This results in failure to convert any string longer
than 1/4 GB, which is becoming an annoying limitation.

As a band-aid to improve matters, allow the allocated output buffer
size to exceed 1GB.  We still insist that the final result fit into
MaxAllocSize (1GB), though.  Perhaps it'd be safe to relax that
restriction, but it'd require close analysis of all callers, which
is daunting (not least because external modules might call these
functions).  For the moment, this should allow a 2X to 4X improvement
in the longest string we can convert, which is a useful gain in
return for quite a simple patch.

Also, once we have successfully converted a long string, repalloc
the output down to the actual string length, returning the excess
to the malloc pool.  This seems worth doing since we can usually
expect to give back several MB if we take this path at all.

This still leaves much to be desired, most notably that the assumption
that MAX_CONVERSION_GROWTH == 4 is very fragile, and yet we have no
guard code verifying that the output buffer isn't overrun.  Fixing
that would require significant changes in the encoding conversion
APIs, so it'll have to wait for some other day.

The present patch seems safely back-patchable, so patch all supported
branches.

Alvaro Herrera and Tom Lane

Discussion: https://postgr.es/m/20190816181418.GA898@alvherre.pgsql
Discussion: https://postgr.es/m/3614.1569359690@sss.pgh.pa.us
      8e10405c