1. 04 Oct, 2016 11 commits
    • Tom Lane's avatar
      Improve (I hope) our autolocation of the Python shared library. · 46ddbbb1
      Tom Lane authored
      Older versions of Python produce garbage (or at least useless) values of
      get_config_vars('LDLIBRARY').  Newer versions produce garbage (or at least
      useless) values of get_config_vars('SO'), which was defeating our configure
      logic that attempted to identify where the Python shlib really is.  The net
      result, at least with a stock Python 3.5 installation on macOS, was that
      we were linking against a static library in the mistaken belief that it was
      a shared library.  This managed to work, if you count statically absorbing
      libpython into plpython.so as working.  But it no longer works as of commit
      d51924be, because now we get separate static copies of libpython in
      plpython.so and hstore_plpython.so, and those can't interoperate on the
      same data.  There are some other infelicities like assuming that nobody
      ever installs a private version of Python on a macOS machine.
      
      Hence, forget about looking in $python_configdir for the Python shlib;
      as far as I can tell no version of Python has ever put one there, and
      certainly no currently-supported version does.  Also, rather than relying
      on get_config_vars('SO'), just try all the possibilities for shlib
      extensions.  Also, rather than trusting Py_ENABLE_SHARED, believe we've
      found a shlib only if it has a recognized extension.  Last, explicitly
      cope with the possibility that the shlib is really in /usr/lib and
      $python_libdir is a red herring --- this is the actual situation on older
      macOS, but we were only accidentally working with it.
      
      Discussion: <5300.1475592228@sss.pgh.pa.us>
      46ddbbb1
    • Robert Haas's avatar
      Fix another Windows compile break. · 6c9c95ed
      Robert Haas authored
      Commit 6f3bd98e is still making
      the buildfarm unhappy.  This time it's mastodon that is complaining.
      6c9c95ed
    • Robert Haas's avatar
      Fix Windows compile break in 6f3bd98e. · 9445d112
      Robert Haas authored
      9445d112
    • Heikki Linnakangas's avatar
      Fix another outdated comment. · d4fca5e6
      Heikki Linnakangas authored
      Preloading is done by logtape.c now.
      d4fca5e6
    • Robert Haas's avatar
      Remove trailing commas from enums. · 23843dcb
      Robert Haas authored
      Buildfarm member mylodon doesn't like them.  Actually, I don't like
      them either, but I failed to notice these before pushing commit
      6f3bd98e.
      23843dcb
    • Robert Haas's avatar
      Adjust worker_spi for 6f3bd98e. · 976a1ce9
      Robert Haas authored
      976a1ce9
    • Robert Haas's avatar
      Extend framework from commit 53be0b1a to report latch waits. · 6f3bd98e
      Robert Haas authored
      WaitLatch, WaitLatchOrSocket, and WaitEventSetWait now taken an
      additional wait_event_info parameter; legal values are defined in
      pgstat.h.  This makes it possible to uniquely identify every point in
      the core code where we are waiting for a latch; extensions can pass
      WAIT_EXTENSION.
      
      Because latches were the major wait primitive not previously covered
      by this patch, it is now possible to see information in
      pg_stat_activity on a large number of important wait events not
      previously addressed, such as ClientRead, ClientWrite, and SyncRep.
      
      Unfortunately, many of the wait events added by this patch will fail
      to appear in pg_stat_activity because they're only used in background
      processes which don't currently appear in pg_stat_activity.  We should
      fix this either by creating a separate view for such information, or
      else by deciding to include them in pg_stat_activity after all.
      
      Michael Paquier and Robert Haas, reviewed by Alexander Korotkov and
      Thomas Munro.
      6f3bd98e
    • Tom Lane's avatar
      Fix hstore_plpython for Python 3. · 490ed1eb
      Tom Lane authored
      In commit d51924be, I overlooked the need to provide linkage for
      PLyUnicode_FromStringAndSize, because that's only used (and indeed
      only exists) in Python 3 builds.
      
      In light of the need to #if this item, rearrange the ordering of
      the code related to each function pointer, so as not to need more
      #if's than absolutely necessary.
      
      Per buildfarm.
      490ed1eb
    • Heikki Linnakangas's avatar
      Update comment. · c86c2d9d
      Heikki Linnakangas authored
      mergepreread()/mergeprereadone() don't exist anymore, the function that
      does roughly the same is now called mergereadnext().
      c86c2d9d
    • Andres Freund's avatar
      Correct logical decoding restore behaviour for subtransactions. · 61633f79
      Andres Freund authored
      Before initializing iteration over a subtransaction's changes, the last
      few changes were not spilled to disk. That's correct if the transaction
      didn't spill to disk, but otherwise... This bug can lead to missed or
      misorderd subtransaction contents when they were spilled to disk.
      
      Move spilling of the remaining in-memory changes to
      ReorderBufferIterTXNInit(), where it can easily be applied to the top
      transaction and, if present, subtransactions.
      
      Since this code had too many bugs already, noticeably increase test
      coverage.
      
      Fixes: #14319
      Reported-By: Huan Ruan
      Discussion: <20160909012610.20024.58169@wrigleys.postgresql.org>
      Backport: 9,4-, where logical decoding was added
      61633f79
    • Tom Lane's avatar
      Convert contrib/hstore_plpython to not use direct linking to other modules. · d51924be
      Tom Lane authored
      Previously, on most platforms, we allowed hstore_plpython's references
      to hstore and plpython to be unresolved symbols at link time, trusting
      the dynamic linker to resolve them when the module is loaded.  This
      has a number of problems, the worst being that the dynamic linker
      does not know where the references come from and can do nothing but
      fail if those other modules haven't been loaded.  We've more or less
      gotten away with that for the limited use-case of datatype transform
      modules, but even there, it requires some awkward hacks, most recently
      commit 83c24920.
      
      Instead, let's not treat these references as linker-resolvable at all,
      but use function pointers that are manually filled in by the module's
      _PG_init function.  There are few enough contact points that this
      doesn't seem unmaintainable, at least for these use-cases.  (Note that
      the same technique wouldn't work at all for decoupling from libpython
      itself, but fortunately that's just a standard shared library and can
      be linked to normally.)
      
      This is an initial patch that just converts hstore_plpython.  If the
      buildfarm doesn't find any fatal problems, I'll work on the other
      transform modules soon.
      
      Tom Lane, per an idea of Andres Freund's.
      
      Discussion: <2652.1475512158@sss.pgh.pa.us>
      d51924be
  2. 03 Oct, 2016 4 commits
    • Tom Lane's avatar
      Show a sensible value in pg_settings.unit for GUC_UNIT_XSEGS variables. · 6bc811c9
      Tom Lane authored
      Commit 88e98230 invented GUC_UNIT_XSEGS for min_wal_size and max_wal_size,
      but neglected to make it display sensibly in pg_settings.unit (by adding a
      case to the switch in GetConfigOptionByNum).  Fix that, and adjust said
      switch to throw a run-time error the next time somebody forgets.
      
      In passing, avoid using a static buffer for the output string --- the rest
      of this function pstrdup's from a local buffer, and I see no very good
      reason why the units code should do it differently and less safely.
      
      Per report from Otar Shavadze.  Back-patch to 9.5 where the new unit type
      was added.
      
      Report: <CAG-jOyA=iNFhN+yB4vfvqh688B7Tr5SArbYcFUAjZi=0Exp-Lg@mail.gmail.com>
      6bc811c9
    • Stephen Frost's avatar
      Fix RLS with COPY (col1, col2) FROM tab · 814b9e9b
      Stephen Frost authored
      Attempting to COPY a subset of columns from a table with RLS enabled
      would fail due to an invalid query being constructed (using a single
      ColumnRef with the list of fields to exact in 'fields', but that's for
      the different levels of an indirection for a single column, not for
      specifying multiple columns).
      
      Correct by building a ColumnRef and then RestTarget for each column
      being requested and then adding those to the targetList for the select
      query.  Include regression tests to hopefully catch if this is broken
      again in the future.
      
      Patch-By: Adam Brightwell
      Reviewed-By: Michael Paquier
      814b9e9b
    • Tom Lane's avatar
      Enforce a specific order for probing library loadability in pg_upgrade. · 83c24920
      Tom Lane authored
      pg_upgrade checks whether all the shared libraries used in the old cluster
      are also available in the new one by issuing LOAD for each library name.
      Previously, it cared not what order it did the LOADs in.  Ideally it
      should not have to care, but currently the transform modules in contrib
      fail unless both the language and datatype modules they depend on are
      loaded first.  A backend-side solution for that looks possible but
      probably not back-patchable, so as a stopgap measure, let's do the LOAD
      tests in order by library name length.  That should fix the problem for
      reasonably-named transform modules, eg "hstore_plpython" will be loaded
      after both "hstore" and "plpython".  (Yeah, it's a hack.)
      
      In a larger sense, having a predictable order of these probes is a good
      thing, since it will make upgrades predictably work or not work in the
      face of inter-library dependencies.  Also, this patch replaces O(N^2)
      de-duplication logic with O(N log N) logic, which could matter in
      installations with very many databases.  So I don't foresee reverting this
      even after we have a proper fix for the library-dependency problem.
      
      In passing, improve a couple of SQL queries used here.
      
      Per complaint from Andrew Dunstan that pg_upgrade'ing the transform contrib
      modules failed.  Back-patch to 9.5 where transform modules were introduced.
      
      Discussion: <f7ac29f3-515c-2a44-21c5-ec925053265f@dunslane.net>
      83c24920
    • Heikki Linnakangas's avatar
      Change the way pre-reading in external sort's merge phase works. · e94568ec
      Heikki Linnakangas authored
      Don't pre-read tuples into SortTuple slots during merge. Instead, use the
      memory for larger read buffers in logtape.c. We're doing the same number
      of READTUP() calls either way, but managing the pre-read SortTuple slots
      is much more complicated. Also, the on-tape representation is more compact
      than SortTuples, so we can fit more pre-read tuples into the same amount
      of memory this way. And we have better cache-locality, when we use just a
      small number of SortTuple slots.
      
      Now that we only hold one tuple from each tape in the SortTuple slots, we
      can greatly simplify the "batch memory" management. We now maintain a
      small set of fixed-sized slots, to hold the tuples, and fall back to
      palloc() for larger tuples. We use this method during all merge phases,
      not just the final merge, and also when randomAccess is requested, and
      also in the TSS_SORTEDONTAPE case. In other words, it's used whenever we
      do an external sort.
      
      Reviewed by Peter Geoghegan and Claudio Freire.
      
      Discussion: <CAM3SWZTpaORV=yQGVCG8Q4axcZ3MvF-05xe39ZvORdU9JcD6hQ@mail.gmail.com>
      e94568ec
  3. 02 Oct, 2016 2 commits
    • Tom Lane's avatar
      Add ALTER EXTENSION ADD/DROP ACCESS METHOD, and use it in pg_upgrade. · e8bdee27
      Tom Lane authored
      Without this, an extension containing an access method is not properly
      dumped/restored during pg_upgrade --- the AM ends up not being a member
      of the extension after upgrading.
      
      Another oversight in commit 473b9328, reported by Andrew Dunstan.
      
      Report: <f7ac29f3-515c-2a44-21c5-ec925053265f@dunslane.net>
      e8bdee27
    • Tom Lane's avatar
      Avoid leaking FDs after an fsync failure. · 728ceba9
      Tom Lane authored
      Fixes errors introduced in commit bc34223b, as detected by Coverity.
      
      In passing, report ENOSPC for a short write while padding a new wal file in
      open_walfile, make certain that close_walfile closes walfile in all cases,
      and improve a couple of comments.
      
      Michael Paquier and Tom Lane
      728ceba9
  4. 01 Oct, 2016 8 commits
    • Tom Lane's avatar
      Do ClosePostmasterPorts() earlier in SubPostmasterMain(). · 3b90e38c
      Tom Lane authored
      In standard Unix builds, postmaster child processes do ClosePostmasterPorts
      immediately after InitPostmasterChild, that is almost immediately after
      being spawned.  This is important because we don't want children holding
      open the postmaster's end of the postmaster death watch pipe.
      
      However, in EXEC_BACKEND builds, SubPostmasterMain was postponing this
      responsibility significantly, in order to make it slightly more convenient
      to pass the right flag value to ClosePostmasterPorts.  This is bad,
      particularly seeing that process_shared_preload_libraries() might invoke
      nearly-arbitrary code.  Rearrange so that we do it as soon as we've
      fetched the socket FDs via read_backend_variables().
      
      Also move the comment explaining about randomize_va_space to before the
      call of PGSharedMemoryReAttach, which is where it's relevant.  The old
      placement was appropriate when the reattach happened inside
      CreateSharedMemoryAndSemaphores, but that was a long time ago.
      
      Back-patch to 9.3; the patch doesn't apply cleanly before that, and
      it doesn't seem worth a lot of effort given that we've had no actual
      field complaints traceable to this.
      
      Discussion: <4157.1475178360@sss.pgh.pa.us>
      3b90e38c
    • Tom Lane's avatar
      Fix bugs in contrib/pg_visibility. · 9a109452
      Tom Lane authored
      collect_corrupt_items() failed to initialize tuple.t_self.  While
      HeapTupleSatisfiesVacuum() doesn't actually use that value, it does
      Assert that it's valid, so that the code would dump core if ip_posid
      chanced to be zero.  (That's somewhat unlikely, which probably explains
      how this got missed.  In any case it wouldn't matter for field use.)
      
      Also, collect_corrupt_items was returning the wrong TIDs, that is the
      contents of t_ctid rather than the tuple's own location.  This would
      be the same thing in simple cases, but it could be wrong if, for
      example, a past update attempt had been rolled back, leaving a live
      tuple whose t_ctid doesn't point at itself.
      
      Also, in pg_visibility(), guard against trying to read a page past
      the end of the rel.  The VM code handles inquiries beyond the end
      of the map by silently returning zeroes, and it seems like we should
      do the same thing here.
      
      I ran into the assertion failure while using pg_visibility to check
      pg_upgrade's behavior, and then noted the other problems while
      reading the code.
      
      Report: <29043.1475288648@sss.pgh.pa.us>
      9a109452
    • Tom Lane's avatar
      Copy-editing for contrib/pg_visibility documentation. · 33596edf
      Tom Lane authored
      Add omitted names for some function parameters.
      Fix some minor grammatical issues.
      33596edf
    • Tom Lane's avatar
      Fix misstatement in comment in Makefile.shlib. · ea046f08
      Tom Lane authored
      There is no need for "all: all-lib" to be placed before inclusion of
      Makefile.shlib.  Makefile.global is what ensures that "all" is the
      default target, and we already document that that has to be included
      first.  Per comment from Pavel Raiskup.
      
      Discussion: <1925924.izSMJEZO3x@unused-4-107.brq.redhat.com>
      ea046f08
    • Tom Lane's avatar
      Fix misplacement of submake-generated-headers prerequisites. · 7107d58e
      Tom Lane authored
      The sequence "configure; cd src/pl/plpython; make -j" failed due to
      trying to compile plpython's .o files before the generated headers
      finished building.  (This is an important real-world case, since it's
      the typical second step when building both plpython2 and plpython3.)
      This happens because the submake-generated-headers target is not
      placed in a way to make it a prerequisite to compiling the .o files.
      Fix that.
      
      Checking other uses of submake-generated-headers, I noted that the one
      attached to pg_regress was similarly misplaced; but it's actually not
      needed at all for pg_regress.o, rather regress.o, so move it to be a
      prerequisite of that.
      
      Back-patch to 9.6 where submake-generated-headers was introduced
      (by commit 548af97f).  It's not immediately clear to me why the
      previous coding didn't have the same issue; but since we've not
      had field reports of plpython make failing, leave it alone in the
      older branches.
      
      Pavel Raiskup and Tom Lane
      
      Discussion: <1925924.izSMJEZO3x@unused-4-107.brq.redhat.com>
      7107d58e
    • Peter Eisentraut's avatar
      Set log_line_prefix and application name in test drivers · a4327296
      Peter Eisentraut authored
      Before pg_regress runs psql, set the application name to the test name.
      Similarly, set the application name to the test file name in the TAP
      tests.  Also, set a default log_line_prefix that show the application
      name, as well as the PID and a time stamp.
      
      That way, the server log output can be correlated to the test input
      files, making debugging a bit easier.
      a4327296
    • Tom Lane's avatar
      Improve error reporting in pg_upgrade's file copying/linking/rewriting. · f002ed2b
      Tom Lane authored
      The previous design for this had copyFile(), linkFile(), and
      rewriteVisibilityMap() returning strerror strings, with the caller
      producing one-size-fits-all error messages based on that.  This made it
      impossible to produce messages that described the failures with any degree
      of precision, especially not short-read problems since those don't set
      errno at all.
      
      Since pg_upgrade has no intention of continuing after any error in this
      area, let's fix this by just letting these functions call pg_fatal() for
      themselves, making it easy for each point of failure to have a suitable
      error message.  Taking this approach also allows dropping cleanup code
      that was unnecessary and was often rather sloppy about preserving errno.
      To not lose relevant info that was reported before, pass in the schema name
      and table name of the current table so that they can be included in the
      error reports.
      
      An additional problem was the use of getErrorText(), which was flat out
      wrong for all but a couple of call sites, because it unconditionally did
      "_dosmaperr(GetLastError())" on Windows.  That's only appropriate when
      reporting an error from a Windows-native API, which only a couple of
      the callers were actually doing.  Thus, even the reported strerror string
      would be unrelated to the actual failure in many cases on Windows.
      To fix, get rid of getErrorText() altogether, and just have call sites
      do strerror(errno) instead, since that's the way all the rest of our
      frontend programs do it.  Add back the _dosmaperr() calls in the two
      places where that's actually appropriate.
      
      In passing, make assorted messages hew more closely to project style
      guidelines, notably by removing initial capitals in not-complete-sentence
      primary error messages.  (I didn't make any effort to clean up places
      I didn't have another reason to touch, though.)
      
      Per discussion of a report from Thomas Kellerer.  Back-patch to 9.6,
      but no further; given the relative infrequency of reports of problems
      here, it's not clear it's worth adapting the patch to older branches.
      
      Patch by me, but with credit to Alvaro Herrera for spotting the issue
      with getErrorText's misuse of _dosmaperr().
      
      Discussion: <nsjrbh$8li$1@blaine.gmane.org>
      f002ed2b
    • Tom Lane's avatar
      Fix multiple portability issues in pg_upgrade's rewriteVisibilityMap(). · 5afcd2aa
      Tom Lane authored
      This is new code in 9.6, and evidently we missed out testing it as
      thoroughly as it should have been.  Bugs fixed here:
      
      1. Use binary not text mode to open the files on Windows.  Before, if
      the visibility map chanced to contain two bytes that looked like \r\n,
      Windows' read() would convert that to \n, which both corrupts the map
      data and causes the file to look shorter than it should.  Unless you
      were *very* unlucky and had an exact multiple of 8K such occurrences
      in each VM file, this would cause pg_upgrade to report a failure,
      though with a rather obscure error message.
      
      2. The code for copying rebuilt bytes into the output was simply wrong.
      It chanced to work okay on little-endian machines but would emit the
      bytes in the wrong order on big-endian, leading to silent corruption
      of the visibility map data.
      
      3. The code was careless about alignment of the working buffers.  Given
      all three of an alignment-picky architecture, a compiler that chooses
      to put the new_vmbuf[] local variable at an odd starting address, and
      a checksum-enabled database, pg_upgrade would dump core.
      
      Point one was reported by Thomas Kellerer, the other two detected by
      code-reading.
      
      Point two is much the nastiest of these issues from an impact standpoint,
      though fortunately it affects only a minority of users.  The Windows issue
      will definitely bite people, but it seems quite unlikely that there would
      be undetected corruption from that.
      
      In addition, I failed to resist the temptation to do some minor cosmetic
      adjustments, mostly improving the comments.
      
      It would be a good idea to try to improve the error reporting here, but
      that seems like material for a separate patch.
      
      Discussion: <nsjrbh$8li$1@blaine.gmane.org>
      5afcd2aa
  5. 30 Sep, 2016 8 commits
  6. 29 Sep, 2016 7 commits
    • Peter Eisentraut's avatar
      Fix compiler warnings · f2af8dc5
      Peter Eisentraut authored
      This was missed in bf5bb2e8, because the
      code is only visible under PG_FLUSH_DATA_WORKS.
      f2af8dc5
    • Tom Lane's avatar
      Allow contrib/file_fdw to read from a program, like COPY FROM PROGRAM. · 8e91e12b
      Tom Lane authored
      This patch just exposes COPY's FROM PROGRAM option in contrib/file_fdw.
      There don't seem to be any security issues with that that are any worse
      than what already exist with file_fdw and COPY; as in the existing cases,
      only superusers are allowed to control what gets executed.
      
      A regression test case might be nice here, but choosing a 100% portable
      command to run is hard.  (We haven't got a test for COPY FROM PROGRAM
      itself, either.)
      
      Corey Huinker and Adam Gomaa, reviewed by Amit Langote
      
      Discussion: <CADkLM=dGDGmaEiZ=UDepzumWg-CVn7r8MHPjr2NArj8S3TsROQ@mail.gmail.com>
      8e91e12b
    • Peter Eisentraut's avatar
      Switch pg_basebackup commands in Postgres.pm to use --nosync · 728a3e73
      Peter Eisentraut authored
      On slow machines, this greatly reduces the I/O pressure induced by the
      tests.
      
      From: Michael Paquier <michael.paquier@gmail.com>
      728a3e73
    • Peter Eisentraut's avatar
      pg_basebackup: Add --nosync option · 6ed2d858
      Peter Eisentraut authored
      This is useful for testing, similar to initdb's --nosync.
      
      From: Michael Paquier <michael.paquier@gmail.com>
      6ed2d858
    • Peter Eisentraut's avatar
      pg_basebackup pg_receivexlog: Issue fsync more carefully · bc34223b
      Peter Eisentraut authored
      Several places weren't careful about fsyncing in the way.  See 1d4a0ab1
      and 606e0f98 for details about required fsyncs.
      
      This adds a couple of functions in src/common/ that have an equivalent
      in the backend: durable_rename(), fsync_parent_path()
      
      From: Michael Paquier <michael.paquier@gmail.com>
      bc34223b
    • Peter Eisentraut's avatar
      Move fsync routines of initdb into src/common/ · bf5bb2e8
      Peter Eisentraut authored
      The intention is to used those in other utilities such as pg_basebackup
      and pg_receivexlog.
      
      From: Michael Paquier <michael.paquier@gmail.com>
      bf5bb2e8
    • Heikki Linnakangas's avatar
      Don't bother to lock bufmgr partitions in pg_buffercache. · 6e654546
      Heikki Linnakangas authored
      That makes the view a lot less disruptive to use on a production system.
      Without the locks, you don't get a consistent snapshot across all buffers,
      but that's OK. It wasn't a very useful guarantee in practice.
      
      Ivan Kartyshov, reviewed by Tomas Vondra and Robert Haas.
      
      Discusssion: <f9d6cab2-73a7-7a84-55a8-07dcb8516ae5@postgrespro.ru>
      6e654546