1. 10 Nov, 2018 5 commits
    • 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
  2. 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
  3. 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
  4. 07 Nov, 2018 9 commits
  5. 06 Nov, 2018 13 commits
  6. 05 Nov, 2018 6 commits