1. 09 Mar, 2015 10 commits
    • Alvaro Herrera's avatar
      Keep CommitTs module in sync in standby and master · 4f3924d9
      Alvaro Herrera authored
      We allow this module to be turned off on restarts, so a restart time
      check is enough to activate or deactivate the module; however, if there
      is a standby replaying WAL emitted from a master which is restarted, but
      the standby isn't, the state in the standby becomes inconsistent and can
      easily be crashed.
      
      Fix by activating and deactivating the module during WAL replay on
      parameter change as well as on system start.
      
      Problem reported by Fujii Masao in
      http://www.postgresql.org/message-id/CAHGQGwFhJ3CnHo1CELEfay18yg_RA-XZT-7D8NuWUoYSZ90r4Q@mail.gmail.com
      
      Author: Petr Jelínek
      4f3924d9
    • Alvaro Herrera's avatar
      Fix crasher bugs in previous commit · e3f1c24b
      Alvaro Herrera authored
      ALTER DEFAULT PRIVILEGES was trying to decode the list of roles in the
      FOR clause as a list of names rather than of RoleSpecs; and the IN
      clause in CREATE ROLE was doing the same thing.  This was evidenced by
      crashes on some buildfarm machines, though on my platform this doesn't
      cause a failure by mere chance; I can reproduce the failures only by
      adding some padding in struct RoleSpecs.
      
      Fix by dereferencing those lists as being of RoleSpecs, not string
      Values.
      e3f1c24b
    • Alvaro Herrera's avatar
      Allow CURRENT/SESSION_USER to be used in certain commands · 31eae602
      Alvaro Herrera authored
      Commands such as ALTER USER, ALTER GROUP, ALTER ROLE, GRANT, and the
      various ALTER OBJECT / OWNER TO, as well as ad-hoc clauses related to
      roles such as the AUTHORIZATION clause of CREATE SCHEMA, the FOR clause
      of CREATE USER MAPPING, and the FOR ROLE clause of ALTER DEFAULT
      PRIVILEGES can now take the keywords CURRENT_USER and SESSION_USER as
      user specifiers in place of an explicit user name.
      
      This commit also fixes some quite ugly handling of special standards-
      mandated syntax in CREATE USER MAPPING, which in particular would fail
      to work in presence of a role named "current_user".
      
      The special role specifiers PUBLIC and NONE also have more consistent
      handling now.
      
      Also take the opportunity to add location tracking to user specifiers.
      
      Authors: Kyotaro Horiguchi.  Heavily reworked by Álvaro Herrera.
      Reviewed by: Rushabh Lathia, Adam Brightwell, Marti Raudsepp.
      31eae602
    • Michael Meskes's avatar
    • Michael Meskes's avatar
      2093eb4d
    • Robert Haas's avatar
      Fix handling of sortKeys field in Tuplesortstate. · 2720e96a
      Robert Haas authored
      Commit 5cefbf5a introduced an
      assumption that this field would always be non-NULL when doing a merge
      pass, but that's not true.  Without this fix, you can crash the server
      by building a hash index that is sufficiently large relative to
      maintenance_work_mem, or by triggering a large datum sort.
      
      Commit 5ea86e6e changed the comments
      for that field to say that it would be set in all cases except for the
      hash index case, but that wasn't (and still isn't) true.
      
      The datum-sort failure was spotted by Tomas Vondra; initial analysis
      of that failure was by Peter Geoghegan.  The remaining issues were
      spotted by me during review of the surrounding code, and the patch is
      all my fault.
      2720e96a
    • Heikki Linnakangas's avatar
      Move WAL-related definitions from dbcommands.h to separate header file. · f1fd515b
      Heikki Linnakangas authored
      This makes it easier to write frontend programs that needs to understand
      the WAL record format of CREATE/DROP DATABASE. dbcommands.h cannot easily
      be #included in a frontend program, because it pulls in other header files
      that need backend stuff, but the new dbcommands_xlog.h header file has
      fewer dependencies.
      f1fd515b
    • Michael Meskes's avatar
      Ignore object files generated by ecpg test suite on Windows · b9e538b1
      Michael Meskes authored
      Patch by Michael Paquier
      b9e538b1
    • Fujii Masao's avatar
      Fix typo in comment. · 828599ac
      Fujii Masao authored
      828599ac
    • Fujii Masao's avatar
      Add missing "goto err" statements in xlogreader.c. · c74c04b8
      Fujii Masao authored
      Spotted by Andres Freund.
      c74c04b8
  2. 08 Mar, 2015 6 commits
    • Peter Eisentraut's avatar
      Sort SUBDIRS variable in src/bin/Makefile · 5a2a48f0
      Peter Eisentraut authored
      The previous order appears to have been historically grown randomness.
      5a2a48f0
    • Tom Lane's avatar
      Cast to (void *) rather than (int *) when passing int64's to PQfn(). · ef75508e
      Tom Lane authored
      This is a possibly-vain effort to silence a Coverity warning about
      bogus endianness dependency.  The code's fine, because it takes care
      of endianness issues for itself, but Coverity sees an int64 being
      passed to an int* argument and not unreasonably suspects something's
      wrong.  I'm not sure if putting the void* cast in the way will shut it
      up; but it can't hurt and seems better from a documentation standpoint
      anyway, since the pointer is not used as an int* in this code path.
      
      Just for a bit of additional safety, verify that the result length
      is 8 bytes as expected.
      
      Back-patch to 9.3 where the code in question was added.
      ef75508e
    • Tom Lane's avatar
      Remove struct PQArgBlock from server-side header libpq/libpq.h. · 01cca2c1
      Tom Lane authored
      This struct is purely a client-side artifact.  Perhaps there was once
      reason for the server to know it, but any such reason is lost in the
      mists of time.  We certainly don't need two independent declarations
      of it.
      01cca2c1
    • Tom Lane's avatar
      Fix documentation for libpq's PQfn(). · 1a0bc4c2
      Tom Lane authored
      The SGML docs claimed that 1-byte integers could be sent or received with
      the "isint" options, but no such behavior has ever been implemented in
      pqGetInt() or pqPutInt().  The in-code documentation header for PQfn() was
      even less in tune with reality, and the code itself used parameter names
      matching neither the SGML docs nor its libpq-fe.h declaration.  Do a bit
      of additional wordsmithing on the SGML docs while at it.
      
      Since the business about 1-byte integers is a clear documentation bug,
      back-patch to all supported branches.
      1a0bc4c2
    • Tom Lane's avatar
      Code cleanup for REINDEX DATABASE/SCHEMA/SYSTEM. · 90c35a9e
      Tom Lane authored
      Fix some minor infelicities.  Some of these things were introduced in
      commit fe263d11, and some are older.
      90c35a9e
    • Tom Lane's avatar
      Fix erroneous error message for REINDEX SYSTEM. · ac091428
      Tom Lane authored
      Missed case in commit fe263d11.
      
      Sawada Masahiko
      ac091428
  3. 07 Mar, 2015 3 commits
    • Noah Misch's avatar
      Build fls.o only when AC_REPLACE_FUNCS so dictates via $(LIBOBJS). · 9d265ae7
      Noah Misch authored
      By building it unconditionally, libpgport inadvertently replaced any
      libc version of the function.  This is essentially a code cleanup; any
      effect on performance is almost surely too small to notice.
      9d265ae7
    • Noah Misch's avatar
      Add CHECK_FOR_INTERRUPTS() to the wait_pid() loop. · 93751570
      Noah Misch authored
      Though the one contemporary caller uses it in a limited way, this
      function could loop indefinitely if pointed to an arbitrary PID.
      93751570
    • Peter Eisentraut's avatar
      Remove rolcatupdate · bb8582ab
      Peter Eisentraut authored
      This role attribute is an ancient PostgreSQL feature, but could only be
      set by directly updating the system catalogs, and it doesn't have any
      clearly defined use.
      
      Author: Adam Brightwell <adam.brightwell@crunchydatasolutions.com>
      bb8582ab
  4. 06 Mar, 2015 3 commits
    • Alvaro Herrera's avatar
      Add some more tests on event triggers · 6510c832
      Alvaro Herrera authored
      Fabien Coelho
      Reviewed by Robert Haas
      6510c832
    • Tom Lane's avatar
      Rethink function argument sorting in pg_dump. · e3bfe6d8
      Tom Lane authored
      Commit 7b583b20 created an unnecessary
      dump failure hazard by applying pg_get_function_identity_arguments()
      to every function in the database, even those that won't get dumped.
      This could result in snapshot-related problems if concurrent sessions are,
      for example, creating and dropping temporary functions, as noted by Marko
      Tiikkaja in bug #12832.  While this is by no means pg_dump's only such
      issue with concurrent DDL, it's unfortunate that we added a new failure
      mode for cases that used to work, and even more so that the failure was
      created for basically cosmetic reasons (ie, to sort overloaded functions
      more deterministically).
      
      To fix, revert that patch and instead sort function arguments using
      information that pg_dump has available anyway, namely the names of the
      argument types.  This will produce a slightly different sort ordering for
      overloaded functions than the previous coding; but applying strcmp
      directly to the output of pg_get_function_identity_arguments really was
      a bit odd anyway.  The sorting will still be name-based and hence
      independent of possibly-installation-specific OID assignments.  A small
      additional benefit is that sorting now works regardless of server version.
      
      Back-patch to 9.3, where the previous commit appeared.
      e3bfe6d8
    • Alvaro Herrera's avatar
      Fix contrib/file_fdw's expected file · c6ee39bc
      Alvaro Herrera authored
      I forgot to update it on yesterday's cf34e373.
      c6ee39bc
  5. 05 Mar, 2015 7 commits
    • Alvaro Herrera's avatar
      Fix user mapping object description · cf34e373
      Alvaro Herrera authored
      We were using "user mapping for user XYZ" as description for user mappings, but
      that's ambiguous because users can have mappings on multiple foreign
      servers; therefore change it to "for user XYZ on server UVW" instead.
      Object identities for user mappings are also updated in the same way, in
      branches 9.3 and above.
      
      The incomplete description string was introduced together with the whole
      SQL/MED infrastructure by commit cae565e5 of 8.4 era, so backpatch all
      the way back.
      cf34e373
    • Alvaro Herrera's avatar
      Silence warning in non-assert-enabled build · bf22d270
      Alvaro Herrera authored
      An OID return value was being used only for a (rather pointless) assert.
      Silence by removing the variable and the assert.
      
      Per note from Peter Geoghegan
      bf22d270
    • Tom Lane's avatar
      Remove comment claiming that PARAM_EXTERN Params always have typmod -1. · 3200b15b
      Tom Lane authored
      This hasn't been true in quite some time, cf plpgsql's make_datum_param().
      3200b15b
    • Fujii Masao's avatar
      Fix typo in comment. · 934d1226
      Fujii Masao authored
      934d1226
    • Tom Lane's avatar
      Avoid unused-variable warning in non-assert builds. · a5c29d37
      Tom Lane authored
      Oversight in my commit b9896198.
      a5c29d37
    • Tom Lane's avatar
      Change plpgsql's cast cache to consider source typmod as significant. · 7f3014dc
      Tom Lane authored
      I had thought that there was no need to maintain separate cache entries
      for different source typmods, but further experimentation shows that there
      is an advantage to doing so in some cases.  In particular, if a domain has
      a typmod (say, "CREATE DOMAIN d AS numeric(20,0)"), failing to notice the
      source typmod leads to applying a length-coercion step even when the
      source has the correct typmod.
      7f3014dc
    • Tom Lane's avatar
      Need to special-case RECORD as well as UNKNOWN in plpgsql's casting logic. · 45f2c2fc
      Tom Lane authored
      This is because can_coerce_type thinks that RECORD can be cast to any
      composite type, but coerce_record_to_complex only works for inputs that are
      RowExprs or whole-row Vars, so we get a hard failure on a CaseTestExpr.
      
      Perhaps these corner cases ought to be fixed so that coerce_to_target_type
      actually returns NULL as per its specification, rather than failing ...
      but for the moment an extra check here is the path of least resistance.
      45f2c2fc
  6. 04 Mar, 2015 4 commits
    • Tom Lane's avatar
      Use standard casting mechanism to convert types in plpgsql, when possible. · 1345cc67
      Tom Lane authored
      plpgsql's historical method for converting datatypes during assignments was
      to apply the source type's output function and then the destination type's
      input function.  Aside from being miserably inefficient in most cases, this
      method failed outright in many cases where a user might expect it to work;
      an example is that "declare x int; ... x := 3.9;" would fail, not round the
      value to 4.
      
      Instead, let's convert by applying the appropriate assignment cast whenever
      there is one.  To avoid breaking compatibility unnecessarily, fall back to
      the I/O conversion method if there is no assignment cast.
      
      So far as I can tell, there is just one case where this method produces a
      different result than the old code in a case where the old code would not
      have thrown an error.  That is assignment of a boolean value to a string
      variable (type text, varchar, or bpchar); the old way gave boolean's output
      representation, ie 't'/'f', while the new way follows the behavior of the
      bool-to-text cast and so gives 'true' or 'false'.  This will need to be
      called out as an incompatibility in the 9.5 release notes.
      
      Aside from handling many conversion cases more sanely, this method is
      often significantly faster than the old way.  In part that's because
      of more effective caching of the conversion info.
      1345cc67
    • Tom Lane's avatar
      Fix cost estimation for indexscans on expensive indexed expressions. · b9896198
      Tom Lane authored
      genericcostestimate() and friends used the cost of the entire indexqual
      expressions as the charge for initial evaluation of indexscan arguments.
      But of course the index column is not evaluated, only the other side
      of the qual expression, so this was a bad overestimate if the index
      column was an expensive expression.
      
      To fix, refactor the logic in this area so that there's a single routine
      charged with deconstructing index quals and figuring out what is the index
      column and what is the comparison expression.  This is more or less free in
      the case of btree indexes, since btcostestimate() was doing equivalent
      deconstruction already.  It probably adds a bit of new overhead in the cases
      of other index types, but not a lot.  (In the case of GIN I think I saved
      something by getting rid of code that wasn't aware that the index column
      associations were already available "for free".)
      
      Per recent gripe from Jeff Janes.
      
      Arguably this is a bug fix, but I'm hesitant to back-patch because of the
      possibility of destabilizing plan choices that people may be happy with.
      b9896198
    • Fujii Masao's avatar
      Fix an obsolete reference to SnapshotNow in comment. · f8b031bc
      Fujii Masao authored
      Peter Geoghegan
      f8b031bc
    • Tom Lane's avatar
      Fix long-obsolete code for separating filter conditions in cost_index(). · 497bac7d
      Tom Lane authored
      This code relied on pointer equality to identify which restriction clauses
      also appear in the indexquals (and, therefore, don't need to be applied as
      simple filter conditions).  That was okay once upon a time, years ago,
      before we introduced the equivalence-class machinery.  Now there's about a
      50-50 chance that an equality clause appearing in the indexquals will be
      the mirror image (commutator) of its mate in the restriction list.  When
      that happens, we'd erroneously think that the clause would be re-evaluated
      at each visited row, and therefore inflate the cost estimate for the
      indexscan by the clause's cost.
      
      Add some logic to catch this case.  It seems to me that it continues not to
      be worthwhile to expend the extra predicate-proof work that createplan.c
      will do on the finally-selected plan, but this case is common enough and
      cheap enough to handle that we should do so.
      
      This will make a small difference (about one cpu_operator_cost per row)
      in simple cases; but in situations where there's an expensive function in
      the indexquals, it can make a very large difference, as seen in recent
      example from Jeff Janes.
      
      This is a long-standing bug, but I'm hesitant to back-patch because of the
      possibility of destabilizing plan choices that people may be happy with.
      497bac7d
  7. 03 Mar, 2015 6 commits
    • Robert Haas's avatar
      Remove residual NULL-pstate handling in addRangeTableEntry. · 5223ddac
      Robert Haas authored
      Passing a NULL pstate wouldn't actually work, because isLockedRefname()
      isn't prepared to cope with it; and there hasn't been any in-core code
      that tries in over a decade.  So just remove the residual NULL handling.
      
      Spotted by Coverity; analysis and patch by Michael Paquier.
      5223ddac
    • Alvaro Herrera's avatar
      Change many routines to return ObjectAddress rather than OID · a2e35b53
      Alvaro Herrera authored
      The changed routines are mostly those that can be directly called by
      ProcessUtilitySlow; the intention is to make the affected object
      information more precise, in support for future event trigger changes.
      Originally it was envisioned that the OID of the affected object would
      be enough, and in most cases that is correct, but upon actually
      implementing the event trigger changes it turned out that ObjectAddress
      is more widely useful.
      
      Additionally, some command execution routines grew an output argument
      that's an object address which provides further info about the executed
      command.  To wit:
      
      * for ALTER DOMAIN / ADD CONSTRAINT, it corresponds to the address of
        the new constraint
      
      * for ALTER OBJECT / SET SCHEMA, it corresponds to the address of the
        schema that originally contained the object.
      
      * for ALTER EXTENSION {ADD, DROP} OBJECT, it corresponds to the address
        of the object added to or dropped from the extension.
      
      There's no user-visible change in this commit, and no functional change
      either.
      
      Discussion: 20150218213255.GC6717@tamriel.snowman.net
      Reviewed-By: Stephen Frost, Andres Freund
      a2e35b53
    • Alvaro Herrera's avatar
      Add comment for "is_internal" parameter · 6f9d7990
      Alvaro Herrera authored
      This was missed in my commit f4c4335a of 9.3 vintage, so backpatch to
      that.
      6f9d7990
    • Tom Lane's avatar
      Reduce json <=> jsonb casts from explicit-only to assignment level. · b67f1ce1
      Tom Lane authored
      There's no reason to make users write an explicit cast to store a
      json value in a jsonb column or vice versa.
      
      We could probably even make these implicit, but that might open us up
      to problems with ambiguous function calls, so for now just do this.
      b67f1ce1
    • Robert Haas's avatar
      pgbench: Fix mistakes in Makefile. · e5f36902
      Robert Haas authored
      My commit 878fdcb8 was not quite
      right.  Tom Lane pointed out one of the mistakes fixed here, and I
      noticed the other myself while reviewing what I'd committed.
      e5f36902
    • Tom Lane's avatar
      Fix busted markup. · d1479011
      Tom Lane authored
      Evidently from commit 878fdcb8.
      Per buildfarm.
      d1479011
  8. 02 Mar, 2015 1 commit
    • Robert Haas's avatar
      pgbench: Add a real expression syntax to \set · 878fdcb8
      Robert Haas authored
      Previously, you could do \set variable operand1 operator operand2, but
      nothing more complicated.  Now, you can \set variable expression, which
      makes it much simpler to do multi-step calculations here.  This also
      adds support for the modulo operator (%), with the same semantics as in
      C.
      
      Robert Haas and Fabien Coelho, reviewed by Álvaro Herrera and
      Stephen Frost
      878fdcb8