1. 06 Sep, 2016 7 commits
    • Peter Eisentraut's avatar
      Add location field to DefElem · 49eb0fd0
      Peter Eisentraut authored
      Add a location field to the DefElem struct, used to parse many utility
      commands.  Update various error messages to supply error position
      information.
      
      To propogate the error position information in a more systematic way,
      create a ParseState in standard_ProcessUtility() and pass that to
      interested functions implementing the utility commands.  This seems
      better than passing the query string and then reassembling a parse state
      ad hoc, which violates the encapsulation of the ParseState type.
      Reviewed-by: default avatarPavel Stehule <pavel.stehule@gmail.com>
      49eb0fd0
    • Tom Lane's avatar
      Doc: small improvements for documentation about VACUUM freezing. · 975768f8
      Tom Lane authored
      Mostly, explain how row xmin's used to be replaced by FrozenTransactionId
      and no longer are.  Do a little copy-editing on the side.
      
      Per discussion with Egor Rogov.  Back-patch to 9.4 where the behavioral
      change occurred.
      
      Discussion: <575D7955.6060209@postgrespro.ru>
      975768f8
    • Tom Lane's avatar
      Guard against possible memory allocation botch in batchmemtuples(). · f032722f
      Tom Lane authored
      Negative availMemLessRefund would be problematic.  It's not entirely
      clear whether the case can be hit in the code as it stands, but this
      seems like good future-proofing in any case.  While we're at it,
      insist that the value be not merely positive but not tiny, so as to
      avoid doing a lot of repalloc work for little gain.
      
      Peter Geoghegan
      
      Discussion: <CAM3SWZRVkuUB68DbAkgw=532gW0f+fofKueAMsY7hVYi68MuYQ@mail.gmail.com>
      f032722f
    • Tom Lane's avatar
      Teach appendShellString() to not quote strings containing "-". · cdc70597
      Tom Lane authored
      Brain fade in commit a00c5831: I was thinking that a string starting with
      "-" could be taken as a switch depending on command line syntax.  That's
      true, but having appendShellString() quote it will not help, so we may as
      well not do so.  Per complaint from Peter Eisentraut.
      cdc70597
    • Tom Lane's avatar
      Repair whitespace in initdb message. · a2ee579b
      Tom Lane authored
      What used to be four spaces somehow turned into a tab and a couple of
      spaces in commit a00c5831, no doubt from overhelpful emacs autoindent.
      Noted by Peter Eisentraut.
      a2ee579b
    • Simon Riggs's avatar
      Fix VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL · dcb12ce8
      Simon Riggs authored
      lazy_truncate_heap() was waiting for
      VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL, but in microseconds
      not milliseconds as originally intended.
      
      Found by code inspection.
      
      Simon Riggs
      dcb12ce8
    • Bruce Momjian's avatar
      C comment: fix file name mention on line 1 · 67e1e2aa
      Bruce Momjian authored
      Author: Amit Langote
      67e1e2aa
  2. 05 Sep, 2016 10 commits
    • Tom Lane's avatar
      Cosmetic code cleanup in commands/extension.c. · 25794e84
      Tom Lane authored
      Some of the comments added by the CREATE EXTENSION CASCADE patch were
      a bit sloppy, and I didn't care for redeclaring the same local variable
      inside a nested block either.  No functional changes.
      25794e84
    • Alvaro Herrera's avatar
      2093f663
    • Tom Lane's avatar
      Make locale-dependent regex character classes work for large char codes. · c54159d4
      Tom Lane authored
      Previously, we failed to recognize Unicode characters above U+7FF as
      being members of locale-dependent character classes such as [[:alpha:]].
      (Actually, the same problem occurs for large pg_wchar values in any
      multibyte encoding, but UTF8 is the only case people have actually
      complained about.)  It's impractical to get Spencer's original code to
      handle character classes or ranges containing many thousands of characters,
      because it insists on considering each member character individually at
      regex compile time, whether or not the character will ever be of interest
      at run time.  To fix, choose a cutoff point MAX_SIMPLE_CHR below which
      we process characters individually as before, and deal with entire ranges
      or classes as single entities above that.  We can actually make things
      cheaper than before for chars below the cutoff, because the color map can
      now be a simple linear array for those chars, rather than the multilevel
      tree structure Spencer designed.  It's more expensive than before for
      chars above the cutoff, because we must do a binary search in a list of
      high chars and char ranges used in the regex pattern, plus call iswalpha()
      and friends for each locale-dependent character class used in the pattern.
      However, multibyte encodings are normally designed to give smaller codes
      to popular characters, so that we can expect that the slow path will be
      taken relatively infrequently.  In any case, the speed penalty appears
      minor except when we have to apply iswalpha() etc. to high character codes
      at runtime --- and the previous coding gave wrong answers for those cases,
      so whether it was faster is moot.
      
      Tom Lane, reviewed by Heikki Linnakangas
      
      Discussion: <15563.1471913698@sss.pgh.pa.us>
      c54159d4
    • Bruce Momjian's avatar
      C comment: align dashes in GroupState node header · f80049f7
      Bruce Momjian authored
      Author: Jim Nasby
      f80049f7
    • Tom Lane's avatar
      Relax transactional restrictions on ALTER TYPE ... ADD VALUE. · 15bc038f
      Tom Lane authored
      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.
      
      Andrew Dunstan and Tom Lane
      
      Discussion: <4075.1459088427@sss.pgh.pa.us>
      15bc038f
    • Simon Riggs's avatar
      Add debug check function LWLockHeldByMeInMode() · 016abf1f
      Simon Riggs authored
      Tests whether my process holds a lock in given mode.
      Add initial usage in MarkBufferDirty().
      
      Thomas Munro
      016abf1f
    • Simon Riggs's avatar
      Document LSN acronym in WAL Internals · ec03f412
      Simon Riggs authored
      We previously didn't mention what an LSN actually was.
      
      Simon Riggs and Michael Paquier
      ec03f412
    • Simon Riggs's avatar
      Dirty replication slots when using sql interface · d851bef2
      Simon Riggs authored
      When pg_logical_slot_get_changes(...) sets confirmed_flush_lsn to the point at
      which replay stopped, it doesn't dirty the replication slot.  So if the replay
      didn't cause restart_lsn or catalog_xmin to change as well, this change will
      not get written out to disk. Even on a clean shutdown.
      
      If Pg crashes or restarts, a subsequent pg_logical_slot_get_changes(...) call
      will see the same changes already replayed since it uses the slot's
      confirmed_flush_lsn as the start point for fetching changes. The caller can't
      specify a start LSN when using the SQL interface.
      
      Mark the slot as dirty after reading changes using the SQL interface so that
      users won't see repeated changes after a clean shutdown. Repeated changes still
      occur when using the walsender interface or after an unclean shutdown.
      
      Craig Ringer
      d851bef2
    • Tom Lane's avatar
      Remove duplicate code from ReorderBufferCleanupTXN(). · b6182081
      Tom Lane authored
      Andres is apparently the only hacker who thinks this code is better as-is.
      I (tgl) follow some of his logic, but the fact that it's setting off
      warnings from static code analyzers seems like a sufficient reason to
      put the complexity into a comment rather than the code.
      
      Aleksander Alekseev
      
      Discussion: <20160404190345.54d84ee8@fujitsu>
      b6182081
    • Tom Lane's avatar
      Add regression test coverage for non-default timezone abbreviation sets. · c7f68bea
      Tom Lane authored
      After further reflection about the mess cleaned up in commit 39b691f2,
      I decided the main bit of test coverage that was still missing was to
      check that the non-default abbreviation-set files we supply are usable.
      Add that.
      
      Back-patch to supported branches, just because it seems like a good
      idea to keep this all in sync.
      c7f68bea
  3. 04 Sep, 2016 4 commits
    • Tom Lane's avatar
      Remove vestigial references to "zic" in favor of "IANA database". · da6ea70c
      Tom Lane authored
      Commit b2cbced9 instituted a policy of referring to the timezone database
      as the "IANA timezone database" in our user-facing documentation.
      Propagate that wording into a couple of places that were still using "zic"
      to refer to the database, which is definitely not right (zic is the
      compilation tool, not the data).
      
      Back-patch, not because this is very important in itself, but because
      we routinely cherry-pick updates to the tznames files and I don't want
      to risk future merge failures.
      da6ea70c
    • Tom Lane's avatar
      Update release notes to mention need for ALTER EXTENSION UPDATE. · 5a072244
      Tom Lane authored
      Maybe we ought to make pg_upgrade do this for you, but it won't happen
      in 9.6, so call out the need for it as a migration consideration.
      5a072244
    • Tom Lane's avatar
      Remove useless pg_strdup() operations. · a2d75b67
      Tom Lane authored
      split_to_stringlist() doesn't modify its first argument nor expect it
      to remain valid after exit, so there's no need to duplicate the optarg
      string at the call sites.  Per Coverity.  (This has been wrong all along,
      but commit 052cc223 changed the useless calls from "strdup" to
      "pg_strdup", which apparently made Coverity think it's a new bug.
      It's not, but it's also not worth back-patching.)
      a2d75b67
    • Heikki Linnakangas's avatar
      Clarify the new Red-Black post-order traversal code a bit. · e21db14b
      Heikki Linnakangas authored
      Coverity complained about the for(;;) loop, because it never actually
      iterated. It was used just to be able to use "break" to exit it early. I
      agree with Coverity, that's a bit confusing, so refactor the code to
      use if-else instead.
      
      While we're at it, use a local variable to hold the "current" node. That's
      shorter and clearer than referring to "iter->last_visited" all the time.
      e21db14b
  4. 03 Sep, 2016 5 commits
    • Tom Lane's avatar
      Improve readability of the output of psql's \timing command. · 6591f422
      Tom Lane authored
      In addition to the existing decimal-milliseconds output value,
      display the same value in mm:ss.fff format if it exceeds one second.
      Tack on hours and even days fields if the interval is large enough.
      This avoids needing mental arithmetic to convert the values into
      customary time units.
      
      Corey Huinker, reviewed by Gerdan Santos; bikeshedding by many
      
      Discussion: <CADkLM=dbC4R8sbbuFXQVBFWoJGQkTEW8RWnC0PbW9nZsovZpJQ@mail.gmail.com>
      6591f422
    • Tom Lane's avatar
      Fix multiple bugs in numeric_poly_deserialize(). · 600dc4c0
      Tom Lane authored
      These were evidently introduced by yesterday's commit 9cca11c9,
      which perhaps needs more review than it got.
      
      Per report from Andreas Seltenreich and additional examination
      of nearby code.
      
      Report: <87oa45qfwq.fsf@credativ.de>
      600dc4c0
    • Tom Lane's avatar
      Fix corrupt GIN_SEGMENT_ADDITEMS WAL records on big-endian hardware. · 60893786
      Tom Lane authored
      computeLeafRecompressWALData() tried to produce a uint16 WAL log field by
      memcpy'ing the first two bytes of an int-sized variable.  That accidentally
      works on little-endian hardware, but not at all on big-endian.  Replay then
      thinks it's looking at an ADDITEMS action with zero entries, and reads the
      first two bytes of the first TID therein as the next segno/action,
      typically leading to "unexpected GIN leaf action" errors during replay.
      Even if replay failed to crash, the resulting GIN index page would surely
      be incorrect.  To fix, just declare the variable as uint16 instead.
      
      Per bug #14295 from Spencer Thomason (much thanks to Spencer for turning
      his problem into a self-contained test case).  This likely also explains
      a previous report of the same symptom from Bernd Helmle.
      
      Back-patch to 9.4 where the problem was introduced (by commit 14d02f0b).
      
      Discussion: <20160826072658.15676.7628@wrigleys.postgresql.org>
      Possible-Report: <2DA7350F7296B2A142272901@eje.land.credativ.lan>
      60893786
    • Simon Riggs's avatar
      New recovery target recovery_target_lsn · 35250b6a
      Simon Riggs authored
      Michael Paquier
      35250b6a
    • Simon Riggs's avatar
      Fix wording of logical decoding concepts · 0c40ab3a
      Simon Riggs authored
      Be specific about conditions under which we emit >1 copy of message
      
      Craig Ringer
      0c40ab3a
  5. 02 Sep, 2016 4 commits
    • Tom Lane's avatar
      Don't require dynamic timezone abbreviations to match underlying time zone. · 39b691f2
      Tom Lane authored
      Previously, we threw an error if a dynamic timezone abbreviation did not
      match any abbreviation recorded in the referenced IANA time zone entry.
      That seemed like a good consistency check at the time, but it turns out
      that a number of the abbreviations in the IANA database are things that
      Olson and crew made up out of whole cloth.  Their current policy is to
      remove such names in favor of using simple numeric offsets.  Perhaps
      unsurprisingly, a lot of these made-up abbreviations have varied in meaning
      over time, which meant that our commit b2cbced9 and later changes made
      them into dynamic abbreviations.  So with newer IANA database versions
      that don't mention these abbreviations at all, we fail, as reported in bug
      #14307 from Neil Anderson.  It's worse than just a few unused-in-the-wild
      abbreviations not working, because the pg_timezone_abbrevs view stops
      working altogether (since its underlying function tries to compute the
      whole view result in one call).
      
      We considered deleting these abbreviations from our abbreviations list, but
      the problem with that is that we can't stay ahead of possible future IANA
      changes.  Instead, let's leave the abbreviations list alone, and treat any
      "orphaned" dynamic abbreviation as just meaning the referenced time zone.
      It will behave a bit differently than it used to, in that you can't any
      longer override the zone's standard vs. daylight rule by using the "wrong"
      abbreviation of a pair, but that's better than failing entirely.  (Also,
      this solution can be interpreted as adding a small new feature, which is
      that any abbreviation a user wants can be defined as referencing a time
      zone name.)
      
      Back-patch to all supported branches, since this problem affects all
      of them when using tzdata 2016f or newer.
      
      Report: <20160902031551.15674.67337@wrigleys.postgresql.org>
      Discussion: <6189.1472820913@sss.pgh.pa.us>
      39b691f2
    • Heikki Linnakangas's avatar
      Move code shared between libpq and backend from backend/libpq/ to common/. · ec136d19
      Heikki Linnakangas authored
      When building libpq, ip.c and md5.c were symlinked or copied from
      src/backend/libpq into src/interfaces/libpq, but now that we have a
      directory specifically for routines that are shared between the server and
      client binaries, src/common/, move them there.
      
      Some routines in ip.c were only used in the backend. Keep those in
      src/backend/libpq, but rename to ifaddr.c to avoid confusion with the file
      that's now in common.
      
      Fix the comment in src/common/Makefile to reflect how libpq actually links
      those files.
      
      There are two more files that libpq symlinks directly from src/backend:
      encnames.c and wchar.c. I don't feel compelled to move those right now,
      though.
      
      Patch by Michael Paquier, with some changes by me.
      
      Discussion: <69938195-9c76-8523-0af8-eb718ea5b36e@iki.fi>
      ec136d19
    • Heikki Linnakangas's avatar
      Speed up SUM calculation in numeric aggregates. · 9cca11c9
      Heikki Linnakangas authored
      This introduces a numeric sum accumulator, which performs better than
      repeatedly calling add_var(). The performance comes from using wider digits
      and delaying carry propagation, tallying positive and negative values
      separately, and avoiding a round of palloc/pfree on every value. This
      speeds up SUM(), as well as other standard aggregates like AVG() and
      STDDEV() that also calculate a sum internally.
      
      Reviewed-by: Andrey Borodin
      Discussion: <c0545351-a467-5b76-6d46-4840d1ea8aa4@iki.fi>
      9cca11c9
    • Heikki Linnakangas's avatar
      Support multiple iterators in the Red-Black Tree implementation. · 9f85784c
      Heikki Linnakangas authored
      While we don't need multiple iterators at the moment, the interface is
      nicer and less dangerous this way.
      
      Aleksander Alekseev, with some changes by me.
      9f85784c
  6. 01 Sep, 2016 2 commits
    • Kevin Grittner's avatar
      Improve tab completion for BEGIN & START|SET TRANSACTION. · 76f9dd4f
      Kevin Grittner authored
      Andreas Karlsson with minor change by me for SET TRANSACTION
      SNAPSHOT.
      76f9dd4f
    • Tom Lane's avatar
      Change API of ShmemAlloc() so it throws error rather than returning NULL. · 6c03d981
      Tom Lane authored
      A majority of callers seem to have believed that this was the API spec
      already, because they omitted any check for a NULL result, and hence
      would crash on an out-of-shared-memory failure.  The original proposal
      was to just add such error checks everywhere, but that does nothing to
      prevent similar omissions in future.  Instead, let's make ShmemAlloc()
      throw the error (so we can remove the caller-side checks that do exist),
      and introduce a new function ShmemAllocNoError() that has the previous
      behavior of returning NULL, for the small number of callers that need
      that and are prepared to do the right thing.  This also lets us remove
      the rather wishy-washy behavior of printing a WARNING for out-of-shmem,
      which never made much sense: either the caller has a strategy for
      dealing with that, or it doesn't.  It's not ShmemAlloc's business to
      decide whether a warning is appropriate.
      
      The v10 release notes will need to call this out as a significant
      source-code change.  It's likely that it will be a bug fix for
      extension callers too, but if not, they'll need to change to using
      ShmemAllocNoError().
      
      This is nominally a bug fix, but the odds that it's fixing any live
      bug are actually rather small, because in general the requests
      being made by the unchecked callers were already accounted for in
      determining the overall shmem size, so really they ought not fail.
      Between that and the possible impact on extensions, no back-patch.
      
      Discussion: <24843.1472563085@sss.pgh.pa.us>
      6c03d981
  7. 31 Aug, 2016 7 commits
    • Tom Lane's avatar
      Improve memory management for PL/Perl functions. · 6f7c0ea3
      Tom Lane authored
      Unlike PL/Tcl, PL/Perl at least made an attempt to clean up after itself
      when a function gets redefined.  But it was still using TopMemoryContext
      for the fn_mcxt of argument/result I/O functions, resulting in the
      potential for memory leaks depending on what those functions did, and the
      retail alloc/free logic was pretty bulky as well.  Fix things to use a
      per-function memory context like the other PLs now do.  Tweak a couple of
      places where things were being done in a not-very-safe order (on the
      principle that a memory leak is better than leaving global state
      inconsistent after an error).  Also make some minor cosmetic adjustments,
      mostly in field names, to make the code look similar to the way PL/Tcl does
      now wherever it's essentially the same logic.
      
      Michael Paquier and Tom Lane
      
      Discussion: <CAB7nPqSOyAsHC6jL24J1B+oK3p=yyNoFU0Vs_B6fd2kdd5g5WQ@mail.gmail.com>
      6f7c0ea3
    • Tom Lane's avatar
      Improve memory management for PL/Tcl functions. · d062245b
      Tom Lane authored
      Formerly, the memory used to represent a PL/Tcl function was allocated with
      malloc() or in TopMemoryContext, and we'd leak it all if the function got
      redefined during the session.  Instead, create a per-function context and
      keep everything in or under that context.  Add a reference-counting
      mechanism (like the one plpgsql has long had) so that we can safely clean
      up an old function definition, either immediately if it's not being
      executed or at the end of the outermost execution.
      
      Currently, we only detect that a cached function is obsolete when we next
      attempt to call that function.  So this covers the updated-definition case
      but leaves cruft around after DROP FUNCTION.  It's not clear whether it's
      worth installing a syscache invalidation callback to watch for drops;
      none of the other PLs do, so for now we won't do it here either.
      
      Michael Paquier and Tom Lane
      
      Discussion: <CAB7nPqSOyAsHC6jL24J1B+oK3p=yyNoFU0Vs_B6fd2kdd5g5WQ@mail.gmail.com>
      d062245b
    • Tom Lane's avatar
      Try to fix portability issue in enum renumbering (again). · 65a588b4
      Tom Lane authored
      The hack embodied in commit 4ba61a48 no longer works after today's change
      to allow DatumGetFloat4/Float4GetDatum to be inlined (commit 14cca1bf).
      Probably what's happening is that the faulty compilers are deciding that
      the now-inlined assignment is a no-op and so they're not required to
      round to float4 width.
      
      We had a bunch of similar issues earlier this year in the degree-based
      trig functions, and eventually settled on using volatile intermediate
      variables as the least ugly method of forcing recalcitrant compilers
      to do what the C standard says (cf commit 82311bcd).  Let's see if
      that method works here.
      
      Discussion: <4640.1472664476@sss.pgh.pa.us>
      65a588b4
    • Tom Lane's avatar
      Remove no-longer-useful SSL-specific Port.count field. · 67922633
      Tom Lane authored
      Since we removed SSL renegotiation, there's no longer any reason to
      keep track of the amount of data transferred over the link.
      
      Daniel Gustafsson
      
      Discussion: <FEA7F89C-ECDF-4799-B789-2F8DDCBA467F@yesql.se>
      67922633
    • Heikki Linnakangas's avatar
      Use static inline functions for float <-> Datum conversions. · 14cca1bf
      Heikki Linnakangas authored
      Now that we are OK with using static inline functions, we can use them
      to avoid function call overhead of pass-by-val versions of Float4GetDatum,
      DatumGetFloat8, and Float8GetDatum. Those functions are only a few CPU
      instructions long, but they could not be written into macros previously,
      because we need a local union variable for the conversion.
      
      I kept the pass-by-ref versions as regular functions. They are very simple
      too, but they call palloc() anyway, so shaving a few instructions from the
      function call doesn't seem so important there.
      
      Discussion: <dbb82a4a-2c15-ba27-dd0a-009d2aa72b77@iki.fi>
      14cca1bf
    • Tom Lane's avatar
      Prevent starting a standalone backend with standby_mode on. · 0e0f43d6
      Tom Lane authored
      This can't really work because standby_mode expects there to be more
      WAL arriving, which there will not ever be because there's no WAL
      receiver process to fetch it.  Moreover, if standby_mode is on then
      hot standby might also be turned on, causing even more strangeness
      because that expects read-only sessions to be executing in parallel.
      Bernd Helmle reported a case where btree_xlog_delete_get_latestRemovedXid
      got confused, but rather than band-aiding individual problems it seems
      best to prevent getting anywhere near this state in the first place.
      Back-patch to all supported branches.
      
      In passing, also fix some omissions of errcodes in other ereport's in
      readRecoveryCommandFile().
      
      Michael Paquier (errcode hacking by me)
      
      Discussion: <00F0B2CEF6D0CEF8A90119D4@eje.credativ.lan>
      0e0f43d6
    • Robert Haas's avatar
      Update comments to reflect code rearrangement. · 530fb68e
      Robert Haas authored
      Commit f9143d10 falsified these.
      
      KaiGai Kohei
      530fb68e
  8. 30 Aug, 2016 1 commit
    • Tom Lane's avatar
      Fix a bunch of places that called malloc and friends with no NULL check. · 052cc223
      Tom Lane authored
      Where possible, use palloc or pg_malloc instead; otherwise, insert
      explicit NULL checks.
      
      Generally speaking, these are places where an actual OOM is quite
      unlikely, either because they're in client programs that don't
      allocate all that much, or they're very early in process startup
      so that we'd likely have had a fork() failure instead.  Hence,
      no back-patch, even though this is nominally a bug fix.
      
      Michael Paquier, with some adjustments by me
      
      Discussion: <CAB7nPqRu07Ot6iht9i9KRfYLpDaF2ZuUv5y_+72uP23ZAGysRg@mail.gmail.com>
      052cc223