1. 01 Feb, 2017 7 commits
    • Tom Lane's avatar
      Provide CatalogTupleDelete() as a wrapper around simple_heap_delete(). · ab028965
      Tom Lane authored
      This extends the work done in commit 2f5c9d9c to provide a more nearly
      complete abstraction layer hiding the details of index updating for catalog
      changes.  That commit only invented abstractions for catalog inserts and
      updates, leaving nearby code for catalog deletes still calling the
      heap-level routines directly.  That seems rather ugly from here, and it
      does little to help if we ever want to shift to a storage system in which
      indexing work is needed at delete time.
      
      Hence, create a wrapper function CatalogTupleDelete(), and replace calls
      of simple_heap_delete() on catalog tuples with it.  There are now very
      few direct calls of [simple_]heap_delete remaining in the tree.
      
      Discussion: https://postgr.es/m/462.1485902736@sss.pgh.pa.us
      ab028965
    • Robert Haas's avatar
      Refactor other replication commands to use DestRemoteSimple. · bbd8550b
      Robert Haas authored
      Commit a84069d9 added a new type of
      DestReceiver to avoid duplicating the existing code for the SHOW
      command, but it turns out we can leverage that new DestReceiver
      type in a few more places, saving some code.
      
      Michael Paquier, reviewed by Andres Freund and by me.
      
      Discussion: http://postgr.es/m/CAB7nPqSdFOQC0evc0r1nJeQyGBqjBrR41MC4rcMqUUpoJaZbtQ%40mail.gmail.com
      Discussion: http://postgr.es/m/CAB7nPqT2K4XFT1JgqufFBjsOc-NUKXg5qBDucHPMbk6Xi1kYaA@mail.gmail.com
      bbd8550b
    • Tom Lane's avatar
      Make psql's \set display variables in alphabetical order. · c3e3844a
      Tom Lane authored
      "\set" with no arguments displays all defined variables, but it does so
      in the order that they appear in variables.c's list, which previously
      was mostly creation order.  That makes the list ugly and hard to find
      things in, and it exposes some psql implementation details to users.
      (For instance, ordinary variables will move to the bottom of the list
      if unset and set again, but variables that have hooks won't.)
      
      Fix that by keeping the list in alphabetical order at all times, which
      isn't much more complicated than breaking out of the insertion search
      loops once we reach an entry that should be after the one to be inserted.
      
      Discussion: https://postgr.es/m/31785.1485900786@sss.pgh.pa.us
      c3e3844a
    • Tom Lane's avatar
      Improve psql's behavior for \set and \unset of its control variables. · 86322dc7
      Tom Lane authored
      This commit improves on the results of commit 511ae628 in two ways:
      
      1. It restores the historical behavior that "\set FOO" is interpreted
      as setting FOO to "on", if FOO is a boolean control variable.  We
      already found one test script that was expecting that behavior, and
      the psql documentation certainly does nothing to discourage people
      from assuming that would work, since it often says just "if FOO is set"
      when describing the effects of a boolean variable.  However, now this
      case will result in actually setting FOO to "on", not an empty string.
      
      2. It arranges for an "\unset" of a control variable to set the value
      back to its default value, rather than becoming apparently undefined.
      The control variables are also initialized that way at psql startup.
      
      In combination, these things guarantee that a control variable always
      has a displayable value that reflects what psql is actually doing.
      That is a pretty substantial usability improvement.
      
      The implementation involves adding a second type of variable hook function
      that is able to replace a proposed new value (including NULL) with another
      one.  We could alternatively have complicated the API of the assign hook,
      but this way seems better since many variables can share the same
      substitution hook function.
      
      Also document the actual behavior of these variables more fully,
      including covering assorted behaviors that were there before but
      never documented.
      
      This patch also includes some minor cleanup that should have been in
      511ae628 but was missed.
      
      Patch by me, but it owes a lot to discussions with Daniel Vérité.
      
      Discussion: https://postgr.es/m/9572.1485821620@sss.pgh.pa.us
      86322dc7
    • Heikki Linnakangas's avatar
      Replace isMD5() with a more future-proof way to check if pw is encrypted. · dbd69118
      Heikki Linnakangas authored
      The rule is that if pg_authid.rolpassword begins with "md5" and has the
      right length, it's an MD5 hash, otherwise it's a plaintext password. The
      idiom has been to use isMD5() to check for that, but that gets awkward,
      when we add new kinds of verifiers, like the verifiers for SCRAM
      authentication in the pending SCRAM patch set. Replace isMD5() with a new
      get_password_type() function, so that when new verifier types are added, we
      don't need to remember to modify every place that currently calls isMD5(),
      to also recognize the new kinds of verifiers.
      
      Also, use the new plain_crypt_verify function in passwordcheck, so that it
      doesn't need to know about MD5, or in the future, about other kinds of
      hashes or password verifiers.
      
      Reviewed by Michael Paquier and Peter Eisentraut.
      
      Discussion: https://www.postgresql.org/message-id/2d07165c-1793-e243-a2a9-e45b624c7580@iki.fi
      dbd69118
    • Heikki Linnakangas's avatar
      Don't create "holes" in BufFiles, in the new logtape code. · 7ac4a389
      Heikki Linnakangas authored
      The "Simplify tape block format" commit ignored the rule that blocks
      returned by ltsGetFreeBlock() must be written out in the same order, at
      least in the first write pass. To fix, relax that requirement, by making
      ltsWriteBlock() to detect if it's about to create a "hole" in the
      underlying BufFile, and fill it with zeros instead.
      
      Reported, analysed, and reviewed by Peter Geoghegan.
      
      Discussion: https://www.postgresql.org/message-id/CAM3SWZRWdNtkhiG0GyiX_1mUAypiK3dV6-6542pYe2iEL-foTA@mail.gmail.com
      7ac4a389
    • Heikki Linnakangas's avatar
      Small fixes to the Perl scripts to create unicode conversion tables. · bc1686f3
      Heikki Linnakangas authored
      Add missing semicolons in UCS_to_* perl scripts.
      For consistency, use "$hashref->{key}" style everywhere.
      
      Kyotaro Horiguchi
      
      Discussion: https://www.postgresql.org/message-id/20170130.153738.139030994.horiguchi.kyotaro@lab.ntt.co.jp
      bc1686f3
  2. 31 Jan, 2017 5 commits
    • Robert Haas's avatar
      Move comment about test slightly closer to test. · 8a815e3f
      Robert Haas authored
      The addition of a TestForOldSnapshot() call here has made the
      referent of this comment slightly less clear, so move the comment
      to compensate.
      
      Amit Kapila (as part of the parallel index scan patch)
      8a815e3f
    • Alvaro Herrera's avatar
      Tweak catalog indexing abstraction for upcoming WARM · 2f5c9d9c
      Alvaro Herrera authored
      Split the existing CatalogUpdateIndexes into two different routines,
      CatalogTupleInsert and CatalogTupleUpdate, which do both the heap
      insert/update plus the index update.  This removes over 300 lines of
      boilerplate code all over src/backend/catalog/ and src/backend/commands.
      The resulting code is much more pleasing to the eye.
      
      Also, by encapsulating what happens in detail during an UPDATE, this
      facilitates the upcoming WARM patch, which is going to add a few more
      lines to the update case making the boilerplate even more boring.
      
      The original CatalogUpdateIndexes is removed; there was only one use
      left, and since it's just three lines, we can as well expand it in place
      there.  We could keep it, but WARM is going to break all the UPDATE
      out-of-core callsites anyway, so there seems to be no benefit in doing
      so.
      
      Author: Pavan Deolasee
      Discussion: https://www.postgr.es/m/CABOikdOcFYSZ4vA2gYfs=M2cdXzXX4qGHeEiW3fu9PCfkHLa2A@mail.gmail.com
      2f5c9d9c
    • Stephen Frost's avatar
      pg_dump: Fix handling of ALTER DEFAULT PRIVILEGES · e2090d9d
      Stephen Frost authored
      In commit 23f34fa4, we changed how ACLs were handled to use the new
      pg_init_privs catalog and to dump out the ACL commands as REVOKE+GRANT
      combinations instead of trying to REVOKE all rights always and then
      GRANT back just the ones which were in place.
      
      Unfortunately, the DEFAULT PRIVILEGES system didn't quite get the
      correct treatment with this change and ended up (incorrectly) only
      including positive GRANTs instead of both the REVOKEs and GRANTs
      necessary to preserve the correct privileges.
      
      There are only a couple cases where such REVOKEs are possible because,
      generally speaking, there's few rights which exist on objects by
      default to be revoked.
      
      Examples of REVOKEs which weren't being correctly preserved are when
      privileges are REVOKE'd from the creator/owner, like so:
      
      ALTER DEFAULT PRIVILEGES
        FOR ROLE myrole
        REVOKE SELECT ON TABLES FROM myrole;
      
      or when other default privileges are being revoked, such as EXECUTE
      rights granted to public for functions:
      
      ALTER DEFAULT PRIVILEGES
        FOR ROLE myrole
        REVOKE EXECUTE ON FUNCTIONS FROM PUBLIC;
      
      Fix this by correctly working out what the correct REVOKE statements are
      (if any) and dump them out, just as we do for everything else.
      
      Noticed while developing additional regression tests for pg_dump, which
      will be landing shortly.
      
      Back-patch to 9.6 where the bug was introduced.
      e2090d9d
    • Stephen Frost's avatar
      perltidy pg_dump TAP tests · 6af8b89a
      Stephen Frost authored
      The pg_dump TAP tests have gotten pretty far from what perltidy thinks
      they should be, so fix that, and in passing use long-form argument names
      with arguments passed via "=" in a similar vein to 58da8334.
      
      No functional changes here, just whitespace and changing runs from
      "-f" to "--file=", and similar.
      6af8b89a
    • Stephen Frost's avatar
      test_pg_dump: perltidy cleanup · 58da8334
      Stephen Frost authored
      As pointed out by Alvaro, we actually use perltidy on the perl scripts
      in the source tree, so go back to the results of a perltidy run for the
      test_pg_dump TAP script.
      
      To make it look slightly less tragic, I changed most of the independent
      arguments into long-form single arguments (eg: -f file.sql changed to be
      --file=file.sql) to avoid having them confusingly split across lines due
      to perltidy.
      
      Back-patch to 9.6, as the last patch was.
      58da8334
  3. 30 Jan, 2017 10 commits
    • Tom Lane's avatar
      Simplify some long-obsolete code in hba.c's next_token(). · 1e5a5d03
      Tom Lane authored
      next_token() oddly set its buffer space consumption limit to one before
      the last char position in the buffer, not the last as you'd expect.
      The reason is there was once an ugly kluge to mark keywords by appending
      a newline to them, potentially requiring one more byte.  Commit e5e2fc84
      removed that kluge, but failed to notice that the length limit could be
      increased.
      
      Also, remove some vestigial handling of newline characters in the buffer.
      That was left over from when this function read the file directly using
      getc().  Commit 7f49a67f changed it to read from a buffer, from which
      tokenize_file had already removed the only possible occurrence of newline,
      but did not simplify this function in consequence.
      
      Also, ensure that we don't return with *lineptr set to someplace past the
      terminating '\0'; that would be catastrophic if a caller were to ask for
      another token from the same line.  This is just latent since no callers
      actually do call again after a "false" return; but considering that it was
      actually costing us extra code to do it wrong, we might as well make it
      bulletproof.
      
      Noted while reviewing pg_hba_file_rules patch.
      1e5a5d03
    • Tom Lane's avatar
      Invent pg_hba_file_rules view to show the content of pg_hba.conf. · de16ab72
      Tom Lane authored
      This view is designed along the same lines as pg_file_settings, to wit
      it shows what is currently in the file, not what the postmaster has
      loaded as the active settings.  That allows it to be used to pre-vet
      edits before issuing SIGHUP.  As with the earlier view, go out of our
      way to allow errors in the file to be reflected in the view, to assist
      that use-case.
      
      (We might at some point invent a view to show the current active settings,
      but this is not that patch; and it's not trivial to do.)
      
      Haribabu Kommi, reviewed by Ashutosh Bapat, Michael Paquier, Simon Riggs,
      and myself
      
      Discussion: https://postgr.es/m/CAJrrPGerH4jiwpcXT1-46QXUDmNp2QDrG9+-Tek_xC8APHShYw@mail.gmail.com
      de16ab72
    • Tom Lane's avatar
      Add a regression test script dedicated to exercising system views. · d002f16c
      Tom Lane authored
      Quite a few of our built-in system views were not exercised anywhere
      in the regression tests.  This is perhaps not so exciting for the ones
      that are simple projections/joins of system catalogs, but for the ones
      that are wrappers for set-returning C functions, the omission translates
      directly to lack of test coverage for those functions.
      
      In many cases, the reason for the omission is that the view doesn't have
      much to do with any specific SQL feature, so there's no natural place to
      test it.  To remedy that, invent a new script sysviews.sql that's dedicated
      to testing SRF-based views.  Move a couple of tests that did fit this
      charter into the new script, and add simple "count(*)" based tests of
      other views within the charter.  That's enough to ensure we at least
      exercise the main code path through the SRF, although it does little to
      prove that the output is sane.
      
      More could be done here, no doubt, and I hope someone will think about
      how we can test these views more thoroughly.  But this is a starting
      point.
      
      Discussion: https://postgr.es/m/19359.1485723741@sss.pgh.pa.us
      d002f16c
    • Tom Lane's avatar
      Make psql reject attempts to set special variables to invalid values. · 511ae628
      Tom Lane authored
      Previously, if the user set a special variable such as ECHO to an
      unrecognized value, psql would bleat but store the new value anyway, and
      then fall back to a default setting for the behavior controlled by the
      variable.  This was agreed to be a not particularly good idea.  With
      this patch, invalid values result in an error message and no change in
      state.
      
      (But this applies only to variables that affect psql's behavior; purely
      informational variables such as ENCODING can still be set to random
      values.)
      
      To do this, modify the API for psql's assign-hook functions so that they
      can return an OK/not OK result, and give them the responsibility for
      printing error messages when they reject a value.  Adjust the APIs for
      ParseVariableBool and ParseVariableNum to support the new behavior
      conveniently.
      
      In passing, document the variable VERSION, which had somehow escaped that.
      And improve the quite-inadequate commenting in psql/variables.c.
      
      Daniel Vérité, reviewed by Rahila Syed, some further tweaking by me
      
      Discussion: https://postgr.es/m/7356e741-fa59-4146-a8eb-cf95fd6b21fb@mm
      511ae628
    • Peter Eisentraut's avatar
      Fix sequence test in cs_CZ locale · 46aae594
      Peter Eisentraut authored
      Rename some objects so that sorted output becomes less locale-dependent.
      46aae594
    • Peter Eisentraut's avatar
      d711532b
    • Tom Lane's avatar
      Update time zone data files to tzdata release 2016j. · 308d8682
      Tom Lane authored
      DST law changes in northern Cyprus (new zone Asia/Famagusta), Russia (new
      zone Europe/Saratov), Tonga, Antarctica/Casey.  Historical corrections for
      Asia/Aqtau, Asia/Atyrau, Asia/Gaza, Asia/Hebron, Italy, Malta.  Replace
      invented zone abbreviation "TOT" for Tonga with numeric UTC offset; but
      as in the past, we'll keep accepting "TOT" for input.
      308d8682
    • Heikki Linnakangas's avatar
      Remove leftover reference to "indirect blocks" in comment. · dbeca61c
      Heikki Linnakangas authored
      Peter Geoghegan
      dbeca61c
    • Stephen Frost's avatar
      Handle ALTER EXTENSION ADD/DROP with pg_init_privs · e54f7572
      Stephen Frost authored
      In commit 6c268df1, pg_init_privs was added to track the initial
      privileges of catalog objects and extensions.  Unfortunately, that
      commit didn't include understanding of ALTER EXTENSION ADD/DROP, which
      allows the objects associated with an extension to be changed after the
      initial CREATE EXTENSION script has been run.
      
      The result of this meant that ACLs for objects added through
      ALTER EXTENSION ADD were not recorded into pg_init_privs and we would
      end up including those ACLs in pg_dump when we shouldn't have.
      
      This commit corrects that by making sure to have pg_init_privs updated
      when ALTER EXTENSION ADD/DROP is run, recording the permissions as they
      are at ALTER EXTENSION ADD time, and removing any if/when ALTER
      EXTENSION DROP is called.
      
      This issue was pointed out by Moshe Jacobson as commentary on bug #14456
      (which was actually a bug about versions prior to 9.6 not handling
      custom ACLs on extensions correctly, an issue now addressed with
      pg_init_privs in 9.6).
      
      Back-patch to 9.6 where pg_init_privs was introduced.
      e54f7572
    • Stephen Frost's avatar
      test_pg_dump TAP test whitespace cleanup · fb94ca77
      Stephen Frost authored
      The formatting of the perl hashes used in the TAP tests for test_pg_dump
      was rather horribly inconsistent and made it more difficult than it
      really should have been to add new tests or adjust what tests are for
      what runs, etc.
      
      Reformat to clean that all up.
      
      Whitespace-only changes.
      fb94ca77
  4. 27 Jan, 2017 8 commits
  5. 26 Jan, 2017 9 commits
  6. 25 Jan, 2017 1 commit