1. 23 Dec, 2016 4 commits
  2. 22 Dec, 2016 14 commits
    • Robert Haas's avatar
      Fix tuple routing in cases where tuple descriptors don't match. · 2ac3ef7a
      Robert Haas authored
      The previous coding failed to work correctly when we have a
      multi-level partitioned hierarchy where tables at successive levels
      have different attribute numbers for the partition key attributes.  To
      fix, have each PartitionDispatch object store a standalone
      TupleTableSlot initialized with the TupleDesc of the corresponding
      partitioned table, along with a TupleConversionMap to map tuples from
      the its parent's rowtype to own rowtype.  After tuple routing chooses
      a leaf partition, we must use the leaf partition's tuple descriptor,
      not the root table's.  To that end, a dedicated TupleTableSlot for
      tuple routing is now allocated in EState.
      
      Amit Langote
      2ac3ef7a
    • Stephen Frost's avatar
      Use TSConfigRelationId in AlterTSConfiguration() · 12bd7dd3
      Stephen Frost authored
      When we are altering a text search configuration, we are getting the
      tuple from pg_ts_config and using its OID, so use TSConfigRelationId
      when invoking any post-alter hooks and setting the object address.
      
      Further, in the functions called from AlterTSConfiguration(), we're
      saving information about the command via
      EventTriggerCollectAlterTSConfig(), so we should be setting
      commandCollected to true.  Also add a regression test to
      test_ddl_deparse for ALTER TEXT SEARCH CONFIGURATION.
      
      Author: Artur Zakirov, a few additional comments by me
      Discussion: https://www.postgresql.org/message-id/57a71eba-f2c7-e7fd-6fc0-2126ec0b39bd%40postgrespro.ru
      
      Back-patch the fix for the InvokeObjectPostAlterHook() call to 9.3 where
      it was introduced, and the fix for the ObjectAddressSet() call and
      setting commandCollected to true to 9.5 where those changes to
      ProcessUtilitySlow() were introduced.
      12bd7dd3
    • Tom Lane's avatar
      Fix CREATE TABLE ... LIKE ... WITH OIDS. · 1ead0208
      Tom Lane authored
      Having a WITH OIDS specification should result in the creation of an OID
      column, but commit b943f502 broke that in the case that there were LIKE
      tables without OIDS.  Commentary in that patch makes it look like this was
      intentional, but if so it was based on a faulty reading of what inheritance
      does: the parent tables can add an OID column, but they can't subtract one.
      AFAICS, the behavior ought to be that you get an OID column if any of the
      inherited tables, LIKE tables, or WITH clause ask for one.
      
      Also, revert that patch's unnecessary split of transformCreateStmt's loop
      over the tableElts list into two passes.  That seems to have been based on
      a misunderstanding as well: we already have two-pass processing here,
      we don't need three passes.
      
      Per bug #14474 from Jeff Dafoe.  Back-patch to 9.6 where the misbehavior
      was introduced.
      
      Report: https://postgr.es/m/20161222145304.25620.47445@wrigleys.postgresql.org
      1ead0208
    • Peter Eisentraut's avatar
      22434dd0
    • Peter Eisentraut's avatar
      909cb78a
    • Tom Lane's avatar
      Fix handling of expanded objects in CoerceToDomain and CASE execution. · cd1b2156
      Tom Lane authored
      When the input value to a CoerceToDomain expression node is a read-write
      expanded datum, we should pass a read-only pointer to any domain CHECK
      expressions and then return the original read-write pointer as the
      expression result.  Previously we were blindly passing the same pointer to
      all the consumers of the value, making it possible for a function in CHECK
      to modify or even delete the expanded value.  (Since a plpgsql function
      will absorb a passed-in read-write expanded array as a local variable
      value, it will in fact delete the value on exit.)
      
      A similar hazard of passing the same read-write pointer to multiple
      consumers exists in domain_check() and in ExecEvalCase, so fix those too.
      
      The fix requires adding MakeExpandedObjectReadOnly calls at the appropriate
      places, which is simple enough except that we need to get the data type's
      typlen from somewhere.  For the domain cases, solve this by redefining
      DomainConstraintRef.tcache as okay for callers to access; there wasn't any
      reason for the original convention against that, other than not wanting the
      API of typcache.c to be any wider than it had to be.  For CASE, there's
      no good solution except to add a syscache lookup during executor start.
      
      Per bug #14472 from Marcos Castedo.  Back-patch to 9.5 where expanded
      values were introduced.
      
      Discussion: https://postgr.es/m/15225.1482431619@sss.pgh.pa.us
      cd1b2156
    • Andres Freund's avatar
      Skip checkpoints, archiving on idle systems. · 6ef2eba3
      Andres Freund authored
      Some background activity (like checkpoints, archive timeout, standby
      snapshots) is not supposed to happen on an idle system. Unfortunately
      so far it was not easy to determine when a system is idle, which
      defeated some of the attempts to avoid redundant activity on an idle
      system.
      
      To make that easier, allow to make individual WAL insertions as not
      being "important". By checking whether any important activity happened
      since the last time an activity was performed, it now is easy to check
      whether some action needs to be repeated.
      
      Use the new facility for checkpoints, archive timeout and standby
      snapshots.
      
      The lack of a facility causes some issues in older releases, but in my
      opinion the consequences (superflous checkpoints / archived segments)
      aren't grave enough to warrant backpatching.
      
      Author: Michael Paquier, editorialized by Andres Freund
      Reviewed-By: Andres Freund, David Steele, Amit Kapila, Kyotaro HORIGUCHI
      Bug: #13685
      Discussion:
          https://www.postgresql.org/message-id/20151016203031.3019.72930@wrigleys.postgresql.org
          https://www.postgresql.org/message-id/CAB7nPqQcPqxEM3S735Bd2RzApNqSNJVietAC=6kfkYv_45dKwA@mail.gmail.com
      Backpatch: -
      6ef2eba3
    • Robert Haas's avatar
      Fix broken error check in _hash_doinsert. · 097e4143
      Robert Haas authored
      You can't just cast a HashMetaPage to a Page, because the meta page
      data is stored after the page header, not at offset 0.  Fortunately,
      this didn't break anything because it happens to find hashm_bsize
      at the offset at which it expects to find pd_pagesize_version, and
      the values are close enough to the same that this works out.
      
      Still, it's a bug, so back-patch to all supported versions.
      
      Mithun Cy, revised a bit by me.
      097e4143
    • Joe Conway's avatar
      Make dblink try harder to form useful error messages · 2f802d95
      Joe Conway authored
      When libpq encounters a connection-level error, e.g. runs out of memory
      while forming a result, there will be no error associated with PGresult,
      but a message will be placed into PGconn's error buffer. postgres_fdw
      takes care to use the PGconn error message when PGresult does not have
      one, but dblink has been negligent in that regard. Modify dblink to mirror
      what postgres_fdw has been doing.
      
      Back-patch to all supported branches.
      
      Author: Joe Conway
      Reviewed-By: Tom Lane
      Discussion: https://postgr.es/m/02fa2d90-2efd-00bc-fefc-c23c00eb671e%40joeconway.com
      2f802d95
    • Robert Haas's avatar
      Code review for ATExecAttachPartition. · 3ee80672
      Robert Haas authored
      Amit Langote.  Most of this reported by Álvaro Herrera.
      3ee80672
    • Joe Conway's avatar
      Protect dblink from invalid options when using postgres_fdw server · c4448683
      Joe Conway authored
      When dblink uses a postgres_fdw server name for its connection, it
      is possible for the connection to have options that are invalid
      with dblink (e.g. "updatable"). The recommended way to avoid this
      problem is to use dblink_fdw servers instead. However there are use
      cases for using postgres_fdw, and possibly other FDWs, for dblink
      connection options, therefore protect against trying to use any
      options that do not apply by using is_valid_dblink_option() when
      building the connection string from the options.
      
      Back-patch to 9.3. Although 9.2 supports FDWs for connection info,
      is_valid_dblink_option() did not yet exist, and neither did
      postgres_fdw, at least in the postgres source tree. Given the lack
      of previous complaints, fixing that seems too invasive/not worth it.
      
      Author: Corey Huinker
      Reviewed-By: Joe Conway
      Discussion: https://postgr.es/m/CADkLM%3DfWyXVEyYcqbcRnxcHutkP45UHU9WD7XpdZaMfe7S%3DRwA%40mail.gmail.com
      c4448683
    • Heikki Linnakangas's avatar
      Simplify tape block format. · 01ec2563
      Heikki Linnakangas authored
      No more indirect blocks. The blocks form a linked list instead.
      
      This saves some memory, because we don't need to have a buffer in memory to
      hold the indirect block (or blocks). To reflect that, TAPE_BUFFER_OVERHEAD
      is reduced from 3 to 1 buffer, which allows using more memory for building
      the initial runs.
      
      Reviewed by Peter Geoghegan and Robert Haas.
      
      Discussion: https://www.postgresql.org/message-id/34678beb-938e-646e-db9f-a7def5c44ada%40iki.fi
      01ec2563
    • Tom Lane's avatar
      Give a useful error message if uuid-ossp is built without preconfiguration. · b86515da
      Tom Lane authored
      Before commit b8cc8f94, it was possible to build contrib/uuid-ossp without
      having told configure you meant to; you could just cd into that directory
      and "make".  That no longer works because the code depends on configure to
      have done header and library probes, but the ensuing error messages are
      not so easy to interpret if you're not an old C hand.  We've gotten a
      couple of complaints recently from people trying to do this the low-tech
      way, so add an explicit #error directing the user to use --with-uuid.
      
      (In principle we might want to do something similar in the other
      optionally-built contrib modules; but I don't think any of the others have
      ever worked without preconfiguration, so there are no bad habits to break
      people of.)
      
      Back-patch to 9.4 where the previous commit came in.
      
      Report: https://postgr.es/m/CAHeEsBf42AWTnk=1qJvFv+mYgRFm07Knsfuc86Ono8nRjf3tvQ@mail.gmail.com
      Report: https://postgr.es/m/CAKYdkBrUaZX+F6KpmzoHqMtiUqCtAW_w6Dgvr6F0WTiopuGxow@mail.gmail.com
      b86515da
    • Michael Meskes's avatar
      Fix buffer overflow on particularly named files and clarify documentation about · 4032ef18
      Michael Meskes authored
      output file naming.
      
      Patch by Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>
      4032ef18
  3. 21 Dec, 2016 14 commits
    • Joe Conway's avatar
      Improve dblink error message when remote does not provide it · ea0aa969
      Joe Conway authored
      When dblink or postgres_fdw detects an error on the remote side of the
      connection, it will try to construct a local error message as best it
      can using libpq's PQresultErrorField(). When no primary message is
      available, it was bailing out with an unhelpful "unknown error". Make
      that message better and more style guide compliant. Per discussion
      on hackers.
      
      Backpatch to 9.2 except postgres_fdw which didn't exist before 9.3.
      
      Discussion: https://postgr.es/m/19872.1482338965%40sss.pgh.pa.us
      ea0aa969
    • Tom Lane's avatar
      Fix detection of unfinished Unicode surrogate pair at end of string. · a8ae1232
      Tom Lane authored
      The U&'...' and U&"..." syntaxes silently discarded a surrogate pair
      start (that is, a code between U+D800 and U+DBFF) if it occurred at
      the very end of the string.  This seems like an obvious oversight,
      since we throw an error for every other invalid combination of surrogate
      characters, including the very same situation in E'...' syntax.
      
      This has been wrong since the pair processing was added (in 9.0),
      so back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/19113.1482337898@sss.pgh.pa.us
      a8ae1232
    • Tom Lane's avatar
      Fix strange behavior (and possible crashes) in full text phrase search. · 89fcea1a
      Tom Lane authored
      In an attempt to simplify the tsquery matching engine, the original
      phrase search patch invented rewrite rules that would rearrange a
      tsquery so that no AND/OR/NOT operator appeared below a PHRASE operator.
      But this approach had numerous problems.  The rearrangement step was
      missed by ts_rewrite (and perhaps other places), allowing tsqueries
      to be created that would cause Assert failures or perhaps crashes at
      execution, as reported by Andreas Seltenreich.  The rewrite rules
      effectively defined semantics for operators underneath PHRASE that were
      buggy, or at least unintuitive.  And because rewriting was done in
      tsqueryin() rather than at execution, the rearrangement was user-visible,
      which is not very desirable --- for example, it might cause unexpected
      matches or failures to match in ts_rewrite.
      
      As a somewhat independent problem, the behavior of nested PHRASE operators
      was only sane for left-deep trees; queries like "x <-> (y <-> z)" did not
      behave intuitively at all.
      
      To fix, get rid of the rewrite logic altogether, and instead teach the
      tsquery execution engine to manage AND/OR/NOT below a PHRASE operator
      by explicitly computing the match location(s) and match widths for these
      operators.
      
      This requires introducing some additional fields into the publicly visible
      ExecPhraseData struct; but since there's no way for third-party code to
      pass such a struct to TS_phrase_execute, it shouldn't create an ABI problem
      as long as we don't move the offsets of the existing fields.
      
      Another related problem was that index searches supposed that "!x <-> y"
      could be lossily approximated as "!x & y", which isn't correct because
      the latter will reject, say, "x q y" which the query itself accepts.
      This required some tweaking in TS_execute_ternary along with the main
      tsquery engine.
      
      Back-patch to 9.6 where phrase operators were introduced.  While this
      could be argued to change behavior more than we'd like in a stable branch,
      we have to do something about the crash hazards and index-vs-seqscan
      inconsistency, and it doesn't seem desirable to let the unintuitive
      behaviors induced by the rewriting implementation stand as precedent.
      
      Discussion: https://postgr.es/m/28215.1481999808@sss.pgh.pa.us
      Discussion: https://postgr.es/m/26706.1482087250@sss.pgh.pa.us
      89fcea1a
    • Stephen Frost's avatar
      Improve ALTER TABLE documentation · 2d1018ca
      Stephen Frost authored
      The ALTER TABLE documentation wasn't terribly clear when it came to
      which commands could be combined together and what it meant when they
      were.
      
      In particular, SET TABLESPACE *can* be combined with other commands,
      when it's operating against a single table, but not when multiple tables
      are being moved with ALL IN TABLESPACE.  Further, the actions are
      applied together but not really in 'parallel', at least today.
      
      Pointed out by: Amit Langote
      
      Improved wording from Tom.
      
      Back-patch to 9.4, where the ALL IN TABLESPACE option was added.
      
      Discussion: https://www.postgresql.org/message-id/14c535b4-13ef-0590-1b98-76af355a0763%40lab.ntt.co.jp
      2d1018ca
    • Stephen Frost's avatar
      Fix dumping of casts and transforms using built-in functions · 2259bf67
      Stephen Frost authored
      In pg_dump.c dumpCast() and dumpTransform(), we would happily ignore the
      cast or transform if it happened to use a built-in function because we
      weren't including the information about built-in functions when querying
      pg_proc from getFuncs().
      
      Modify the query in getFuncs() to also gather information about
      functions which are used by user-defined casts and transforms (where
      "user-defined" means "has an OID >= FirstNormalObjectId").  This also
      adds to the TAP regression tests for 9.6 and master to cover these
      types of objects.
      
      Back-patch all the way for casts, back to 9.5 for transforms.
      
      Discussion: https://www.postgresql.org/message-id/flat/20160504183952.GE10850%40tamriel.snowman.net
      2259bf67
    • Stephen Frost's avatar
      For 8.0 servers, get last built-in oid from pg_database · 19990918
      Stephen Frost authored
      We didn't start ensuring that all built-in objects had OIDs less than
      16384 until 8.1, so for 8.0 servers we still need to query the value out
      of pg_database.  We need this, in particular, to distinguish which casts
      were built-in and which were user-defined.
      
      For HEAD, we only worry about going back to 8.0, for the back-branches,
      we also ensure that 7.0-7.4 work.
      
      Discussion: https://www.postgresql.org/message-id/flat/20160504183952.GE10850%40tamriel.snowman.net
      19990918
    • Dean Rasheed's avatar
      Fix order of operations in CREATE OR REPLACE VIEW. · 58b13626
      Dean Rasheed authored
      When CREATE OR REPLACE VIEW acts on an existing view, don't update the
      view options until after the view query has been updated.
      
      This is necessary in the case where CREATE OR REPLACE VIEW is used on
      an existing view that is not updatable, and the new view is updatable
      and specifies the WITH CHECK OPTION. In this case, attempting to apply
      the new options to the view before updating its query fails, because
      the options are applied using the ALTER TABLE infrastructure which
      checks that WITH CHECK OPTION is only applied to an updatable view.
      
      If new columns are being added to the view, that is also done using
      the ALTER TABLE infrastructure, but it is important that that still be
      done before updating the view query, because the rules system checks
      that the query columns match those on the view relation. Added a
      comment to explain that, in case someone is tempted to move that to
      where the view options are now being set.
      
      Back-patch to 9.4 where WITH CHECK OPTION was added.
      
      Report: https://postgr.es/m/CAEZATCUp%3Dz%3Ds4SzZjr14bfct_bdJNwMPi-gFi3Xc5k1ntbsAgQ%40mail.gmail.com
      58b13626
    • Robert Haas's avatar
      Convert elog() to ereport() and do some wordsmithing. · cd510f04
      Robert Haas authored
      It's not entirely clear that we should log a message here at all, but
      it's certainly wrong to use elog() for a message that should clearly
      be translatable.
      
      Amit Langote
      cd510f04
    • Robert Haas's avatar
      Refactor partition tuple routing code to reduce duplication. · 1fc5c494
      Robert Haas authored
      Amit Langote
      1fc5c494
    • Robert Haas's avatar
      Fix corner-case bug in WaitEventSetWaitBlock on Windows. · 3b790d25
      Robert Haas authored
      If we do not reset the FD_READ event, WaitForMultipleObjects won't
      return it again again unless we've meanwhile read from the socket,
      which is generally true but not guaranteed.  WaitEventSetWaitBlock
      itself may fail to return the event to the caller if the latch is
      also set, and even if we changed that, the caller isn't obliged to
      handle all returned events at once.  On non-Windows systems, the
      socket-read event is purely level-triggered, so this issue does
      not exist.  To fix, make Windows reset the event when needed.
      
      This bug was introduced by 98a64d0b,
      and causes hangs when trying to use the pldebugger extension.
      
      Patch by Amit Kapial.  Reported and tested by Ashutosh Sharma, who
      also provided some analysis.  Further analysis by Michael Paquier.
      3b790d25
    • Robert Haas's avatar
      Refactor merge path generation code. · 59649c3f
      Robert Haas authored
      This shouldn't change the set of paths that get generated in any
      way, but it is preparatory work for further changes to allow a
      partial path to be merge-joined witih a non-partial path to produce
      a partial join path.
      
      Dilip Kumar, with cosmetic adjustments by me.
      59649c3f
    • Peter Eisentraut's avatar
      Reorder pg_sequence columns to avoid alignment issue · f3b421da
      Peter Eisentraut authored
      On AIX, doubles are aligned at 4 bytes, but int64 is aligned at 8 bytes.
      Our code assumes that doubles have alignment that can also be applied to
      int64, but that fails in this case.  One effect is that
      heap_form_tuple() writes tuples in a different layout than
      Form_pg_sequence expects.
      
      Rather than rewrite the whole alignment code, work around the issue by
      reordering the columns in pg_sequence so that the first int64 column
      naturally comes out at an 8-byte boundary.
      f3b421da
    • Fujii Masao's avatar
      Forbid invalid combination of options in pg_basebackup. · ecbdc4c5
      Fujii Masao authored
      Commit 56c7d8d4 allowed pg_basebackup
      to stream WAL in tar mode. But there is the restriction that WAL
      streaming in tar mode works only when the value - (dash) is not
      specified as output directory. This means that the combination of
      three options "-D -", "-F t" and "-X stream" is invalid. However,
      previously, even when those options were specified at the same time,
      pg_basebackup background process unexpectedly started streaming WAL.
      And then it exited with an error.
      
      This commit changes pg_basebackup so that it errors out on such
      invalid combination of options at the beginning.
      
      Reviewed by Magnus Hagander, and patch by me.
      ecbdc4c5
    • Tom Lane's avatar
      Fix minor oversights in nodeAgg.c. · c080b223
      Tom Lane authored
      aggstate->evalproj is always set up by ExecInitAgg, so there's no
      need to test.  Doing so led Coverity to think that we might be
      intending "slot" to be possibly NULL here, and it quite properly
      complained that the rest of combine_aggregates() wasn't prepared
      for that.
      
      Also fix a couple of obvious thinkos in Asserts checking that
      "inputoff" isn't past the end of the slot.
      
      Errors introduced in commit 8ed3f11b, so no need for back-patch.
      c080b223
  4. 20 Dec, 2016 4 commits
  5. 19 Dec, 2016 4 commits
    • Robert Haas's avatar
      Provide a DSA area for all parallel queries. · e13029a5
      Robert Haas authored
      This will allow future parallel query code to dynamically allocate
      storage shared by all participants.
      
      Thomas Munro, with assorted changes by me.
      e13029a5
    • Tom Lane's avatar
      Fix handling of phrase operator removal while removing tsquery stopwords. · 26044384
      Tom Lane authored
      The distance of a removed phrase operator should propagate up to a
      parent phrase operator if there is one, but this only worked correctly
      in left-deep trees.  Throwing in a few parentheses confused it completely,
      as indeed was illustrated by bizarre results in existing regression test
      cases.
      
      To fix, track unaccounted-for distances that should propagate to the left
      and to the right of the current node, rather than trying to make it work
      with only one returned distance.
      
      Also make some adjustments to behave as well as we can for cases of
      intermixed phrase and regular (AND/OR) operators.  I don't think it's
      possible to be 100% correct for that without a rethinking of the tsquery
      representation; for example, maybe we should just not drop stopword nodes
      at all underneath phrase operators.  But this is better than it was,
      and changing tsquery representation wouldn't be safely back-patchable.
      
      While at it, I simplified the API of the clean_fakeval_intree function
      a bit by getting rid of the "char *result" output parameter; that wasn't
      doing anything that wasn't redundant with whether the result node is
      NULL or not, and testing for NULL seems a lot clearer/safer.
      
      This is part of a larger project to fix various infelicities in the
      phrase-search implementation, but this part seems comittable on its own.
      
      Back-patch to 9.6 where phrase operators were introduced.
      
      Discussion: https://postgr.es/m/28215.1481999808@sss.pgh.pa.us
      Discussion: https://postgr.es/m/26706.1482087250@sss.pgh.pa.us
      26044384
    • Robert Haas's avatar
      Fix locking problem in _hash_squeezebucket() / _hash_freeovflpage(). · dd728826
      Robert Haas authored
      A bucket squeeze operation needs to lock each page of the bucket
      before releasing the prior page, but the previous coding fumbled the
      locking when freeing an overflow page during a bucket squeeze
      operation.  Commit 6d46f478
      introduced this bug.
      
      Amit Kapila, with help from Kuntal Ghosh and Dilip Kumar, after
      an initial trouble report by Jeff Janes.  Reviewed by me.  I also
      fixed a problem with a comment.
      dd728826
    • Robert Haas's avatar
      Remove unused file. · 668dbbec
      Robert Haas authored
      This was added in 10540974, but has
      never been used for anything as far as I can tell.  There seems to
      be no reason to keep it.
      668dbbec