1. 25 Jan, 2019 6 commits
    • Tom Lane's avatar
      Split QTW_EXAMINE_RTES flag into QTW_EXAMINE_RTES_BEFORE/_AFTER. · 18c0da88
      Tom Lane authored
      This change allows callers of query_tree_walker() to choose whether
      to visit an RTE before or after visiting the contents of the RTE
      (i.e., prefix or postfix tree order).  All existing users of
      QTW_EXAMINE_RTES want the QTW_EXAMINE_RTES_BEFORE behavior, but
      an upcoming patch will want QTW_EXAMINE_RTES_AFTER, and it seems
      like a potentially useful change on its own.
      
      Andreas Karlsson (extracted from CTE inlining patch)
      
      Discussion: https://postgr.es/m/8810.1542402910@sss.pgh.pa.us
      18c0da88
    • Tom Lane's avatar
      Teach nulltestsel() that system columns are never NULL. · ff750ce2
      Tom Lane authored
      While it's perhaps unlikely that users would write an explicit test
      like "ctid IS NULL", this function is also used in range estimation,
      and an incorrect answer can throw off the results for tight ranges.
      Anyway it's not much code so we might as well do it.
      
      Edmund Horner
      
      Discussion: https://postgr.es/m/CAMyN-kCa3BFUFrCTtQeprxTU1anCd3Pua7zXstGCKq4pXgjukw@mail.gmail.com
      ff750ce2
    • Tom Lane's avatar
      Fix possibly-uninitialized-variable warning from commit 9556aa01. · 6119060d
      Tom Lane authored
      Heikki's compiler doesn't complain about end_ptr, apparently,
      but mine does.
      
      In passing, I failed to resist the temptation to remove the
      no-longer-used fldnum variable, and relocate chunk_len's
      declaration to a narrower scope.
      6119060d
    • Heikki Linnakangas's avatar
      Use single-byte Boyer-Moore-Horspool search even with multibyte encodings. · 9556aa01
      Heikki Linnakangas authored
      The old implementation first converted the input strings to arrays of
      wchars, and performed the conversion on those. However, the conversion is
      expensive, and for a large input string, consumes a lot of memory.
      Allocating the large arrays also meant that these functions could not be
      used on strings larger 1 GB / pg_encoding_max_length() (256 MB for UTF-8).
      
      Avoid the conversion, and instead use the single-byte algorithm even with
      multibyte encodings. That can get fooled, if there is a matching byte
      sequence in the middle of a multi-byte character, so to eliminate false
      positives like that, we verify any matches by walking the string character
      by character with pg_mblen(). Also, if the caller needs the position of
      the match, as a character-offset, we also need to walk the string to count
      the characters.
      
      Performance testing shows that walking the whole string with pg_mblen() is
      somewhat slower than converting the whole string to wchars. It's still
      often a win, though, because we don't need to do it if there is no match,
      and even when there is, we only need to walk up to the point where the
      match is, not the whole string. Even in the worst case, there would be
      room for optimization: Much of the CPU time in the current loop with
      pg_mblen() is function call overhead, and could be improved by inlining
      pg_mblen() and/or the encoding-specific mblen() functions. But I didn't
      attempt to do that as part of this patch.
      
      Most of the callers of text_position_setup/next functions were actually
      not interested in the position of the match, counted in characters. To
      cater for them, refactor the text_position_next() interface into two
      parts: searching for the next match (text_position_next()), and returning
      the current match's position as a pointer (text_position_get_match_ptr())
      or as a character offset (text_position_get_match_pos()). Getting the
      pointer to the match is a more convenient API for many callers, and with
      UTF-8, it allows skipping the character-walking step altogether, because
      UTF-8 can't have false matches even when treated like raw byte strings.
      
      Reviewed-by: John Naylor
      Discussion: https://www.postgresql.org/message-id/3173d989-bc1c-fc8a-3b69-f24246f73876%40iki.fi
      9556aa01
    • Heikki Linnakangas's avatar
      Fix comments that claimed that mblen() only looks at first byte. · a5be6e9a
      Heikki Linnakangas authored
      GB18030's mblen() function looks at the first and the second byte of the
      multibyte character, to determine its length. copy.c had made the
      assumption that mblen() only looks at the first byte, but it turns out to
      work out fine, because of the way the GB18030 encoding works. COPY will
      see a 4-byte encoded character as two 2-byte encoded characters, which is
      enough for COPY's purposes. It cannot mix those up with delimiter or
      escaping characters, because only single-byte ASCII characters are
      supported as delimiters or escape characters.
      
      Discussion: https://www.postgresql.org/message-id/7704d099-9643-2a55-fb0e-becd64400dcb%40iki.fi
      a5be6e9a
    • Peter Eisentraut's avatar
      Allow generalized expression syntax for partition bounds · 7c079d74
      Peter Eisentraut authored
      Previously, only literals were allowed.  This change allows general
      expressions, including functions calls, which are evaluated at the
      time the DDL command is executed.
      
      Besides offering some more functionality, it simplifies the parser
      structures and removes some inconsistencies in how the literals were
      handled.
      
      Author: Kyotaro Horiguchi, Tom Lane, Amit Langote
      Reviewed-by: default avatarPeter Eisentraut <peter.eisentraut@2ndquadrant.com>
      Discussion: https://www.postgresql.org/message-id/flat/9f88b5e0-6da2-5227-20d0-0d7012beaa1c@lab.ntt.co.jp/
      7c079d74
  2. 24 Jan, 2019 10 commits
    • Tom Lane's avatar
      Remove _configthreadlocale() calls in ecpg test suite. · e3565fd6
      Tom Lane authored
      This essentially reverts commits a772624b and 04fbe0e4, which
      added "_configthreadlocale(_ENABLE_PER_THREAD_LOCALE)" calls to the
      thread-related ecpg test programs.  That was nothing but a hack,
      because we shouldn't expect that ecpg-using applications have
      done that for us; and now that we've inserted such calls into
      ecpglib, the tests should still pass without it.
      
      (If they don't, it would be good to know that.)
      
      HEAD only; there seems no big need to change this in the
      back branches.
      
      Discussion: https://postgr.es/m/22937.1548307384@sss.pgh.pa.us
      e3565fd6
    • Tom Lane's avatar
      Remove infinite-loop hazards in ecpg test suite. · d5a1fde3
      Tom Lane authored
      A report from Andrew Dunstan showed that an ecpglib breakage that
      causes repeated query failures could lead to infinite loops in some
      ecpg test scripts, because they contain "while(1)" loops with no
      exit condition other than successful test completion.  That might
      be all right for manual testing, but it seems entirely unacceptable
      for automated test environments such as our buildfarm.  We don't
      want buildfarm owners to have to intervene manually when a test
      goes wrong.
      
      To fix, just change all those while(1) loops to exit after at most
      100 iterations (which is more than any of them expect to iterate).
      This seems sufficient since we'd see discrepancies in the test output
      if any loop executed the wrong number of times.
      
      I tested this by dint of intentionally breaking ecpg_do_prologue
      to always fail, and verifying that the tests still got to completion.
      
      Back-patch to all supported branches, since the whole point of this
      exercise is to protect the buildfarm against future mistakes.
      
      Discussion: https://postgr.es/m/18693.1548302004@sss.pgh.pa.us
      d5a1fde3
    • Peter Eisentraut's avatar
      PL/pgSQL: Add statement ID to statement structures · bbd5c207
      Peter Eisentraut authored
      This can be used by a profiler as the index for an array of
      per-statement metrics.
      
      Author: Pavel Stehule <pavel.stehule@gmail.com>
      Reviewed-by: default avatarPeter Eisentraut <peter.eisentraut@2ndquadrant.com>
      Discussion: https://www.postgresql.org/message-id/flat/CAFj8pRDRCjN6rpM9ZccU7Ta_afsNX7mg9=n34F+r445Nt9v2tA@mail.gmail.com/
      bbd5c207
    • Peter Eisentraut's avatar
      Fix whitespace · bf2fb2e0
      Peter Eisentraut authored
      bf2fb2e0
    • Alvaro Herrera's avatar
      Fix droppability of constraints upon partition detach · efd9366d
      Alvaro Herrera authored
      We were failing to set conislocal correctly for constraints in
      partitions after partition detach, leading to those constraints becoming
      undroppable.  Fix by setting the flag correctly.  Existing databases
      might contain constraints with the conislocal wrongly set to false, for
      partitions that were detached; this situation should be fixable by
      applying an UPDATE on pg_constraint to set conislocal true.  This
      problem should otherwise be innocuous and should disappear across a
      dump/restore or pg_upgrade.
      
      Secondarily, when constraint drop was attempted in a partitioned table,
      ATExecDropConstraint would try to recurse to partitions after doing
      performDeletion() of the constraint in the partitioned table itself; but
      since the constraint in the partitions are dropped by the initial call
      of performDeletion() (because of following dependencies), the recursion
      step would fail since it would not find the constraint, causing the
      whole operation to fail.  Fix by preventing recursion.
      
      Reported-by: Amit Langote
      Diagnosed-by: Amit Langote
      Author: Amit Langote, Álvaro Herrera
      Discussion: https://postgr.es/m/f2b8ead5-4131-d5a8-8016-2ea0a31250af@lab.ntt.co.jp
      efd9366d
    • Tom Lane's avatar
      Fix portability problem in pgbench. · e6c3ba7f
      Tom Lane authored
      The pgbench regression test supposed that srandom() with a specific value
      would result in deterministic output from random(), as required by POSIX.
      It emerges however that OpenBSD is too smart to be constrained by mere
      standards, so their random() emits nondeterministic output anyway.
      While a workaround does exist, what seems like a better fix is to stop
      relying on the platform's srandom()/random() altogether, so that what
      you get from --random-seed=N is not merely deterministic but platform
      independent.  Hence, use a separate pg_jrand48() random sequence in
      place of random().
      
      Also adjust the regression test case that's supposed to detect
      nondeterminism so that it's more likely to detect it; the original
      choice of random_zipfian parameter tended to produce the same output
      all the time even if the underlying behavior wasn't deterministic.
      
      In passing, improve pgbench's docs about random_zipfian().
      
      Back-patch to v11 where this code was introduced.
      
      Fabien Coelho and Tom Lane
      
      Discussion: https://postgr.es/m/4615.1547792324@sss.pgh.pa.us
      e6c3ba7f
    • Alvaro Herrera's avatar
      Simplify coding to detach constraints when detaching partition · 19184fcc
      Alvaro Herrera authored
      The original coding was too baroque and led to an use-after-release
      mistake, noticed by buildfarm member prion.
      
      Discussion: https://postgr.es/m/21693.1548305934@sss.pgh.pa.us
      19184fcc
    • Etsuro Fujita's avatar
      postgres_fdw: Account for tlist eval costs in estimate_path_cost_size(). · fd1afdba
      Etsuro Fujita authored
      Previously, estimate_path_cost_size() didn't account for tlist eval
      costs, except when costing a foreign-grouping path using local
      statistics, but such costs should be accounted for when costing that path
      using remote estimates, because some of the tlist expressions might be
      evaluated locally.  Also, such costs should be accounted for in the case
      of a foreign-scan or foreign-join path, because the tlist might contain
      PlaceHolderVars, which postgres_fdw currently evaluates locally.
      
      This also fixes an oversight in my commit f8f6e446.
      
      Like that commit, apply this to HEAD only to avoid destabilizing existing
      plan choices.
      
      Author: Etsuro Fujita
      Discussion: https://postgr.es/m/5BFD3EAD.2060301%40lab.ntt.co.jp
      fd1afdba
    • Tom Lane's avatar
      Blind attempt to fix _configthreadlocale() failures on MinGW. · 2cf91ccb
      Tom Lane authored
      Apparently, some builds of MinGW contain a version of
      _configthreadlocale() that always returns -1, indicating failure.
      Rather than treating that as a curl-up-and-die condition, soldier on
      as though the function didn't exist.  This leaves us without thread
      safety on such MinGW versions, but we didn't have it anyway.
      
      Discussion: https://postgr.es/m/d06a16bc-52d6-9f0d-2379-21242d7dbe81@2ndQuadrant.com
      2cf91ccb
    • Alvaro Herrera's avatar
      Detach constraints when partitions are detached · ae366aa5
      Alvaro Herrera authored
      I (Álvaro) forgot to do this in eb7ed3f3, leading to undroppable
      constraints after partitions are detached.  Repair.
      
      Reported-by: Amit Langote
      Author: Amit Langote
      Discussion: https://postgr.es/m/c1c9b688-b886-84f7-4048-1e4ebe9b1d06@lab.ntt.co.jp
      ae366aa5
  3. 23 Jan, 2019 5 commits
  4. 22 Jan, 2019 8 commits
  5. 21 Jan, 2019 11 commits
    • Tom Lane's avatar
      Remove useless bms_copy step in RelationGetIndexAttrBitmap. · 8f9e934a
      Tom Lane authored
      Seems to be from a bad case of copy-and-paste-itis in commit 665d1fad.
      It wouldn't be quite so annoying if it didn't contradict the comment
      half a dozen lines above.
      
      David Rowley
      
      Discussion: https://postgr.es/m/CAKJS1f95Dyf8Qkdz4W+PbCmT-HTb54tkqUCC8isa2RVgSJ_pXQ@mail.gmail.com
      8f9e934a
    • Alvaro Herrera's avatar
      Create action triggers when partitions are detached · 0464fdf0
      Alvaro Herrera authored
      Detaching a partition from a partitioned table that's constrained by
      foreign keys requires additional action triggers on the referenced side;
      otherwise, DELETE/UPDATE actions there fail to notice rows in the table
      that was partition, and so are incorrectly allowed through.  With this
      commit, those triggers are now created.  Conversely, when a table that
      has a foreign key is attached as a partition to a table that also has
      the same foreign key, those action triggers are no longer needed, so we
      remove them.
      
      Add a minimal test case verifying (part of) this.
      
      Authors: Amit Langote, Álvaro Herrera
      Discussion: https://postgr.es/m/f2b8ead5-4131-d5a8-8016-2ea0a31250af@lab.ntt.co.jp
      0464fdf0
    • Alvaro Herrera's avatar
      Flush relcache entries when their FKs are meddled with · 17554409
      Alvaro Herrera authored
      Back in commit 100340e2, we made relcache entries keep lists of the
      foreign keys applying to the relation -- but we forgot to update
      CacheInvalidateHeapTuple to flush those entries when new FKs got created
      or existing ones updated/deleted.  No bugs appear to have been reported
      that would be explained by this ommission, but I noticed the problem
      while working on an unrelated bugfix which clearly showed it.  Fix by
      adding relcache flush on relevant foreign key changes.
      
      Backpatch to 9.6, like the aforementioned commit.
      
      Discussion: https://postgr.es/m/201901211927.7mmhschxlejh@alvherre.pgsql
      Reviewed-by: Tom Lane
      17554409
    • Tom Lane's avatar
      Second try at fixing ecpglib thread-safety problem. · ee27584c
      Tom Lane authored
      While Windows (allegedly) has _configthreadlocale() pretty far back,
      it seems MinGW didn't acquire support for that till more recently.
      Fortunately, we can use an autoconf probe on that toolchain,
      instead of guessing whether it's there.  (Hm, I wonder whether Cygwin
      will need this also.)
      
      Per buildfarm.
      
      Discussion: https://postgr.es/m/20190121193512.tdmcnic2yjxlufaw@alap3.anarazel.de
      ee27584c
    • Andres Freund's avatar
      Fix "Remove superfluous tqual.h includes" by adding back one include. · 527114e5
      Andres Freund authored
      I removed one include too many in e7cc78ad, not sure why that
      escaped my test script.
      
      Author: Andres Freund
      527114e5
    • Tom Lane's avatar
      Fix sepgsql regression test. · 071e1189
      Tom Lane authored
      Message order in the expected output changes due to commit f1ad067f.
      Per buildfarm.
      
      Discussion: https://postgr.es/m/20190121201134.dyx6anto6akflh5d@alap3.anarazel.de
      071e1189
    • Andres Freund's avatar
      Remove superfluous tqual.h includes. · e7cc78ad
      Andres Freund authored
      Most of these had been obsoleted by 568d4138 / the SnapshotNow
      removal.
      
      This is is preparation for moving most of tqual.[ch] into either
      snapmgr.h or heapam.h, which in turn is in preparation for pluggable
      table AMs.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
      e7cc78ad
    • Andres Freund's avatar
    • Andres Freund's avatar
      Replace heapam.h includes with {table, relation}.h where applicable. · 111944c5
      Andres Freund authored
      A lot of files only included heapam.h for relation_open, heap_open etc
      - replace the heapam.h include in those files with the narrower
      header.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20190111000539.xbv7s6w7ilcvm7dp@alap3.anarazel.de
      111944c5
    • Andres Freund's avatar
      Introduce access/{table.h, relation.h}, for generic functions from heapam.h. · 4b21acf5
      Andres Freund authored
      access/heapam contains functions that are very storage specific (say
      heap_insert() and a lot of lower level functions), and fairly generic
      infrastructure like relation_open(), heap_open() etc.  In the upcoming
      pluggable storage work we're introducing a layer between table
      accesses in general and heapam, to allow for different storage
      methods. For a bit cleaner separation it thus seems advantageous to
      move generic functions like the aforementioned to their own headers.
      
      access/relation.h will contain relation_open() etc, and access/table.h
      will contain table_open() (formerly known as heap_open()). I've decided
      for table.h not to include relation.h, but we might change that at a
      later stage.
      
      relation.h already exists in another directory, but the other
      plausible name (rel.h) also conflicts. It'd be nice if there were a
      non-conflicting name, but nobody came up with a suggestion. It's
      possible that the appropriate way to address the naming conflict would
      be to rename nodes/relation.h, which isn't particularly well named.
      
      To avoid breaking a lot of extensions that just use heap_open() etc,
      table.h has macros mapping the old names to the new ones, and heapam.h
      includes relation, table.h.  That also allows to keep the
      bulk renaming of existing callers in a separate commit.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20190111000539.xbv7s6w7ilcvm7dp@alap3.anarazel.de
      4b21acf5
    • Tom Lane's avatar
      Sort the dependent objects before recursing in findDependentObjects(). · f1ad067f
      Tom Lane authored
      Historically, the notices output by DROP CASCADE tended to come out
      in uncertain order, and in some cases you might get different claims
      about which object depends on which other one.  This is because we
      just traversed the dependency tree in the order in which pg_depend
      entries are seen, and nbtree has never promised anything about the
      order of equal-keyed index entries.  We've put up with that for years,
      hacking regression tests when necessary to prevent them from emitting
      unstable output.  However, it's a problem for pending work that will
      change nbtree's behavior for equal keys, as that causes unexpected
      changes in the regression test results.
      
      Hence, adjust findDependentObjects to sort the results of each
      indexscan before processing them.  The sort is on descending OID of
      the dependent objects, hence more or less reverse creation order.
      While this rule could still result in bogus regression test failures
      if an OID wraparound occurred mid-test, that seems unlikely to happen
      in any plausible development or packaging-test scenario.
      
      This is enough to ensure output stability for ordinary DROP CASCADE
      commands, but not for DROP OWNED BY, because that has a different
      code path with the same problem.  We might later choose to sort in
      the DROP OWNED BY code as well, but this patch doesn't do so.
      
      I've also not done anything about reverting the existing hacks to
      suppress unstable DROP CASCADE output in specific regression tests.
      It might be worth undoing those, but it seems like a distinct question.
      
      The first indexscan loop in findDependentObjects is not touched,
      meaning there is a hazard of unstable error reports from that too.
      However, said hazard is not the fault of that code: it was designed
      on the assumption that there could be at most one "owning" object
      to complain about, and that assumption does not seem unreasonable.
      The recent patch that added the possibility of multiple
      DEPENDENCY_INTERNAL_AUTO links broke that assumption, but we should
      fix that situation not band-aid around it.  That's a matter for
      another patch, though.
      
      Discussion: https://postgr.es/m/12244.1547854440@sss.pgh.pa.us
      f1ad067f