1. 04 Nov, 2013 3 commits
  2. 03 Nov, 2013 4 commits
    • Tom Lane's avatar
      Get rid of more cases of the "must detoast before output function" meme. · e36ce0c7
      Tom Lane authored
      I missed that json.c was doing this too, because for some bizarre reason
      it wasn't doing it adjacent to the output function call.
      e36ce0c7
    • Tom Lane's avatar
      Prevent memory leaks from accumulating across printtup() calls. · b006f4dd
      Tom Lane authored
      Historically, printtup() has assumed that it could prevent memory leakage
      by pfree'ing the string result of each output function and manually
      managing detoasting of toasted values.  This amounts to assuming that
      datatype output functions never leak any memory internally; an assumption
      we've already decided to be bogus elsewhere, for example in COPY OUT.
      range_out in particular is known to leak multiple kilobytes per call, as
      noted in bug #8573 from Godfried Vanluffelen.  While we could go in and fix
      that leak, it wouldn't be very notationally convenient, and in any case
      there have been and undoubtedly will again be other leaks in other output
      functions.  So what seems like the best solution is to run the output
      functions in a temporary memory context that can be reset after each row,
      as we're doing in COPY OUT.  Some quick experimentation suggests this is
      actually a tad faster than the retail pfree's anyway.
      
      This patch fixes all the variants of printtup, except for debugtup()
      which is used in standalone mode.  It doesn't seem worth worrying
      about query-lifespan leaks in standalone mode, and fixing that case
      would be a bit tedious since debugtup() doesn't currently have any
      startup or shutdown functions.
      
      While at it, remove manual detoast management from several other
      output-function call sites that had copied it from printtup().  This
      doesn't make a lot of difference right now, but in view of recent
      discussions about supporting "non-flattened" Datums, we're going to
      want that code gone eventually anyway.
      
      Back-patch to 9.2 where range_out was introduced.  We might eventually
      decide to back-patch this further, but in the absence of known major
      leaks in older output functions, I'll refrain for now.
      b006f4dd
    • Michael Meskes's avatar
    • Kevin Grittner's avatar
      Acquire appropriate locks when rewriting during RMV. · 2a781d57
      Kevin Grittner authored
      Since the query has not been freshly parsed when executing REFRESH
      MATERIALIZED VIEW, locks must be explicitly taken before rewrite.
      
      Backpatch to 9.3.
      
      Andres Freund
      2a781d57
  3. 02 Nov, 2013 2 commits
    • Kevin Grittner's avatar
      Fix subquery reference to non-populated MV in CMV. · be420fa0
      Kevin Grittner authored
      A subquery reference to a matview should be allowed by CREATE
      MATERIALIZED VIEW WITH NO DATA, just like a direct reference is.
      
      Per bug report from Laurent Sartran.
      
      Backpatch to 9.3.
      be420fa0
    • Tom Lane's avatar
      Retry after buffer locking failure during SPGiST index creation. · 24ace405
      Tom Lane authored
      The original coding thought this case was impossible, but it can happen
      if the bgwriter or checkpointer processes decide to write out an index
      page while creation is still proceeding, leading to a bogus "unexpected
      spgdoinsert() failure" error.  Problem reported by Jonathan S. Katz.
      
      Teodor Sigaev
      24ace405
  4. 01 Nov, 2013 4 commits
    • Tom Lane's avatar
      Ensure all files created for a single BufFile have the same resource owner. · bffd1ce9
      Tom Lane authored
      Callers expect that they only have to set the right resource owner when
      creating a BufFile, not during subsequent operations on it.  While we could
      insist this be fixed at the caller level, it seems more sensible for the
      BufFile to take care of it.  Without this, some temp files belonging to
      a BufFile can go away too soon, eg at the end of a subtransaction,
      leading to errors or crashes.
      
      Reported and fixed by Andres Freund.  Back-patch to all active branches.
      bffd1ce9
    • Tom Lane's avatar
      Remove CTimeZone/HasCTZSet, root and branch. · 45f64f1b
      Tom Lane authored
      These variables no longer have any useful purpose, since there's no reason
      to special-case brute force timezones now that we have a valid
      session_timezone setting for them.  Remove the variables, and remove the
      SET/SHOW TIME ZONE code that deals with them.
      
      The user-visible impact of this is that SHOW TIME ZONE will now show a
      POSIX-style zone specification, in the form "<+-offset>-+offset", rather
      than an interval value when a brute-force zone has been set.  While perhaps
      less intuitive, this is a better definition than before because it's
      actually possible to give that string back to SET TIME ZONE and get the
      same behavior, unlike what used to happen.
      
      We did not previously mention the angle-bracket syntax when describing
      POSIX timezone specifications; add some documentation so that people
      can figure out what these strings do.  (There's still quite a lot of
      undocumented functionality there, but anybody who really cares can
      go read the POSIX spec to find out about it.  In practice most people
      seem to prefer Olsen-style city names anyway.)
      45f64f1b
    • Tom Lane's avatar
      Remove internal uses of CTimeZone/HasCTZSet. · 1c8a7f61
      Tom Lane authored
      The only remaining places where we actually look at CTimeZone/HasCTZSet
      are abstime2tm() and timestamp2tm().  Now that session_timezone is always
      valid, we can remove these special cases.  The caller-visible impact of
      this is that these functions now always return a valid zone abbreviation
      if requested, whereas before they'd return a NULL pointer if a brute-force
      timezone was in use.  In the existing code, the only place I can find that
      changes behavior is to_char(), whose TZ format code will now print
      something useful rather than nothing for such zones.  (In the places where
      the returned zone abbreviation is passed to EncodeDateTime, the lack of
      visible change is because we've chosen the abbreviation used for these
      zones to match what EncodeTimezone would have printed.)
      
      It's likely that there is now a fair amount of removable dead code around
      the call sites, namely anything that's meant to cope with getting a NULL
      timezone abbreviation, but I've not made an effort to root that out.
      
      This could be back-patched if we decide we'd like to fix to_char()'s
      behavior in the back branches, but there doesn't seem to be much
      enthusiasm for that at present.
      1c8a7f61
    • Tom Lane's avatar
      Fix some odd behaviors when using a SQL-style simple GMT offset timezone. · 631dc390
      Tom Lane authored
      Formerly, when using a SQL-spec timezone setting with a fixed GMT offset
      (called a "brute force" timezone in the code), the session_timezone
      variable was not updated to match the nominal timezone; rather, all code
      was expected to ignore session_timezone if HasCTZSet was true.  This is
      of course obviously fragile, though a search of the code finds only
      timeofday() failing to honor the rule.  A bigger problem was that
      DetermineTimeZoneOffset() supposed that if its pg_tz parameter was
      pointer-equal to session_timezone, then HasCTZSet should override the
      parameter.  This would cause datetime input containing an explicit zone
      name to be treated as referencing the brute-force zone instead, if the
      zone name happened to match the session timezone that had prevailed
      before installing the brute-force zone setting (as reported in bug #8572).
      The same malady could affect AT TIME ZONE operators.
      
      To fix, set up session_timezone so that it matches the brute-force zone
      specification, which we can do using the POSIX timezone definition syntax
      "<abbrev>offset", and get rid of the bogus lookaside check in
      DetermineTimeZoneOffset().  Aside from fixing the erroneous behavior in
      datetime parsing and AT TIME ZONE, this will cause the timeofday() function
      to print its result in the user-requested time zone rather than some
      previously-set zone.  It might also affect results in third-party
      extensions, if there are any that make use of session_timezone without
      considering HasCTZSet, but in all cases the new behavior should be saner
      than before.
      
      Back-patch to all supported branches.
      631dc390
  5. 31 Oct, 2013 1 commit
  6. 30 Oct, 2013 1 commit
    • Robert Haas's avatar
      Avoid too-large shift on 32-bit Windows. · 343bb134
      Robert Haas authored
      Apparently, shifts greater than or equal to the width of the type
      are undefined, and can surprisingly produce a non-zero value.
      
      Amit Kapila, with a comment by me.
      343bb134
  7. 29 Oct, 2013 2 commits
    • Tom Lane's avatar
      Fix old typo in comment. · 6756c8ad
      Tom Lane authored
      NFAs have children, but their individual states don't.
      6756c8ad
    • Tom Lane's avatar
      Prevent using strncpy with src == dest in TupleDescInitEntry. · 9a9473f3
      Tom Lane authored
      The C and POSIX standards state that strncpy's behavior is undefined when
      source and destination areas overlap.  While it remains dubious whether any
      implementations really misbehave when the pointers are exactly equal, some
      platforms are now starting to force the issue by complaining when an
      undefined call occurs.  (In particular OS X 10.9 has been seen to dump core
      here, though the exact set of circumstances needed to trigger that remain
      elusive.  Similar behavior can be expected to be optional on Linux and
      other platforms in the near future.)  So tweak the code to explicitly do
      nothing when nothing need be done.
      
      Back-patch to all active branches.  In HEAD, this also lets us get rid of
      an exception in valgrind.supp.
      
      Per discussion of a report from Matthias Schmitt.
      9a9473f3
  8. 28 Oct, 2013 5 commits
  9. 26 Oct, 2013 1 commit
    • Tom Lane's avatar
      Use unaligned output in selected regression queries to reduce diff noise. · 9c339eb4
      Tom Lane authored
      The rules regression test prints all known views and rules, which is a set
      that changes regularly.  Previously, a change in one rule would frequently
      lead to whitespace changes across the entire output of this query, which is
      painful to verify and causes undesirable conflicts between unrelated patch
      sets.  Use \a mode to improve matters.  Also use \t mode to suppress the
      total-rows count, which was also a source of unnecessary patch conflicts.
      
      Likewise modify the output mode for the list of indexed tables generated
      in sanity_check.sql.  There might be other places where we should use this
      idea, but these are the ones that have caused the most problems.
      
      Andres Freund
      9c339eb4
  10. 25 Oct, 2013 6 commits
    • Tom Lane's avatar
      Improve pqexpbuffer.c to use modern vsnprintf implementations efficiently. · 9f9d9b51
      Tom Lane authored
      When using a C99-compliant vsnprintf, we can use its report of the required
      buffer size to avoid making multiple loops through the formatting logic.
      This is similar to the changes recently made in stringinfo.c, but we can't
      use psprintf.c here because in libpq we don't want to exit() on error.
      (The behavior pqexpbuffer.c has historically used is to mark the
      PQExpBuffer as "broken", ie empty, if it runs into any fatal problem.)
      
      To avoid duplicating code more than necessary, I refactored
      printfPQExpBuffer and appendPQExpBuffer to share a subroutine that's
      very similar to psprintf.c's pvsnprintf in spirit.
      9f9d9b51
    • Tom Lane's avatar
      Suppress -0 in the C field of lines computed by line_construct_pts(). · 43fe90f6
      Tom Lane authored
      It's not entirely clear why some PPC machines are generating -0 here, since
      the underlying computation should be exactly 0 - 0.  Perhaps there's some
      wider-than-nominal-precision calculations happening?  Anyway, the best way
      to avoid platform-dependent results seems to be to explicitly reset -0 to
      regular zero.
      43fe90f6
    • Tom Lane's avatar
      Revert "Tweak "line" test to avoid negative zeros on some platforms" · 1f7a4791
      Tom Lane authored
      This reverts commit a0a546f0.
      It seems better to tweak the code to suppress -0 results during
      line_construct_pts(), which I'll do in the next commit.
      1f7a4791
    • Peter Eisentraut's avatar
    • Tom Lane's avatar
      Ignore SIGSYS during initdb. · 5e1e47c7
      Tom Lane authored
      This prevents the recently-added probe for shm_open() from crashing
      on platforms that are impolite enough to deliver a signal rather than
      returning ENOSYS for an unimplemented kernel call.  At least on the
      one known example (HPUX 10.20), ignoring SIGSYS does result in the
      desired behavior of getting an ENOSYS error return instead.
      
      Per discussion, we might later wish to do this in the backend as well,
      but for now it seems sufficient to do it in initdb.
      5e1e47c7
    • Tom Lane's avatar
      Use improved vsnprintf calling logic in more places. · 3147acd6
      Tom Lane authored
      When we are using a C99-compliant vsnprintf implementation (which should be
      most places, these days) it is worth the trouble to make use of its report
      of how large the buffer needs to be to succeed.  This patch adjusts
      stringinfo.c and some miscellaneous usages in pg_dump to do that, relying
      on the logic recently added in libpgcommon's psprintf.c.  Since these
      places want to know the number of bytes written once we succeed, modify the
      API of pvsnprintf() to report that.
      
      There remains near-duplicate logic in pqexpbuffer.c, but since that code
      is in libpq, psprintf.c's approach of exit()-on-error isn't appropriate
      for use there.  Also note that I didn't bother touching the multitude
      of places that call (v)snprintf without any attempt to provide a resizable
      buffer.
      
      Release-note-worthy incompatibility: the API of appendStringInfoVA()
      changed.  If there's any third-party code that's calling that directly,
      it will need tweaking along the same lines as in this patch.
      
      David Rowley and Tom Lane
      3147acd6
  11. 24 Oct, 2013 5 commits
    • Heikki Linnakangas's avatar
      Increase the number of different values used when seeding random(). · 98c50656
      Heikki Linnakangas authored
      When a backend process is forked, we initialize the system's random number
      generator with srandom(). The seed used is derived from the backend's pid
      and the timestamp. However, we only used the microseconds part of the
      timestamp, and it was XORed with the pid, so the total range of different
      seed values chosen was 0-999999. That's quite limited.
      
      Change the code to also use the seconds part of the timestamp in the seed,
      and shift the microseconds so that all 32 bits of the seed are used.
      
      Honza Horak
      98c50656
    • Heikki Linnakangas's avatar
      Improve documentation of random() function. · 75fdcec1
      Heikki Linnakangas authored
      Move random() and setseed() to a separate table, to have them grouped
      together. Also add a notice that random() is not cryptographically secure.
      
      Original patch by Honza Horak, although I didn't use his version.
      75fdcec1
    • Heikki Linnakangas's avatar
      Plug memory leak when reloading config file. · 138184ad
      Heikki Linnakangas authored
      The absolute path to config file was not pfreed. There are probably more
      small leaks here and there in the config file reload code and assign hooks,
      and in practice no-one reloads the config files frequently enough for it to
      be a problem, but this one is trivial enough that might as well fix it.
      
      Backpatch to 9.3 where the leak was introduced.
      138184ad
    • Heikki Linnakangas's avatar
      bb598456
    • Heikki Linnakangas's avatar
      Fix typos in comments. · 4d6d425a
      Heikki Linnakangas authored
      4d6d425a
  12. 23 Oct, 2013 3 commits
    • Robert Haas's avatar
      Simplify tab completion rules for views and foreign tables. · 5c4dd2cd
      Robert Haas authored
      Since an increasing number of views and foreign tables are now able
      to be updated, complete with any table, view, or foreign table in
      the relevant contexts.  This avoids the need to use a complex
      query that may be both confusing to end-users and nonperformant
      to construct the list of possible completions.
      
      Dean Rasheed, persuant to a complaint from Bernd Helme and a
      suggestion from Peter Eisentraut
      5c4dd2cd
    • Heikki Linnakangas's avatar
      Fix two bugs in setting the vm bit of empty pages. · 83eb5400
      Heikki Linnakangas authored
      Use a critical section when setting the all-visible flag on an empty page,
      and WAL-logging it. log_newpage_buffer() contains an assertion that it
      must be called inside a critical section, and it's the right thing to do
      when modifying a buffer anyway.
      
      Also, the page should be marked dirty before calling log_newpage_buffer(),
      per the comment in log_newpage_buffer() and src/backend/access/transam/README.
      
      Patch by Andres Freund, in response to my report. Backpatch to 9.2, like
      the patch that introduced these bugs (a6370fd9).
      83eb5400
    • Tom Lane's avatar
      Suppress a couple of compiler warnings seen with older gcc versions. · 5f1ab461
      Tom Lane authored
      To wit,
      bgworker.c: In function `RegisterDynamicBackgroundWorker':
      bgworker.c:761: warning: `generation' might be used uninitialized in this function
      dsm_impl.c: In function `dsm_impl_op':
      dsm_impl.c:197: warning: control reaches end of non-void function
      
      Neither of these represent actual bugs, but we may as well tweak the code
      so that more compilers can tell that.  This won't change the generated code
      on compilers that do recognize that the cases are unreachable.
      5f1ab461
  13. 22 Oct, 2013 3 commits
    • Tom Lane's avatar
      Replace pg_asprintf() with psprintf(). · 2c66f992
      Tom Lane authored
      This eliminates an awkward coding pattern that's also unnecessarily
      inconsistent with backend coding.  psprintf() is now the thing to
      use everywhere.
      2c66f992
    • Tom Lane's avatar
      Get rid of use of asprintf() in favor of a more portable implementation. · 09a89cb5
      Tom Lane authored
      asprintf(), aside from not being particularly portable, has a fundamentally
      badly-designed API; the psprintf() function that was added in passing in
      the previous patch has a much better API choice.  Moreover, the NetBSD
      implementation that was borrowed for the previous patch doesn't work with
      non-C99-compliant vsnprintf, which is something we still have to cope with
      on some platforms; and it depends on va_copy which isn't all that portable
      either.  Get rid of that code in favor of an implementation similar to what
      we've used for many years in stringinfo.c.  Also, move it into libpgcommon
      since it's not really libpgport material.
      
      I think this patch will be enough to turn the buildfarm green again, but
      there's still cosmetic work left to do, namely get rid of pg_asprintf()
      in favor of using psprintf().  That will come in a followon patch.
      09a89cb5
    • Peter Eisentraut's avatar
      586a8fc7