1. 12 Jan, 2016 3 commits
    • Tom Lane's avatar
      Remove no-longer-needed old-style check for incompatible plpythons. · 796d1e88
      Tom Lane authored
      Commit 866566a6 introduced a new mechanism for incompatible
      plpythons to detect each other.  I left the old mechanism in place,
      because it seems possible that a plpython predating that commit might be
      used with one postdating it.  (This would require updating plpython3 but
      not plpython2 or vice versa, but that seems well within the realm of
      possibility.)  However, surely it will not be able to happen in 9.6 or
      later, so we can delete the old mechanism in HEAD.
      796d1e88
    • Tom Lane's avatar
      Use LOAD not actual code execution to pull in plpython library. · fb6fcbd3
      Tom Lane authored
      Commit 866566a6 is insufficient to prevent dump/reload failures
      when using transform modules in a database with both plpython2 and
      plpython3 installed.  The reason is that the transform extension scripts
      use DO blocks as a mechanism to pull in the libpython library before
      creating the transform function.  It's necessary to preload the library
      because the dynamic loader won't do it for us on every platform, leading
      to "unresolved symbol" failures when the transform library is loaded.
      But it's *not* necessary to execute Python code, and doing so will
      provoke a multiple-Pythons-are-loaded error even after the preceding
      commit.
      
      To fix, use LOAD instead of a DO block.  That requires superuser privilege,
      but creation of a C function does anyway.  It also embeds knowledge of
      the underlying library name for each PL language; but that's wired into
      the initdb-time contents of pg_pltemplate too, so that doesn't seem like
      a large problem either.  Note that CREATE TRANSFORM as such doesn't call
      the language module at all.
      
      Per a report from Paul Jones.  Back-patch to 9.5 where transform modules
      were introduced.
      fb6fcbd3
    • Tom Lane's avatar
      Avoid dump/reload problems when using both plpython2 and plpython3. · 866566a6
      Tom Lane authored
      Commit 80371601 installed a safeguard against loading plpython2
      and plpython3 at the same time, but asserted that both could still be
      used in the same database, just not in the same session.  However, that's
      not actually all that practical because dumping and reloading will fail
      (since both libraries necessarily get loaded into the restoring session).
      pg_upgrade is even worse, because it checks for missing libraries by
      loading every .so library mentioned in the entire installation into one
      session, so that you can have only one across the whole cluster.
      
      We can improve matters by not throwing the error immediately in _PG_init,
      but only when and if we're asked to do something that requires calling
      into libpython.  This ameliorates both of the above situations, since
      while execution of CREATE LANGUAGE, CREATE FUNCTION, etc will result in
      loading plpython, it isn't asked to do anything interesting (at least
      not if check_function_bodies is off, as it will be during a restore).
      
      It's possible that this opens some corner-case holes in which a crash
      could be provoked with sufficient effort.  However, since plpython
      only exists as an untrusted language, any such crash would require
      superuser privileges, making it "don't do that" not a security issue.
      To reduce the hazards in this area, the error is still FATAL when it
      does get thrown.
      
      Per a report from Paul Jones.  Back-patch to 9.2, which is as far back
      as the patch applies without work.  (It could be made to work in 9.1,
      but given the lack of previous complaints, I'm disinclined to expend
      effort so far back.  We've been pretty desultory about support for
      Python 3 in 9.1 anyway.)
      866566a6
  2. 11 Jan, 2016 2 commits
  3. 09 Jan, 2016 7 commits
    • Tom Lane's avatar
      Remove a useless PG_GETARG_DATUM() call from jsonb_build_array. · 820bdccc
      Tom Lane authored
      This loop uselessly fetched the argument after the one it's currently
      looking at.  No real harm is done since we couldn't possibly fetch off
      the end of memory, but it's confusing to the reader.
      
      Also remove a duplicate (and therefore confusing) PG_ARGISNULL check in
      jsonb_build_object.
      
      I happened to notice these things while trolling for missed null-arg
      checks earlier today.  Back-patch to 9.5, not because there is any
      real bug, but just because 9.5 and HEAD are still in sync in this
      file and we might as well keep them so.
      
      In passing, re-pgindent.
      820bdccc
    • Tom Lane's avatar
      Add some checks on "char"-type columns to type_sanity and opr_sanity. · 3ef16c46
      Tom Lane authored
      I noticed that the sanity checks in the regression tests omitted to
      check a couple of "poor man's enum" columns that you'd reasonably
      expect them to check.
      
      There are other "char"-type columns in system catalogs that are not
      covered by either type_sanity or opr_sanity, e.g. pg_rewrite.ev_type.
      However, those catalogs are not populated with any manually-created
      data during bootstrap, so it seems less necessary to check them
      this way.
      3ef16c46
    • Tom Lane's avatar
      Clean up some lack-of-STRICT issues in the core code, too. · 26d538dc
      Tom Lane authored
      A scan for missed proisstrict markings in the core code turned up
      these functions:
      
      brin_summarize_new_values
      pg_stat_reset_single_table_counters
      pg_stat_reset_single_function_counters
      pg_create_logical_replication_slot
      pg_create_physical_replication_slot
      pg_drop_replication_slot
      
      The first three of these take OID, so a null argument will normally look
      like a zero to them, resulting in "ERROR: could not open relation with OID
      0" for brin_summarize_new_values, and no action for the pg_stat_reset_XXX
      functions.  The other three will dump core on a null argument, though this
      is mitigated by the fact that they won't do so until after checking that
      the caller is superuser or has rolreplication privilege.
      
      In addition, the pg_logical_slot_get/peek[_binary]_changes family was
      intentionally marked nonstrict, but failed to make nullness checks on all
      the arguments; so again a null-pointer-dereference crash is possible but
      only for superusers and rolreplication users.
      
      Add the missing ARGISNULL checks to the latter functions, and mark the
      former functions as strict in pg_proc.  Make that change in the back
      branches too, even though we can't force initdb there, just so that
      installations initdb'd in future won't have the issue.  Since none of these
      bugs rise to the level of security issues (and indeed the pg_stat_reset_XXX
      functions hardly misbehave at all), it seems sufficient to do this.
      
      In addition, fix some order-of-operations oddities in the slot_get_changes
      family, mostly cosmetic, but not the part that moves the function's last
      few operations into the PG_TRY block.  As it stood, there was significant
      risk for an error to exit without clearing historical information from
      the system caches.
      
      The slot_get_changes bugs go back to 9.4 where that code was introduced.
      Back-patch appropriate subsets of the pg_proc changes into all active
      branches, as well.
      26d538dc
    • Tom Lane's avatar
      Clean up code for widget_in() and widget_out(). · 1cb63c79
      Tom Lane authored
      Given syntactically wrong input, widget_in() could call atof() with an
      indeterminate pointer argument, typically leading to a crash; or if it
      didn't do that, it might return a NULL pointer, which again would lead
      to a crash since old-style C functions aren't supposed to do things
      that way.  Fix that by correcting the off-by-one syntax test and
      throwing a proper error rather than just returning NULL.
      
      Also, since widget_in and widget_out have been marked STRICT for a
      long time, their tests for null inputs are just dead code; remove 'em.
      In the oldest branches, also improve widget_out to use snprintf not
      sprintf, just to be sure.
      
      In passing, get rid of a long-since-useless sprintf into a local buffer
      that nothing further is done with, and make some other minor coding
      style cleanups.
      
      In the intended regression-testing usage of these functions, none of
      this is very significant; but if the regression test database were
      left around in a production installation, these bugs could amount
      to a minor security hazard.
      
      Piotr Stefaniak, Michael Paquier, and Tom Lane
      1cb63c79
    • Simon Riggs's avatar
      Revoke change to rmgr desc of btree vacuum · b6028426
      Simon Riggs authored
      Per discussion with Andres Freund
      b6028426
    • Tom Lane's avatar
      Add STRICT to some C functions created by the regression tests. · 529baf6a
      Tom Lane authored
      These functions readily crash when passed a NULL input value.  The tests
      themselves do not pass NULL values to them; but when the regression
      database is used as a basis for fuzz testing, they cause a lot of noise.
      Also, if someone were to leave a regression database lying about in a
      production installation, these would create a minor security hazard.
      
      Andreas Seltenreich
      529baf6a
    • Simon Riggs's avatar
      Avoid pin scan for replay of XLOG_BTREE_VACUUM · 687f2cd7
      Simon Riggs authored
      Replay of XLOG_BTREE_VACUUM during Hot Standby was previously thought to require
      complex interlocking that matched the requirements on the master. This required
      an O(N) operation that became a significant problem with large indexes, causing
      replication delays of seconds or in some cases minutes while the
      XLOG_BTREE_VACUUM was replayed.
      
      This commit skips the “pin scan” that was previously required, by observing in
      detail when and how it is safe to do so, with full documentation. The pin scan
      is skipped only in replay; the VACUUM code path on master is not touched here.
      
      The current commit still performs the pin scan for toast indexes, though this
      can also be avoided if we recheck scans on toast indexes. Later patch will
      address this.
      
      No tests included. Manual tests using an additional patch to view WAL records
      and their timing have shown the change in WAL records and their handling has
      successfully reduced replication delay.
      687f2cd7
  4. 08 Jan, 2016 6 commits
    • Alvaro Herrera's avatar
      Revert "Blind attempt at a Cygwin fix" · 46317211
      Alvaro Herrera authored
      This reverts commit e9282e95, which blew
      up in a pretty spectacular way.  Re-introduce the original code while we
      search for a real fix.
      46317211
    • Alvaro Herrera's avatar
      Blind attempt at a Cygwin fix · e9282e95
      Alvaro Herrera authored
      Further portability fix for a9676139.  Mingw- and MSVC-based builds
      appear to be working fine, but Cygwin needs an extra tweak whereby the
      new win32security.c file is explicitely added to the list of files to
      build in pgport, per Cygwin members brolga and lorikeet.
      
      Author: Michael Paquier
      e9282e95
    • Magnus Hagander's avatar
      Fix typo in comment · 2650486e
      Magnus Hagander authored
      Tatsuro Yamada
      2650486e
    • Magnus Hagander's avatar
      Remove reundand include of TestLib · c662ef1d
      Magnus Hagander authored
      Kyotaro HORIGUCHI
      c662ef1d
    • Tom Lane's avatar
      Marginal cleanup of GROUPING SETS code in grouping_planner(). · a54676ac
      Tom Lane authored
      Improve comments and make it a shade less messy.  I think we might want
      to move all of this somewhere else later, but it needs to be more
      readable first.
      
      In passing, re-pgindent the file, affecting some recently-added comments
      concerning parallel query planning.
      a54676ac
    • Tom Lane's avatar
      Delay creation of subplan tlist until after create_plan(). · c44d0138
      Tom Lane authored
      Once upon a time it was necessary for grouping_planner() to determine
      the tlist it wanted from the scan/join plan subtree before it called
      query_planner(), because query_planner() would actually make a Plan using
      that.  But we refactored things a long time ago to delay construction of
      the Plan tree till later, so there's no need to build that tlist until
      (and indeed unless) we're ready to plaster it onto the Plan.  The only
      thing query_planner() cares about is what Vars are going to be needed for
      the tlist, and it can perfectly well get that by looking at the real tlist
      rather than some masticated version.
      
      Well, actually, there is one minor glitch in that argument, which is that
      make_subplanTargetList also adds Vars appearing only in HAVING to the
      tlist it produces.  So now we have to account for HAVING explicitly in
      build_base_rel_tlists.  But that just adds a few lines of code, and
      I doubt it moves the needle much on processing time; we might be doing
      pull_var_clause() twice on the havingQual, but before we had it scanning
      dummy tlist entries instead.
      
      This is a very small down payment on rationalizing grouping_planner
      enough so it can be refactored.
      c44d0138
  5. 07 Jan, 2016 8 commits
    • Alvaro Herrera's avatar
      Fix order of arguments to va_start() · f81c966d
      Alvaro Herrera authored
      f81c966d
    • Tom Lane's avatar
      Fix unobvious interaction between -X switch and subdirectory creation. · b41fb650
      Tom Lane authored
      Turns out the only reason initdb -X worked is that pg_mkdir_p won't
      whine if you point it at something that's a symlink to a directory.
      Otherwise, the attempt to create pg_xlog/ just like all the other
      subdirectories would have failed.  Let's be a little more explicit
      about what's happening.  Oversight in my patch for bug #13853
      (mea culpa for not testing -X ...)
      b41fb650
    • Alvaro Herrera's avatar
      Add win32security to LIBOBJS · fa838b55
      Alvaro Herrera authored
      This seems to fix Mingw's compile that was broken in a9676139, as
      evidenced by buildfarm.
      fa838b55
    • Tom Lane's avatar
      Use plain mkdir() not pg_mkdir_p() to create subdirectories of PGDATA. · 33b054bc
      Tom Lane authored
      When we're creating subdirectories of PGDATA during initdb, we know darn
      well that the parent directory exists (or should exist) and that the new
      subdirectory doesn't (or shouldn't).  There is therefore no need to use
      anything more complicated than mkdir().  Using pg_mkdir_p() just opens us
      up to unexpected failure modes, such as the one exhibited in bug #13853
      from Nuri Boardman.  It's not very clear why pg_mkdir_p() went wrong there,
      but it is clear that we didn't need to be trying to create parent
      directories in the first place.  We're not even saving any code, as proven
      by the fact that this patch nets out at minus five lines.
      
      Since this is a response to a field bug report, back-patch to all branches.
      33b054bc
    • Alvaro Herrera's avatar
      pgstat: add WAL receiver status view & SRF · b1a9bad9
      Alvaro Herrera authored
      This new view provides insight into the state of a running WAL receiver
      in a HOT standby node.
      The information returned includes the PID of the WAL receiver process,
      its status (stopped, starting, streaming, etc), start LSN and TLI, last
      received LSN and TLI, timestamp of last message send and receipt, latest
      end-of-WAL LSN and time, and the name of the slot (if any).
      
      Access to the detailed data is only granted to superusers; others only
      get the PID.
      
      Author: Michael Paquier
      Reviewer: Haribabu Kommi
      b1a9bad9
    • Tom Lane's avatar
      Remove vestigial CHECK_FOR_INTERRUPTS call. · 6b1a837f
      Tom Lane authored
      Commit e710b65c inserted code in md5_crypt_verify to disable and later
      re-enable interrupts, with a CHECK_FOR_INTERRUPTS call as part of the
      second step, to process any interrupts that had been held off.  Commit
      6647248e removed the interrupt disable/re-enable code, but left behind
      the CHECK_FOR_INTERRUPTS, even though this is now an entirely random,
      pointless place for one.  md5_crypt_verify doesn't run long enough to
      need such a check, and if it did, this would still be the wrong place
      to put one.
      6b1a837f
    • Tom Lane's avatar
      Provide more detail in postmaster log for password authentication failures. · 5e0b5dca
      Tom Lane authored
      We tell people to examine the postmaster log if they're unsure why they are
      getting auth failures, but actually only a few relatively-uncommon failure
      cases were given their own log detail messages in commit 64e43c59.
      Expand on that so that every failure case detected within md5_crypt_verify
      gets a specific log detail message.  This should cover pretty much every
      ordinary password auth failure cause.
      
      So far I've not noticed user demand for a similar level of auth detail
      for the other auth methods, but sooner or later somebody might want to
      work on them.  This is not that patch, though.
      5e0b5dca
    • Alvaro Herrera's avatar
      Windows: Make pg_ctl reliably detect service status · a9676139
      Alvaro Herrera authored
      pg_ctl is using isatty() to verify whether the process is running in a
      terminal, and if not it sends its output to Windows' Event Log ... which
      does the wrong thing when the output has been redirected to a pipe, as
      reported in bug #13592.
      
      To fix, make pg_ctl use the code we already have to detect service-ness:
      in the master branch, move src/backend/port/win32/security.c to src/port
      (with suitable tweaks so that it runs properly in backend and frontend
      environments); pg_ctl already has access to pgport so it Just Works.  In
      older branches, that's likely to cause trouble, so instead duplicate the
      required code in pg_ctl.c.
      
      Author: Michael Paquier
      Bug report and diagnosis: Egon Kocjan
      Backpatch: all supported branches
      a9676139
  6. 06 Jan, 2016 2 commits
    • Tom Lane's avatar
      In initdb's post-bootstrap phase, drop temp tables explicitly. · dad08994
      Tom Lane authored
      Although these temp tables will get removed from template1 at the end of
      the standalone-backend run, that's too late to keep them from getting
      copied into the template0 and postgres databases, now that we use only a
      single backend run for the whole sequence.  While no real harm is done
      by the extra copies (since they'd be deleted on first use of the temp
      schema), it's still unsightly, and it would mean some wasted cycles for
      every database creation for the life of the installation.
      
      Oversight in commit c4a8812c.  Noticed by Amit Langote.
      dad08994
    • Tom Lane's avatar
      Comment typo fix. · 4bf87169
      Tom Lane authored
      Per Amit Langote.
      4bf87169
  7. 05 Jan, 2016 11 commits
    • Tatsuo Ishii's avatar
      Fix typo in create_transform.sgml. · 65681d08
      Tatsuo Ishii authored
      65681d08
    • Alvaro Herrera's avatar
      Add scale(numeric) · abb17339
      Alvaro Herrera authored
      Author: Marko Tiikkaja
      abb17339
    • Tom Lane's avatar
      Remove some ancient and unmaintained encoding-conversion test cruft. · 419400c5
      Tom Lane authored
      In commit 92119191 I claimed that we weren't testing encoding
      conversion functions, but further poking around reveals that we did
      have an equivalent though hard-wired set of tests in conversion.sql.
      AFAICS there is no advantage to doing it like that as compared to letting
      the catalog contents drive the test, so let the opr_sanity addition stand
      and remove the now-redundant tests in conversion.sql.
      
      Also, remove some infrastructure in src/backend/utils/mb/conversion_procs
      for building conversion.sql's list of tests.  That was unmaintained, and
      had not corresponded to the actual contents of conversion.sql since 2007
      or perhaps even further back.
      419400c5
    • Tom Lane's avatar
      Sort $(wildcard) output where needed for reproducible build output. · 3343ea9e
      Tom Lane authored
      The order of inclusion of .o files makes a difference in linker output;
      not a functional difference, but still a bitwise difference, which annoys
      some packagers who would like reproducible builds.
      
      Report and patch by Christoph Berg
      3343ea9e
    • Alvaro Herrera's avatar
      Make pg_receivexlog silent with 9.3 and older servers · 4aecd22d
      Alvaro Herrera authored
      A pointless and confusing error message is shown to the user when
      attempting to identify a 9.3 or older remote server with a 9.5/9.6
      pg_receivexlog, because the return signature of IDENTIFY_SYSTEM was
      changed in 9.4.  There's no good reason for the warning message, so
      shuffle code around to keep it quiet.
      
      (pg_recvlogical is also affected by this commit, but since it obviously
      cannot work with 9.3 that doesn't actually matter much.)
      
      Backpatch to 9.5.
      
      Reported by Marco Nenciarini, who also wrote the initial patch.  Further
      tweaked by Robert Haas and Fujii Masao; reviewed by Michael Paquier and
      Craig Ringer.
      4aecd22d
    • Tom Lane's avatar
      In opr_sanity regression test, check for unexpected uses of cstring. · 92119191
      Tom Lane authored
      In light of commit ea0d494d, it seems like a good idea to add
      a regression test that will complain about random functions taking or
      returning cstring.  Only I/O support functions and encoding conversion
      functions should be declared that way.
      
      While at it, add some checks that encoding conversion functions are
      declared properly.  Since pg_conversion isn't populated manually,
      it's not quite as necessary to check its contents as it is for catalogs
      like pg_proc; but one thing we definitely have not tested in the past
      is whether the identified conproc for a conversion actually does that
      conversion vs. some other one.
      92119191
    • Tom Lane's avatar
      Make the to_reg*() functions accept text not cstring. · ea0d494d
      Tom Lane authored
      Using cstring as the input type was a poor decision, because that's not
      really a full-fledged type.  In particular, it lacks implicit coercions
      from text or varchar, meaning that usages like to_regproc('foo'||'bar')
      wouldn't work; basically the only case that did work without explicit
      casting was a simple literal constant argument.
      
      The lack of field complaints about this suggests that hardly anyone
      is using these functions, so hopefully fixing it won't cause much of
      a compatibility problem.  They've only been there since 9.4, anyway.
      
      Petr Korobeinikov
      ea0d494d
    • Alvaro Herrera's avatar
      Make pg_shseclabel available in early backend startup · efa318bc
      Alvaro Herrera authored
      While the in-core authentication mechanism doesn't need to access
      pg_shseclabel at all, it's reasonable to think that an authentication
      hook will want to look at the label for the role logging in, or for rows
      in other catalogs used during the authentication phase of startup.
      
      Catalog version bumped, because this changes the "is nailed" status for
      pg_shseclabel.
      
      Author: Adam Brightwell
      efa318bc
    • Tom Lane's avatar
      Add to_regnamespace() and to_regrole() to the documentation. · 83be1844
      Tom Lane authored
      Commits cb9fa802 and 0c90f676 added these functions,
      but did not bother with documentation.
      83be1844
    • Tom Lane's avatar
      Convert psql's tab completion for backslash commands to the new style. · 4f18010a
      Tom Lane authored
      This requires adding some more infrastructure to handle both case-sensitive
      and case-insensitive matching, as well as the ability to match a prefix of
      a previous word.  So it ends up being about a wash line-count-wise, but
      it's just as big a readability win here as in the SQL tab completion rules.
      
      Michael Paquier, some adjustments by me
      4f18010a
    • Tom Lane's avatar
      In psql's tab completion, change most TailMatches patterns to Matches. · 9b181b03
      Tom Lane authored
      In the refactoring in commit d37b816d,
      we mostly kept to the original design whereby only the last few words
      on the line were matched to identify a completable pattern.  However,
      after commit d854118c, there's really
      no reason to do it like that: where it's sensible, we can use patterns
      that expect to match the entire input line.  And mostly, it's sensible.
      Matching the entire line greatly reduces the odds of a false match that
      leads to offering irrelevant completions.  Moreover (though I've not
      tried to measure this), it should make tab completion faster since
      many of the patterns will be discarded after a single integer comparison
      that finds that the wrong number of words appear on the line.
      
      There are certain identifiable places where we still need to use
      TailMatches because the statement in question is allowed to appear
      embedded in a larger statement.  These are just a small minority of
      the existing patterns, though, so the benefit of switching where
      possible is large.
      
      It's possible that this patch has removed some within-line matching
      behaviors that are in fact desirable, but we can put those back when
      we get complaints.  Most of the removed behaviors are certainly silly.
      
      Michael Paquier, with some further adjustments by me
      9b181b03
  8. 04 Jan, 2016 1 commit
    • Tom Lane's avatar
      Docs: provide a concrete discussion and example for RLS race conditions. · 7debf360
      Tom Lane authored
      Commit 43cd468c added some wording to create_policy.sgml purporting
      to warn users against a race condition of the sort that had been noted some
      time ago by Peter Geoghegan.  However, that warning was far too vague to be
      useful (or at least, I completely failed to grasp what it was on about).
      Since the problem case occurs with a security design pattern that lots of
      people are likely to try to use, we need to be as clear as possible about
      it.  Provide a concrete example in the main-line docs in place of the
      original warning.
      7debf360