1. 26 Feb, 2015 2 commits
    • Stephen Frost's avatar
      Add hasRowSecurity to copyfuncs/outfuncs · 62a4a1af
      Stephen Frost authored
      The RLS patch added a hasRowSecurity field to PlannerGlobal and
      PlannedStmt but didn't update nodes/copyfuncs.c and nodes/outfuncs.c to
      reflect those additional fields.
      
      Correct that by adding entries to the appropriate functions for those
      fields.
      
      Pointed out by Robert.
      62a4a1af
    • Stephen Frost's avatar
      Add locking clause for SB views for update/delete · 6f9bd50e
      Stephen Frost authored
      In expand_security_qual(), we were handling locking correctly when a
      PlanRowMark existed, but not when we were working with the target
      relation (which doesn't have any PlanRowMarks, but the subquery created
      for the security barrier quals still needs to lock the rows under it).
      
      Noted by Etsuro Fujita when working with the Postgres FDW, which wasn't
      properly issuing a SELECT ... FOR UPDATE to the remote side under a
      DELETE.
      
      Back-patch to 9.4 where updatable security barrier views were
      introduced.
      
      Per discussion with Etsuro and Dean Rasheed.
      6f9bd50e
  2. 25 Feb, 2015 3 commits
    • Tom Lane's avatar
      Fix over-optimistic caching in fetch_array_arg_replace_nulls(). · 77903ede
      Tom Lane authored
      When I rewrote this in commit 56a79a86,
      I forgot that it's possible for the input array type to change from one
      call to the next (this can happen when applying the function to
      pg_statistic columns, for instance).  Fix that.
      77903ede
    • Tom Lane's avatar
      Fix dumping of views that are just VALUES(...) but have column aliases. · e9f1c01b
      Tom Lane authored
      The "simple" path for printing VALUES clauses doesn't work if we need
      to attach nondefault column aliases, because there's noplace to do that
      in the minimal VALUES() syntax.  So modify get_simple_values_rte() to
      detect nondefault aliases and treat that as a non-simple case.  This
      further exposes that the "non-simple" path never actually worked;
      it didn't produce valid syntax.  Fix that too.  Per bug #12789 from
      Curtis McEnroe, and analysis by Andrew Gierth.
      
      Back-patch to all supported branches.  Before 9.3, this also requires
      back-patching the part of commit 092d7ded
      that created get_simple_values_rte() to begin with; inserting the extra
      test into the old factorization of that logic would've been too messy.
      e9f1c01b
    • Michael Meskes's avatar
      Remove null-pointer checks that are not needed. · 8794bf1c
      Michael Meskes authored
      If a pointer is guaranteed to carry information there is no need to check
      for NULL again. Patch by Michael Paquier.
      8794bf1c
  3. 24 Feb, 2015 4 commits
    • Tom Lane's avatar
      Improve parser's one-extra-token lookahead mechanism. · d809fd00
      Tom Lane authored
      There are a couple of places in our grammar that fail to be strict LALR(1),
      by requiring more than a single token of lookahead to decide what to do.
      Up to now we've dealt with that by using a filter between the lexer and
      parser that merges adjacent tokens into one in the places where two tokens
      of lookahead are necessary.  But that creates a number of user-visible
      anomalies, for instance that you can't name a CTE "ordinality" because
      "WITH ordinality AS ..." triggers folding of WITH and ORDINALITY into one
      token.  I realized that there's a better way.
      
      In this patch, we still do the lookahead basically as before, but we never
      merge the second token into the first; we replace just the first token by
      a special lookahead symbol when one of the lookahead pairs is seen.
      
      This requires a couple extra productions in the grammar, but it involves
      fewer special tokens, so that the grammar tables come out a bit smaller
      than before.  The filter logic is no slower than before, perhaps a bit
      faster.
      
      I also fixed the filter logic so that when backing up after a lookahead,
      the current token's terminator is correctly restored; this eliminates some
      weird behavior in error message issuance, as is shown by the one change in
      existing regression test outputs.
      
      I believe that this patch entirely eliminates odd behaviors caused by
      lookahead for WITH.  It doesn't really improve the situation for NULLS
      followed by FIRST/LAST unfortunately: those sequences still act like a
      reserved word, even though there are cases where they should be seen as two
      ordinary identifiers, eg "SELECT nulls first FROM ...".  I experimented
      with additional grammar hacks but couldn't find any simple solution for
      that.  Still, this is better than before, and it seems much more likely
      that we *could* somehow solve the NULLS case on the basis of this filter
      behavior than the previous one.
      d809fd00
    • Peter Eisentraut's avatar
      Error when creating names too long for tar format · 23a78352
      Peter Eisentraut authored
      The tar format (at least the version we are using), does not support
      file names or symlink targets longer than 99 bytes.  Until now, the tar
      creation code would silently truncate any names that are too long.  (Its
      original application was pg_dump, where this never happens.)  This
      creates problems when running base backups over the replication
      protocol.
      
      The most important problem is when a tablespace path is longer than 99
      bytes, which will result in a truncated tablespace path being backed up.
      Less importantly, the basebackup protocol also promises to back up any
      other files it happens to find in the data directory, which would also
      lead to file name truncation if someone put a file with a long name in
      there.
      
      Now both of these cases result in an error during the backup.
      
      Add tests that fail when a too-long file name or symlink is attempted to
      be backed up.
      Reviewed-by: default avatarRobert Hass <robertmhaas@gmail.com>
      23a78352
    • Heikki Linnakangas's avatar
      347c7432
    • Heikki Linnakangas's avatar
      Fix typo in README. · dd58c609
      Heikki Linnakangas authored
      Kyotaro Horiguchi
      dd58c609
  4. 23 Feb, 2015 10 commits
    • Peter Eisentraut's avatar
      Fix invalid DocBook XML · b007bee1
      Peter Eisentraut authored
      b007bee1
    • Alvaro Herrera's avatar
      Fix stupid merge errors in previous commit · d1712d01
      Alvaro Herrera authored
      Brown paper bag installed permanently.
      d1712d01
    • Tom Lane's avatar
      Further tweaking of raw grammar output to distinguish different inputs. · 56be925e
      Tom Lane authored
      Use a different A_Expr_Kind for LIKE/ILIKE/SIMILAR TO constructs, so that
      they can be distinguished from direct invocation of the underlying
      operators.  Also, postpone selection of the operator name when transforming
      "x IN (select)" to "x = ANY (select)", so that those syntaxes can be told
      apart at parse analysis time.
      
      I had originally thought I'd also have to do something special for the
      syntaxes IS NOT DISTINCT FROM, IS NOT DOCUMENT, and x NOT IN (SELECT...),
      which the grammar translates as though they were NOT (construct).
      On reflection though, we can distinguish those cases reliably by noting
      whether the parse location shown for the NOT is the same as for its child
      node.  This only requires tweaking the parse locations for NOT IN, which
      I've done here.
      
      These changes should have no effect outside the parser; they're just in
      support of being able to give accurate warnings for planned operator
      precedence changes.
      56be925e
    • 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
  5. 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
  6. 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
  7. 20 Feb, 2015 5 commits