1. 27 Mar, 2019 6 commits
    • Tom Lane's avatar
      Suppress uninitialized-variable warning. · a51cc7e9
      Tom Lane authored
      Apparently Andres' compiler is smart enough to see that hpage
      must be initialized before use ... but mine isn't.
      a51cc7e9
    • Michael Paquier's avatar
      Improve error handling of column references in expression transformation · ecfed4a1
      Michael Paquier authored
      Column references are not allowed in default expressions and partition
      bound expressions, and are restricted as such once the transformation of
      their expressions is done.  However, trying to use more complex column
      references can lead to confusing error messages.  For example, trying to
      use a two-field column reference name for default expressions and
      partition bounds leads to "missing FROM-clause entry for table", which
      makes no sense in their respective context.
      
      In order to make the errors generated more useful, this commit adds more
      verbose messages when transforming column references depending on the
      context.  This has a little consequence though: for example an
      expression using an aggregate with a column reference as argument would
      cause an error to be generated for the column reference, while the
      aggregate was the problem reported before this commit because column
      references get transformed first.
      
      The confusion exists for default expressions for a long time, and the
      problem is new as of v12 for partition bounds.  Still per the lack of
      complaints on the matter no backpatch is done.
      
      The patch has been written by Amit Langote and me, and Tom Lane has
      provided the improvement of the documentation for default expressions on
      the CREATE TABLE page.
      
      Author: Amit Langote, Michael Paquier
      Reviewed-by: Tom Lane
      Discussion: https://postgr.es/m/20190326020853.GM2558@paquier.xyz
      ecfed4a1
    • Thomas Munro's avatar
      Fix off-by-one error in txid_status(). · d2fd7f74
      Thomas Munro authored
      The transaction ID returned by GetNextXidAndEpoch() is in the future,
      so we can't attempt to access its status or we might try to read a
      CLOG page that doesn't exist.  The > vs >= confusion probably stemmed
      from the choice of a variable name containing the word "last" instead
      of "next", so fix that too.
      
      Back-patch to 10 where the function arrived.
      
      Author: Thomas Munro
      Discussion: https://postgr.es/m/CA%2BhUKG%2Buua_BV5cyfsioKVN2d61Lukg28ECsWTXKvh%3DBtN2DPA%40mail.gmail.com
      d2fd7f74
    • Michael Paquier's avatar
      Switch some palloc/memset calls to palloc0 · 1983af8e
      Michael Paquier authored
      Some code paths have been doing some allocations followed by an
      immediate memset() to initialize the allocated area with zeros, this is
      a bit overkill as there are already interfaces to do both things in one
      call.
      
      Author: Daniel Gustafsson
      Reviewed-by: Michael Paquier
      Discussion: https://postgr.es/m/vN0OodBPkKs7g2Z1uyk3CUEmhdtspHgYCImhlmSxv1Xn6nY1ZnaaGHL8EWUIQ-NEv36tyc4G5-uA3UXUF2l4sFXtK_EQgLN1hcgunlFVKhA=@yesql.se
      1983af8e
    • Michael Paquier's avatar
      Switch function current_schema[s]() to be parallel-unsafe · 5bde1651
      Michael Paquier authored
      When invoked for the first time in a session, current_schema() and
      current_schemas() can finish by creating a temporary schema.  Currently
      those functions are parallel-safe, however if for a reason or another
      they get launched across multiple parallel workers, they would fail when
      attempting to create a temporary schema as temporary contexts are not
      supported in this case.
      
      The original issue has been spotted by buildfarm members crake and
      lapwing, after commit c5660e0a has introduced the first regression tests
      based on current_schema() in the tree.  After that, 396676b0 has
      introduced a workaround to avoid parallel plans but that was not
      completely right either.
      
      Catversion is bumped.
      
      Author: Michael Paquier
      Reviewed-by: Daniel Gustafsson
      Discussion: https://postgr.es/m/20190118024618.GF1883@paquier.xyz
      5bde1651
    • Tomas Vondra's avatar
      Track unowned relations in doubly-linked list · 6ca015f9
      Tomas Vondra authored
      Relations dropped in a single transaction are tracked in a list of
      unowned relations.  With large number of dropped relations this resulted
      in poor performance at the end of a transaction, when the relations are
      removed from the singly linked list one by one.
      
      Commit b4166911 attempted to address this issue (particularly when it
      happens during recovery) by removing the relations in a reverse order,
      resulting in O(1) lookups in the list of unowned relations.  This did
      not work reliably, though, and it was possible to trigger the O(N^2)
      behavior in various ways.
      
      Instead of trying to remove the relations in a specific order with
      respect to the linked list, which seems rather fragile, switch to a
      regular doubly linked.  That allows us to remove relations cheaply no
      matter where in the list they are.
      
      As b4166911 was a bugfix, backpatched to all supported versions, do the
      same thing here.
      
      Reviewed-by: Alvaro Herrera
      Discussion: https://www.postgresql.org/message-id/flat/80c27103-99e4-1d0c-642c-d9f3b94aaa0a%402ndquadrant.com
      Backpatch-through: 9.4
      6ca015f9
  2. 26 Mar, 2019 13 commits
    • Andres Freund's avatar
      Compute XID horizon for page level index vacuum on primary. · 558a9165
      Andres Freund authored
      Previously the xid horizon was only computed during WAL replay. That
      had two major problems:
      1) It relied on knowing what the table pointed to looks like. That was
         easy enough before the introducing of tableam (we knew it had to be
         heap, although some trickery around logging the heap relfilenodes
         was required). But to properly handle table AMs we need
         per-database catalog access to look up the AM handler, which
         recovery doesn't allow.
      2) Not knowing the xid horizon also makes it hard to support logical
         decoding on standbys. When on a catalog table, we need to be able
         to conflict with slots that have an xid horizon that's too old. But
         computing the horizon by visiting the heap only works once
         consistency is reached, but we always need to be able to detect
         conflicts.
      
      There's also a secondary problem, in that the current method performs
      redundant work on every standby. But that's counterbalanced by
      potentially computing the value when not necessary (either because
      there's no standby, or because there's no connected backends).
      
      Solve 1) and 2) by moving computation of the xid horizon to the
      primary and by involving tableam in the computation of the horizon.
      
      To address the potentially increased overhead, increase the efficiency
      of the xid horizon computation for heap by sorting the tids, and
      eliminating redundant buffer accesses. When prefetching is available,
      additionally perform prefetching of buffers.  As this is more of a
      maintenance task, rather than something routinely done in every read
      only query, we add an arbitrary 10 to the effective concurrency -
      thereby using IO concurrency, when not globally enabled.  That's
      possibly not the perfect formula, but seems good enough for now.
      
      Bumps WAL format, as latestRemovedXid is now part of the records, and
      the heap's relfilenode isn't anymore.
      
      Author: Andres Freund, Amit Khandekar, Robert Haas
      Reviewed-By: Robert Haas
      Discussion:
          https://postgr.es/m/20181212204154.nsxf3gzqv3gesl32@alap3.anarazel.de
          https://postgr.es/m/20181214014235.dal5ogljs3bmlq44@alap3.anarazel.de
          https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
      558a9165
    • Alvaro Herrera's avatar
      Fix partitioned index creation bug with dropped columns · 126d6312
      Alvaro Herrera authored
      ALTER INDEX .. ATTACH PARTITION fails if the partitioned table where the
      index is defined contains more dropped columns than its partition, with
      this message:
        ERROR:  incorrect attribute map
      The cause was that one caller of CompareIndexInfo was passing the number
      of attributes of the partition rather than the parent, which confused
      the length check.  Repair.
      
      This can cause pg_upgrade to fail when used on such a database.  Leave
      some more objects around after regression tests, so that the case is
      detected by pg_upgrade test suite.
      
      Remove some spurious empty lines noticed while looking for other cases
      of the same problem.
      
      Discussion: https://postgr.es/m/20190326213924.GA2322@alvherre.pgsql
      126d6312
    • Tom Lane's avatar
      Build "other rels" of appendrel baserels in a separate step. · 53bcf5e3
      Tom Lane authored
      Up to now, otherrel RelOptInfos were built at the same time as baserel
      RelOptInfos, thanks to recursion in build_simple_rel().  However,
      nothing in query_planner's preprocessing cares at all about otherrels,
      only baserels, so we don't really need to build them until just before
      we enter make_one_rel.  This has two benefits:
      
      * create_lateral_join_info did a lot of extra work to propagate
      lateral-reference information from parents to the correct children.
      But if we delay creation of the children till after that, it's
      trivial (and much harder to break, too).
      
      * Since we have all the restriction quals correctly assigned to
      parent appendrels by this point, it'll be possible to do plan-time
      pruning and never make child RelOptInfos at all for partitions that
      can be pruned away.  That's not done here, but will be later on.
      
      Amit Langote, reviewed at various times by Dilip Kumar, Jesper Pedersen,
      Yoshikazu Imai, and David Rowley
      
      Discussion: https://postgr.es/m/9d7c5112-cb99-6a47-d3be-cf1ee6862a1d@lab.ntt.co.jp
      53bcf5e3
    • Tom Lane's avatar
      Add ORDER BY to more ICU regression test cases. · 8994cc6f
      Tom Lane authored
      Commit c77e1220 didn't fully fix the problem.  Per buildfarm
      and local testing.
      8994cc6f
    • Tom Lane's avatar
      Fix oversight in data-type change for autovacuum_vacuum_cost_delay. · 7c366ac9
      Tom Lane authored
      Commit caf626b2 missed that the relevant reloptions entry needs
      to be moved from the intRelOpts[] array to realRelOpts[].
      Somewhat surprisingly, it seems to work anyway, perhaps because
      the desired default and limit values are all integers.  We ought
      to have either a simpler data structure or better cross-checking
      here, but that's for another patch.
      
      Nikolay Shaplov
      
      Discussion: https://postgr.es/m/4861742.12LTaSB3sv@x200m
      7c366ac9
    • Alvaro Herrera's avatar
      psql: Schema-qualify typecast in one \d query · 1d21ba8a
      Alvaro Herrera authored
      Bug introduced in my commit bc87f22e
      1d21ba8a
    • Tom Lane's avatar
      Get rid of duplicate child RTE for a partitioned table. · e8d5dd6b
      Tom Lane authored
      We've been creating duplicate RTEs for partitioned tables just
      because we do so for regular inheritance parent tables.  But unlike
      regular-inheritance parents which are themselves regular tables
      and thus need to be scanned, partitioned tables don't need the
      extra RTE.
      
      This makes the conditions for building a child RTE the same as those
      for building an AppendRelInfo, allowing minor simplification in
      expand_single_inheritance_child.  Since the planner's actual processing
      is driven off the AppendRelInfo list, nothing much changes beyond that,
      we just have one fewer useless RTE entry.
      
      Amit Langote, reviewed and hacked a bit by me
      
      Discussion: https://postgr.es/m/9d7c5112-cb99-6a47-d3be-cf1ee6862a1d@lab.ntt.co.jp
      e8d5dd6b
    • Alvaro Herrera's avatar
      Improve psql's \d display of foreign key constraints · 1af25ca0
      Alvaro Herrera authored
      When used on a partition containing foreign keys coming from one of its
      ancestors, \d would (rather unhelpfully) print the details about the
      pg_constraint row in the partition.  This becomes a bit frustrating when
      the user tries things like dropping the FK in the partition; instead,
      show the details for the foreign key on the table where it is defined.
      
      Also, when a table is referenced by a foreign key on a partitioned
      table, we would show multiple "Referenced by" lines, one for each
      partition, which gets unwieldy pretty fast.  Modify that so that it
      shows only one line for the ancestor partitioned table where the FK is
      defined.
      
      Discussion: https://postgr.es/m/20181204143834.ym6euxxxi5aeqdpn@alvherre.pgsql
      Reviewed-by: Tom Lane, Amit Langote, Peter Eisentraut
      1af25ca0
    • Magnus Hagander's avatar
      Fix typo · 05295e36
      Magnus Hagander authored
      Author: Daniel Gustafsson <daniel@yesql.se>
      05295e36
    • Peter Eisentraut's avatar
      Fix misplaced const · c8c885b7
      Peter Eisentraut authored
      These instances were apparently trying to carry the const qualifier
      from the arguments through the complex casts, but for that the const
      qualifier was misplaced.
      c8c885b7
    • Andres Freund's avatar
      Remove heap_hot_search(). · 2ac1b2b1
      Andres Freund authored
      After 71bdc99d, "tableam: Add helper for indexes to check if a
      corresponding table tuples exist." there's no in-core user left. As
      there's unlikely to be an external user, and such an external user
      could easily be adjusted to use table_index_fetch_tuple_check(),
      remove heap_hot_search().
      
      Per complaint from Peter Geoghegan
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/CAH2-Wzn0Oq4ftJrTqRAsWy2WGjv0QrJcwoZ+yqWsF_Z5vjUBFw@mail.gmail.com
      2ac1b2b1
    • Michael Paquier's avatar
      Fix crash when using partition bound expressions · cdde886d
      Michael Paquier authored
      Since 7c079d74, partition bounds are able to use generalized expression
      syntax when processed, treating "minvalue" and "maxvalue" as specific
      cases as they get passed down for transformation as a column references.
      
      The checks for infinite bounds in range expressions have been lax
      though, causing crashes when trying to use column reference names with
      more than one field.  Here is an example causing a crash:
      CREATE TABLE list_parted (a int) PARTITION BY LIST (a);
      CREATE TABLE part_list_crash PARTITION OF list_parted
        FOR VALUES IN (somename.somename);
      
      Note that the creation of the second relation should fail as partition
      bounds cannot have column references in their expressions, so when
      finding an expression which does not match the expected infinite bounds,
      then this commit lets the generic transformation machinery check after
      it.  The error message generated in this case references as well a
      missing RTE, which is confusing.  This problem will be treated
      separately as it impacts as well default expressions for some time, and
      for now only the cases where a crash can happen are fixed.
      
      While on it, extend the set of regression tests in place for list
      partition bounds and add an extra set for range partition bounds.
      
      Reported-by: Alexander Lakhin
      Author: Michael Paquier
      Reviewed-by: Amit Langote
      Discussion: https://postgr.es/m/15668-0377b1981aa1a393@postgresql.org
      cdde886d
    • Andres Freund's avatar
      tableam: Add table_get_latest_tid, to wrap heap_get_latest_tid. · 2e3da03e
      Andres Freund authored
      This primarily is to allow WHERE CURRENT OF to continue to work as it
      currently does. It's not clear to me that these semantics make sense
      for every AM, but it works for the in-core heap, and the out of core
      zheap. We can refine it further at a later point if necessary.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
      2e3da03e
  3. 25 Mar, 2019 20 commits
  4. 24 Mar, 2019 1 commit
    • Tom Lane's avatar
      Un-hide most cascaded-drop details in regression test results. · 940311e4
      Tom Lane authored
      Now that the ordering of DROP messages ought to be stable everywhere,
      we should not need these kluges of hiding DETAIL output just to avoid
      unstable ordering.  Hiding it's not great for test coverage, so
      let's undo that where possible.
      
      In a small number of places, it's necessary to leave it in, for
      example because the output might include a variable pg_temp_nnn
      schema name.  I also left things alone in places where the details
      would depend on other regression test scripts, e.g. plpython_drop.sql.
      
      Perhaps buildfarm experience will show this to be a bad idea,
      but if so I'd like to know why.
      
      Discussion: https://postgr.es/m/E1h6eep-0001Mw-Vd@gemulon.postgresql.org
      940311e4