1. 09 Feb, 2017 6 commits
    • Robert Haas's avatar
      Remove all references to "xlog" from SQL-callable functions in pg_proc. · 806091c9
      Robert Haas authored
      Commit f82ec32a renamed the pg_xlog
      directory to pg_wal.  To make things consistent, and because "xlog" is
      terrible terminology for either "transaction log" or "write-ahead log"
      rename all SQL-callable functions that contain "xlog" in the name to
      instead contain "wal".  (Note that this may pose an upgrade hazard for
      some users.)
      
      Similarly, rename the xlog_position argument of the functions that
      create slots to be called wal_position.
      
      Discussion: https://www.postgresql.org/message-id/CA+Tgmob=YmA=H3DbW1YuOXnFVgBheRmyDkWcD9M8f=5bGWYEoQ@mail.gmail.com
      806091c9
    • Robert Haas's avatar
      simplehash: Additional tweaks to make specifying an allocator work. · 72257f95
      Robert Haas authored
      Even if we don't emit definitions for SH_ALLOCATE and SH_FREE, we
      still need prototypes.  The user can't define them before including
      simplehash.h because SH_TYPE isn't available yet.
      
      For the allocator to be able to access private_data, it needs to
      become an argument to SH_CREATE.  Previously we relied on callers
      to set that after returning from SH_CREATE, but SH_CREATE calls
      SH_ALLOCATE before returning.
      
      Dilip Kumar, reviewed by me.
      72257f95
    • Robert Haas's avatar
      Fix race condition in ConditionVariablePrepareToSleep. · 3f3d60d3
      Robert Haas authored
      Thomas Munro
      3f3d60d3
    • Robert Haas's avatar
      pageinspect: Fix hash_bitmap_info not to read the underlying page. · fc8219dc
      Robert Haas authored
      It did that to verify that the page was an overflow page rather than
      anything else, but that means that checking the status of all the
      overflow bits requires reading the entire index.  So don't do that.
      The new code validates that the page is not a primary bucket page
      or bitmap page by looking at the metapage, so that using this on
      large numbers of pages can be reasonably efficient.
      
      Ashutosh Sharma, per a complaint from me, and with further
      modifications by me.
      fc8219dc
    • Tom Lane's avatar
      Allow index AMs to cache data across aminsert calls within a SQL command. · 86d911ec
      Tom Lane authored
      It's always been possible for index AMs to cache data across successive
      amgettuple calls within a single SQL command: the IndexScanDesc.opaque
      field is meant for precisely that.  However, no comparable facility
      exists for amortizing setup work across successive aminsert calls.
      This patch adds such a feature and teaches GIN, GIST, and BRIN to use it
      to amortize catalog lookups they'd previously been doing on every call.
      (The other standard index AMs keep everything they need in the relcache,
      so there's little to improve there.)
      
      For GIN, the overall improvement in a statement that inserts many rows
      can be as much as 10%, though it seems a bit less for the other two.
      In addition, this makes a really significant difference in runtime
      for CLOBBER_CACHE_ALWAYS tests, since in those builds the repeated
      catalog lookups are vastly more expensive.
      
      The reason this has been hard up to now is that the aminsert function is
      not passed any useful place to cache per-statement data.  What I chose to
      do is to add suitable fields to struct IndexInfo and pass that to aminsert.
      That's not widening the index AM API very much because IndexInfo is already
      within the ken of ambuild; in fact, by passing the same info to aminsert
      as to ambuild, this is really removing an inconsistency in the AM API.
      
      Discussion: https://postgr.es/m/27568.1486508680@sss.pgh.pa.us
      86d911ec
    • Andres Freund's avatar
      Add explicit ORDER BY to a few tests that exercise hash-join code. · 7c5d8c16
      Andres Freund authored
      A proposed patch, also by Thomas and in the same thread, would change
      the output order of these.  Independent of the follow-up patches
      getting committed, nailing down the order in these specific tests at
      worst seems harmless.
      
      Author: Thomas Munro
      Discussion: https://postgr.es/m/CAEepm=1D4-tP7j7UAgT_j4ZX2j4Ehe1qgZQWFKBMb8F76UW5Rg@mail.gmail.com
      7c5d8c16
  2. 08 Feb, 2017 4 commits
    • Tom Lane's avatar
      Fix roundoff problems in float8_timestamptz() and make_interval(). · 8f93bd85
      Tom Lane authored
      When converting a float value to integer microseconds, we should be careful
      to round the value to the nearest integer, typically with rint(); simply
      assigning to an int64 variable will truncate, causing apparently off-by-one
      values in cases that should work.  Most places in the datetime code got
      this right, but not these two.
      
      float8_timestamptz() is new as of commit e511d878 (9.6).  Previous
      versions effectively depended on interval_mul() to do roundoff correctly,
      which it does, so this fixes an accuracy regression in 9.6.
      
      The problem in make_interval() dates to its introduction in 9.4.  Aside
      from being careful to round not truncate, let's incorporate the hours and
      minutes inputs into the result with exact integer arithmetic, rather than
      risk introducing roundoff error where there need not have been any.
      
      float8_timestamptz() problem reported by Erik Nordström, though this is
      not his proposed patch.  make_interval() problem found by me.
      
      Discussion: https://postgr.es/m/CAHuQZDS76jTYk3LydPbKpNfw9KbACmD=49dC4BrzHcfPv6yA1A@mail.gmail.com
      8f93bd85
    • Robert Haas's avatar
      Add WAL consistency checking facility. · a507b869
      Robert Haas authored
      When the new GUC wal_consistency_checking is set to a non-empty value,
      it triggers recording of additional full-page images, which are
      compared on the standby against the results of applying the WAL record
      (without regard to those full-page images).  Allowable differences
      such as hints are masked out, and the resulting pages are compared;
      any difference results in a FATAL error on the standby.
      
      Kuntal Ghosh, based on earlier patches by Michael Paquier and Heikki
      Linnakangas.  Extensively reviewed and revised by Michael Paquier and
      by me, with additional reviews and comments from Amit Kapila, Álvaro
      Herrera, Simon Riggs, and Peter Eisentraut.
      a507b869
    • Peter Eisentraut's avatar
    • Peter Eisentraut's avatar
      doc: Some improvements in CREATE SUBSCRIPTION ref page · e35bbea7
      Peter Eisentraut authored
      Add link to description of libpq connection strings.  Add link to
      explanation of replication access control.  This currently points to the
      description of streaming replication access control, which is currently
      the same as for logical replication, but that might be refined later.
      Also remove plain-text passwords from the examples, to not encourage
      that dubious practice.
      
      based on suggestions from Simon Riggs
      e35bbea7
  3. 07 Feb, 2017 7 commits
    • Robert Haas's avatar
      Revise the way the element allocator for a simplehash is specified. · c3c4f6e1
      Robert Haas authored
      This method is more elegant and more efficient.
      
      Per a suggestion from Andres Freund, who also briefly reviewed
      the patch.
      c3c4f6e1
    • Tom Lane's avatar
      Speed up "brin" regression test a little bit. · 242066cc
      Tom Lane authored
      In the large DO block, collect row TIDs into array variables instead of
      creating and dropping a pile of temporary tables.  In a normal build,
      this reduces the brin test script's runtime from about 1.1 sec to 0.4 sec
      on my workstation.  That's not all that exciting perhaps, but in a
      CLOBBER_CACHE_ALWAYS test build, the runtime drops from 20 min to 17 min,
      which is a little more useful.  In combination with some other changes
      I plan to propose, this will help provide a noticeable reduction in
      cycle time for CLOBBER_CACHE_ALWAYS buildfarm critters.
      242066cc
    • Robert Haas's avatar
      Avoid redefining simplehash_allocate/simplehash_free. · ac8eb972
      Robert Haas authored
      There's no generic guard against multiple inclusion in this file,
      for good reason.  But these typedefs need one, as per a report
      from Jeff Janes.
      ac8eb972
    • Robert Haas's avatar
      Allow the element allocator for a simplehash to be specified. · 565903af
      Robert Haas authored
      This is infrastructure for a pending patch to allow parallel bitmap
      heap scans.
      
      Dilip Kumar, reviewed (in earlier versions) by Andres Freund and
      (more recently) by me.  Some further renaming by me, also.
      565903af
    • Robert Haas's avatar
      Fix compiler warning. · 94708c0e
      Robert Haas authored
      Mithun Cy, per a report by Erik Rijkers
      94708c0e
    • Robert Haas's avatar
      Cache hash index's metapage in rel->rd_amcache. · 293e24e5
      Robert Haas authored
      This avoids a very significant amount of buffer manager traffic and
      contention when scanning hash indexes, because it's no longer
      necessary to lock and pin the metapage for every scan.  We do need
      some way of figuring out when the cache is too stale to use any more,
      so that when we lock the primary bucket page to which the cached
      metapage points us, we can tell whether a split has occurred since we
      cached the metapage data.  To do that, we use the hash_prevblkno field
      in the primary bucket page, which would otherwise always be set to
      InvalidBuffer.
      
      This patch contains code so that it will continue working (although
      less efficiently) with hash indexes built before this change, but
      perhaps we should consider bumping the hash version and ripping out
      the compatibility code.  That decision can be made later, though.
      
      Mithun Cy, reviewed by Jesper Pedersen, Amit Kapila, and by me.
      Before committing, I made a number of cosmetic changes to the last
      posted version of the patch, adjusted _hash_getcachedmetap to be more
      careful about order of operation, and made some necessary updates to
      the pageinspect documentation and regression tests.
      293e24e5
    • Tom Lane's avatar
      Correct thinko in last-minute release note item. · 39c3ca51
      Tom Lane authored
      The CREATE INDEX CONCURRENTLY bug can only be triggered by row updates,
      not inserts, since the problem would arise from an update incorrectly
      being made HOT.  Noted by Alvaro.
      39c3ca51
  4. 06 Feb, 2017 11 commits
    • Tom Lane's avatar
      64ee636a
    • Peter Eisentraut's avatar
      doc: Document sequence function privileges better · 696af9ab
      Peter Eisentraut authored
      Document the privileges required for each of the sequence functions.
      This was already in the GRANT reference page, but also add it to the
      function description for easier reference.
      696af9ab
    • Peter Eisentraut's avatar
      Avoid permission failure in pg_sequences.last_value · ab82340a
      Peter Eisentraut authored
      Before, reading pg_sequences.last_value would fail unless the user had
      appropriate sequence permissions, which would make the pg_sequences view
      cumbersome to use.  Instead, return null instead of the real value when
      there are no permissions.
      
      From: Michael Paquier <michael.paquier@gmail.com>
      Reported-by: default avatarShinoda, Noriyoshi <noriyoshi.shinoda@hpe.com>
      ab82340a
    • Tom Lane's avatar
      Release note updates. · ad6af3fc
      Tom Lane authored
      Add item for last-minute CREATE INDEX CONCURRENTLY fix.
      Repair a couple of misspellings of patch authors' names.
      
      Back-branch updates will follow shortly, but I thought I'd
      commit this separately just to make it more visible.
      ad6af3fc
    • Tom Lane's avatar
      Avoid returning stale attribute bitmaps in RelationGetIndexAttrBitmap(). · 2aaec654
      Tom Lane authored
      The problem with the original coding here is that we might receive (and
      clear) a relcache invalidation signal for the target relation down inside
      one of the index_open calls we're doing.  Since the target is open, we
      would not drop the relcache entry, just reset its rd_indexvalid and
      rd_indexlist fields.  But RelationGetIndexAttrBitmap() kept going, and
      would eventually cache and return potentially-obsolete attribute bitmaps.
      
      The case where this matters is where the inval signal was from a CREATE
      INDEX CONCURRENTLY telling us about a new index on a formerly-unindexed
      column.  (In all other cases, the lock we hold on the target rel should
      prevent any concurrent change in index state.)  Even just returning the
      stale attribute bitmap is not such a problem, because it shouldn't matter
      during the transaction in which we receive the signal.  What hurts is
      caching the stale data, because it can survive into later transactions,
      breaking CREATE INDEX CONCURRENTLY's expectation that later transactions
      will not create new broken HOT chains.  The upshot is that there's a window
      for building corrupted indexes during CREATE INDEX CONCURRENTLY.
      
      This patch fixes the problem by rechecking that the set of index OIDs
      is still the same at the end of RelationGetIndexAttrBitmap() as it was
      at the start.  If not, we loop back and try again.  That's a little
      more than is strictly necessary to fix the bug --- in principle, we
      could return the stale data but not cache it --- but it seems like a
      bad idea on general principles for relcache to return data it knows
      is stale.
      
      There might be more hazards of the same ilk, or there might be a better
      way to fix this one, but this patch definitely improves matters and seems
      unlikely to make anything worse.  So let's push it into today's releases
      even as we continue to study the problem.
      
      Pavan Deolasee and myself
      
      Discussion: https://postgr.es/m/CABOikdM2MUq9cyZJi1KyLmmkCereyGp5JQ4fuwKoyKEde_mzkQ@mail.gmail.com
      2aaec654
    • Peter Eisentraut's avatar
      doc: Update CREATE DATABASE examples · 549f7473
      Peter Eisentraut authored
      The example of using CREATE DATABASE with the ENCODING option did not
      work anymore (except in special circumstances) and did not represent a
      good general-purpose example, so write some new examples.
      
      Reported-by: marc+pgsql@milestonerdl.com
      549f7473
    • Tom Lane's avatar
      Update comment in relcache.c. · a5931834
      Tom Lane authored
      Commit 665d1fad introduced rd_pkindex, and made RelationGetIndexList
      responsible for updating it, but didn't bother to fix
      RelationGetIndexList's header comment to say so.
      a5931834
    • Peter Eisentraut's avatar
      Add missing newline to error messages · afcb0c97
      Peter Eisentraut authored
      Also improve the message style a bit while we're here.
      afcb0c97
    • Heikki Linnakangas's avatar
      Fix typo also in expected output. · d93b7535
      Heikki Linnakangas authored
      Commit 181bdb90 fixed the typo in the .sql file, but forgot to update the
      expected output.
      d93b7535
    • Heikki Linnakangas's avatar
      Fix typo in variable name. · d02d9853
      Heikki Linnakangas authored
      Masahiko Sawada
      d02d9853
    • Heikki Linnakangas's avatar
      Fix typos in comments. · 181bdb90
      Heikki Linnakangas authored
      Backpatch to all supported versions, where applicable, to make backpatching
      of future fixes go more smoothly.
      
      Josh Soref
      
      Discussion: https://www.postgresql.org/message-id/CACZqfqCf+5qRztLPgmmosr-B0Ye4srWzzw_mo4c_8_B_mtjmJQ@mail.gmail.com
      181bdb90
  5. 04 Feb, 2017 2 commits
  6. 03 Feb, 2017 10 commits
    • Robert Haas's avatar
      Improve grammar of message about two-phase state files. · 38c363ad
      Robert Haas authored
      When there's only one two-phase state file, there's also only one
      long-running prepared transaction.  Adjust the message text
      accordingly.
      
      Nikhil Sontakke
      
      Discussion: http://postgr.es/m/CAMGcDxcmR_DWZXXndGoPzVQx=B17A5=RviEA1qNaF=FWLy5Whw@mail.gmail.com
      38c363ad
    • Robert Haas's avatar
      pageinspect: More type-sanity surgery on the new hash index code. · 871ec0e3
      Robert Haas authored
      Uniformly expose unsigned quantities using the next-wider signed
      integer type (since we have no unsigned types at the SQL level).
      At the SQL level, this results a change to report itemoffset as
      int4 rather than int2.  Also at the SQL level, report one value
      that is an OID as type oid.  Under the hood, uniformly use macros
      that match the SQL output type as to both width and signedness.
      871ec0e3
    • Robert Haas's avatar
      pgstattuple: Add pgstathashindex. · e759854a
      Robert Haas authored
      Since pgstattuple v1.5 hasn't been released yet, no need for a new
      extension version.  The new function exposes statistics about hash
      indexes similar to what other pgstatindex functions return for other
      index types.
      
      Ashutosh Sharma, reviewed by Kuntal Ghosh.  Substantial further
      revisions by me.
      e759854a
    • Fujii Masao's avatar
      Be sure to release LogicalRepLauncherLock in DROP SUBSCRIPTION command. · 39b8cc99
      Fujii Masao authored
      Previously DROP SUBSCRIPTION command forgot to release the lock at all.
      
      Original patches by Kyotaro Horiguchi and Michael Paquier,
      but I didn't use them.
      Discussion: http://postgr.es/m/20170201.173623.66249355.horiguchi.kyotaro@lab.ntt.co.jp
      39b8cc99
    • Tom Lane's avatar
      In pageinspect/hashfuncs.c, avoid crashes on alignment-picky machines. · 14e9b18f
      Tom Lane authored
      On machines with MAXALIGN = 8, the payload of a bytea is not maxaligned,
      since it will start 4 bytes into a palloc'd value.  On alignment-picky
      hardware, this will cause failures in accesses to 8-byte-wide values
      within the page.  We already encountered this problem when we introduced
      GIN index inspection functions, and fixed it in commit 84ad68d6.  Make
      use of the same function for hash indexes.
      
      A small difficulty is that up to now contrib/pageinspect has not shared
      any functions at all across files.  To support that, introduce a common
      header file "pageinspect.h" for the module.
      
      Also, move get_page_from_raw() out of ginfuncs.c, where it didn't
      especially belong, and put it in rawpage.c which seems a more natural home.
      
      Per buildfarm.
      
      Discussion: https://postgr.es/m/17311.1486134714@sss.pgh.pa.us
      14e9b18f
    • Robert Haas's avatar
      pageinspect: Remove platform-dependent values from hash tests. · 29e312bc
      Robert Haas authored
      Per a report from Tom Lane, the ffactor reported by hash_metapage_info
      and the free_size reported by hash_page_stats vary by platform.
      
      Ashutosh Sharma and Robert Haas
      29e312bc
    • Tom Lane's avatar
      Fix a bunch more portability bugs in commit 08bf6e52. · c6eeb67d
      Tom Lane authored
      It seems like somebody used a dartboard while choosing integer widths
      for the various values taken and returned by these functions ... and
      then threw a fresh set of darts while writing the SQL declarations.
      
      This patch brings the C code into line with what the SQL declarations
      say, which is enough to make it not dump core on the particular 32-bit
      machine I'm testing on.  But I think we could do with another round
      of looking at what the datum widths *should* be.  For instance, it's
      not all that sensible that hash_bitmap_info decided to use int64 to
      represent a BlockNumber input when get_raw_page doesn't do it that way.
      
      There's also a remaining problem that the expected outputs from the
      test script are platform-dependent, but I'll leave that issue for
      somebody else.
      
      Per buildfarm.
      c6eeb67d
    • Robert Haas's avatar
      pageinspect: Try to fix some bugs in previous commit. · ed807fda
      Robert Haas authored
      Commit 08bf6e52 seems not to have
      used the correct *GetDatum and PG_GETARG_* macros for the SQL types
      in some cases, and some of the SQL types seem to have been poorly
      chosen, too.  Try to fix it.  I'm not sure if this is the reason
      why the buildfarm is currently unhappy with this code, but it
      seems like a good place to start.
      
      Buildfarm unhappiness reported by Tom Lane.
      ed807fda
    • Tom Lane's avatar
      Clean up psql's behavior for a few more control variables. · fd6cd698
      Tom Lane authored
      Modify FETCH_COUNT to always have a defined value, like other control
      variables, mainly so it will always appear in "\set" output.
      
      Add hooks to force HISTSIZE to be defined and require it to have an
      integer value.  (I don't see any point in allowing it to be set to
      non-integral values.)
      
      Add hooks to force IGNOREEOF to be defined and require it to have an
      integer value.  Unlike the other cases, here we're trying to be
      bug-compatible with a rather bogus externally-defined behavior, so I think
      we need to continue to allow "\set IGNOREEOF whatever".  Fix it so that
      the substitution hook silently replace non-numeric values with "10",
      so that the stored value always reflects what we're really doing.
      
      Add a dummy assign hook for HISTFILE, just so it's always in
      variables.c's list.  We can't require it to be defined always, because
      that would break the interaction with the PSQL_HISTORY environment
      variable, so there isn't any change in visible behavior here.
      
      Remove tab-complete.c's private list of known variable names, since that's
      really a maintenance nuisance.  Given the preceding changes, there are no
      control variables it won't show anyway.  This does mean that if for some
      reason you've unset one of the status variables (DBNAME, HOST, etc), that
      variable would not appear in tab completion for \set.  But I think that's
      fine, for at least two reasons: we shouldn't be encouraging people to use
      those variables as regular variables, and if someone does do so anyway,
      why shouldn't it act just like a regular variable?
      
      Remove ugly and no-longer-used-anywhere GetVariableNum().  In general,
      future additions of integer-valued control variables should follow the
      paradigm of adding an assign hook using ParseVariableNum(), so there's
      no reason to expect we'd need this again later.
      
      Discussion: https://postgr.es/m/17516.1485973973@sss.pgh.pa.us
      fd6cd698
    • Tom Lane's avatar
      Avoid improbable null pointer dereference in pgpassfileWarning(). · 8ac0365c
      Tom Lane authored
      Coverity complained that we might pass a null pointer to strcmp()
      if PQresultErrorField were to return NULL.  That shouldn't be possible,
      since the server is supposed to always provide some SQLSTATE or other
      in an error message.  But we usually defend against such hazards, and
      it only takes a little more code to do so here.
      
      There's no good reason to think this is a live bug, so no back-patch.
      8ac0365c