1. 26 Mar, 2019 12 commits
    • 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
  2. 25 Mar, 2019 20 commits
  3. 24 Mar, 2019 8 commits
    • 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
    • Tom Lane's avatar
      Sort dependent objects before reporting them in DROP ROLE. · af6550d3
      Tom Lane authored
      Commit 8aa9dd74 didn't quite finish the job in this area after all,
      because DROP ROLE has a code path distinct from DROP OWNED BY, and
      it was still reporting dependent objects in whatever order the index
      scan returned them in.
      
      Buildfarm experience shows that index ordering of equal-keyed objects is
      significantly less stable than before in the wake of using heap TIDs as
      tie-breakers.  So if we try to hide the unstable ordering by suppressing
      DETAIL reports, we're just going to end up having to do that for every
      DROP that reports multiple objects.  That's not great from a coverage
      or problem-detection standpoint, and it's something we'll inevitably
      forget in future patches, leading to more iterations of fixing-an-
      unstable-result.  So let's just bite the bullet and sort here too.
      
      Discussion: https://postgr.es/m/E1h6eep-0001Mw-Vd@gemulon.postgresql.org
      af6550d3
    • Peter Geoghegan's avatar
      Remove dead code from nbtsplitloc.c. · 59ab3be9
      Peter Geoghegan authored
      It doesn't make sense to consider the possibility that there will only
      be one candidate split point when choosing among split points to find
      the split with the lowest penalty.  This is a vestige of an earlier
      version of the patch that became commit fab25024.
      
      Issue spotted while rereviewing coverage of the nbtree patch series
      using gcov.
      59ab3be9
    • Tom Lane's avatar
      Avoid double-free in vacuumlo error path. · bd9396a0
      Tom Lane authored
      The code would do "PQclear(res)" twice if lo_unlink failed, evidently
      due to careless thinking about how far out a "break" would break.
      Remove the extra PQclear and adjust the loop logic so that we'll fall
      out of both levels of loop after an error, as was clearly the intent.
      
      Spotted by Coverity.  I have no idea why it took this long to notice,
      since the bug has been there since commit 67ccbb08.  Accordingly,
      back-patch to all supported branches.
      bd9396a0
    • Michael Paquier's avatar
      Make current_logfiles use permissions assigned to files in data directory · 276d2e6c
      Michael Paquier authored
      Since its introduction in 19dc233c, current_logfiles has been assigned
      the same permissions as a log file, which can be enforced with
      log_file_mode.  This setup can lead to incompatibility problems with
      group access permissions as current_logfiles is not located in the log
      directory, but at the root of the data folder.  Hence, if group
      permissions are used but log_file_mode is more restrictive, a backup
      with a user in the group having read access could fail even if the log
      directory is located outside of the data folder.
      
      Per discussion with the folks mentioned below, we have concluded that
      current_logfiles should not be treated as a log file as it only stores
      metadata related to log files, and that it should use the same
      permissions as all other files in the data directory.  This solution has
      the merit to be simple and fixes all the interaction problems between
      group access and log_file_mode.
      
      Author: Haribabu Kommi
      Reviewed-by: Stephen Frost, Robert Haas, Tom Lane, Michael Paquier
      Discussion: https://postgr.es/m/CAJrrPGcEotF1P7AWoeQyD3Pqr-0xkQg_Herv98DjbaMj+naozw@mail.gmail.com
      Backpatch-through: 11, where group access has been added.
      276d2e6c
    • Peter Eisentraut's avatar
      Transaction chaining · 280a408b
      Peter Eisentraut authored
      Add command variants COMMIT AND CHAIN and ROLLBACK AND CHAIN, which
      start new transactions with the same transaction characteristics as the
      just finished one, per SQL standard.
      
      Support for transaction chaining in PL/pgSQL is also added.  This
      functionality is especially useful when running COMMIT in a loop in
      PL/pgSQL.
      Reviewed-by: default avatarFabien COELHO <coelho@cri.ensmp.fr>
      Discussion: https://www.postgresql.org/message-id/flat/28536681-324b-10dc-ade8-ab46f7645a5a@2ndquadrant.com
      280a408b
    • Andres Freund's avatar
      Remove spurious return. · b2db2770
      Andres Freund authored
      Per buildfarm member anole.
      
      Author: Andres Freund
      b2db2770
    • Andres Freund's avatar
      tableam: Add tuple_{insert, delete, update, lock} and use. · 5db6df0c
      Andres Freund authored
      This adds new, required, table AM callbacks for insert/delete/update
      and lock_tuple. To be able to reasonably use those, the EvalPlanQual
      mechanism had to be adapted, moving more logic into the AM.
      
      Previously both delete/update/lock call-sites and the EPQ mechanism had
      to have awareness of the specific tuple format to be able to fetch the
      latest version of a tuple. Obviously that needs to be abstracted
      away. To do so, move the logic that find the latest row version into
      the AM. lock_tuple has a new flag argument,
      TUPLE_LOCK_FLAG_FIND_LAST_VERSION, that forces it to lock the last
      version, rather than the current one.  It'd have been possible to do
      so via a separate callback as well, but finding the last version
      usually also necessitates locking the newest version, making it
      sensible to combine the two. This replaces the previous use of
      EvalPlanQualFetch().  Additionally HeapTupleUpdated, which previously
      signaled either a concurrent update or delete, is now split into two,
      to avoid callers needing AM specific knowledge to differentiate.
      
      The move of finding the latest row version into tuple_lock means that
      encountering a row concurrently moved into another partition will now
      raise an error about "tuple to be locked" rather than "tuple to be
      updated/deleted" - which is accurate, as that always happens when
      locking rows. While possible slightly less helpful for users, it seems
      like an acceptable trade-off.
      
      As part of this commit HTSU_Result has been renamed to TM_Result, and
      its members been expanded to differentiated between updating and
      deleting. HeapUpdateFailureData has been renamed to TM_FailureData.
      
      The interface to speculative insertion is changed so nodeModifyTable.c
      does not have to set the speculative token itself anymore. Instead
      there's a version of tuple_insert, tuple_insert_speculative, that
      performs the speculative insertion (without requiring a flag to signal
      that fact), and the speculative insertion is either made permanent
      with table_complete_speculative(succeeded = true) or aborted with
      succeeded = false).
      
      Note that multi_insert is not yet routed through tableam, nor is
      COPY. Changing multi_insert requires changes to copy.c that are large
      enough to better be done separately.
      
      Similarly, although simpler, CREATE TABLE AS and CREATE MATERIALIZED
      VIEW are also only going to be adjusted in a later commit.
      
      Author: Andres Freund and Haribabu Kommi
      Discussion:
          https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
          https://postgr.es/m/20190313003903.nwvrxi7rw3ywhdel@alap3.anarazel.de
          https://postgr.es/m/20160812231527.GA690404@alvherre.pgsql
      5db6df0c