Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 13 Oct, 2020 1 commit
  3. 12 Oct, 2020 9 commits
  5. 11 Oct, 2020 1 commit
  7. 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
  9. 09 Oct, 2020 3 commits
  11. 08 Oct, 2020 5 commits
  13. 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
      Clean up after newly-added tests for pg_test_fsync and pg_test_timing. · 6c05e5b7
      Tom Lane authored 
      Oversight in 4d29e6db.
      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
  15. 06 Oct, 2020 9 commits
  17. 05 Oct, 2020 2 commits