1. 05 Oct, 2017 7 commits
  2. 04 Oct, 2017 5 commits
  3. 03 Oct, 2017 3 commits
    • Tom Lane's avatar
      Allow multiple tables to be specified in one VACUUM or ANALYZE command. · 11d8d72c
      Tom Lane authored
      Not much to say about this; does what it says on the tin.
      
      However, formerly, if there was a column list then the ANALYZE action was
      implied; now it must be specified, or you get an error.  This is because
      it would otherwise be a bit unclear what the user meant if some tables
      have column lists and some don't.
      
      Nathan Bossart, reviewed by Michael Paquier and Masahiko Sawada, with some
      editorialization by me
      
      Discussion: https://postgr.es/m/E061A8E3-5E3D-494D-94F0-E8A9B312BBFC@amazon.com
      11d8d72c
    • Tom Lane's avatar
      Fix race condition with unprotected use of a latch pointer variable. · 45f9d086
      Tom Lane authored
      Commit 597a87cc introduced a latch pointer variable to replace use
      of a long-lived shared latch in the shared WalRcvData structure.
      This was not well thought out, because there are now hazards of the
      pointer variable changing while it's being inspected by another
      process.  This could obviously lead to a core dump in code like
      
      	if (WalRcv->latch)
      		SetLatch(WalRcv->latch);
      
      and there's a more remote risk of a torn read, if we have any
      platforms where reading/writing a pointer is not atomic.
      
      An actual problem would occur only if the walreceiver process
      exits (gracefully) while the startup process is trying to
      signal it, but that seems well within the realm of possibility.
      
      To fix, treat the pointer variable (not the referenced latch)
      as being protected by the WalRcv->mutex spinlock.  There
      remains a race condition that we could apply SetLatch to a
      process latch that no longer belongs to the walreceiver, but
      I believe that's harmless: at worst it'd cause an extra wakeup
      of the next process to use that PGPROC structure.
      
      Back-patch to v10 where the faulty code was added.
      
      Discussion: https://postgr.es/m/22735.1507048202@sss.pgh.pa.us
      45f9d086
    • Alvaro Herrera's avatar
      Fix coding rules violations in walreceiver.c · 89e434b5
      Alvaro Herrera authored
      1. Since commit b1a9bad9 we had pstrdup() inside a
      spinlock-protected critical section; reported by Andreas Seltenreich.
      Turn those into strlcpy() to stack-allocated variables instead.
      Backpatch to 9.6.
      
      2. Since commit 9ed551e0 we had a pfree() uselessly inside a
      spinlock-protected critical section.  Tom Lane noticed in code review.
      Move down.  Backpatch to 9.6.
      
      3. Since commit 64233902 we had GetCurrentTimestamp() (a kernel
      call) inside a spinlock-protected critical section.  Tom Lane noticed in
      code review.  Move it up.  Backpatch to 9.2.
      
      4. Since commit 1bb25580 we did elog(PANIC) while holding spinlock.
      Tom Lane noticed in code review.  Release spinlock before dying.
      Backpatch to 9.2.
      
      Discussion: https://postgr.es/m/87h8vhtgj2.fsf@ansel.ydns.eu
      89e434b5
  4. 02 Oct, 2017 4 commits
  5. 01 Oct, 2017 8 commits
  6. 30 Sep, 2017 5 commits
    • Tom Lane's avatar
      Fix pg_dump to assign domain array type OIDs during pg_upgrade. · 2632bcce
      Tom Lane authored
      During a binary upgrade, all type OIDs are supposed to be assigned by
      pg_dump based on their values in the old cluster.  But now that domains
      have arrays, there's nothing to base the arrays' type OIDs on, if we're
      upgrading from a pre-v11 cluster.  Make pg_dump search for an unused type
      OID to use for this purpose.  Per buildfarm.
      
      Discussion: https://postgr.es/m/E1dyLlE-0002gT-H5@gemulon.postgresql.org
      2632bcce
    • Tom Lane's avatar
      Support arrays over domains. · c12d570f
      Tom Lane authored
      Allowing arrays with a domain type as their element type was left un-done
      in the original domain patch, but not for any very good reason.  This
      omission leads to such surprising results as array_agg() not working on
      a domain column, because the parser can't identify a suitable output type
      for the polymorphic aggregate.
      
      In order to fix this, first clean up the APIs of coerce_to_domain() and
      some internal functions in parse_coerce.c so that we consistently pass
      around a CoercionContext along with CoercionForm.  Previously, we sometimes
      passed an "isExplicit" boolean flag instead, which is strictly less
      information; and coerce_to_domain() didn't even get that, but instead had
      to reverse-engineer isExplicit from CoercionForm.  That's contrary to the
      documentation in primnodes.h that says that CoercionForm only affects
      display and not semantics.  I don't think this change fixes any live bugs,
      but it makes things more consistent.  The main reason for doing it though
      is that now build_coercion_expression() receives ccontext, which it needs
      in order to be able to recursively invoke coerce_to_target_type().
      
      Next, reimplement ArrayCoerceExpr so that the node does not directly know
      any details of what has to be done to the individual array elements while
      performing the array coercion.  Instead, the per-element processing is
      represented by a sub-expression whose input is a source array element and
      whose output is a target array element.  This simplifies life in
      parse_coerce.c, because it can build that sub-expression by a recursive
      invocation of coerce_to_target_type().  The executor now handles the
      per-element processing as a compiled expression instead of hard-wired code.
      The main advantage of this is that we can use a single ArrayCoerceExpr to
      handle as many as three successive steps per element: base type conversion,
      typmod coercion, and domain constraint checking.  The old code used two
      stacked ArrayCoerceExprs to handle type + typmod coercion, which was pretty
      inefficient, and adding yet another array deconstruction to do domain
      constraint checking seemed very unappetizing.
      
      In the case where we just need a single, very simple coercion function,
      doing this straightforwardly leads to a noticeable increase in the
      per-array-element runtime cost.  Hence, add an additional shortcut evalfunc
      in execExprInterp.c that skips unnecessary overhead for that specific form
      of expression.  The runtime speed of simple cases is within 1% or so of
      where it was before, while cases that previously required two levels of
      array processing are significantly faster.
      
      Finally, create an implicit array type for every domain type, as we do for
      base types, enums, etc.  Everything except the array-coercion case seems
      to just work without further effort.
      
      Tom Lane, reviewed by Andrew Dunstan
      
      Discussion: https://postgr.es/m/9852.1499791473@sss.pgh.pa.us
      c12d570f
    • Andres Freund's avatar
      Fix copy & pasto in 510b8cbf. · 248e3375
      Andres Freund authored
      Reported-By: Peter Geoghegan
      248e3375
    • Andres Freund's avatar
      Fix typo. · f1424123
      Andres Freund authored
      Reported-By: Thomas Munro and Jesper Pedersen
      f1424123
    • Andres Freund's avatar
      Extend & revamp pg_bswap.h infrastructure. · 510b8cbf
      Andres Freund authored
      Upcoming patches are going to address performance issues that involve
      slow system provided ntohs/htons etc. To address that expand
      pg_bswap.h to provide pg_ntoh{16,32,64}, pg_hton{16,32,64} and
      optimize their respective implementations by using compiler intrinsics
      for gcc compatible compilers and msvc. Fall back to manual
      implementations using shifts etc otherwise.
      
      Additionally remove multiple evaluation hazards from the existing
      BSWAP32/64 macros, by replacing them with inline functions when
      necessary. In the course of that the naming scheme is changed to
      pg_bswap16/32/64.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20170927172019.gheidqy6xvlxb325@alap3.anarazel.de
      510b8cbf
  7. 29 Sep, 2017 8 commits
    • Peter Eisentraut's avatar
      Use Py_RETURN_NONE where suitable · 0008a106
      Peter Eisentraut authored
      This is more idiomatic style and available as of Python 2.4, which is
      our minimum.
      0008a106
    • Tom Lane's avatar
      Fix inadequate locking during get_rel_oids(). · 19de0ab2
      Tom Lane authored
      get_rel_oids used to not take any relation locks at all, but that stopped
      being a good idea with commit 3c3bb993, which inserted a syscache lookup
      into the function.  A concurrent DROP TABLE could now produce "cache lookup
      failed", which we don't want to have happen in normal operation.  The best
      solution seems to be to transiently take a lock on the relation named by
      the RangeVar (which also makes the result of RangeVarGetRelid a lot less
      spongy).  But we shouldn't hold the lock beyond this function, because we
      don't want VACUUM to lock more than one table at a time.  (That would not
      be a big problem right now, but it will become one after the pending
      feature patch to allow multiple tables to be named in VACUUM.)
      
      In passing, adjust vacuum_rel and analyze_rel to document that we don't
      trust the passed RangeVar to be accurate, and allow the RangeVar to
      possibly be NULL --- which it is anyway for a whole-database VACUUM,
      though we accidentally didn't crash for that case.
      
      The passed RangeVar is in fact inaccurate when dealing with a child
      partition, as of v10, and it has been wrong for a whole long time in the
      case of vacuum_rel() recursing to a TOAST table.  None of these things
      present visible bugs up to now, because the passed RangeVar is in fact
      only consulted for autovacuum logging, and in that particular context it's
      always accurate because autovacuum doesn't let vacuum.c expand partitions
      nor recurse to toast tables.  Still, this seems like trouble waiting to
      happen, so let's nail the door at least partly shut.  (Further cleanup
      is planned, in HEAD only, as part of the pending feature patch.)
      
      Fix some sadly inaccurate/obsolete comments too.  Back-patch to v10.
      
      Michael Paquier and Tom Lane
      
      Discussion: https://postgr.es/m/25023.1506107590@sss.pgh.pa.us
      19de0ab2
    • Robert Haas's avatar
      psql: Don't try to print a partition constraint we didn't fetch. · 69c16983
      Robert Haas authored
      If \d rather than \d+ is used, then verbose is false and we don't ask
      the server for the partition constraint; so we shouldn't print it in
      that case either.
      
      Maksim Milyutin, per a report from Jesper Pedersen.  Reviewed by
      Jesper Pedersen and Amit Langote.
      
      Discussion: http://postgr.es/m/2af5fc4d-7bcc-daa8-4fe6-86274bea363c@redhat.com
      69c16983
    • Robert Haas's avatar
      pgbench: If we fail to send a command to the server, fail. · e55d9643
      Robert Haas authored
      This beats the old behavior of busy-waiting hands down.
      
      Oversight in commit 12788ae4.
      
      Report by Pavan Deolasee. Patch by Fabien Coelho.  Reviewed by
      Pavan Deolasee.
      
      Discussion: http://postgr.es/m/CABOikdPhfXTypckMC1Ux6Ko+hKBWwUBA=EXsvamXYSg8M9J94w@mail.gmail.com
      e55d9643
    • Peter Eisentraut's avatar
      psql: Update \d sequence display · 2a14b960
      Peter Eisentraut authored
      For \d sequencename, the psql code just did SELECT * FROM sequencename
      to get the information to display, but this does not contain much
      interesting information anymore in PostgreSQL 10, because the metadata
      has been moved to a separate system catalog.
      
      This patch creates a newly designed sequence display that is not merely
      an extension of the general relation/table display as it was previously.
      
      Example:
      
      PostgreSQL 9.6:
      
      => \d foobar
                 Sequence "public.foobar"
          Column     |  Type   |        Value
      ---------------+---------+---------------------
       sequence_name | name    | foobar
       last_value    | bigint  | 1
       start_value   | bigint  | 1
       increment_by  | bigint  | 1
       max_value     | bigint  | 9223372036854775807
       min_value     | bigint  | 1
       cache_value   | bigint  | 1
       log_cnt       | bigint  | 0
       is_cycled     | boolean | f
       is_called     | boolean | f
      
      PostgreSQL 10 before this change:
      
      => \d foobar
         Sequence "public.foobar"
         Column   |  Type   | Value
      ------------+---------+-------
       last_value | bigint  | 1
       log_cnt    | bigint  | 0
       is_called  | boolean | f
      
      New:
      
      => \d foobar
                                 Sequence "public.foobar"
        Type  | Start | Minimum |       Maximum       | Increment | Cycles? | Cache
      --------+-------+---------+---------------------+-----------+---------+-------
       bigint |     1 |       1 | 9223372036854775807 |         1 | no      |     1
      Reviewed-by: default avatarFabien COELHO <coelho@cri.ensmp.fr>
      2a14b960
    • Tom Lane's avatar
      Marginal improvement for generated code in execExprInterp.c. · 136ab7c5
      Tom Lane authored
      Avoid the coding pattern "*op->resvalue = f();", as some compilers think
      that requires them to evaluate "op->resvalue" before the function call.
      Unless there are lots of free registers, this can lead to a useless
      register spill and reload across the call.
      
      I changed all the cases like this in ExecInterpExpr(), but didn't bother
      in the out-of-line opcode eval subroutines, since those are presumably
      not as performance-critical.
      
      Discussion: https://postgr.es/m/2508.1506630094@sss.pgh.pa.us
      136ab7c5
    • Peter Eisentraut's avatar
      Add background worker type · 5373bc2a
      Peter Eisentraut authored
      Add bgw_type field to background worker structure.  It is intended to be
      set to the same value for all workers of the same type, so they can be
      grouped in pg_stat_activity, for example.
      
      The backend_type column in pg_stat_activity now shows bgw_type for a
      background worker.  The ps listing also no longer calls out that a
      process is a background worker but just show the bgw_type.  That way,
      being a background worker is more of an implementation detail now that
      is not shown to the user.  However, most log messages still refer to
      'background worker "%s"'; otherwise constructing sensible and
      translatable log messages would become tricky.
      Reviewed-by: default avatarMichael Paquier <michael.paquier@gmail.com>
      Reviewed-by: default avatarDaniel Gustafsson <daniel@yesql.se>
      5373bc2a
    • Robert Haas's avatar
      Remove replacement selection sort. · 8b304b8b
      Robert Haas authored
      At the time replacement_sort_tuples was introduced, there were still
      cases where replacement selection sort noticeably outperformed using
      quicksort even for the first run.  However, those cases seem to have
      evaporated as a result of further improvements made since that time
      (and perhaps also advances in CPU technology).  So remove replacement
      selection and the controlling GUC entirely.  This makes tuplesort.c
      noticeably simpler and probably paves the way for further
      optimizations someone might want to do later.
      
      Peter Geoghegan, with review and testing by Tomas Vondra and me.
      
      Discussion: https://postgr.es/m/CAH2-WzmmNjG_K0R9nqYwMq3zjyJJK+hCbiZYNGhAy-Zyjs64GQ@mail.gmail.com
      8b304b8b