1. 22 Jul, 2017 1 commit
  2. 21 Jul, 2017 8 commits
    • Tom Lane's avatar
      Doc: update versioning information in libpq.sgml. · e22efaab
      Tom Lane authored
      The descriptions of PQserverVersion and PQlibVersion hadn't been updated
      for the new two-part version-numbering approach.  Fix that.
      
      In passing, remove some trailing whitespace elsewhere in the file.
      e22efaab
    • Robert Haas's avatar
      pg_rewind: Fix some problems when copying files >2GB. · a46fe6e8
      Robert Haas authored
      When incrementally updating a file larger than 2GB, the old code could
      either fail outright (if the client asked the server for bytes beyond
      the 2GB boundary) or fail to copy all the blocks that had actually
      been modified (if the server reported a file size to the client in
      excess of 2GB), resulting in data corruption.  Generally, such files
      won't occur anyway, but they might if using a non-default segment size
      or if there the directory contains stray files unrelated to
      PostgreSQL.  Fix by a more prudent choice of data types.
      
      Even with these improvements, this code still uses a mix of different
      types (off_t, size_t, uint64, int64) to represent file sizes and
      offsets, not all of which necessarily have the same width or
      signedness, so further cleanup might be in order here.  However, at
      least now they all have the potential to be 64 bits wide on 64-bit
      platforms.
      
      Kuntal Ghosh and Michael Paquier, with a tweak by me.
      
      Discussion: http://postgr.es/m/CAGz5QC+8gbkz=Brp0TgoKNqHWTzonbPtPex80U0O6Uh_bevbaA@mail.gmail.com
      a46fe6e8
    • Tom Lane's avatar
      Stabilize postgres_fdw regression tests. · 88f48b57
      Tom Lane authored
      The new test cases added in commit 8bf58c0d turn out to have output
      that can vary depending on the lc_messages setting prevailing on the
      test server.  Hide the remote end's error messages to ensure stable
      output.  This isn't a terribly desirable solution; we'd rather know
      that the connection failed for the expected reason and not some other
      one.  But there seems little choice for the moment.
      
      Per buildfarm.
      
      Discussion: https://postgr.es/m/18419.1500658570@sss.pgh.pa.us
      88f48b57
    • Robert Haas's avatar
      pg_rewind: Fix busted sanity check. · 063ff921
      Robert Haas authored
      As written, the code would only fail the sanity check if none of the
      columns returned by the server were of the expected type, but we want
      it to fail if even one column is not of the expected type.
      
      Discussion: http://postgr.es/m/CA+TgmoYuY5zW7JEs+1hSS1D=V5K8h1SQuESrq=bMNeo0B71Sfw@mail.gmail.com
      063ff921
    • Tom Lane's avatar
      Re-establish postgres_fdw connections after server or user mapping changes. · 8bf58c0d
      Tom Lane authored
      Previously, postgres_fdw would keep on using an existing connection even
      if the user did ALTER SERVER or ALTER USER MAPPING commands that should
      affect connection parameters.  Teach it to watch for catcache invals
      on these catalogs and re-establish connections when the relevant catalog
      entries change.  Per bug #14738 from Michal Lis.
      
      In passing, clean up some rather crufty decisions in commit ae9bfc5d
      about where fields of ConnCacheEntry should be reset.  We now reset
      all the fields whenever we open a new connection.
      
      Kyotaro Horiguchi, reviewed by Ashutosh Bapat and myself.
      Back-patch to 9.3 where postgres_fdw appeared.
      
      Discussion: https://postgr.es/m/20170710113917.7727.10247@wrigleys.postgresql.org
      8bf58c0d
    • Teodor Sigaev's avatar
      Fix double shared memory allocation. · 7e1fb4c5
      Teodor Sigaev authored
      SLRU buffer lwlocks are allocated twice by oversight in commit
      fe702a7b where that locks were moved to
      separate tranche. The bug doesn't have user-visible effects except small
      overspending of shared memory.
      
      Backpatch to 9.6 where it was introduced.
      
      Alexander Korotkov with small editorization by me.
      7e1fb4c5
    • Dean Rasheed's avatar
      Make the new partition regression tests locale-independent. · 68f785fd
      Dean Rasheed authored
      The order of partitions listed by \d+ is in general locale-dependent.
      Rename the partitions in the test added by d363d42b to force them to
      be listed in a consistent order.
      68f785fd
    • Dean Rasheed's avatar
      Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition bounds. · d363d42b
      Dean Rasheed authored
      Previously, UNBOUNDED meant no lower bound when used in the FROM list,
      and no upper bound when used in the TO list, which was OK for
      single-column range partitioning, but problematic with multiple
      columns. For example, an upper bound of (10.0, UNBOUNDED) would not be
      collocated with a lower bound of (10.0, UNBOUNDED), thus making it
      difficult or impossible to define contiguous multi-column range
      partitions in some cases.
      
      Fix this by using MINVALUE and MAXVALUE instead of UNBOUNDED to
      represent a partition column that is unbounded below or above
      respectively. This syntax removes any ambiguity, and ensures that if
      one partition's lower bound equals another partition's upper bound,
      then the partitions are contiguous.
      
      Also drop the constraint prohibiting finite values after an unbounded
      column, and just document the fact that any values after MINVALUE or
      MAXVALUE are ignored. Previously it was necessary to repeat UNBOUNDED
      multiple times, which was needlessly verbose.
      
      Note: Forces a post-PG 10 beta2 initdb.
      
      Report by Amul Sul, original patch by Amit Langote with some
      additional hacking by me.
      
      Discussion: https://postgr.es/m/CAAJ_b947mowpLdxL3jo3YLKngRjrq9+Ej4ymduQTfYR+8=YAYQ@mail.gmail.com
      d363d42b
  3. 20 Jul, 2017 3 commits
  4. 19 Jul, 2017 2 commits
  5. 18 Jul, 2017 5 commits
    • Tom Lane's avatar
      Improve make_tsvector() to handle empty input, and simplify its callers. · 04a2c7f4
      Tom Lane authored
      It seemed a bit silly that each caller of make_tsvector() was laboriously
      special-casing the situation where no lexemes were found, when it would
      be easy and much more bullet-proof to make make_tsvector() handle that.
      04a2c7f4
    • Tom Lane's avatar
      Fix serious performance problems in json(b) to_tsvector(). · b4c6d31c
      Tom Lane authored
      In an off-list followup to bug #14745, Bob Jones complained that
      to_tsvector() on a 2MB jsonb value took an unreasonable amount of
      time and space --- enough to draw the wrath of the OOM killer on
      his machine.  On my machine, his example proved to require upwards
      of 18 seconds and 4GB, which seemed pretty bogus considering that
      to_tsvector() on the same data treated as text took just a couple
      hundred msec and 10 or so MB.
      
      On investigation, the problem is that the implementation scans each
      string element of the json(b) and converts it to tsvector separately,
      then applies tsvector_concat() to join those separate tsvectors.
      The unreasonable memory usage came from leaking every single one of
      the transient tsvectors --- but even without that mistake, this is an
      O(N^2) or worse algorithm, because tsvector_concat() has to repeatedly
      process the words coming from earlier elements.
      
      We can fix it by accumulating all the lexeme data and applying
      make_tsvector() just once.  As a side benefit, that also makes the
      desired adjustment of lexeme positions far cheaper, because we can
      just tweak the running "pos" counter between JSON elements.
      
      In passing, try to make the explanation of that tweak more intelligible.
      (I didn't think that a barely-readable comment far removed from the
      actual code was helpful.)  And do some minor other code beautification.
      b4c6d31c
    • Tom Lane's avatar
      Doc: fix thinko in v10 release notes. · fb9bd4b0
      Tom Lane authored
      s/log_destination/log_directory/, per Jov in bug #14749.
      
      Report: https://postgr.es/m/20170718082444.9229.99690@wrigleys.postgresql.org
      fb9bd4b0
    • Robert Haas's avatar
      Reverse-convert row types in ExecWithCheckOptions. · c85ec643
      Robert Haas authored
      Just as we already do in ExecConstraints, and for the same reason:
      to improve the quality of error messages.
      
      Etsuro Fujita, reviewed by Amit Langote
      
      Discussion: http://postgr.es/m/56e0baa8-e458-2bbb-7936-367f7d832e43@lab.ntt.co.jp
      c85ec643
    • Robert Haas's avatar
      Use a real RT index when setting up partition tuple routing. · f81a91db
      Robert Haas authored
      Before, we always used a dummy value of 1, but that's not right when
      the partitioned table being modified is inside of a WITH clause
      rather than part of the main query.
      
      Amit Langote, reported and reviewd by Etsuro Fujita, with a comment
      change by me.
      
      Discussion: http://postgr.es/m/ee12f648-8907-77b5-afc0-2980bcb0aa37@lab.ntt.co.jp
      f81a91db
  6. 17 Jul, 2017 6 commits
  7. 16 Jul, 2017 2 commits
  8. 15 Jul, 2017 2 commits
    • Tom Lane's avatar
      Improve comments for execExpr.c's handling of FieldStore subexpressions. · de2af6e0
      Tom Lane authored
      Given this code's general eagerness to use subexpressions' output variables
      as temporary workspace, it's not exactly clear that it is safe for
      FieldStore to tell a newval subexpression that it can write into the same
      variable that is being supplied as a potential input.  Document the chain
      of assumptions needed for that to be safe.
      de2af6e0
    • Tom Lane's avatar
      Improve comments for execExpr.c's isAssignmentIndirectionExpr(). · e9b64824
      Tom Lane authored
      I got confused about why this function doesn't need to recursively
      search the expression tree for a CaseTestExpr node.  After figuring
      that out, add a comment to save the next person some time.
      e9b64824
  9. 14 Jul, 2017 4 commits
    • Alvaro Herrera's avatar
      pg_upgrade i18n: Fix "%s server/cluster" wording · 837255cc
      Alvaro Herrera authored
      The original wording was impossible to translate correctly.
      
      Discussion: https://postgr.es/m/20170523002827.lzc2jkzh2gubclqb@alvherre.pgsql
      837255cc
    • Tom Lane's avatar
      Code review for NextValueExpr expression node type. · decb08eb
      Tom Lane authored
      Add missing infrastructure for this node type, notably in ruleutils.c where
      its lack could demonstrably cause EXPLAIN to fail.  Add outfuncs/readfuncs
      support.  (outfuncs support is useful today for debugging purposes.  The
      readfuncs support may never be needed, since at present it would only
      matter for parallel query and NextValueExpr should never appear in a
      parallelizable query; but it seems like a bad idea to have a primnode type
      that isn't fully supported here.)  Teach planner infrastructure that
      NextValueExpr is a volatile, parallel-unsafe, non-leaky expression node
      with cost cpu_operator_cost.  Given its limited scope of usage, there
      *might* be no live bug today from the lack of that knowledge, but it's
      certainly going to bite us on the rear someday.  Teach pg_stat_statements
      about the new node type, too.
      
      While at it, also teach cost_qual_eval() that MinMaxExpr, SQLValueFunction,
      XmlExpr, and CoerceToDomain should be charged as cpu_operator_cost.
      Failing to do this for SQLValueFunction was an oversight in my commit
      0bb51aa9.  The others are longer-standing oversights, but no time like the
      present to fix them.  (In principle, CoerceToDomain could have cost much
      higher than this, but it doesn't presently seem worth trying to examine the
      domain's constraints here.)
      
      Modify execExprInterp.c to execute NextValueExpr as an out-of-line
      function; it seems quite unlikely to me that it's worth insisting that
      it be inlined in all expression eval methods.  Besides, providing the
      out-of-line function doesn't stop anyone from inlining if they want to.
      
      Adjust some places where NextValueExpr support had been inserted with the
      aid of a dartboard rather than keeping it in the same order as elsewhere.
      
      Discussion: https://postgr.es/m/23862.1499981661@sss.pgh.pa.us
      decb08eb
    • Tom Lane's avatar
      Fix broken link-command-line ordering for libpgfeutils. · c95275fc
      Tom Lane authored
      In the frontend Makefiles that pull in libpgfeutils, we'd generally
      done it like this:
      
      LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)
      
      That method is badly broken, as seen in bug #14742 from Chris Ruprecht.
      The -L flag for src/fe_utils ends up being placed after whatever random
      -L flags are in LDFLAGS already.  That puts us at risk of pulling in
      libpgfeutils.a from some previous installation rather than the freshly
      built one in src/fe_utils.  Also, the lack of an "override" is hazardous
      if someone tries to specify some LDFLAGS on the make command line.
      
      The correct way to do it is like this:
      
      override LDFLAGS := -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) $(LDFLAGS)
      
      so that libpgfeutils, along with libpq, libpgport, and libpgcommon, are
      guaranteed to be pulled in from the build tree and not from any referenced
      system directory, because their -L flags will appear first.
      
      In some places we'd been even lazier and done it like this:
      
      LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils -lpq
      
      which is subtly wrong in an additional way: on platforms where we can't
      restrict the symbols exported by libpq.so, it allows libpgfeutils to
      latch onto libpgport and libpgcommon symbols from libpq.so, rather than
      directly from those static libraries as intended.  This carries hazards
      like those explained in the comments for the libpq_pgport macro.
      
      In addition to fixing the broken libpgfeutils usages, I tried to
      standardize on using $(libpq_pgport) like so:
      
      override LDFLAGS := $(libpq_pgport) $(LDFLAGS)
      
      even where libpgfeutils is not in the picture.  This makes no difference
      right now but will hopefully discourage future mistakes of the same ilk.
      And it's more like the way we handle CPPFLAGS in libpq-using Makefiles.
      
      In passing, just for consistency, make pgbench include PTHREAD_LIBS the
      same way everyplace else does, ie just after LIBS rather than in some
      random place in the command line.  This might have practical effect if
      there are -L switches in that macro on some platform.
      
      It looks to me like the MSVC build scripts are not affected by this
      error, but someone more familiar with them than I might want to double
      check.
      
      Back-patch to 9.6 where libpgfeutils was introduced.  In 9.6, the hazard
      this error creates is that a reinstallation might link to the prior
      installation's copy of libpgfeutils.a and thereby fail to absorb a
      minor-version bug fix.
      
      Discussion: https://postgr.es/m/20170714125106.9231.13772@wrigleys.postgresql.org
      c95275fc
    • Heikki Linnakangas's avatar
      Fix pg_basebackup output to stdout on Windows. · 8046465c
      Heikki Linnakangas authored
      When writing a backup to stdout with pg_basebackup on Windows, put stdout
      to binary mode. Any CR bytes in the output will otherwise be output
      incorrectly as CR+LF.
      
      In the passing, standardize on using "_setmode" instead of "setmode", for
      the sake of consistency. They both do the same thing, but according to
      MSDN documentation, setmode is deprecated.
      
      Fixes bug #14634, reported by Henry Boehlert. Patch by Haribabu Kommi.
      Backpatch to all supported versions.
      
      Discussion: https://www.postgresql.org/message-id/20170428082818.24366.13134@wrigleys.postgresql.org
      8046465c
  10. 13 Jul, 2017 3 commits
    • Tom Lane's avatar
      Fix dumping of FUNCTION RTEs that contain non-function-call expressions. · a3ca72ae
      Tom Lane authored
      The grammar will only accept something syntactically similar to a function
      call in a function-in-FROM expression.  However, there are various ways
      to input something that ruleutils.c won't deparse that way, potentially
      leading to a view or rule that fails dump/reload.  Fix by inserting a
      dummy CAST around anything that isn't going to deparse as a function
      (which is one of the ways to get something like that in there in the
      first place).
      
      In HEAD, also make use of the infrastructure added by this to avoid
      emitting unnecessary parentheses in CREATE INDEX deparsing.  I did
      not change that in back branches, thinking that people might find it
      to be unexpected/unnecessary behavioral change.
      
      In HEAD, also fix incorrect logic for when to add extra parens to
      partition key expressions.  Somebody apparently thought they could
      get away with simpler logic than pg_get_indexdef_worker has, but
      they were wrong --- a counterexample is PARTITION BY LIST ((a[1])).
      Ignoring the prettyprint flag for partition expressions isn't exactly
      a nice solution anyway.
      
      This has been broken all along, so back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/10477.1499970459@sss.pgh.pa.us
      a3ca72ae
    • Alvaro Herrera's avatar
      Fix typo in v10 release notes · 2036f71b
      Alvaro Herrera authored
      The new functions return a list of files in the corresponding directory,
      not the name of the directory itself.
      
      Pointed out by Gianni Ciolli.
      2036f71b
    • Heikki Linnakangas's avatar
      Fix race between GetNewTransactionId and GetOldestActiveTransactionId. · 74fc8386
      Heikki Linnakangas authored
      The race condition goes like this:
      
      1. GetNewTransactionId advances nextXid e.g. from 100 to 101
      2. GetOldestActiveTransactionId reads the new nextXid, 101
      3. GetOldestActiveTransactionId loops through the proc array. There are no
         active XIDs there, so it returns 101 as the oldest active XID.
      4. GetNewTransactionid stores XID 100 to MyPgXact->xid
      
      So, GetOldestActiveTransactionId returned XID 101, even though 100 only
      just started and is surely still running.
      
      This would be hard to hit in practice, and even harder to spot any ill
      effect if it happens. GetOldestActiveTransactionId is only used when
      creating a checkpoint in a master server, and the race condition can only
      happen on an online checkpoint, as there are no backends running during a
      shutdown checkpoint. The oldestActiveXid value of an online checkpoint is
      only used when starting up a hot standby server, to determine the starting
      point where pg_subtrans is initialized from. For the race condition to
      happen, there must be no other XIDs in the proc array that would hold back
      the oldest-active XID value, which means that the missed XID must be a top
      transaction's XID. However, pg_subtrans is not used for top XIDs, so I
      believe an off-by-one error is in fact inconsequential. Nevertheless, let's
      fix it, as it's clearly wrong and the fix is simple.
      
      This has been wrong ever since hot standby was introduced, so backport to
      all supported versions.
      
      Discussion: https://www.postgresql.org/message-id/e7258662-82b6-7a45-56d4-99b337a32bf7@iki.fi
      74fc8386
  11. 12 Jul, 2017 4 commits
    • Tom Lane's avatar
      Fix ruleutils.c for domain-over-array cases, too. · bc2d716a
      Tom Lane authored
      Further investigation shows that ruleutils isn't quite up to speed either
      for cases where we have a domain-over-array: it needs to be prepared to
      look past a CoerceToDomain at the top level of field and element
      assignments, else it decompiles them incorrectly.  Potentially this would
      result in failure to dump/reload a rule, if it looked like the one in the
      new test case.  (I also added a test for EXPLAIN; that output isn't broken,
      but clearly we need more test coverage here.)
      
      Like commit b1cb32fb, this bug is reachable in cases we already support,
      so back-patch all the way.
      bc2d716a
    • Heikki Linnakangas's avatar
      Reduce memory usage of tsvector type analyze function. · da11977d
      Heikki Linnakangas authored
      compute_tsvector_stats() detoasted and kept in memory every tsvector value
      in the sample, but that can be a lot of memory. The original bug report
      described a case using over 10 gigabytes, with statistics target of 10000
      (the maximum).
      
      To fix, allocate a separate copy of just the lexemes that we keep around,
      and free the detoasted tsvector values as we go. This adds some palloc/pfree
      overhead, when you have a lot of distinct lexemes in the sample, but it's
      better than running out of memory.
      
      Fixes bug #14654 reported by James C. Reviewed by Tom Lane. Backport to
      all supported versions.
      
      Discussion: https://www.postgresql.org/message-id/20170514200602.1451.46797@wrigleys.postgresql.org
      da11977d
    • Alvaro Herrera's avatar
      commit_ts test: Set node name in test · ca793c59
      Alvaro Herrera authored
      Otherwise, the script output has a lot of pointless warnings.
      
      This was forgotten in 9def031b
      ca793c59
    • Tom Lane's avatar
      Avoid integer overflow while sifting-up a heap in tuplesort.c. · 512f67c8
      Tom Lane authored
      If the number of tuples in the heap exceeds approximately INT_MAX/2,
      this loop's calculation "2*i+1" could overflow, resulting in a crash.
      Fix it by using unsigned int rather than int for the relevant local
      variables; that shouldn't cost anything extra on any popular hardware.
      Per bug #14722 from Sergey Koposov.
      
      Original patch by Sergey Koposov, modified by me per a suggestion
      from Heikki Linnakangas to use unsigned int not int64.
      
      Back-patch to 9.4, where tuplesort.c grew the ability to sort as many
      as INT_MAX tuples in-memory (commit 263865a4).
      
      Discussion: https://postgr.es/m/20170629161637.1478.93109@wrigleys.postgresql.org
      512f67c8