Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 24 Nov, 2020 1 commit
  3. 23 Nov, 2020 9 commits
    • David Rowley's avatar
      Improve compiler code layout in elog/ereport ERROR calls · 913ec71d
      David Rowley authored 
      Here we use a bit of preprocessor trickery to coax supporting compilers
into laying out their generated code so that the code that's in the same
branch as elog(ERROR)/ereport(ERROR) calls is moved away from the hot
path.  Effectively, this reduces the size of the hot code meaning that it
can sit on fewer cache lines.

Performance improvements of between 10-15% have been seen on highly CPU
bound workloads using pgbench's TPC-b benchmark.

What's achieved here is very similar to putting the error condition inside
an unlikely() macro. For example;

if (unlikely(x < 0))
    elog(ERROR, "invalid x value");

now there's no need to make use of unlikely() here as the common macro
used by elog and ereport will now see that elevel is >= ERROR and make use
of a pg_attribute_cold marked version of errstart().

When elevel < ERROR or if it cannot be determined to be constant, the
original behavior is maintained.

Author: David Rowley
Reviewed-by: Andres Freund, Peter Eisentraut
Discussion: https://postgr.es/m/CAApHDvrVpasrEzLL2er7p9iwZFZ%3DJj6WisePcFeunwfrV0js_A%40mail.gmail.com
      913ec71d
    • David Rowley's avatar
      Define pg_attribute_cold and pg_attribute_hot macros · 697e1d02
      David Rowley authored 
      For compilers supporting __has_attribute and __has_attribute (hot/cold).

__has_attribute is supported on gcc >= 5, clang >= 2.9 and icc >= 17.

A followup commit will implement some usages of these macros.

Author: David Rowley
Reviewed-by: Andres Freund, Peter Eisentraut
Discussion: https://postgr.es/m/CAApHDvrVpasrEzLL2er7p9iwZFZ%3DJj6WisePcFeunwfrV0js_A%40mail.gmail.com
      697e1d02
    • Tom Lane's avatar
      Remove unnecessary #include. · 3b9b01f7
      Tom Lane authored 
      Justin Pryzby

Discussion: https://postgr.es/m/20201123205505.GJ24052@telsasoft.com
      3b9b01f7
    • Alvaro Herrera's avatar
      Don't hold ProcArrayLock longer than needed in rare cases · 450c8230
      Alvaro Herrera authored 
      While cancelling an autovacuum worker, we hold ProcArrayLock while
formatting a debugging log string.  We can make this shorter by saving
the data we need to produce the message and doing the formatting outside
the locked region.

This isn't terribly critical, as it only occurs pretty rarely: when a
backend runs deadlock detection and it happens to be blocked by a
autovacuum running autovacuum.  Still, there's no need to cause a hiccup
in ProcArrayLock processing, which can be very high-traffic in some
cases.

While at it, rework code so that we only print the string when it is
really going to be used, as suggested by Michael Paquier.

Discussion: https://postgr.es/m/20201118214127.GA3179@alvherre.pgsqlReviewed-by: default avatarMichael Paquier <michael@paquier.xyz>
      450c8230
    • Tom Lane's avatar
      Rename the "point is strictly above/below point" comparison operators. · 0cc99327
      Tom Lane authored 
      Historically these were called >^ and <^, but that is inconsistent
with the similar box, polygon, and circle operators, which are named
|>> and <<| respectively.  Worse, the >^ and <^ names are used for
*not* strict above/below tests for the box type.

Hence, invent new operators following the more common naming.  The
old operators remain available for now, and are still accepted by
the relevant index opclasses too.  But there's a deprecation notice,
so maybe we can get rid of them someday.

Emre Hasegeli, reviewed by Pavel Borisov

Discussion: https://postgr.es/m/24348.1587444160@sss.pgh.pa.us
      0cc99327
    • Tom Lane's avatar
      Improve wording of two error messages related to generated columns. · d36228a9
      Tom Lane authored 
      Clarify that you can "insert" into a generated column as long as what
you're inserting is a DEFAULT placeholder.

