1. 15 Oct, 2020 1 commit
  2. 14 Oct, 2020 6 commits
  3. 13 Oct, 2020 5 commits
    • Tom Lane's avatar
      Paper over regression failures in infinite_recurse() on PPC64 Linux. · ae0f7b11
      Tom Lane authored
      Our infinite_recurse() test to verify sane stack-overrun behavior
      is affected by a bug of the Linux kernel on PPC64: it will get SIGSEGV
      if it receives a signal when the stack depth is (a) over 1MB and
      (b) within a few kB of filling the current physical stack allocation.
      See https://bugzilla.kernel.org/show_bug.cgi?id=205183.
      
      Since this test is a bit time-consuming and we run it in parallel with
      test scripts that do a lot of DDL, it can be expected to get an sinval
      catchup interrupt at some point, leading to failure if the timing is
      wrong.  This has caused more than 100 buildfarm failures over the
      past year or so.
      
      While a fix exists for the kernel bug, it might be years before that
      propagates into all production kernels, particularly in some of the
      older distros we have in the buildfarm.  For now, let's just back off
      and not run this test on Linux PPC64; that loses nothing in test
      coverage so far as our own code is concerned.
      
      To do that, split this test into a new script infinite_recurse.sql
      and skip the test when the platform name is powerpc64...-linux-gnu.
      
      Back-patch to v12.  Branches before that have not been seen to get
      this failure.  No doubt that's because the "errors" test was not
      run in parallel with other tests before commit 798070ec, greatly
      reducing the odds of an sinval catchup being necessary.
      
      I also back-patched 3c855354 into v12, just so the new regression
      script would look the same in all branches having it.
      
      Discussion: https://postgr.es/m/3479046.1602607848@sss.pgh.pa.us
      Discussion: https://postgr.es/m/20190723162703.GM22387%40telsasoft.com
      ae0f7b11
    • Heikki Linnakangas's avatar
      Create ResultRelInfos later in InitPlan, index them by RT index. · 1375422c
      Heikki Linnakangas authored
      Instead of allocating all the ResultRelInfos upfront in one big array,
      allocate them in ExecInitModifyTable(). es_result_relations is now an
      array of ResultRelInfo pointers, rather than an array of structs, and it
      is indexed by the RT index.
      
      This simplifies things: we get rid of the separate concept of a "result
      rel index", and don't need to set it in setrefs.c anymore. This also
      allows follow-up optimizations (not included in this commit yet) to skip
      initializing ResultRelInfos for target relations that were not needed at
      runtime, and removal of the es_result_relation_info pointer.
      
      The EState arrays of regular result rels and root result rels are merged
      into one array. Similarly, the resultRelations and rootResultRelations
      lists in PlannedStmt are merged into one. It's not actually clear to me
      why they were kept separate in the first place, but now that the
      es_result_relations array is indexed by RT index, it certainly seems
      pointless.
      
      The PlannedStmt->resultRelations list is now only needed for
      ExecRelationIsTargetRelation(). One visible effect of this change is that
      ExecRelationIsTargetRelation() will now return 'true' also for the
      partition root, if a partitioned table is updated. That seems like a good
      thing, although the function isn't used in core code, and I don't see any
      reason for an FDW to call it on a partition root.
      
      Author: Amit Langote
      Discussion: https://www.postgresql.org/message-id/CA%2BHiwqGEmiib8FLiHMhKB%2BCH5dRgHSLc5N5wnvc4kym%2BZYpQEQ%40mail.gmail.com
      1375422c
    • Amit Kapila's avatar
      Fix the unstable output of tests added by commit 8fccf758. · 2050832d
      Amit Kapila authored
      The test cases added by that commit were trying to test the exact number of
      times a particular transaction has spilled. However, that number can vary if
      any background transaction (say by autovacuum) happens in parallel to the main
      transaction. So let's not try to verify the exact count.
      
      Author: Amit Kapila
      Reviewed-by: Sawada Masahiko
      Discussion: https://postgr.es/m/CA+fd4k5_pPAYRTDrO2PbtTOe0eHQpBvuqmCr8ic39uTNmR49Eg@mail.gmail.com
      2050832d
    • Peter Eisentraut's avatar
      doc: Expand recursive query documentation · 323ae003
      Peter Eisentraut authored
      Break the section up with subsection headings.  Add examples for depth-
      and breadth-first search ordering.  For consistency with the SQL
      search clause, start the depth counting at 0 instead of 1 in the
      examples.
      
      Discussion: https://www.postgresql.org/message-id/c5603982-0088-7f14-0caa-fdcd0c837b57@2ndquadrant.com
      323ae003
    • Amit Kapila's avatar
      Add tests for logical replication spilled stats. · 8fccf758
      Amit Kapila authored
      Commit 98681675 added a mechanism to track statistics corresponding to
      the spilling of changes from ReorderBuffer but didn't add any tests.
      
      Author: Amit Kapila and Sawada Masahiko
      Discussion: https://postgr.es/m/CA+fd4k5_pPAYRTDrO2PbtTOe0eHQpBvuqmCr8ic39uTNmR49Eg@mail.gmail.com
      8fccf758
  4. 12 Oct, 2020 9 commits
  5. 11 Oct, 2020 1 commit
  6. 10 Oct, 2020 5 commits
    • Tom Lane's avatar
      Band-aid new postgres_fdw test case to remove error text dependency. · 85d08b8b
      Tom Lane authored
      Buildfarm member lorikeet is still failing the test from commit
      32a9c0bd, but now it's down to the should-have-foreseen-it problem
      that the error message isn't what the expected-output file expects.
      Let's see if we can get stable results by printing just the SQLSTATE.
      I believe we'll reliably see ERRCODE_CONNECTION_FAILURE, since
      pgfdw_report_error() will report that for any libpq-originated error.
      
      There may be a better way to do this, but I'd like to get the
      buildfarm back to green before we discuss further improvements.
      
      Discussion: https://postgr.es/m/E1kPc9v-0005L4-2l@gemulon.postgresql.org
      Discussion: https://postgr.es/m/2621622.1602184554@sss.pgh.pa.us
      85d08b8b
    • Tom Lane's avatar
      Remove pointless error-code checking in pg_dump/parallel.c. · eeb01eb1
      Tom Lane authored
      Commit fe27009c tried to make parallel.c's Windows implementation of
      piperead() translate Windows socket errors to Unix, but that didn't
      actually work because TranslateSocketError() is backend-internal code
      (and not even public there).  But on closer inspection, the sole
      caller of this function doesn't actually care whether the result is
      zero or negative, much less inspect the errno.  So the whole exercise
      is totally useless, and has been since this code was introduced.
      Rip it out and just call recv() directly.
      
      Per buildfarm.
      
      Discussion: https://postgr.es/m/2621622.1602184554@sss.pgh.pa.us
      eeb01eb1
    • Tom Lane's avatar
      Minor cleanup for win32stat.c. · 961e07b8
      Tom Lane authored
      Fix silly typo in previous commit.
      
      Discussion: https://postgr.es/m/CAC+AXB0g44SbvSpC86o_1HWh8TAU2pZrMRW6tJT-dkijotx5Qg@mail.gmail.com
      961e07b8
    • Tom Lane's avatar
      Minor cleanup for win32stat.c. · c94cfb38
      Tom Lane authored
      Ensure that CloseHandle() can't clobber the errno we set for
      failure exits, and make a couple of tweaks for pgindent.
      
      Juan José Santamaría Flecha
      
      Discussion: https://postgr.es/m/CAC+AXB0g44SbvSpC86o_1HWh8TAU2pZrMRW6tJT-dkijotx5Qg@mail.gmail.com
      c94cfb38
    • Tom Lane's avatar
      Recognize network-failure errnos as indicating hard connection loss. · fe27009c
      Tom Lane authored
      Up to now, only ECONNRESET (and EPIPE, in most but not quite all places)
      received special treatment in our error handling logic.  This patch
      changes things so that related error codes such as ECONNABORTED are
      also recognized as indicating that the connection's dead and unlikely
      to come back.
      
      We continue to think, however, that only ECONNRESET and EPIPE should be
      reported as probable server crashes; the other cases indicate network
      connectivity problems but prove little about the server's state.  Thus,
      there's no change in the error message texts that are output for such
      cases.  The key practical effect is that errcode_for_socket_access()
      will report ERRCODE_CONNECTION_FAILURE rather than
      ERRCODE_INTERNAL_ERROR for a network failure.  It's expected that this
      will fix buildfarm member lorikeet's failures since commit 32a9c0bd,
      as that seems to be due to not treating ECONNABORTED equivalently to
      ECONNRESET.
      
      The set of errnos treated this way now includes ECONNABORTED, EHOSTDOWN,
      EHOSTUNREACH, ENETDOWN, ENETRESET, and ENETUNREACH.  Several of these
      were second-class citizens in terms of their handling in places like
      get_errno_symbol(), so upgrade the infrastructure where necessary.
      
      As committed, this patch assumes that all these symbols are defined
      everywhere.  POSIX specifies all of them except EHOSTDOWN, but that
      seems to exist on all platforms of interest; we'll see what the
      buildfarm says about that.
      
      Probably this should be back-patched, but let's see what the buildfarm
      thinks of it first.
      
      Fujii Masao and Tom Lane
      
      Discussion: https://postgr.es/m/2621622.1602184554@sss.pgh.pa.us
      fe27009c
  7. 09 Oct, 2020 3 commits
  8. 08 Oct, 2020 5 commits
  9. 07 Oct, 2020 5 commits
    • Tom Lane's avatar
      Fix optimization hazard in gram.y's makeOrderedSetArgs(), redux. · 8d2a01ae
      Tom Lane authored
      It appears that commit cf63c641, which intended to prevent
      misoptimization of the result-building step in makeOrderedSetArgs,
      didn't go far enough: buildfarm member hornet's version of xlc
      is now optimizing back to the old, broken behavior in which
      list_length(directargs) is fetched only after list_concat() has
      changed that value.  I'm not entirely convinced whether that's
      an undeniable compiler bug or whether it can be justified by a
      sufficiently aggressive interpretation of C sequence points.
      So let's just change the code to make it harder to misinterpret.
      
      Back-patch to all supported versions, just in case.
      
      Discussion: https://postgr.es/m/1830491.1601944935@sss.pgh.pa.us
      8d2a01ae
    • Tom Lane's avatar
      Prevent internal overflows in date-vs-timestamp and related comparisons. · 3db322ea
      Tom Lane authored
      The date-vs-timestamp, date-vs-timestamptz, and timestamp-vs-timestamptz
      comparators all worked by promoting the first type to the second and
      then doing a simple same-type comparison.  This works fine, except
      when the conversion result is out of range, in which case we throw an
      entirely avoidable error.  The sources of such failures are
      (a) type date can represent dates much farther in the future than
      the timestamp types can;
      (b) timezone rotation might cause a just-in-range timestamp value to
      become a just-out-of-range timestamptz value.
      
      Up to now we just ignored these corner-case issues, but now we have
      an actual user complaint (bug #16657 from Huss EL-Sheikh), so let's
      do something about it.
      
      It turns out that commit 52ad1e65 already built all the necessary
      infrastructure to support error-free comparisons, but neglected to
      actually use it in the main-line code paths.  Fix that, do a little
      bit of code style review, and remove the now-duplicate logic in
      jsonpath_exec.c.
      
      Back-patch to v13 where 52ad1e65 came in.  We could take this back
      further by back-patching said infrastructure, but given the small
      number of complaints so far, I don't feel a great need to.
      
      Discussion: https://postgr.es/m/16657-cde2f876d8cc7971@postgresql.org
      3db322ea
    • Tom Lane's avatar
      6c05e5b7
    • Tom Lane's avatar
      Rethink recent fix for pg_dump's handling of extension config tables. · 9e5f1f21
      Tom Lane authored
      Commit 3eb3d3e7 was a few bricks shy of a load: while it correctly
      set the table's "interesting" flag when deciding to dump the data of
      an extension config table, it was not correct to clear that flag
      if we concluded we shouldn't dump the data.  This led to the crash
      reported in bug #16655, because in fact we'll traverse dumpTableSchema
      anyway for all extension tables (to see if they have user-added
      seclabels or RLS policies).
      
      The right thing to do is to force "interesting" true in makeTableDataInfo,
      and otherwise leave the flag alone.  (Doing it there is more future-proof
      in case additional calls are added, and it also avoids setting the flag
      unnecessarily if that function decides the table is non-dumpable.)
      
      This investigation also showed that while only the --inserts code path
      had an obvious failure in the case considered by 3eb3d3e7, the COPY
      code path also has a problem with not having loaded table subsidiary
      data.  That causes fmtCopyColumnList to silently return an empty string
      instead of the correct column list.  That accidentally mostly works,
      which perhaps is why we didn't notice this before.  It would only fail
      if the restore column order is different from the dump column order,
      which only happens in weird inheritance cases, so it's not surprising
      nobody had hit the case with an extension config table.  Nonetheless,
      it's a bug, and it goes a long way back, not just to v12 where the
      --inserts code path started to have a problem with this.
      
      In hopes of catching such cases a bit sooner in future, add some
      Asserts that "interesting" has been set in both dumpTableData and
      dumpTableSchema.  Adjust the test case added by 3eb3d3e7 so that it
      checks the COPY rather than INSERT form of that bug, allowing it to
      detect the longer-standing symptom.
      
      Per bug #16655 from Cameron Daniel.  Back-patch to all supported
      branches.
      
      Discussion: https://postgr.es/m/16655-5c92d6b3a9438137@postgresql.org
      Discussion: https://postgr.es/m/18048b44-3414-b983-8c7c-9165b177900d@2ndQuadrant.com
      9e5f1f21
    • Amit Kapila's avatar
      Display the names of missing columns in error during logical replication. · f0770709
      Amit Kapila authored
      In logical replication when a subscriber is missing some columns, it
      currently emits an error message that says "some" columns are missing, but
      it doesn't specify the missing column names. Change that to display
      missing column names which makes an error to be more informative to the
      user.
      
      We have decided not to backpatch this commit as this is a minor usability
      improvement and no user has reported this.
      
      Reported-by: Bharath Rupireddy
      Author: Bharath Rupireddy
      Reviewed-by: Kyotaro Horiguchi and Amit Kapila
      Discussion: https://postgr.es/m/CALj2ACVkW-EXH_4pmBK8tNeHRz5ksUC4WddGactuCjPiBch-cg@mail.gmail.com
      f0770709