1. 13 Nov, 2018 1 commit
  2. 12 Nov, 2018 6 commits
    • Michael Paquier's avatar
      Remove CommandCounterIncrement() after processing ON COMMIT DELETE · 52b70b1c
      Michael Paquier authored
      This comes from f9b5b41e, which is part of one the original commits that
      implemented ON COMMIT actions.  By looking at the truncation code, any
      CCI needed happens locally when rebuilding indexes, so it looks safe to
      just remove this final incrementation.
      
      Author: Michael Paquier
      Reviewed-by: Álvaro Herrera
      Discussion: https://postgr.es/m/20181109024731.GF2652@paquier.xyz
      52b70b1c
    • Tom Lane's avatar
      Simplify null-element handling in extension_config_remove(). · 3de49148
      Tom Lane authored
      There's no point in asking deconstruct_array() for a null-flags
      array when we already checked the array has no nulls, and aren't
      going to examine the output anyhow.  Not asking for this output
      should make the code marginally faster, and it's also more
      robust since if there somehow were nulls, deconstruct_array()
      would throw an error.
      
      Daniel Gustafsson
      
      Discussion: https://postgr.es/m/289FFB8B-7AAB-48B5-A497-6E0D41D7BA47@yesql.se
      3de49148
    • Tom Lane's avatar
      Limit the number of index clauses considered in choose_bitmap_and(). · e3f005d9
      Tom Lane authored
      classify_index_clause_usage() is O(N^2) in the number of distinct index
      qual clauses it considers, because of its use of a simple search list to
      store them.  For nearly all queries, that's fine because only a few clauses
      will be considered.  But Alexander Kuzmenkov reported a machine-generated
      query with 80000 (!) index qual clauses, which caused this code to take
      forever.  Somewhat remarkably, this is the only O(N^2) behavior we now
      have for such a query, so let's fix it.
      
      We can get rid of the O(N^2) runtime for cases like this without much
      damage to the functionality of choose_bitmap_and() by separating out
      paths with "too many" qual or pred clauses, and deeming them to always
      be nonredundant with other paths.  Then their clauses needn't go into
      the search list, so it doesn't get too long, but we don't lose the
      ability to consider bitmap AND plans altogether.  I set the threshold
      for "too many" to be 100 clauses per path, which should be plenty to
      ensure no change in planning behavior for normal queries.
      
      There are other things we could do to make this go faster, but it's not
      clear that it's worth any additional effort.  80000 qual clauses require
      a whole lot of work in many other places, too.
      
      The code's been like this for a long time, so back-patch to all supported
      branches.  The troublesome query only works back to 9.5 (in 9.4 it fails
      with stack overflow in the parser); so I'm not sure that fixing this in
      9.4 has any real-world benefit, but perhaps it does.
      
      Discussion: https://postgr.es/m/90c5bdfa-d633-dabe-9889-3cf3e1acd443@postgrespro.ru
      e3f005d9
    • Michael Paquier's avatar
      ffb68980
    • Peter Eisentraut's avatar
      doc: Small punctuation improvement · fc151211
      Peter Eisentraut authored
      fc151211
    • Peter Eisentraut's avatar
      doc: Small run-time pruning doc fix · 253b3f67
      Peter Eisentraut authored
      A note in ddl.sgml used to mention that run-time pruning was only
      implemented for Append.  When we got MergeAppend support, this was
      updated to mention that MergeAppend is supported too.  This is
      slightly weird as it's not all that obvious what exactly isn't
      supported when we mention:
      
          <para>
           Both of these behaviors are likely to be changed in a future release
           of <productname>PostgreSQL</productname>.
          </para>
      
      This patch updates this to mention that ModifyTable is unsupported,
      which makes the above fragment make sense again.
      
      Author: David Rowley <david.rowley@2ndquadrant.com>
      253b3f67
  3. 11 Nov, 2018 2 commits
  4. 10 Nov, 2018 8 commits
    • Peter Eisentraut's avatar
      Apply RI trigger skipping tests also for DELETE · 69ee2ff9
      Peter Eisentraut authored
      The tests added in cfa0f425 to skip
      firing an RI trigger if any old key value is NULL can also be applied
      for DELETE.  This should give a performance gain in those cases, and it
      also saves a lot of duplicate code in the actual RI triggers.  (That
      code was already dead code for the UPDATE cases.)
      Reviewed-by: default avatarDaniel Gustafsson <daniel@yesql.se>
      69ee2ff9
    • Peter Eisentraut's avatar
      Remove dead foreign key optimization code · 34479d9a
      Peter Eisentraut authored
      The ri_KeysEqual() calls in the foreign-key trigger functions to
      optimize away some updates are useless because since
      adfeef55 those triggers are not enqueued
      at all.  (It's also not useful to keep these checks as some kind of
      backstop, since it's also semantically correct to just run the full
      check even with equal keys.)
      Reviewed-by: default avatarDaniel Gustafsson <daniel@yesql.se>
      34479d9a
    • Andres Freund's avatar
      Combine two flag tests in GetSnapshotData(). · 5fde047f
      Andres Freund authored
      Previously the code checked PROC_IN_LOGICAL_DECODING and
      PROC_IN_VACUUM separately. As the relevant variable is marked as
      volatile, the compiler cannot combine the two tests.  As
      GetSnapshotData() is pretty hot in a number of workloads, it's
      worthwhile to fix that.
      
      It'd also be a good idea to get rid of the volatiles altogether. But
      for one that's a larger patch, and for another, the code after this
      change still seems at least as easy to read as before.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20181005172955.wyjb4fzcdzqtaxjq@alap3.anarazel.de
      5fde047f
    • Andres Freund's avatar
      docs: Adapt wal_segment_size docs to fc49e24f. · 5fc1670b
      Andres Freund authored
      Before this change the docs weren't adapted to the fact that
      wal_segment_size is now measured in bytes, rather than multiples of
      wal_block_size.
      
      Author: David Steele
      Discussion: https://postgr.es/m/68ea97d6-2ed9-f339-e57d-ab3a33caf3b1@pgmasters.net
      Backpatch: 11-, like fc49e24f itself.
      5fc1670b
    • Tom Lane's avatar
      Fix error-cleanup mistakes in exec_stmt_call(). · f26c06a4
      Tom Lane authored
      Commit 15c72934 was a couple bricks shy of a load: we need to
      ensure that expr->plan gets reset to NULL on any error exit,
      if it's not supposed to be saved.  Also ensure that the
      stmt->target calculation gets redone if needed.
      
      The easy way to exhibit a problem is to set up code that
      violates the writable-argument restriction and then execute
      it twice.  But error exits out of, eg, setup_param_list()
      could also break it.  Make the existing PG_TRY block cover
      all of that code to be sure.
      
      Per report from Pavel Stehule.
      
      Discussion: https://postgr.es/m/CAFj8pRAeXNTO43W2Y0Cn0YOVFPv1WpYyOqQrrzUiN6s=dn7gCg@mail.gmail.com
      f26c06a4
    • Tom Lane's avatar
      Fix missing role dependencies for some schema and type ACLs. · fa2952d8
      Tom Lane authored
      This patch fixes several related cases in which pg_shdepend entries were
      never made, or were lost, for references to roles appearing in the ACLs of
      schemas and/or types.  While that did no immediate harm, if a referenced
      role were later dropped, the drop would be allowed and would leave a
      dangling reference in the object's ACL.  That still wasn't a big problem
      for normal database usage, but it would cause obscure failures in
      subsequent dump/reload or pg_upgrade attempts, taking the form of
      attempts to grant privileges to all-numeric role names.  (I think I've
      seen field reports matching that symptom, but can't find any right now.)
      
      Several cases are fixed here:
      
      1. ALTER DOMAIN SET/DROP DEFAULT would lose the dependencies for any
      existing ACL entries for the domain.  This case is ancient, dating
      back as far as we've had pg_shdepend tracking at all.
      
      2. If a default type privilege applies, CREATE TYPE recorded the
      ACL properly but forgot to install dependency entries for it.
      This dates to the addition of default privileges for types in 9.2.
      
      3. If a default schema privilege applies, CREATE SCHEMA recorded the
      ACL properly but forgot to install dependency entries for it.
      This dates to the addition of default privileges for schemas in v10
      (commit ab89e465).
      
      Another somewhat-related problem is that when creating a relation
      rowtype or implicit array type, TypeCreate would apply any available
      default type privileges to that type, which we don't really want
      since such an object isn't supposed to have privileges of its own.
      (You can't, for example, drop such privileges once they've been added
      to an array type.)
      
      ab89e465 is also to blame for a race condition in the regression tests:
      privileges.sql transiently installed globally-applicable default
      privileges on schemas, which sometimes got absorbed into the ACLs of
      schemas created by concurrent test scripts.  This should have resulted
      in failures when privileges.sql tried to drop the role holding such
      privileges; but thanks to the bug fixed here, it instead led to dangling
      ACLs in the final state of the regression database.  We'd managed not to
      notice that, but it became obvious in the wake of commit da906766, which
      allowed the race condition to occur in pg_upgrade tests.
      
      To fix, add a function recordDependencyOnNewAcl to encapsulate what
      callers of get_user_default_acl need to do; while the original call
      sites got that right via ad-hoc code, none of the later-added ones
      have.  Also change GenerateTypeDependencies to generate these
      dependencies, which requires adding the typacl to its parameter list.
      (That might be annoying if there are any extensions calling that
      function directly; but if there are, they're most likely buggy in the
      same way as the core callers were, so they need work anyway.)  While
      I was at it, I changed GenerateTypeDependencies to accept most of its
      parameters in the form of a Form_pg_type pointer, making its parameter
      list a bit less unwieldy and mistake-prone.
      
      The test race condition is fixed just by wrapping the addition and
      removal of default privileges into a single transaction, so that that
      state is never visible externally.  We might eventually prefer to
      separate out tests of default privileges into a script that runs by
      itself, but that would be a bigger change and would make the tests
      run slower overall.
      
      Back-patch relevant parts to all supported branches.
      
      Discussion: https://postgr.es/m/15719.1541725287@sss.pgh.pa.us
      fa2952d8
    • Andres Freund's avatar
      Remove ineffective check against dropped columns from slot_getattr(). · c670d0fa
      Andres Freund authored
      Before this commit slot_getattr() checked for dropped
      columns (returning NULL in that case), but only after checking for
      previously deformed columns. As slot_deform_tuple() does not contain
      such a check, the check in slot_getattr() would often not have been
      reached, depending on previous use of the slot.
      
      These days locking and plan invalidation ought to ensure that dropped
      columns are not accessed in query plans. Therefore this commit just
      drops the insufficient check in slot_getattr().  It's possible that
      we'll find some holes againt use of dropped columns, but if so, those
      need to be addressed independent of slot_getattr(), as most accesses
      don't go through that function anyway.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20181107174403.zai7fedgcjoqx44p@alap3.anarazel.de
      c670d0fa
    • Andres Freund's avatar
      Don't require return slots for nodes without projection. · 1ef6bd29
      Andres Freund authored
      In a lot of nodes the return slot is not required. That can either be
      because the node doesn't do any projection (say an Append node), or
      because the node does perform projections but the projection is
      optimized away because the projection would yield an identical row.
      
      Slots aren't that small, especially for wide rows, so it's worthwhile
      to avoid creating them.  It's not possible to just skip creating the
      slot - it's currently used to determine the tuple descriptor returned
      by ExecGetResultType().  So separate the determination of the result
      type from the slot creation.  The work previously done internally
      ExecInitResultTupleSlotTL() can now also be done separately with
      ExecInitResultTypeTL() and ExecInitResultSlot().  That way nodes that
      aren't guaranteed to need a result slot, can use
      ExecInitResultTypeTL() to determine the result type of the node, and
      ExecAssignScanProjectionInfo() (via
      ExecConditionalAssignProjectionInfo()) determines that a result slot
      is needed, it is created with ExecInitResultSlot().
      
      Besides the advantage of avoiding to create slots that then are
      unused, this is necessary preparation for later patches around tuple
      table slot abstraction. In particular separating the return descriptor
      and slot is a prerequisite to allow JITing of tuple deforming with
      knowledge of the underlying tuple format, and to avoid unnecessarily
      creating JITed tuple deforming for virtual slots.
      
      This commit removes a redundant argument from
      ExecInitResultTupleSlotTL(). While this commit touches a lot of the
      relevant lines anyway, it'd normally still not worthwhile to cause
      breakage, except that aforementioned later commits will touch *all*
      ExecInitResultTupleSlotTL() callers anyway (but fits worse
      thematically).
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20181105210039.hh4vvi4vwoq5ba2q@alap3.anarazel.de
      1ef6bd29
  5. 09 Nov, 2018 3 commits
    • Michael Paquier's avatar
      Fix incorrect routine name in xlog_heapam.h · 3ce12018
      Michael Paquier authored
      s/xl_heap_delete/xl_heap_truncate/ in a comment block referring to flags
      for truncation.
      
      Discussion: https://postgr.es/m/20180413034734.GE1552@paquier.xyz
      3ce12018
    • Alvaro Herrera's avatar
      Indicate session name in isolationtester notices · a28e10e8
      Alvaro Herrera authored
      When a session under isolationtester produces printable notices (NOTICE,
      WARNING) we were just printing them unadorned, which can be confusing
      when debugging.  Prefix them with the session name, which makes things
      clearer.
      
      Author: Álvaro Herrera
      Reviewed-by: Hari Babu Kommi
      Discussion: https://postgr.es/m/20181024213451.75nh3f3dctmcdbfq@alvherre.pgsql
      a28e10e8
    • Michael Paquier's avatar
      Fix dependency handling of partitions and inheritance for ON COMMIT · 319a8101
      Michael Paquier authored
      This commit fixes a set of issues with ON COMMIT actions when used on
      partitioned tables and tables with inheritance children:
      - Applying ON COMMIT DROP on a partitioned table with partitions or on a
      table with inheritance children caused a failure at commit time, with
      complains about the children being already dropped as all relations are
      dropped one at the same time.
      - Applying ON COMMIT DELETE on a partition relying on a partitioned
      table which uses ON COMMIT DROP would cause the partition truncation to
      fail as the parent is removed first.
      
      The solution to the first problem is to handle the removal of all the
      dependencies in one go instead of dropping relations one-by-one, based
      on a suggestion from Álvaro Herrera.  So instead all the relation OIDs
      to remove are gathered and then processed in one round of multiple
      deletions.
      
      The solution to the second problem is to reorder the actions, with
      truncation happening first and relation drop done after.  Even if it
      means that a partition could be first truncated, then immediately
      dropped if its partitioned table is dropped, this has the merit to keep
      the code simple as there is no need to do existence checks on the
      relations to drop.
      
      Contrary to a manual TRUNCATE on a partitioned table, ON COMMIT DELETE
      does not cascade to its partitions.  The ON COMMIT action defined on
      each partition gets the priority.
      
      Author: Michael Paquier
      Reviewed-by: Amit Langote, Álvaro Herrera, Robert Haas
      Discussion: https://postgr.es/m/68f17907-ec98-1192-f99f-8011400517f5@lab.ntt.co.jp
      Backpatch-through: 10
      319a8101
  6. 08 Nov, 2018 4 commits
    • Tom Lane's avatar
      Disallow setting client_min_messages higher than ERROR. · 3d360e20
      Tom Lane authored
      Previously it was possible to set client_min_messages to FATAL or PANIC,
      which had the effect of suppressing transmission of regular ERROR messages
      to the client.  Perhaps that seemed like a useful option in the past, but
      the trouble with it is that it breaks guarantees that are explicitly made
      in our FE/BE protocol spec about how a query cycle can end.  While libpq
      and psql manage to cope with the omission, that's mostly because they
      are not very bright; client libraries that have more semantic knowledge
      are likely to get confused.  Notably, pgODBC doesn't behave very sanely.
      Let's fix this by getting rid of the ability to set client_min_messages
      above ERROR.
      
      In HEAD, just remove the FATAL and PANIC options from the set of allowed
      enum values for client_min_messages.  (This change also affects
      trace_recovery_messages, but that's OK since these aren't useful values
      for that variable either.)
      
      In the back branches, there was concern that rejecting these values might
      break applications that are explicitly setting things that way.  I'm
      pretty skeptical of that argument, but accommodate it by accepting these
      values and then internally setting the variable to ERROR anyway.
      
      In all branches, this allows a couple of tiny simplifications in the
      logic in elog.c, so do that.
      
      Also respond to the point that was made that client_min_messages has
      exactly nothing to do with the server's logging behavior, and therefore
      does not belong in the "When To Log" subsection of the documentation.
      The "Statement Behavior" subsection is a better match, so move it there.
      
      Jonah Harris and Tom Lane
      
      Discussion: https://postgr.es/m/7809.1541521180@sss.pgh.pa.us
      Discussion: https://postgr.es/m/15479-ef0f4cc2fd995ca2@postgresql.org
      3d360e20
    • Alvaro Herrera's avatar
      Revise attribute handling code on partition creation · 705d433f
      Alvaro Herrera authored
      The original code to propagate NOT NULL and default expressions
      specified when creating a partition was mostly copy-pasted from
      typed-tables creation, but not being a great match it contained some
      duplicity, inefficiency and bugs.
      
      This commit fixes the bug that NOT NULL constraints declared in the
      parent table would not be honored in the partition.  One reported issue
      that is not fixed is that a DEFAULT declared in the child is not used
      when inserting through the parent.  That would amount to a behavioral
      change that's better not back-patched.
      
      This rewrite makes the code simpler:
      
      1. instead of checking for duplicate column names in its own block,
      reuse the original one that already did that;
      
      2. instead of concatenating the list of columns from parent and the one
      declared in the partition and scanning the result to (incorrectly)
      propagate defaults and not-null constraints, just scan the latter
      searching the former for a match, and merging sensibly.  This works
      because we know the list in the parent is already correct and there can
      only be one parent.
      
      This rewrite makes ColumnDef->is_from_parent unused, so it's removed
      on branch master; on released branches, it's kept as an unused field in
      order not to cause ABI incompatibilities.
      
      This commit also adds a test case for creating partitions with
      collations mismatching that on the parent table, something that is
      closely related to the code being patched.  No code change is introduced
      though, since that'd be a behavior change that could break some (broken)
      working applications.
      
      Amit Langote wrote a less invasive fix for the original
      NOT NULL/defaults bug, but while I kept the tests he added, I ended up
      not using his original code.  Ashutosh Bapat reviewed Amit's fix.  Amit
      reviewed mine.
      
      Author: Álvaro Herrera, Amit Langote
      Reviewed-by: Ashutosh Bapat, Amit Langote
      Reported-by: Jürgen Strobel (bug #15212)
      Discussion: https://postgr.es/m/152746742177.1291.9847032632907407358@wrigleys.postgresql.org
      705d433f
    • Andrew Dunstan's avatar
      Adjust valgrind fix in commit 517b0d0b · 12d5f39b
      Andrew Dunstan authored
      lousyjack still wasn't happy. I have tested this modification and it
      worked.
      12d5f39b
    • Michael Paquier's avatar
  7. 07 Nov, 2018 9 commits
  8. 06 Nov, 2018 7 commits