Also, use ERRCODE_GENERATED_ALWAYS in place of ERRCODE_SYNTAX_ERROR;
there doesn't seem to be any reason to use the less specific errcode.

Discussion: https://postgr.es/m/9q0sgcr416t.fsf@gmx.us
      d36228a9
    • Alvaro Herrera's avatar
      Make some sanity-check elogs more verbose · fe051291
      Alvaro Herrera authored 
      A few sanity checks in funcapi.c were not mentioning all the possible
clauses for failure, confusing developers who fat-fingered catalog data
additions.  Make the errors more detailed to avoid wasting time in
pinpointing mistakes.

Per complaint from Craig Ringer.
Reviewed-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAMsr+YH7Kd87A3cU5m_wKo46HPQ46zFv5wesFNL0YWxkGhGv3g@mail.gmail.com
      fe051291
    • Heikki Linnakangas's avatar
      Fix a few comments that referred to copy.c. · 68b1a487
      Heikki Linnakangas authored 
      Missed these in the previous commit.
      68b1a487
    • Heikki Linnakangas's avatar
      Split copy.c into four files. · c532d15d
      Heikki Linnakangas authored 
      Copy.c has grown really large. Split it into more manageable parts:

- copy.c now contains only a few functions that are common to COPY FROM
  and COPY TO.

- copyto.c contains code for COPY TO.

- copyfrom.c contains code for initializing COPY FROM, and inserting the
  tuples to the correct table.

- copyfromparse.c contains code for reading from the client/file/program,
  and parsing the input text/CSV/binary format into tuples.

All of these parts are fairly complicated, and fairly independent of each
other. There is a patch being discussed to implement parallel COPY FROM,
which will add a lot of new code to the COPY FROM path, and another patch
which would allow INSERTs to use the same multi-insert machinery as COPY
FROM, both of which will require refactoring that code. With those two
patches, there's going to be a lot of code churn in copy.c anyway, so now
seems like a good time to do this refactoring.

The CopyStateData struct is also split. All the formatting options, like
FORMAT, QUOTE, ESCAPE, are put in a new CopyFormatOption struct, which
is used by both COPY FROM and TO. Other state data are kept in separate
CopyFromStateData and CopyToStateData structs.

Reviewed-by: Soumyadeep Chakraborty, Erik Rijkers, Vignesh C, Andres Freund
Discussion: https://www.postgresql.org/message-id/8e15b560-f387-7acc-ac90-763986617bfb%40iki.fi
      c532d15d
  5. 22 Nov, 2020 1 commit
  7. 21 Nov, 2020 5 commits
  9. 20 Nov, 2020 6 commits
  11. 19 Nov, 2020 6 commits
    • Tom Lane's avatar
      Remove undocumented IS [NOT] OF syntax. · 926fa801
      Tom Lane authored 
      This feature was added a long time ago, in 7c1e67bd and eb121ba2,
but never documented in any user-facing way.  (Documentation added
in 6126d3e7 was commented out almost immediately, in 8272fc3f.)
That's because, while this syntax is defined by SQL:99, our
implementation is only vaguely related to the standard's semantics.
The standard appears to intend a run-time not parse-time test, and
it definitely intends that the test should understand subtype
relationships.

No one has stepped up to fix that in the intervening years, but
people keep coming across the code and asking why it's not documented.
Let's just get rid of it: if anyone ever wants to make it work per
spec, they can easily recover whatever parts of this code are still
of value from our git history.

If there's anyone out there who's actually using this despite its
undocumented status, they can switch to using pg_typeof() instead,
eg. "pg_typeof(something) = 'mytype'::regtype".  That gives
essentially the same semantics as what our IS OF code did.
(We didn't have that function last time this was discussed, or
we would have ripped out IS OF then.)

Discussion: https://postgr.es/m/CAKFQuwZ2pTc-DSkOiTfjauqLYkNREeNZvWmeg12Q-_69D+sYZA@mail.gmail.com
Discussion: https://postgr.es/m/BAY20-F23E9F2B4DAB3E4E88D3623F99B0@phx.gbl
Discussion: https://postgr.es/m/3E7CF81D.1000203@joeconway.com
      926fa801
    • Tom Lane's avatar
      Further fixes for CREATE TABLE LIKE: cope with self-referential FKs. · 97390fe8
      Tom Lane authored 
      Commit 50289819 was too careless about the order of execution of the
