1. 25 Nov, 2020 3 commits
    • Amit Kapila's avatar
      Remove obsolete comment atop ri_PlanCheck. · 805b8163
      Amit Kapila authored
      Commit 5b7ba75f removed the unused parameter but forgot to update the
      nearby comments.
      
      Author: Li Japin
      Backpatch-through: 13, where it was introduced
      Discussion: https://postgr.es/m/0E2F62A2-B2F1-4052-83AE-F0BEC8A75789@hotmail.com
      805b8163
    • David Rowley's avatar
      Stop gap fix for __attribute__((cold)) compiler bug in MinGW 8.1 · 687f6163
      David Rowley authored
      The buildfarm animal walleye, running MinGW 8.1 has been having problems
      ever since 697e1d02 and 913ec71d went in.  This appears to be a bug in
      assembler which was fixed in a later version.
      
      For now, in order to get that animal running green again, let's just
      define pg_attribute_cold and pg_attribute_hot to be empty macros on that
      compiler.  Hopefully, we can get the support of the owner of the animal to
      upgrade to a less buggy compiler and revert this at a later date.
      
      Discussion: https://postgr.es/m/286560.1606233316@sss.pgh.pa.us
      687f6163
    • Michael Paquier's avatar
      Remove catalog function currtid() · 7b94e999
      Michael Paquier authored
      currtid() and currtid2() are an undocumented set of functions whose sole
      known user is the Postgres ODBC driver, able to retrieve the latest TID
      version for a tuple given by the caller of those functions.
      
      As used by Postgres ODBC, currtid() is a shortcut able to retrieve the
      last TID loaded into a backend by passing an OID of 0 (magic value)
      after a tuple insertion.  This is removed in this commit, as it became
      obsolete after the driver began using "RETURNING ctid" with inserts, a
      clause supported since Postgres 8.2 (using RETURNING is better for
      performance anyway as it reduces the number of round-trips to the
      backend).
      
      currtid2() is still used by the driver, so this remains around for now.
      Note that this function is kept in its original shape for backward
      compatibility reasons.
      
      Per discussion with many people, including Andres Freund, Peter
      Eisentraut, Álvaro Herrera, Hiroshi Inoue, Tom Lane and myself.
      
      Bump catalog version.
      
      Discussion: https://postgr.es/m/20200603021448.GB89559@paquier.xyz
      7b94e999
  2. 24 Nov, 2020 9 commits
  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
  4. 22 Nov, 2020 1 commit
  5. 21 Nov, 2020 5 commits
  6. 20 Nov, 2020 6 commits
  7. 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
  8. 18 Nov, 2020 1 commit
    • Alvaro Herrera's avatar
      Relax lock level for setting PGPROC->statusFlags · 27838981
      Alvaro Herrera authored
      We don't actually need a lock to set PGPROC->statusFlags itself; what we
      do need is a shared lock on either XidGenLock or ProcArrayLock in order to
      ensure MyProc->pgxactoff keeps still while we modify the mirror array in
      ProcGlobal->statusFlags.  Some places were using an exclusive lock for
      that, which is excessive.  Relax those to use shared lock only.
      
      procarray.c has a couple of places with somewhat brittle assumptions
      about PGPROC changes: ProcArrayEndTransaction uses only shared lock, so
      it's permissible to change MyProc only.  On the other hand,
      ProcArrayEndTransactionInternal also changes other procs, so it must
      hold exclusive lock.  Add asserts to ensure those assumptions continue
      to hold.
      
      Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
      Reviewed-by: default avatarMichael Paquier <michael@paquier.xyz>
      Discussion: https://postgr.es/m/20201117155501.GA13805@alvherre.pgsql
      27838981