1. 23 Feb, 2015 7 commits
    • Alvaro Herrera's avatar
      Support more commands in event triggers · 296f3a60
      Alvaro Herrera authored
      COMMENT, SECURITY LABEL, and GRANT/REVOKE now also fire
      ddl_command_start and ddl_command_end event triggers, when they operate
      on database-local objects.
      
      Reviewed-By: Michael Paquier, Andres Freund, Stephen Frost
      296f3a60
    • Heikki Linnakangas's avatar
      Replace checkpoint_segments with min_wal_size and max_wal_size. · 88e98230
      Heikki Linnakangas authored
      Instead of having a single knob (checkpoint_segments) that both triggers
      checkpoints, and determines how many checkpoints to recycle, they are now
      separate concerns. There is still an internal variable called
      CheckpointSegments, which triggers checkpoints. But it no longer determines
      how many segments to recycle at a checkpoint. That is now auto-tuned by
      keeping a moving average of the distance between checkpoints (in bytes),
      and trying to keep that many segments in reserve. The advantage of this is
      that you can set max_wal_size very high, but the system won't actually
      consume that much space if there isn't any need for it. The min_wal_size
      sets a floor for that; you can effectively disable the auto-tuning behavior
      by setting min_wal_size equal to max_wal_size.
      
      The max_wal_size setting is now the actual target size of WAL at which a
      new checkpoint is triggered, instead of the distance between checkpoints.
      Previously, you could calculate the actual WAL usage with the formula
      "(2 + checkpoint_completion_target) * checkpoint_segments + 1". With this
      patch, you set the desired WAL usage with max_wal_size, and the system
      calculates the appropriate CheckpointSegments with the reverse of that
      formula. That's a lot more intuitive for administrators to set.
      
      Reviewed by Amit Kapila and Venkata Balaji N.
      88e98230
    • Heikki Linnakangas's avatar
      Renumber GUC_* constants. · 0fec0003
      Heikki Linnakangas authored
      This moves all the regular flags back together (for aesthetic reasons), and
      makes room for more GUC_UNIT_* types.
      0fec0003
    • Heikki Linnakangas's avatar
      Refactor unit conversions code in guc.c. · 1b630264
      Heikki Linnakangas authored
      Replace the if-switch-case constructs with two conversion tables,
      containing all the supported conversions between human-readable unit
      strings and the base units used in GUC variables. This makes the code
      easier to read, and makes adding new units simpler.
      1b630264
    • Andres Freund's avatar
      Guard against spurious signals in LockBufferForCleanup. · bc208a5a
      Andres Freund authored
      When LockBufferForCleanup() has to wait for getting a cleanup lock on a
      buffer it does so by setting a flag in the buffer header and then wait
      for other backends to signal it using ProcWaitForSignal().
      Unfortunately LockBufferForCleanup() missed that ProcWaitForSignal() can
      return for other reasons than the signal it is hoping for. If such a
      spurious signal arrives the wait flags on the buffer header will still
      be set. That then triggers "ERROR: multiple backends attempting to wait
      for pincount 1".
      
      The fix is simple, unset the flag if still set when retrying. That
      implies an additional spinlock acquisition/release, but that's unlikely
      to matter given the cost of waiting for a cleanup lock.  Alternatively
      it'd have been possible to move responsibility for maintaining the
      relevant flag to the waiter all together, but that might have had
      negative consequences due to possible floods of signals. Besides being
      more invasive.
      
      This looks to be a very longstanding bug. The relevant code in
      LockBufferForCleanup() hasn't changed materially since its introduction
      and ProcWaitForSignal() was documented to return for unrelated reasons
      since 8.2.  The master only patch series removing ImmediateInterruptOK
      made it much easier to hit though, as ProcSendSignal/ProcWaitForSignal
      now uses a latch shared with other tasks.
      
      Per discussion with Kevin Grittner, Tom Lane and me.
      
      Backpatch to all supported branches.
      
      Discussion: 11553.1423805224@sss.pgh.pa.us
      bc208a5a
    • Fujii Masao's avatar
      Add GUC to control the time to wait before retrieving WAL after failed attempt. · 5d2b45e3
      Fujii Masao authored
      Previously when the standby server failed to retrieve WAL files from any sources
      (i.e., streaming replication, local pg_xlog directory or WAL archive), it always
      waited for five seconds (hard-coded) before the next attempt. For example,
      this is problematic in warm-standby because restore_command can fail
      every five seconds even while new WAL file is expected to be unavailable for
      a long time and flood the log files with its error messages.
      
      This commit adds new parameter, wal_retrieve_retry_interval, to control that
      wait time.
      
      Alexey Vasiliev and Michael Paquier, reviewed by Andres Freund and me.
      5d2b45e3
    • Heikki Linnakangas's avatar
      Fix potential deadlock with libpq non-blocking mode. · 2a3f6e36
      Heikki Linnakangas authored
      If libpq output buffer is full, pqSendSome() function tries to drain any
      incoming data. This avoids deadlock, if the server e.g. sends a lot of
      NOTICE messages, and blocks until we read them. However, pqSendSome() only
      did that in blocking mode. In non-blocking mode, the deadlock could still
      happen.
      
      To fix, take a two-pronged approach:
      
      1. Change the documentation to instruct that when PQflush() returns 1, you
      should wait for both read- and write-ready, and call PQconsumeInput() if it
      becomes read-ready. That fixes the deadlock, but applications are not going
      to change overnight.
      
      2. In pqSendSome(), drain the input buffer before returning 1. This
      alleviates the problem for applications that only wait for write-ready. In
      particular, a slow but steady stream of NOTICE messages during COPY FROM
      STDIN will no longer cause a deadlock. The risk remains that the server
      attempts to send a large burst of data and fills its output buffer, and at
      the same time the client also sends enough data to fill its output buffer.
      The application will deadlock if it goes to sleep, waiting for the socket
      to become write-ready, before the server's data arrives. In practice,
      NOTICE messages and such that the server might be sending are usually
      short, so it's highly unlikely that the server would fill its output buffer
      so quickly.
      
      Backpatch to all supported versions.
      2a3f6e36
  2. 22 Feb, 2015 5 commits
    • Tom Lane's avatar
      Add parse location fields to NullTest and BooleanTest structs. · c063da17
      Tom Lane authored
      We did not need a location tag on NullTest or BooleanTest before, because
      no error messages referred directly to their locations.  That's planned
      to change though, so add these fields in a separate housekeeping commit.
      
      Catversion bump because stored rules may change.
      c063da17
    • Tom Lane's avatar
      Get rid of multiple applications of transformExpr() to the same tree. · 6a75562e
      Tom Lane authored
      transformExpr() has for many years had provisions to do nothing when
      applied to an already-transformed expression tree.  However, this was
      always ugly and of dubious reliability, so we'd be much better off without
      it.  The primary historical reason for it was that gram.y sometimes
      returned multiple links to the same subexpression, which is no longer true
      as of my BETWEEN fixes.  We'd also grown some lazy hacks in CREATE TABLE
      LIKE (failing to distinguish between raw and already-transformed index
      specifications) and one or two other places.
      
      This patch removes the need for and support for re-transforming already
      transformed expressions.  The index case is dealt with by adding a flag
      to struct IndexStmt to indicate that it's already been transformed;
      which has some benefit anyway in that tablecmds.c can now Assert that
      transformation has happened rather than just assuming.  The other main
      reason was some rather sloppy code for array type coercion, which can
      be fixed (and its performance improved too) by refactoring.
      
      I did leave transformJoinUsingClause() still constructing expressions
      containing untransformed operator nodes being applied to Vars, so that
      transformExpr() still has to allow Var inputs.  But that's a much narrower,
      and safer, special case than before, since Vars will never appear in a raw
      parse tree, and they don't have any substructure to worry about.
      
      In passing fix some oversights in the patch that added CREATE INDEX
      IF NOT EXISTS (missing processing of IndexStmt.if_not_exists).  These
      appear relatively harmless, but still sloppy coding practice.
      6a75562e
    • Tom Lane's avatar
      Represent BETWEEN as a special node type in raw parse trees. · 34af082f
      Tom Lane authored
      Previously, gram.y itself converted BETWEEN into AND (or AND/OR) nests of
      expression comparisons.  This was always as bogus as could be, but fixing
      it hasn't risen to the top of the to-do list.  The present patch invents an
      A_Expr representation for BETWEEN expressions, and does the expansion to
      comparison trees in parse_expr.c which is at least a slightly saner place
      to be doing semantic conversions.  There should be no change in the post-
      parse-analysis results.
      
      This does nothing for the semantic issues with BETWEEN (dubious connection
      to btree-opclass semantics, and multiple evaluation of possibly volatile
      subexpressions) ... but it's a necessary preliminary step before we could
      fix any of that.  The main immediate benefit is that preserving BETWEEN as
      an identifiable raw-parse-tree construct will enable better error messages.
      
      While at it, fix the code so that multiply-referenced subexpressions are
      physically duplicated before being passed through transformExpr().  This
      gets rid of one of the principal reasons why transformExpr() has
      historically had to allow already-processed input.
      34af082f
    • Jeff Davis's avatar
      Rename variable in AllocSetContextCreate to be consistent. · 74811c40
      Jeff Davis authored
      Everywhere else in the file, "context" is of type MemoryContext and
      "set" is of type AllocSet. AllocSetContextCreate uses a variable of
      type AllocSet, so rename it from "context" to "set".
      74811c40
    • Jeff Davis's avatar
      In array_agg(), don't create a new context for every group. · b419865a
      Jeff Davis authored
      Previously, each new array created a new memory context that started
      out at 8kB. This is incredibly wasteful when there are lots of small
      groups of just a few elements each.
      
      Change initArrayResult() and friends to accept a "subcontext" argument
      to indicate whether the caller wants the ArrayBuildState allocated in
      a new subcontext or not. If not, it can no longer be released
      separately from the rest of the memory context.
      
      Fixes bug report by Frank van Vugt on 2013-10-19.
      
      Tomas Vondra. Reviewed by Ali Akbar, Tom Lane, and me.
      b419865a
  3. 21 Feb, 2015 11 commits
    • Tom Lane's avatar
      Try to fix busted gettimeofday() code. · e9fd5545
      Tom Lane authored
      Per buildfarm, we have to match the _stdcall property of the system
      functions.
      e9fd5545
    • Tom Lane's avatar
      Use FLEXIBLE_ARRAY_MEMBER in Windows-specific code. · 332f02f8
      Tom Lane authored
      Be a tad more paranoid about overlength input, too.
      332f02f8
    • Andres Freund's avatar
      Force some system catalog table columns to be marked NOT NULL. · 82a532b3
      Andres Freund authored
      In a manual pass over the catalog declaration I found a number of
      columns which the boostrap automatism didn't mark NOT NULL even though
      they actually were. Add BKI_FORCE_NOT_NULL markings to them.
      
      It's usually not critical if a system table column is falsely determined
      to be nullable as the code should always catch relevant cases. But it's
      good to have a extra layer in place.
      
      Discussion: 20150215170014.GE15326@awork2.anarazel.de
      82a532b3
    • Andres Freund's avatar
      Allow forcing nullness of columns during bootstrap. · eb68379c
      Andres Freund authored
      Bootstrap determines whether a column is null based on simple builtin
      rules. Those work surprisingly well, but nonetheless a few existing
      columns aren't set correctly. Additionally there is at least one patch
      sent to hackers where forcing the nullness of a column would be helpful.
      
      The boostrap format has gained FORCE [NOT] NULL for this, which will be
      emitted by genbki.pl when BKI_FORCE_(NOT_)?NULL is specified for a
      column in a catalog header.
      
      This patch doesn't change the marking of any existing columns.
      
      Discussion: 20150215170014.GE15326@awork2.anarazel.de
      eb68379c
    • Tom Lane's avatar
      0627eff3
    • Tom Lane's avatar
      Use FLEXIBLE_ARRAY_MEMBER in a number of other places. · 2e211211
      Tom Lane authored
      I think we're about done with this...
      2e211211
    • Tom Lane's avatar
      Use FLEXIBLE_ARRAY_MEMBER for HeapTupleHeaderData.t_bits[]. · e1a11d93
      Tom Lane authored
      This requires changing quite a few places that were depending on
      sizeof(HeapTupleHeaderData), but it seems for the best.
      
      Michael Paquier, some adjustments by me
      e1a11d93
    • Tom Lane's avatar
      Minor code beautification in conninfo_uri_parse_params(). · 3d9b6f31
      Tom Lane authored
      Reading this made me itch, so clean the logic a bit.
      3d9b6f31
    • Tom Lane's avatar
      Fix misparsing of empty value in conninfo_uri_parse_params(). · b26e2081
      Tom Lane authored
      After finding an "=" character, the pointer was advanced twice when it
      should only advance once.  This is harmless as long as the value after "="
      has at least one character; but if it doesn't, we'd miss the terminator
      character and include too much in the value.
      
      In principle this could lead to reading off the end of memory.  It does not
      seem worth treating as a security issue though, because it would happen on
      client side, and besides client logic that's taking conninfo strings from
      untrusted sources has much worse security problems than this.
      
      Report and patch received off-list from Thomas Fanghaenel.
      Back-patch to 9.2 where the faulty code was introduced.
      b26e2081
    • Robert Haas's avatar
      Don't require users of src/port/gettimeofday.c to initialize it. · 64235fec
      Robert Haas authored
      Commit 8001fe67 introduced this
      requirement, but per discussion, we want to avoid requirements of
      this type to make things easier on the calling code.  An especially
      important consideration is that this may be used in frontend code,
      not just the backend.
      
      Asif Naeem, reviewed by Michael Paquier
      64235fec
    • Tom Lane's avatar
      Some more FLEXIBLE_ARRAY_MEMBER fixes. · f2874feb
      Tom Lane authored
      f2874feb
  4. 20 Feb, 2015 12 commits
    • Tom Lane's avatar
      Fix statically allocated struct with FLEXIBLE_ARRAY_MEMBER member. · 33b2a2c9
      Tom Lane authored
      clang complains about this, not unreasonably, so define another struct
      that's explicitly for a WordEntryPos with exactly one element.
      
      While at it, get rid of pretty dubious use of a static variable for
      more than one purpose --- if it were being treated as const maybe
      I'd be okay with this, but it isn't.
      33b2a2c9
    • Tom Lane's avatar
      Use FLEXIBLE_ARRAY_MEMBER in some more places. · 33a3b03d
      Tom Lane authored
      Fix a batch of structs that are only visible within individual .c files.
      
      Michael Paquier
      33a3b03d
    • Tom Lane's avatar
      Use FLEXIBLE_ARRAY_MEMBER in struct RecordIOData. · c110eff1
      Tom Lane authored
      I (tgl) fixed this last night in rowtypes.c, but I missed that the
      code had been copied into a couple of other places.
      
      Michael Paquier
      c110eff1
    • Tom Lane's avatar
      Use FLEXIBLE_ARRAY_MEMBER in struct varlena. · e38b1eb0
      Tom Lane authored
      This forces some minor coding adjustments in tuptoaster.c and inv_api.c,
      but the new coding there is cleaner anyway.
      
      Michael Paquier
      e38b1eb0
    • Alvaro Herrera's avatar
      Remove unnecessary and unreliable test · 8902f792
      Alvaro Herrera authored
      8902f792
    • Alvaro Herrera's avatar
      Update PGSTAT_FILE_FORMAT_ID · 3b14bb77
      Alvaro Herrera authored
      Previous commit should have bumped it but didn't.  Oops.
      
      Per note from Tom.
      3b14bb77
    • Alvaro Herrera's avatar
      Have TRUNCATE update pgstat tuple counters · d42358ef
      Alvaro Herrera authored
      This works by keeping a per-subtransaction record of the ins/upd/del
      counters before the truncate, and then resetting them; this record is
      useful to return to the previous state in case the truncate is rolled
      back, either in a subtransaction or whole transaction.  The state is
      propagated upwards as subtransactions commit.
      
      When the per-table data is sent to the stats collector, a flag indicates
      to reset the live/dead counters to zero as well.
      
      Catalog version bumped due to the change in pgstat format.
      
      Author: Alexander Shulgin
      Discussion: 1007.1207238291@sss.pgh.pa.us
      Discussion: 548F7D38.2000401@BlueTreble.com
      Reviewed-by: Álvaro Herrera, Jim Nasby
      d42358ef
    • Tom Lane's avatar
      Some more FLEXIBLE_ARRAY_MEMBER hacking. · 5740be6d
      Tom Lane authored
      5740be6d
    • Tom Lane's avatar
      Remove unused variable. · 9aa53bbd
      Tom Lane authored
      Per buildfarm.
      9aa53bbd
    • Tom Lane's avatar
      Use "#ifdef CATALOG_VARLEN" to protect nullable fields of pg_authid. · 692bd09a
      Tom Lane authored
      This gives a stronger guarantee than a mere comment against accessing these
      fields as simple struct members.  Since rolpassword is in fact varlena,
      it's not clear why these didn't get marked from the beginning, but let's
      do it now.
      
      Michael Paquier
      692bd09a
    • Tom Lane's avatar
      Use FLEXIBLE_ARRAY_MEMBER in a bunch more places. · 09d8d110
      Tom Lane authored
      Replace some bogus "x[1]" declarations with "x[FLEXIBLE_ARRAY_MEMBER]".
      Aside from being more self-documenting, this should help prevent bogus
      warnings from static code analyzers and perhaps compiler misoptimizations.
      
      This patch is just a down payment on eliminating the whole problem, but
      it gets rid of a lot of easy-to-fix cases.
      
      Note that the main problem with doing this is that one must no longer rely
      on computing sizeof(the containing struct), since the result would be
      compiler-dependent.  Instead use offsetof(struct, lastfield).  Autoconf
      also warns against spelling that offsetof(struct, lastfield[0]).
      
      Michael Paquier, review and additional fixes by me.
      09d8d110
    • Tom Lane's avatar
      Add pg_stat_get_snapshot_timestamp() to show statistics snapshot timestamp. · 2fb7a75f
      Tom Lane authored
      Per discussion, this could be useful for purposes such as programmatically
      detecting a nonresponding stats collector.  We already have the timestamp
      anyway, it's just a matter of providing a SQL-accessible function to fetch
      it.
      
      Matt Kelly, reviewed by Jim Nasby
      2fb7a75f
  5. 19 Feb, 2015 4 commits
    • Heikki Linnakangas's avatar
      Remove dead structs. · 634618ec
      Heikki Linnakangas authored
      These are not used with the new WAL format anymore. GIN split records are
      simply always recorded as full-page images.
      
      Michael Paquier
      634618ec
    • Tom Lane's avatar
      Update assorted TOAST-related documentation. · 9bb955c8
      Tom Lane authored
      While working on documentation for expanded arrays, I noticed a number of
      details in the TOAST-related documentation that were already inaccurate or
      obsolete.  This should be fixed independently of whether expanded arrays
      get in or not.  One issue is that the already existing indirect-pointer
      facility was not documented at all.  Also, the documentation says that you
      only need to use VARSIZE/SET_VARSIZE if you've made your variable-length
      type TOAST-aware, but actually we've forced that business on all varlena
      types even if they've opted out of TOAST by setting storage = plain.
      Wordsmith a few other things too, like an amusingly archaic claim that
      there are few 64-bit machines.
      
      I thought about back-patching this, but since all this doco is oriented
      to hackers and C-coded extension authors, fixing it in HEAD is probably
      good enough.
      9bb955c8
    • Tom Lane's avatar
      Split array_push into separate array_append and array_prepend functions. · 56a79a86
      Tom Lane authored
      There wasn't any good reason for a single C function to implement both
      these SQL functions: it saved very little code overall, and it required
      significant pushups to re-determine at runtime which case applied.  Redoing
      it as two functions ends up with just slightly more lines of code, but it's
      simpler to understand, and faster too because we need not repeat syscache
      lookups on every call.
      
      An important side benefit is that this eliminates the only case in which
      different aliases of the same C function had both anyarray and anyelement
      arguments at the same position, which would almost always be a mistake.
      The opr_sanity regression test will now notice such mistakes since there's
      no longer a valid case where it happens.
      56a79a86
    • Peter Eisentraut's avatar
      Fix Perl coding error in msvc build system · d30292b8
      Peter Eisentraut authored
      Code like
      
          open(P, "cl /? 2>&1 |") || die "cl command not found";
      
      does not actually catch any errors, because the exit status of the
      command before the pipe is ignored.  The fix is to look at $?.
      
      This also gave the opportunity to clean up the logic of this code a bit.
      d30292b8
  6. 18 Feb, 2015 1 commit
    • Alvaro Herrera's avatar
      Fix opclass/opfamily identity strings · 9c7dd350
      Alvaro Herrera authored
      The original representation uses "opcname for amname", which is good
      enough; but if we replace "for" with "using", we can apply the returned
      identity directly in a DROP command, as in
      
      DROP OPERATOR CLASS opcname USING amname
      
      This slightly simplifies code using object identities to programatically
      execute commands on these kinds of objects.
      
      Note backwards-incompatible change:
      The previous representation dates back to 9.3 when object identities
      were introduced by commit f8348ea3, but we don't want to change the
      behavior on released branches unnecessarily and so this is not
      backpatched.
      9c7dd350