additional ALTER TABLE operations generated by expandTableLikeClause.
It just stuck them all at the end, which seems okay for most purposes.
But it falls down in the case where LIKE is importing a primary key
or unique index and the outer CREATE TABLE includes a FOREIGN KEY
constraint that needs to depend on that index.  Weird as that is,
it used to work, so we ought to keep it working.

To fix, make parse_utilcmd.c insert LIKE clauses between index-creation
and FK-creation commands in the transformed list of commands, and change
utility.c so that the commands generated by expandTableLikeClause are
executed immediately not at the end.  One could imagine scenarios where
this wouldn't work either; but currently expandTableLikeClause only
makes column default expressions, CHECK constraints, and indexes, and
this ordering seems fine for those.

Per bug #16730 from Sofoklis Papasofokli.  Like the previous patch,
back-patch to all supported branches.

Discussion: https://postgr.es/m/16730-b902f7e6e0276b30@postgresql.org
      97390fe8
    • Peter Eisentraut's avatar
      Rename object in test to avoid conflict · afaccbba
      Peter Eisentraut authored 
      In 01e658fa, the hash_func test
creates a type t1, but apparently a test running in parallel might
also use that name, depending on timing.  Rename the type to avoid the
issue.
      afaccbba
    • Peter Eisentraut's avatar
      Hash support for row types · 01e658fa
      Peter Eisentraut authored 
      Add hash functions for the record type as well as a hash operator
family and operator class for the record type.  This enables all the
hash functionality for the record type such as hash-based plans for
UNION/INTERSECT/EXCEPT DISTINCT, recursive queries using UNION
DISTINCT, hash joins, and hash partitioning.
Reviewed-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/38eccd35-4e2d-6767-1b3c-dada1eac3124%402ndquadrant.com
      01e658fa
    • Thomas Munro's avatar
      Add BarrierArriveAndDetachExceptLast(). · 7888b099
      Thomas Munro authored 
      Provide a way for one process to continue the remaining phases of a
(previously) parallel computation alone.  Later patches will use this to
extend Parallel Hash Join.

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: default avatarThomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKG%2BA6ftXPz4oe92%2Bx8Er%2BxpGZqto70-Q_ERwRaSyA%3DafNg%40mail.gmail.com
      7888b099
    • Michael Paquier's avatar
      Improve failure detection with array parsing in pg_dump · 13b58f89
      Michael Paquier authored 
      Similarly to 3636efa1, the checks done in pg_dump when parsing array
values from catalogs have been too lax.  Under memory pressure, it could
be possible, though very unlikely, to finish with dumps that miss some
data like:
- Statistics for indexes
- Run-time configuration of functions
- Configuration of extensions
- Publication list for a subscription

No backpatch is done as this is not going to be a problem in practice.
For example, if an OOM causes an array parsing to fail, a follow-up code
path of pg_dump would most likely complain with an allocation failure
due to the memory pressure.

Author: Michael Paquier
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20201111061319.GE2276@paquier.xyz
      13b58f89
  13. 18 Nov, 2020 5 commits
  15. 17 Nov, 2020 5 commits
    • Peter Geoghegan's avatar
      Deprecate nbtree's BTP_HAS_GARBAGE flag. · cf2acaf4
      Peter Geoghegan authored 
      Streamline handling of the various strategies that we have to avoid a
page split in nbtinsert.c.  When it looks like a leaf page is about to
overflow, we now perform deleting LP_DEAD items and deduplication in one
central place.  This greatly simplifies _bt_findinsertloc().

This has an independently useful consequence: nbtree no longer relies on
the BTP_HAS_GARBAGE page level flag/hint for anything important.  We
still set and unset the flag in the same way as before, but it's no
longer treated as a gating condition when considering if we should check
for already-set LP_DEAD bits.  This happens at the point where the page
looks like it might have to be split anyway, so simply checking the
LP_DEAD bits in passing is practically free.  This avoids missing
LP_DEAD bits just because the page-level hint is unset, which is
probably reasonably common (e.g. it happens when VACUUM unsets the
page-level flag without actually removing index tuples whose LP_DEAD-bit
was set recently, after the VACUUM operation began but before it reached
the leaf page in question).

