1. 06 Apr, 2014 3 commits
    • Simon Riggs's avatar
      Isolation test files for ALTER TABLE patch · f14a6bbe
      Simon Riggs authored
      f14a6bbe
    • Simon Riggs's avatar
      Reduce lock levels of some ALTER TABLE cmds · e5550d5f
      Simon Riggs authored
      VALIDATE CONSTRAINT
      
      CLUSTER ON
      SET WITHOUT CLUSTER
      
      ALTER COLUMN SET STATISTICS
      ALTER COLUMN SET ()
      ALTER COLUMN RESET ()
      
      All other sub-commands use AccessExclusiveLock
      
      Simon Riggs and Noah Misch
      
      Reviews by Robert Haas and Andres Freund
      e5550d5f
    • Tom Lane's avatar
      Improve contrib/pg_trgm's heuristics for regexp index searches. · 80a5cf64
      Tom Lane authored
      When extracting trigrams from a regular expression for search of a GIN or
      GIST trigram index, it's useful to penalize (preferentially discard)
      trigrams that contain whitespace, since those are typically far more common
      in the index than trigrams not containing whitespace.  Of course, this
      should only be a preference not a hard rule, since we might otherwise end
      up with no trigrams to search for.  The previous coding tended to produce
      fairly inefficient trigram search sets for anchored regexp patterns, as
      reported by Erik Rijkers.  This patch penalizes whitespace-containing
      trigrams, and also reduces the target number of extracted trigrams, since
      experience suggests that the original coding tended to select too many
      trigrams to search for.
      
      Alexander Korotkov, reviewed by Tom Lane
      80a5cf64
  2. 05 Apr, 2014 6 commits
    • Tom Lane's avatar
      Block signals earlier during postmaster startup. · 5d8117e1
      Tom Lane authored
      Formerly, we set up the postmaster's signal handling only when we were
      about to start launching subprocesses.  This is a bad idea though, as
      it means that for example a SIGINT arriving before that will kill the
      postmaster instantly, perhaps leaving lockfiles, socket files, shared
      memory, etc laying about.  We'd rather that such a signal caused orderly
      postmaster termination including releasing of those resources.  A simple
      fix is to move the PostmasterMain stanza that initializes signal handling
      to an earlier point, before we've created any such resources.  Then, an
      early-arriving signal will be blocked until we're ready to deal with it
      in the usual way.  (The only part that really needs to be moved up is
      blocking of signals, but it seems best to keep the signal handler
      installation calls together with that; for one thing this ensures the
      kernel won't drop any signals we wished to get.  The handlers won't get
      invoked in any case until we unblock signals in ServerLoop.)
      
      Per a report from MauMau.  He proposed changing the way "pg_ctl stop"
      works to deal with this, but that'd just be masking one symptom not
      fixing the core issue.
      
      It's been like this since forever, so back-patch to all supported branches.
      5d8117e1
    • Heikki Linnakangas's avatar
      Fix another palloc in critical section. · ffbba6ee
      Heikki Linnakangas authored
      Also add a regression test for a GIN index with enough items with the same
      key, so that a GIN posting tree gets created. Apparently none of the
      existing GIN tests were large enough for that.
      
      This code is new, no backpatching required.
      ffbba6ee
    • Tom Lane's avatar
      Fix processing of PGC_BACKEND GUC parameters on Windows. · 6862ca69
      Tom Lane authored
      EXEC_BACKEND builds (i.e., Windows) failed to absorb values of PGC_BACKEND
      parameters if they'd been changed post-startup via the config file.  This
      for example prevented log_connections from working if it were turned on
      post-startup.  The mechanism for handling this case has always been a bit
      of a kluge, and it wasn't revisited when we implemented EXEC_BACKEND.
      While in a normal forking environment new backends will inherit the
      postmaster's value of such settings, EXEC_BACKEND backends have to read
      the settings from the CONFIG_EXEC_PARAMS file, and they were mistakenly
      rejecting them.  So this case has always been broken in the Windows port;
      so back-patch to all supported branches.
      
      Amit Kapila
      6862ca69
    • Tom Lane's avatar
      ecpg/ecpglib must build the src/port files it uses with -DFRONTEND. · 44c5d387
      Tom Lane authored
      Remarkably, this hasn't been noticed before, though it surely should
      have been happening since around the fall of the Byzantine empire.
      Commit 438b529604 changed path.c to depend on FRONTEND, and that exposed
      the omission, per buildfarm reports.
      
      I'm suspicious that some other subdirectories are missing this too,
      but this one change is enough to make ecpg tests pass for me.
      44c5d387
    • Tom Lane's avatar
      Fix tablespace creation WAL replay to work on Windows. · abe075df
      Tom Lane authored
      The code segment that removes the old symlink (if present) wasn't clued
      into the fact that on Windows, symlinks are junction points which have
      to be removed with rmdir().
      
      Backpatch to 9.0, where the failing code was introduced.
      
      MauMau, reviewed by Muhammad Asif Naeem and Amit Kapila
      abe075df
    • Tom Lane's avatar
      Allow "-C variable" and "--describe-config" even to root users. · b203c57b
      Tom Lane authored
      There's no really compelling reason to refuse to do these read-only,
      non-server-starting options as root, and there's at least one good
      reason to allow -C: pg_ctl uses -C to find out the true data directory
      location when pointed at a config-only directory.  On Windows, this is
      done before dropping administrator privileges, which means that pg_ctl
      fails for administrators if and only if a config-only layout is used.
      
      Since the root-privilege check is done so early in startup, it's a bit
      awkward to check for these switches.  Make the somewhat arbitrary
      decision that we'll only skip the root check if -C is the first switch.
      This is not just to make the code a bit simpler: it also guarantees that
      we can't misinterpret a --boot mode switch.  (While AuxiliaryProcessMain
      doesn't currently recognize any such switch, it might have one in the
      future.)  This is no particular problem for pg_ctl, and since the whole
      behavior is undocumented anyhow, it's not a documentation issue either.
      (--describe-config only works as the first switch anyway, so this is
      no restriction for that case either.)
      
      Back-patch to 9.2 where pg_ctl first began to use -C.
      
      MauMau, heavily edited by me
      b203c57b
  3. 04 Apr, 2014 9 commits
    • Tom Lane's avatar
      Preserve errno across free(). · 2209c0f8
      Tom Lane authored
      Dept. of second thoughts: free() isn't guaranteed not to change errno.
      Make sure we report the right error if getcwd() fails.
      2209c0f8
    • Tom Lane's avatar
      Make sure -D is an absolute path when starting server on Windows. · 9aca5125
      Tom Lane authored
      This is needed because Windows services may get started with a different
      current directory than where pg_ctl is executed.  We want relative -D
      paths to be interpreted relative to pg_ctl's CWD, similarly to what
      happens on other platforms.
      
      In support of this, move the backend's make_absolute_path() function
      into src/port/path.c (where it probably should have been long since)
      and get rid of the rather inferior version in pg_regress.
      
      Kumar Rajeev Rastogi, reviewed by MauMau
      9aca5125
    • Tom Lane's avatar
      Fix bogus time printout in walreceiver's debug log messages. · 8120c745
      Tom Lane authored
      The displayed sendtime and receipttime were always exactly equal, because
      somebody forgot that timestamptz_to_str returns a static buffer (thereby
      simplifying life for most callers, at the cost of complicating it for those
      who need two results concurrently).  Apply the same pstrdup solution used
      by the other call sites with this issue.  Back-patch to 9.2 where the
      faulty code was introduced.  Per bug #9849 from Haruka Takatsuka, though
      this is not exactly his patch.
      
      Possibly we should change timestamptz_to_str's API, but I wouldn't want
      to do so in the back branches.
      8120c745
    • Robert Haas's avatar
      Fix some compiler warnings that clang emits with -pedantic. · 59202fae
      Robert Haas authored
      Andres Freund
      59202fae
    • Heikki Linnakangas's avatar
      Move multixid allocation out of critical section. · b1236f4b
      Heikki Linnakangas authored
      It can fail if you run out of memory.
      
      This call was added in 9.3, so backpatch to 9.3 only.
      b1236f4b
    • Heikki Linnakangas's avatar
      In checkpoint, move the check for in-progress xacts out of critical section. · d9e7873b
      Heikki Linnakangas authored
      GetVirtualXIDsDelayingChkpt calls palloc, which isn't safe in a critical
      section. I thought I covered this case with the exemption for the
      checkpointer, but CreateCheckPoint is also called from the startup process.
      d9e7873b
    • Heikki Linnakangas's avatar
      Add an Assertion that you don't palloc within a critical section. · 4a170ee9
      Heikki Linnakangas authored
      This caught a bunch of cases doing that already, which I just fixed in
      previous commit. This is the assertion itself.
      
      Per Tom Lane's idea.
      4a170ee9
    • Heikki Linnakangas's avatar
      Avoid allocations in critical sections. · 877b0887
      Heikki Linnakangas authored
      If a palloc in a critical section fails, it becomes a PANIC.
      877b0887
    • Tom Lane's avatar
      Fix non-equivalence of VARIADIC and non-VARIADIC function call formats. · c7b35395
      Tom Lane authored
      For variadic functions (other than VARIADIC ANY), the syntaxes foo(x,y,...)
      and foo(VARIADIC ARRAY[x,y,...]) should be considered equivalent, since the
      former is converted to the latter at parse time.  They have indeed been
      equivalent, in all releases before 9.3.  However, commit 75b39e79 made an
      ill-considered decision to record which syntax had been used in FuncExpr
      nodes, and then to make equal() test that in checking node equality ---
      which caused the syntaxes to not be seen as equivalent by the planner.
      This is the underlying cause of bug #9817 from Dmitry Ryabov.
      
      It might seem that a quick fix would be to make equal() disregard
      FuncExpr.funcvariadic, but the same commit made that untenable, because
      the field actually *is* semantically significant for some VARIADIC ANY
      functions.  This patch instead adopts the approach of redefining
      funcvariadic (and aggvariadic, in HEAD) as meaning that the last argument
      is a variadic array, whether it got that way by parser intervention or was
      supplied explicitly by the user.  Therefore the value will always be true
      for non-ANY variadic functions, restoring the principle of equivalence.
      (However, the planner will continue to consider use of VARIADIC as a
      meaningful difference for VARIADIC ANY functions, even though some such
      functions might disregard it.)
      
      In HEAD, this change lets us simplify the decompilation logic in
      ruleutils.c, since the funcvariadic/aggvariadic flag tells directly whether
      to print VARIADIC.  However, in 9.3 we have to continue to cope with
      existing stored rules/views that might contain the previous definition.
      Fortunately, this just means no change in ruleutils.c, since its existing
      behavior effectively ignores funcvariadic for all cases other than VARIADIC
      ANY functions.
      
      In HEAD, bump catversion to reflect the fact that FuncExpr.funcvariadic
      changed meanings; this is sort of pro forma, since I don't believe any
      built-in views are affected.
      
      Unfortunately, this patch doesn't magically fix everything for affected
      9.3 users.  After installing 9.3.5, they might need to recreate their
      rules/views/indexes containing variadic function calls in order to get
      everything consistent with the new definition.  As in the cited bug,
      the symptom of a problem would be failure to use a nominally matching
      index that has a variadic function call in its definition.  We'll need
      to mention this in the 9.3.5 release notes.
      c7b35395
  4. 03 Apr, 2014 4 commits
    • Tom Lane's avatar
      Code review for commit d26888bc. · 741364bf
      Tom Lane authored
      Mostly, copy-edit the comments; but also fix it to not reject domains over
      arrays.
      741364bf
    • Tom Lane's avatar
      Fix documentation about joining pg_locks to other views. · 42c6236f
      Tom Lane authored
      The advice to join to pg_prepared_xacts via the transaction column was not
      updated when the transaction column was replaced by virtualtransaction.
      Since it's not quite obvious how to do that join, give an explicit example.
      For consistency also give an example for the adjacent case of joining to
      pg_stat_activity.  And link-ify the view references too, just because we
      can.  Per bug #9840 from Alexey Bashtanov.
      
      Michael Paquier and Tom Lane
      42c6236f
    • Tom Lane's avatar
      Avoid promising that "ADD COLUMN ... DEFAULT NULL" is free. · 879808e5
      Tom Lane authored
      The system realizes that DEFAULT NULL is dummy in simple cases, but not if
      a cast function (such as a length coercion) needs to be applied.  It's
      dubious that suppressing that function call would be appropriate, anyway.
      For the moment, let's just adjust the docs to say that you should omit the
      DEFAULT clause if you don't want a rewrite to happen.  Per gripe from Amit
      Langote.
      879808e5
    • Heikki Linnakangas's avatar
      Avoid palloc in critical section in GiST WAL-logging. · 04e298b8
      Heikki Linnakangas authored
      Memory allocation can fail if you run out of memory, and inside a critical
      section that will lead to a PANIC. Use conservatively-sized arrays in stack
      instead.
      
      There was previously no explicit limit on the number of pages a GiST split
      can produce, it was only limited by the number of LWLocks that can be held
      simultaneously (100 at the moment). This patch adds an explicit limit of 75
      pages. That should be plenty, a typical split shouldn't produce more than
      2-3 page halves.
      
      The bug has been there forever, but only backpatch down to 9.1. The code
      was changed significantly in 9.1, and it doesn't seem worth the risk or
      trouble to adapt this for 9.0 and 8.4.
      04e298b8
  5. 02 Apr, 2014 3 commits
    • Tom Lane's avatar
      Fix assorted issues in client host name lookup. · fc752505
      Tom Lane authored
      The code for matching clients to pg_hba.conf lines that specify host names
      (instead of IP address ranges) failed to complain if reverse DNS lookup
      failed; instead it silently didn't match, so that you might end up getting
      a surprising "no pg_hba.conf entry for ..." error, as seen in bug #9518
      from Mike Blackwell.  Since we don't want to make this a fatal error in
      situations where pg_hba.conf contains a mixture of host names and IP
      addresses (clients matching one of the numeric entries should not have to
      have rDNS data), remember the lookup failure and mention it as DETAIL if
      we get to "no pg_hba.conf entry".  Apply the same approach to forward-DNS
      lookup failures, too, rather than treating them as immediate hard errors.
      
      Along the way, fix a couple of bugs that prevented us from detecting an
      rDNS lookup error reliably, and make sure that we make only one rDNS lookup
      attempt; formerly, if the lookup attempt failed, the code would try again
      for each host name entry in pg_hba.conf.  Since more or less the whole
      point of this design is to ensure there's only one lookup attempt not one
      per entry, the latter point represents a performance bug that seems
      sufficient justification for back-patching.
      
      Also, adjust src/port/getaddrinfo.c so that it plays as well as it can
      with this code.  Which is not all that well, since it does not have actual
      support for rDNS lookup, but at least it should return the expected (and
      required by spec) error codes so that the main code correctly perceives the
      lack of functionality as a lookup failure.  It's unlikely that PG is still
      being used in production on any machines that require our getaddrinfo.c,
      so I'm not excited about working harder than this.
      
      To keep the code in the various branches similar, this includes
      back-patching commits c424d0d1 and
      1997f34d into 9.2 and earlier.
      
      Back-patch to 9.1 where the facility for hostnames in pg_hba.conf was
      introduced.
      fc752505
    • Tom Lane's avatar
      De-anonymize the union in JsonbValue. · f33a71a7
      Tom Lane authored
      Needed for strict C89 compliance.
      f33a71a7
    • Tom Lane's avatar
      Fix bugs in manipulation of PgBackendStatus.st_clienthostname. · 682c5bbe
      Tom Lane authored
      Initialization of this field was not being done according to the
      st_changecount protocol (it has to be done within the changecount increment
      range, not outside).  And the test to see if the value should be reported
      as null was wrong.  Noted while perusing uses of Port.remote_hostname.
      
      This was wrong from the introduction of this code (commit 4a25bc14),
      so back-patch to 9.1.
      682c5bbe
  6. 01 Apr, 2014 6 commits
  7. 31 Mar, 2014 6 commits
    • Robert Haas's avatar
      Mark FastPathStrongRelationLocks volatile. · 4bc15a8b
      Robert Haas authored
      Otherwise, the compiler might decide to move modifications to data
      within this structure outside the enclosing SpinLockAcquire /
      SpinLockRelease pair, leading to shared memory corruption.
      
      This may or may not explain a recent lmgr-related buildfarm failure
      on prairiedog, but it needs to be fixed either way.
      4bc15a8b
    • Robert Haas's avatar
      test_decoding: Update .gitignore · 0f95b723
      Robert Haas authored
      Commit 7317d8d9 changed the set of
      things that need to be ignored, but neglected to update .gitignore.
      0f95b723
    • Robert Haas's avatar
      Count buffers dirtied due to hints in pgBufferUsage.shared_blks_dirtied. · 066254ce
      Robert Haas authored
      Previously, such buffers weren't counted, with the possible result that
      EXPLAIN (BUFFERS) and pg_stat_statements would understate the true
      number of blocks dirtied by an SQL statement.
      
      Back-patch to 9.2, where this counter was introduced.
      
      Amit Kapila
      066254ce
    • Robert Haas's avatar
      Fix thinko in logical decoding code. · 3f0e4be4
      Robert Haas authored
      Andres Freund
      3f0e4be4
    • Heikki Linnakangas's avatar
      Rewrite the way GIN posting lists are packed on a page, to reduce WAL volume. · 14d02f0b
      Heikki Linnakangas authored
      Inserting (in retail) into the new 9.4 format GIN posting tree created much
      larger WAL records than in 9.3. The previous strategy to WAL logging was
      basically to log the whole page on each change, with the exception of
      completely unmodified segments up to the first modified one. That was not
      too bad when appending to the end of the page, as only the last segment had
      to be WAL-logged, but per Fujii Masao's testing, even that produced 2x the
      WAL volume that 9.3 did.
      
      The new strategy is to keep track of changes to the posting lists in a more
      fine-grained fashion, and also make the repacking" code smarter to avoid
      decoding and re-encoding segments unnecessarily.
      14d02f0b
    • Heikki Linnakangas's avatar
      Rename GinLogicValue to GinTernaryValue. · 0cfa34c2
      Heikki Linnakangas authored
      It's more descriptive. Also, get rid of the enum, and use #defines instead,
      per Greg Stark's suggestion.
      0cfa34c2
  8. 30 Mar, 2014 1 commit
    • Andrew Dunstan's avatar
      Use separate output dirs for test_decoding's two runs. · 7317d8d9
      Andrew Dunstan authored
      contrib/test_decoding's "make check" runs two sets of tests. Unless we
      specify separate output directories for each set the isolation tests
      will overwrite the output from the  normal regression set. Doing this
      will help the buildfarm collect complete logs.
      7317d8d9
  9. 29 Mar, 2014 2 commits
    • Bruce Momjian's avatar
      psql: display "Replica Identity" only for FULL and NOTHING · 9d661164
      Bruce Momjian authored
      INDEX is already displayed on the index, and we now exclude pg_catalog.
      DEFAULT is not displayed.
      9d661164
    • Tom Lane's avatar
      Fix dumping of a materialized view that depends on a table's primary key. · 62215de2
      Tom Lane authored
      It is possible for a view or materialized view to depend on a table's
      primary key, if the view query relies on functional dependency to
      abbreviate a GROUP BY list.  This is problematic for pg_dump since we
      ordinarily want to dump view definitions in the pre-data section but
      indexes in post-data.  pg_dump knows how to deal with this situation for
      regular views, by breaking the view's ON SELECT rule apart from the view
      proper.  But it had not been taught what to do about materialized views,
      and in fact mistakenly dumped them as regular views in such cases, as
      seen in bug #9616 from Jesse Denardo.
      
      If we had CREATE OR REPLACE MATERIALIZED VIEW, we could fix this in a
      manner analogous to what's done for regular views; but we don't yet,
      and we'd not back-patch such a thing into 9.3 anyway.  As a hopefully-
      temporary workaround, break the circularity by postponing the matview
      into post-data altogether when this case occurs.
      62215de2