1. 16 Oct, 2018 1 commit
    • Andres Freund's avatar
      Move TupleTableSlots boolean member into one flag variable. · c5257345
      Andres Freund authored
      There's several reasons for this change:
      1) It reduces the total size of TupleTableSlot / reduces alignment
         padding, making the commonly accessed members fit into a single
         cacheline (but we currently do not force proper alignment, so
         that's not yet guaranteed to be helpful)
      2) Combining the booleans into a flag allows to combine read/writes
         from memory.
      3) With the upcoming slot abstraction changes, it allows to have core
         and extended flags, in a memory efficient way.
      
      Author: Ashutosh Bapat and Andres Freund
      Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
      c5257345
  2. 15 Oct, 2018 6 commits
    • Andres Freund's avatar
      Move generic slot support functions from heaptuple.c into execTuples.c. · 9d906f11
      Andres Freund authored
      heaptuple.c was never a particular good fit for slot_getattr(),
      slot_getsomeattrs() and slot_getmissingattrs(), but in upcoming
      changes slots will be made more abstract (allowing slots that contain
      different types of tuples), making it clearly the wrong place.
      
      Note that slot_deform_tuple() remains in it's current place, as it
      clearly deals with a HeapTuple.  getmissingattrs() also remains, but
      it's less clear that that's correct - but execTuples.c wouldn't be the
      right place.
      
      Author: Ashutosh Bapat.
      Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
      9d906f11
    • Thomas Munro's avatar
      Move the replication lag tracker into heap memory. · e73ca79f
      Thomas Munro authored
      Andres Freund complained about the 128KB of .bss occupied by LagTracker.
      It's only needed in the walsender process, so allocate it in heap
      memory there.
      
      Author: Thomas Munro
      Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya%40alap3.anarazel.de
      e73ca79f
    • Tom Lane's avatar
      Check for stack overrun in standard_ProcessUtility(). · d48da369
      Tom Lane authored
      ProcessUtility can recurse, and indeed can be driven to infinite
      recursion, so it ought to have a check_stack_depth() call.  This
      covers the reported bug (portal trying to execute itself) and a bunch
      of other cases that could perhaps arise somewhere.
      
      Per bug #15428 from Malthe Borch.  Back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/15428-b3c2915ec470b033@postgresql.org
      d48da369
    • Peter Eisentraut's avatar
      pgbench: Report errors during run better · 5b75a4f8
      Peter Eisentraut authored
      When an error occurs during a benchmark run, exit with a nonzero exit
      code and write a message at the end.  Previously, it would just print
      the error message when it happened but then proceed to print the run
      summary normally and exit with status 0.  To still allow
      distinguishing setup from run-time errors, we use exit status 2 for
      the new state, whereas existing errors during pgbench initialization
      use exit status 1.
      Reviewed-by: default avatarFabien COELHO <coelho@cri.ensmp.fr>
      5b75a4f8
    • Peter Eisentraut's avatar
      Make spelling of "acknowledgment" consistent · 35584fd0
      Peter Eisentraut authored
      I used the preferred U.S. spelling, as we do in other cases.
      35584fd0
    • Peter Eisentraut's avatar
      Fixes for "Glyph not available" warnings from FOP · 9274c577
      Peter Eisentraut authored
      With the PostgreSQL 11 release notes acknowledgments list, FOP reported
      
      WARNING: Glyph "?" (0x144, nacute) not available in font "Times-Roman".
      WARNING: Glyph "?" (0x15e, Scedilla) not available in font "Times-Roman".
      WARNING: Glyph "?" (0x15f, scedilla) not available in font "Times-Roman".
      WARNING: Glyph "?" (0x131, dotlessi) not available in font "Times-Roman".
      
      This is because we have some new contributors whose names use letters
      that we haven't used before, and apparently FOP can't handle them out
      of the box.
      
      For now, just fix this by "unaccenting" those names.  In the future,
      maybe this can be fixed better with a different font configuration.
      
      There is also another warning
      
      WARNING: Glyph "?" (0x3c0, pi) not available in font "Times-Roman".
      
      but that existed in previous releases and is not touched here.
      9274c577
  3. 14 Oct, 2018 7 commits
    • Alexander Korotkov's avatar
      Add missed tag in bloom.sgml · 981b64f8
      Alexander Korotkov authored
      Backpatch commits don't contain this error.
      981b64f8
    • Alexander Korotkov's avatar
      contrib/bloom documentation improvement · 98afb839
      Alexander Korotkov authored
      This commit documents rounding of "length" parameter and absence of support
      for unique indexes and NULLs searching.  Backpatch to 9.6 where contrib/bloom
      was introduced.
      
      Discussion: https://postgr.es/m/CAF4Au4wPQQ7EHVSnzcLjsbY3oLSzVk6UemZLD1Sbmwysy3R61g%40mail.gmail.com
      Author: Oleg Bartunov with minor editorialization by me
      Backpatch-through: 9.6
      98afb839
    • Tom Lane's avatar
      Make some subquery-using test cases a bit more robust. · b403ea43
      Tom Lane authored
      These test cases could be adversely affected by an upcoming change
      to allow pullup of FROM-less subqueries.  Tweak them to ensure that
      they'll continue to test what they did before.
      
      Discussion: https://postgr.es/m/5395.1539275668@sss.pgh.pa.us
      b403ea43
    • Tom Lane's avatar
      Use PlaceHolderVars within the quals of a FULL JOIN. · 7d4a10e2
      Tom Lane authored
      This prevents failures in cases where we pull up a constant or var-free
      expression from a subquery and put it into a full join's qual.  That can
      result in not recognizing the qual as containing a mergejoin-able or
      hashjoin-able condition.  A PHV prevents the problem because it is still
      recognized as belonging to the side of the join the subquery is in.
      
      I'm not very sure about the net effect of this change on plan quality.
      In "typical" cases where the join keys are Vars, nothing changes.
      In an affected case, the PHV-wrapped expression is less likely to be seen
      as equal to PHV-less instances below the join, but more likely to be seen
      as equal to similar expressions above the join, so it may end up being a
      wash.  In the one existing case where there's any visible change in a
      regression-test plan, it amounts to referencing a lower computation of a
      COALESCE result instead of recomputing it, which seems like a win.
      
      Given my uncertainty about that and the lack of field complaints,
      no back-patch, even though this is a very ancient problem.
      
      Discussion: https://postgr.es/m/32090.1539378124@sss.pgh.pa.us
      7d4a10e2
    • Tom Lane's avatar
      Clean up/tighten up coercibility checks in opr_sanity regression test. · e9f42d52
      Tom Lane authored
      With the removal of the old abstime type, there are no longer any cases
      in this test where we need to use the weaker castcontext-ignoring form
      of binary coercibility check.  (The other major source of such headaches,
      apparently-incompatible hash functions, is now hashvalidate()'s problem
      not this test script's problem.)  Hence, just use binary_coercible()
      everywhere, and remove the comments explaining why we don't do so ---
      which were broken anyway by cda6a8d0.
      
      I left physically_coercible() in place but renamed it to better
      match what it's actually testing, and added some comments.
      
      Also, in test queries that have an assumption about the maximum number
      of function arguments they need to handle, add a clause to make them fail
      if someday there's a relevant function with more arguments.  Otherwise
      we're likely not to notice that we need to extend the queries.
      
      Discussion: https://postgr.es/m/27637.1539388060@sss.pgh.pa.us
      e9f42d52
    • Michael Paquier's avatar
      Avoid duplicate XIDs at recovery when building initial snapshot · 1df21ddb
      Michael Paquier authored
      On a primary, sets of XLOG_RUNNING_XACTS records are generated on a
      periodic basis to allow recovery to build the initial state of
      transactions for a hot standby.  The set of transaction IDs is created
      by scanning all the entries in ProcArray.  However it happens that its
      logic never counted on the fact that two-phase transactions finishing to
      prepare can put ProcArray in a state where there are two entries with
      the same transaction ID, one for the initial transaction which gets
      cleared when prepare finishes, and a second, dummy, entry to track that
      the transaction is still running after prepare finishes.  This way
      ensures a continuous presence of the transaction so as callers of for
      example TransactionIdIsInProgress() are always able to see it as alive.
      
      So, if a XLOG_RUNNING_XACTS takes a standby snapshot while a two-phase
      transaction finishes to prepare, the record can finish with duplicated
      XIDs, which is a state expected by design.  If this record gets applied
      on a standby to initial its recovery state, then it would simply fail,
      so the odds of facing this failure are very low in practice.  It would
      be tempting to change the generation of XLOG_RUNNING_XACTS so as
      duplicates are removed on the source, but this requires to hold on
      ProcArrayLock for longer and this would impact all workloads,
      particularly those using heavily two-phase transactions.
      
      XLOG_RUNNING_XACTS is also actually used only to initialize the standby
      state at recovery, so instead the solution is taken to discard
      duplicates when applying the initial snapshot.
      
      Diagnosed-by: Konstantin Knizhnik
      Author: Michael Paquier
      Discussion: https://postgr.es/m/0c96b653-4696-d4b4-6b5d-78143175d113@postgrespro.ru
      Backpatch-through: 9.3
      1df21ddb
    • Tom Lane's avatar
      8f850f2c
  4. 13 Oct, 2018 3 commits
  5. 12 Oct, 2018 6 commits
    • Tom Lane's avatar
      Another round of portability hacking on ECPG regression tests. · 240cd6bc
      Tom Lane authored
      Removing the separate Windows expected-files in commit f1885386
      turns out to have been too optimistic: on most (but not all!) of our
      Windows buildfarm members, the tests still print floats with three
      exponent digits, because they're invoking the native printf()
      not snprintf.c.
      
      But rather than put back the extra expected-files, let's hack
      the three tests in question so that they adjust float formatting
      the same way snprintf.c does.
      
      Discussion: https://postgr.es/m/18890.1539374107@sss.pgh.pa.us
      240cd6bc
    • Tom Lane's avatar
      Simplify use of AllocSetContextCreate() wrapper macro. · 13cd7209
      Tom Lane authored
      We can allow this macro to accept either abbreviated or non-abbreviated
      allocation parameters by making use of __VA_ARGS__.  As noted by Andres
      Freund, it's unlikely that any compiler would have __builtin_constant_p
      but not __VA_ARGS__, so this gives up little or no error checking, and
      it avoids a minor but annoying API break for extensions.
      
      With this change, there is no reason for anybody to call
      AllocSetContextCreateExtended directly, so in HEAD I renamed it to
      AllocSetContextCreateInternal.  It's probably too late for an ABI
      break like that in 11, though.
      
      Discussion: https://postgr.es/m/20181012170355.bhxi273skjt6sag4@alap3.anarazel.de
      13cd7209
    • Tom Lane's avatar
      Remove dead reference to ecpg resultmap file. · 24a2c436
      Tom Lane authored
      I missed this in my prior commit because it doesn't matter in non-VPATH
      builds.
      
      Per buildfarm.
      24a2c436
    • Alvaro Herrera's avatar
      Correct attach/detach logic for FKs in partitions · c7d43c4d
      Alvaro Herrera authored
      There was no code to handle foreign key constraints on partitioned
      tables in the case of ALTER TABLE DETACH; and if you happened to ATTACH
      a partition that already had an equivalent constraint, that one was
      ignored and a new constraint was created.  Adding this to the fact that
      foreign key cloning reuses the constraint name on the partition instead
      of generating a new name (as it probably should, to cater to SQL
      standard rules about constraint naming within schemas), the result was a
      pretty poor user experience -- the most visible failure was that just
      detaching a partition and re-attaching it failed with an error such as
      
        ERROR:  duplicate key value violates unique constraint "pg_constraint_conrelid_contypid_conname_index"
        DETAIL:  Key (conrelid, contypid, conname)=(26702, 0, test_result_asset_id_fkey) already exists.
      
      because it would try to create an identically-named constraint in the
      partition.  To make matters worse, if you tried to drop the constraint
      in the now-independent partition, that would fail because the constraint
      was still seen as dependent on the constraint in its former parent
      partitioned table:
        ERROR:  cannot drop inherited constraint "test_result_asset_id_fkey" of relation "test_result_cbsystem_0001_0050_monthly_2018_09"
      
      This fix attacks the problem from two angles: first, when the partition
      is detached, the constraint is also marked as independent, so the drop
      now works.  Second, when the partition is re-attached, we scan existing
      constraints searching for one matching the FK in the parent, and if one
      exists, we link that one to the parent constraint.  So we don't end up
      with a duplicate -- and better yet, we don't need to scan the referenced
      table to verify that the constraint holds.
      
      To implement this I made a small change to previously planner-only
      struct ForeignKeyCacheInfo to contain the constraint OID; also relcache
      now maintains the list of FKs for partitioned tables too.
      
      Backpatch to 11.
      
      Reported-by: Michael Vitale (bug #15425)
      Discussion: https://postgr.es/m/15425-2dbc9d2aa999f816@postgresql.org
      c7d43c4d
    • Tom Lane's avatar
      Make float exponent output on Windows look the same as elsewhere. · f1885386
      Tom Lane authored
      Windows, alone among our supported platforms, likes to emit three-digit
      exponent fields even when two digits would do.  Adjust such results to
      look like the way everyone else does it.  Eliminate a bunch of variant
      expected-output files that were needed only because of this quirk.
      
      Discussion: https://postgr.es/m/2934.1539122454@sss.pgh.pa.us
      f1885386
    • Michael Paquier's avatar
      Add TAP tests for pg_verify_checksums · b34e84f1
      Michael Paquier authored
      All options available in the utility get coverage:
      - Tests with disabled page checksums.
      - Tests with enabled test checksums.
      - Emulation of corruption and broken checksums with a full scan and
      single relfilenode scan.
      
      This patch has been contributed mainly by Michael Banck and Magnus
      Hagander with things presented on various threads, and I have gathered
      all the contents into a single patch.
      
      Author: Michael Banck, Magnus Hagander, Michael Paquier
      Reviewed-by: Peter Eisentraut
      Discussion: https://postgr.es/m/20181005012645.GE1629@paquier.xyz
      b34e84f1
  6. 11 Oct, 2018 3 commits
  7. 10 Oct, 2018 4 commits
    • Andres Freund's avatar
      Fix logical decoding error when system table w/ toast is repeatedly rewritten. · e9edc1ba
      Andres Freund authored
      Repeatedly rewriting a mapped catalog table with VACUUM FULL or
      CLUSTER could cause logical decoding to fail with:
      ERROR, "could not map filenode \"%s\" to relation OID"
      
      To trigger the problem the rewritten catalog had to have live tuples
      with toasted columns.
      
      The problem was triggered as during catalog table rewrites the
      heap_insert() check that prevents logical decoding information to be
      emitted for system catalogs, failed to treat the new heap's toast table
      as a system catalog (because the new heap is not recognized as a
      catalog table via RelationIsLogicallyLogged()). The relmapper, in
      contrast to the normal catalog contents, does not contain historical
      information. After a single rewrite of a mapped table the new relation
      is known to the relmapper, but if the table is rewritten twice before
      logical decoding occurs, the relfilenode cannot be mapped to a
      relation anymore.  Which then leads us to error out.   This only
      happens for toast tables, because the main table contents aren't
      re-inserted with heap_insert().
      
      The fix is simple, add a new heap_insert() flag that prevents logical
      decoding information from being emitted, and accept during decoding
      that there might not be tuple data for toast tables.
      
      Unfortunately that does not fix pre-existing logical decoding
      errors. Doing so would require not throwing an error when a filenode
      cannot be mapped to a relation during decoding, and that seems too
      likely to hide bugs.  If it's crucial to fix decoding for an existing
      slot, temporarily changing the ERROR in ReorderBufferCommit() to a
      WARNING appears to be the best fix.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20180914021046.oi7dm4ra3ot2g2kt@alap3.anarazel.de
      Backpatch: 9.4-, where logical decoding was introduced
      e9edc1ba
    • Andres Freund's avatar
      Force synchronous commit to be enabled for all test_decoding tests. · ef493055
      Andres Freund authored
      Without that the tests fail when forced to be run against a cluster
      with synchronous_commit = off (as the WAL might not yet be flushed to
      disk by the point logical decoding gets called, and thus the expected
      output breaks). Most tests already do that, add it to a few newer tests.
      
      Author: Andres Freund
      ef493055
    • Peter Eisentraut's avatar
      Slightly correct context check for event triggers · f82d4d66
      Peter Eisentraut authored
      The previous check for a "complete query" omitted the new
      PROCESS_UTILITY_QUERY_NONATOMIC value.  This didn't actually make a
      difference in practice, because only CALL and SET from PL/pgSQL run in
      this state, but it's more correct to include it anyway.
      
      Discussion: https://www.postgresql.org/message-id/4566041d-2567-74d2-d135-19ff6a20fe51%402ndquadrant.com
      f82d4d66
    • Peter Eisentraut's avatar
      Test that event triggers work in functions and procedures · ae307861
      Peter Eisentraut authored
      This ensures that we have coverage of all the ProcessUtilityContext
      variants.
      ae307861
  8. 09 Oct, 2018 7 commits
    • Peter Eisentraut's avatar
      Turn transaction_isolation into GUC enum · f8c10f61
      Peter Eisentraut authored
      It was previously a string setting that was converted into an enum by
      custom code, but using the GUC enum facility seems much simpler and
      doesn't change any functionality, except that
      
          set transaction_isolation='default';
      
      no longer works, but that was never documented and doesn't work with
      any other transaction characteristics.  (Note that this is not the
      same as RESET or SET TO DEFAULT, which still work.)
      Reviewed-by: default avatarHeikki Linnakangas <hlinnaka@iki.fi>
      Discussion: https://www.postgresql.org/message-id/457db615-e84c-4838-310e-43841eb806e5@iki.fi
      f8c10f61
    • Tom Lane's avatar
      Make src/common/exec.c's error logging less ugly. · b6b297d2
      Tom Lane authored
      This code used elog where it really ought to use ereport, mainly so that
      it can report a SQLSTATE different from ERRCODE_INTERNAL_ERROR.  There
      were some other random deviations from typical error report practice too.
      
      In addition, we can make some cleanups that were impractical six months
      ago:
      
      * Use one variadic macro, instead of several with different numbers
      of arguments, reducing the temptation to force-fit messages into
      particular numbers of arguments;
      
      * Use %m, even in the frontend case, simplifying the code.
      
      Discussion: https://postgr.es/m/6025.1527351693@sss.pgh.pa.us
      b6b297d2
    • Greg Stark's avatar
      Add "B" suffix for bytes to docs · 36e9d413
      Greg Stark authored
      6e7baa32 and b06d8e58 added "B" as a valid suffix for
      GUC_UNIT_BYTES but neglected to add it to the docs.
      36e9d413
    • Tom Lane's avatar
      Remove no-longer-needed variant expected regression result files. · 6eb4378d
      Tom Lane authored
      numerology_1.out and float8-small-is-zero_1.out differ from their
      base files only in showing plain zero rather than minus zero for
      some results.  I believe that in the wake of commit 6eb3eb57,
      we will print minus zero as such on all IEEE-float platforms
      (and non-IEEE floats are going to cause many more regression diffs
      than this, anyway).  Hence we should be able to remove these and
      eliminate a bit of maintenance pain.  Let's see if the buildfarm
      agrees.
      
      Discussion: https://postgr.es/m/29037.1539021687@sss.pgh.pa.us
      6eb4378d
    • Tom Lane's avatar
      Select appropriate PG_PRINTF_ATTRIBUTE for recent NetBSD. · aed9fa0b
      Tom Lane authored
      NetBSD-current generates a large number of warnings about "%m" not
      being appropriate to use with *printf functions.  While that's true
      for their native printf, it's surely not true for snprintf.c, so I
      think they have misunderstood gcc's definition of the "gnu_printf"
      archetype.  Nonetheless, choosing "__syslog__" instead silences the
      warnings; so teach configure about that.
      
      Since this is only a cosmetic warning issue (and anyway it depends
      on previous hacking to be self-consistent), no back-patch.
      
      Discussion: https://postgr.es/m/16785.1539046036@sss.pgh.pa.us
      aed9fa0b
    • Michael Paquier's avatar
      Add pg_ls_archive_statusdir function · c4810162
      Michael Paquier authored
      This function lists the contents of the WAL archive status directory,
      and is intended to be used by monitoring tools.  Unlike pg_ls_dir(),
      access to it can be granted to non-superusers so that those monitoring
      tools can observe the principle of least privilege.  Access is also
      given by default to members of pg_monitor.
      
      Author:  Christoph Moench-Tegeder
      Reviewed-by: Aya Iwata
      Discussion: https://postgr.es/m/20180930205920.GA64534@elch.exwg.net
      c4810162
    • Tom Lane's avatar
      Convert some long lists in configure.in to one-line-per-entry style. · bfa6c5a0
      Tom Lane authored
      The idea here is that patches that add items to these lists will often
      be easier to rebase over other additions to the same lists, because
      they won't be trying to touch the very same line of configure.in.
      
      There will still be merge conflicts in the configure script, but that
      can be fixed just by re-running autoconf (or by leaving configure out
      of the submitted patch to begin with ...)
      
      Implementation note: use of m4_normalize() is necessary to get rid of
      the newlines, else incorrect shell syntax will be emitted.  But with
      that hack, the generated configure script is identical to what it
      was before.
      
      Discussion: https://postgr.es/m/19344.1539050134@sss.pgh.pa.us
      bfa6c5a0
  9. 08 Oct, 2018 3 commits
    • Thomas Munro's avatar
      Relax transactional restrictions on ALTER TYPE ... ADD VALUE (redux). · 212fab99
      Thomas Munro authored
      Originally committed as 15bc038f (plus some follow-ups), this was
      reverted in 28e07270 due to a problem discovered in parallel
      workers.  This new version corrects that problem by sending the
      list of uncommitted enum values to parallel workers.
      
      Here follows the original commit message describing the change:
      
      To prevent possibly breaking indexes on enum columns, we must keep
      uncommitted enum values from getting stored in tables, unless we
      can be sure that any such column is new in the current transaction.
      
      Formerly, we enforced this by disallowing ALTER TYPE ... ADD VALUE
      from being executed at all in a transaction block, unless the target
      enum type had been created in the current transaction.  This patch
      removes that restriction, and instead insists that an uncommitted enum
      value can't be referenced unless it belongs to an enum type created
      in the same transaction as the value.  Per discussion, this should be
      a bit less onerous.  It does require each function that could possibly
      return a new enum value to SQL operations to check this restriction,
      but there aren't so many of those that this seems unmaintainable.
      
      Author: Andrew Dunstan and Tom Lane, with parallel query fix by Thomas Munro
      Reviewed-by: Tom Lane
      Discussion: https://postgr.es/m/CAEepm%3D0Ei7g6PaNTbcmAh9tCRahQrk%3Dr5ZWLD-jr7hXweYX3yg%40mail.gmail.com
      Discussion: https://postgr.es/m/4075.1459088427%40sss.pgh.pa.us
      212fab99
    • Tom Lane's avatar
      Fix omissions in snprintf.c's coverage of standard *printf functions. · 7767aadd
      Tom Lane authored
      A warning on a NetBSD box revealed to me that pg_waldump/compat.c
      is using vprintf(), which snprintf.c did not provide coverage for.
      This is not good if we want to have uniform *printf behavior, and
      it's pretty silly to omit when it's a one-line function.
      
      I also noted that snprintf.c has pg_vsprintf() but for some reason
      it was not exposed to the outside world, creating another way in
      which code might accidentally invoke the platform *printf family.
      
      Let's just make sure that we replace all eight of the POSIX-standard
      printf family.
      
      Also, upgrade plperl.h and plpython.h to make sure that they do
      their undefine/redefine rain dance for all eight, not some random
      maybe-sufficient subset thereof.
      7767aadd
    • Tom Lane's avatar
      Advance transaction timestamp for intra-procedure transactions. · 82ff0cc9
      Tom Lane authored
      Per discussion, this behavior seems less astonishing than not doing so.
      
      Peter Eisentraut and Tom Lane
      
      Discussion: https://postgr.es/m/20180920234040.GC29981@momjian.us
      82ff0cc9