Note that this isn't a big behavioral change compared to PostgreSQL 13.
We were already checking for set LP_DEAD bits regardless of whether the
BTP_HAS_GARBAGE page level flag was set before we considered doing a
deduplication pass.  This commit only goes slightly further by doing the
same check for all indexes, even indexes where deduplication won't be
performed.

We don't completely remove the BTP_HAS_GARBAGE flag.  We still rely on
it as a gating condition with pg_upgrade'd indexes from before B-tree
version 4/PostgreSQL 12.  That makes sense because we sometimes have to
make a choice among pages full of duplicates when inserting a tuple with
pre version 4 indexes.  It probably still pays to avoid accessing the
line pointer array of a page there, since it won't yet be clear whether
we'll insert on to the page in question at all, let alone split it as a
result.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: default avatarVictor Yegorov <vyegorov@gmail.com>
Discussion: https://postgr.es/m/CAH2-Wz%3DYpc1PDdk8OVJDChGJBjT06%3DA0Mbv9HyTLCsOknGcUFg%40mail.gmail.com
      cf2acaf4
    • Alvaro Herrera's avatar
      indexcmds.c: reorder function prototypes · 7684b6fb
      Alvaro Herrera authored 
      ... out of an overabundance of neatnikism, perhaps.
      7684b6fb
    • Peter Geoghegan's avatar
      nbtree: Rename nbtinsert.c variables for consistency. · a034f8b6
      Peter Geoghegan authored 
      Stop naming special area/opaque pointer variables 'lpageop' in contexts
where it doesn't make sense.  This is a holdover from a time when logic
that performs tasks that are now spread across _bt_insertonpg(),
_bt_findinsertloc(), and _bt_split() was more centralized.  'lpageop'
denotes "left page", which doesn't make sense outside of contexts in
which there isn't also a right page.

Also acquire page flag variables up front within _bt_insertonpg().  This
makes it closer to _bt_split() following refactoring commit bc3087b6.
This allows the page split and retail insert paths to both make use of
the same variables.
      a034f8b6
    • Amit Kapila's avatar
      Fix 'skip-empty-xacts' option in test_decoding for streaming mode. · 9653f24a
      Amit Kapila authored 
      In streaming mode, the transaction can be decoded in multiple streams and
those streams can be interleaved with streams of other transactions. So,
we can't remember the transaction's write status in the logical decoding
context because that might get changed due to some other transactions and
lead to wrong answers for 'skip-empty-xacts' option. We decided to keep
each transaction's write status in the ReorderBufferTxn to avoid
interleaved streams changing the status of some unrelated transactions.

Diagnosed-by: Amit Kapila
Author: Dilip Kumar
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CAA4eK1LR7=XNM_TLmpZMFuV8ZQpoxkem--NZJYf8YXmesbvwLA@mail.gmail.com
      9653f24a
    • Tom Lane's avatar
      Don't Insert() a VFD entry until it's fully built. · 2bd49b49
      Tom Lane authored 
      Otherwise, if FDDEBUG is enabled, the debugging output fails because
it tries to read the fileName, which isn't set up yet (and should in
fact always be NULL).

AFAICT, this has been wrong since Berkeley.  Before 96bf88d5,
it would accidentally fail to crash on platforms where snprintf()
is forgiving about being passed a NULL pointer for %s; but the
file name intended to be included in the debug output wouldn't
ever have shown up.

Report and fix by Greg Nancarrow.  Although this is only visibly
broken in custom-made builds, it still seems worth back-patching
to all supported branches, as the FDDEBUG code is pretty useless
as it stands.

Discussion: https://postgr.es/m/CAJcOf-cUDgm9qYtC_B6XrC6MktMPNRby2p61EtSGZKnfotMArw@mail.gmail.com
      2bd49b49
  17. 16 Nov, 2020 2 commits