1. 28 Sep, 2018 9 commits
  2. 27 Sep, 2018 5 commits
    • Tom Lane's avatar
      Fix assorted bugs in pg_get_partition_constraintdef(). · aaf10f32
      Tom Lane authored
      It failed if passed a nonexistent relation OID, or one that was a non-heap
      relation, because of blindly applying heap_open to a user-supplied OID.
      This is not OK behavior for a SQL-exposed function; we have a project
      policy that we should return NULL in such cases.  Moreover, since
      pg_get_partition_constraintdef ought now to work on indexes, restricting
      it to heaps is flat wrong anyway.
      
      The underlying function generate_partition_qual() wasn't on board with
      indexes having partition quals either, nor for that matter with rels
      having relispartition set but yet null relpartbound.  (One wonders
      whether the person who wrote the function comment blocks claiming that
      these functions allow a missing relpartbound had ever tested it.)
      
      Fix by testing relispartition before opening the rel, and by using
      relation_open not heap_open.  (If any other relkinds ever grow the
      ability to have relispartition set, the code will work with them
      automatically.)  Also, don't reject null relpartbound in
      generate_partition_qual.
      
      Back-patch to v11, and all but the null-relpartbound change to v10.
      (It's not really necessary to change generate_partition_qual at all
      in v10, but I thought s/heap_open/relation_open/ would be a good
      idea anyway just to keep the code in sync with later branches.)
      
      Per report from Justin Pryzby.
      
      Discussion: https://postgr.es/m/20180927200020.GJ776@telsasoft.com
      aaf10f32
    • Alexander Korotkov's avatar
      Minor formatting cleanup for 2a636834 · 4ec90f53
      Alexander Korotkov authored
      4ec90f53
    • Alexander Korotkov's avatar
    • Andres Freund's avatar
      Clean up in the wake of TupleDescGetSlot() removal / 10763358. · 27e082b0
      Andres Freund authored
      The previous commit wasn't careful enough to remove all traces of
      TupleDescGetSlot().
      
      Besides fixing the oversight of not removing TupleDescGetSlot()'s
      declaration, this also removes FuncCallContext->slot. That was
      documented to be for use in combination with TupleDescGetSlot(), a
      cursory search over extensions finds no users, and there doesn't seem
      to be convincing reasons to keep it around. If we later in the v12
      release cycle find users, we can re-consider this part of the commit.
      
      Reported-By: Michael Paquier
      Discussion: https://postgr.es/m/20180926000413.GC1659@paquier.xyz
      27e082b0
    • Tom Lane's avatar
      Build src/port files as a library with -fPIC, and use that in libpq. · ea53100d
      Tom Lane authored
      libpq and ecpg need shared-library-friendly versions of assorted src/port/
      and src/common/ modules.  Up to now, they got those by symlinking the
      individual source files and compiling them locally.  That's baroque, and a
      pain to maintain, and it results in some amount of duplicated compile work.
      It might've made sense when only a couple of files were needed, but the
      list has grown and grown and grown :-(
      
      It makes more sense to have the originating directory build a third variant
      of libpgport.a/libpgcommon.a containing modules built with $(CFLAGS_SL),
      and just link that into the shared library.  Unused files won't get linked,
      so the end result should be the same.
      
      This patch makes a down payment on that idea by having src/port/ build
      such a library and making libpq use it.  If the buildfarm doesn't expose
      fatal problems with the approach, I'll extend it to the other cases.
      
      Discussion: https://postgr.es/m/13022.1538003440@sss.pgh.pa.us
      ea53100d
  3. 26 Sep, 2018 15 commits
    • Tom Lane's avatar
      Fix another portability issue from commit 758ce9b7. · ce4887bd
      Tom Lane authored
      strerror.c now requires strlcpy() in some cases, and a couple of the
      ecpg libraries did not have that at hand.  Pull it in from src/port/
      following the usual recipe.  Per buildfarm.
      ce4887bd
    • Michael Paquier's avatar
      Switch flags tracking pending interrupts to sig_atomic_t · ba16aade
      Michael Paquier authored
      Those previously used bool, which should be safe on any modern
      platforms, however the C standard is clear that it is better to use
      sig_atomic_t for variables manipulated in signal handlers.  This commit
      adds at the same time PGDLLIMPORT to ClientConnectionLost.
      
      Author: Michael Paquier
      Reviewed-by: Tom Lane, Chris Travers, Andres Freund
      Discussion: https://postgr.es/m/20180925011311.GD1354@paquier.xyz
      ba16aade
    • Tom Lane's avatar
      Try another way to detect the result type of strerror_r(). · 751f532b
      Tom Lane authored
      The method we've traditionally used, of redeclaring strerror_r() to
      see if the compiler complains of inconsistent declarations, turns out
      not to work reliably because some compilers only report a warning,
      not an error.  Amazingly, this has gone undetected for years, even
      though it certainly breaks our detection of whether strerror_r
      succeeded.
      
      Let's instead test whether the compiler will take the result of
      strerror_r() as a switch() argument.  It's possible this won't
      work universally either, but it's the best idea I could come up with
      on the spur of the moment.
      
      We should probably back-patch this once the dust settles, but
      first let's see what the buildfarm thinks of it.
      
      Discussion: https://postgr.es/m/10877.1537993279@sss.pgh.pa.us
      751f532b
    • Tom Lane's avatar
      Clean up *printf macros to avoid conflict with format archetypes. · 8b91d258
      Tom Lane authored
      We must define the macro "printf" with arguments, else it can mess
      up format archetype attributes in builds where PG_PRINTF_ATTRIBUTE
      is just "printf".  Fortunately, that's easy to do now that we're
      requiring C99; we can use __VA_ARGS__.
      
      On the other hand, it's better not to use __VA_ARGS__ for the rest
      of the *printf crew, so that one can take the addresses of those
      functions without surprises.
      
      I'd proposed doing this some time ago, but forgot to make it happen;
      buildfarm failures subsequent to 96bf88d5 reminded me.
      
      Discussion: https://postgr.es/m/22709.1535135640@sss.pgh.pa.us
      Discussion: https://postgr.es/m/20180926190934.ea4xvzhkayuw7gkx@alap3.anarazel.de
      8b91d258
    • Tom Lane's avatar
      Fix link failures due to snprintf/strerror changes. · a6b88d68
      Tom Lane authored
      snprintf.c requires isnan(), which requires -lm on some platforms.
      libpq never bothered with -lm before, but now it needs it.
      
      strerror.c tries to translate a string or two, which requires -lintl.
      We'd managed never to need that anywhere in ecpg/pgtypeslib/ before,
      but now we do.
      
      Per buildfarm and a report from Peter Eisentraut.
      
      Discussion: https://postgr.es/m/20180926190934.ea4xvzhkayuw7gkx@alap3.anarazel.de
      Discussion: https://postgr.es/m/f67b5008-9f01-057f-2bff-558cb53af851@2ndquadrant.com
      a6b88d68
    • Peter Eisentraut's avatar
      Recurse to sequences on ownership change for all relkinds · 0320ddaf
      Peter Eisentraut authored
      When a table ownership is changed, we must apply that also to any owned
      sequences.  (Otherwise, it would result in a situation that cannot be
      restored, because linked sequences must have the same owner as the
      table.)  But this was previously only applied to regular tables and
      materialized views.  But it should also apply to at least foreign
      tables.  This patch removes the relkind check altogether, because it
      doesn't save very much and just introduces the possibility of similar
      omissions.
      
      Bug: #15238
      Reported-by: default avatarChristoph Berg <christoph.berg@credativ.de>
      0320ddaf
    • Tom Lane's avatar
      Implement %m in src/port/snprintf.c, and teach elog.c to rely on that. · d6c55de1
      Tom Lane authored
      I started out with the idea that we needed to detect use of %m format specs
      in contexts other than elog/ereport calls, because we couldn't rely on that
      working in *printf calls.  But a better answer is to fix things so that it
      does work.  Now that we're using snprintf.c all the time, we can implement
      %m in that and we've fixed the problem.
      
      This requires also adjusting our various printf-wrapping functions so that
      they ensure "errno" is preserved when they call snprintf.c.
      
      Remove elog.c's handmade implementation of %m, and let it rely on
      snprintf to support the feature.  That should provide some performance
      gain, though I've not attempted to measure it.
      
      There are a lot of places where we could now simplify 'printf("%s",
      strerror(errno))' into 'printf("%m")', but I'm not in any big hurry
      to make that happen.
      
      Patch by me, reviewed by Michael Paquier
      
      Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us
      d6c55de1
    • Tom Lane's avatar
      Always use our own versions of *printf(). · 96bf88d5
      Tom Lane authored
      We've spent an awful lot of effort over the years in coping with
      platform-specific vagaries of the *printf family of functions.  Let's just
      forget all that mess and standardize on always using src/port/snprintf.c.
      This gets rid of a lot of configure logic, and it will allow a saner
      approach to dealing with %m (though actually changing that is left for
      a follow-on patch).
      
      Preliminary performance testing suggests that as it stands, snprintf.c is
      faster than the native printf functions for some tasks on some platforms,
      and slower for other cases.  A pending patch will improve that, though
      cases with floating-point conversions will doubtless remain slower unless
      we want to put a *lot* of effort into that.  Still, we've not observed
      that *printf is really a performance bottleneck for most workloads, so
      I doubt this matters much.
      
      Patch by me, reviewed by Michael Paquier
      
      Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us
      96bf88d5
    • Tom Lane's avatar
      Incorporate strerror_r() into src/port/snprintf.c, too. · 758ce9b7
      Tom Lane authored
      This provides the features that used to exist in useful_strerror()
      for users of strerror_r(), too.  Also, standardize on the GNU convention
      that strerror_r returns a char pointer that may not be NULL.
      
      I notice that libpq's win32.c contains a variant version of strerror_r
      that probably ought to be folded into strerror.c.  But lacking a
      Windows environment, I should leave that to somebody else.
      
      Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us
      758ce9b7
    • Tom Lane's avatar
      Convert elog.c's useful_strerror() into a globally-used strerror wrapper. · 26e9d4d4
      Tom Lane authored
      elog.c has long had a private strerror wrapper that handles assorted
      possible failures or deficiencies of the platform's strerror.  On Windows,
      it also knows how to translate Winsock error codes, which the native
      strerror does not.  Move all this code into src/port/strerror.c and
      define strerror() as a macro that invokes it, so that both our frontend
      and backend code will have all of this behavior.
      
      I believe this constitutes an actual bug fix on Windows, since AFAICS
      our frontend code did not report Winsock error codes properly before this.
      However, the main point is to lay the groundwork for implementing %m
      in src/port/snprintf.c: the behavior we want %m to have is this one,
      not the native strerror's.
      
      Note that this throws away the prior use of src/port/strerror.c,
      which was to implement strerror() on platforms lacking it.  That's
      been dead code for nigh twenty years now, since strerror() was
      already required by C89.
      
      We should likewise cause strerror_r to use this behavior, but
      I'll tackle that separately.
      
      Patch by me, reviewed by Michael Paquier
      
      Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us
      26e9d4d4
    • Peter Eisentraut's avatar
      Update dummy CREATE ASSERTION grammar · a49ceda6
      Peter Eisentraut authored
      While we are probably still far away from fully implementing
      assertions, all patch proposals appear to take issue with the existing
      dummy grammar CREATE/DROP ASSERTION productions, so update those a
      little bit.  Rename the rule, use any_name instead of name, and remove
      some unused code.  Also remove the production for DROP ASSERTION,
      since that would most likely be handled via the generic DROP support.
      
      extracted from a patch by Joe Wildish
      a49ceda6
    • Tomas Vondra's avatar
      Improve test coverage of geometric types · a3d28448
      Tomas Vondra authored
      This commit significantly increases test coverage of geo_ops.c, adding
      tests for various issues addressed by 2e2a392d (which went undetected
      for a long time, at least partially due to not being covered).
      
      This also removes alternative results expecting -0 on some platforms.
      Instead the functions are should return the same results everywhere,
      transforming -0 to 0 if needed.
      
      The tests are added to geometric.sql file, sorted by the left hand side
      of the operators. There are many cross datatype operators, so this seems
      like the best solution.
      
      Author: Emre Hasegeli
      Reviewed-by: Tomas Vondra
      
      Discussion: https://www.postgresql.org/message-id/CAE2gYzxF7-5djV6-cEvqQu-fNsnt%3DEqbOURx7ZDg%2BVv6ZMTWbg%40mail.gmail.com
      a3d28448
    • Tomas Vondra's avatar
      Fix problems in handling the line data type · 2e2a392d
      Tomas Vondra authored
      According to the source history, the internal format of line data type
      has changed, but various functions working with it did were not updated
      and thus were producing wrong results.
      
      This patch addresses various such issues, in particular:
      
      * Reject invalid specification A=B=0 on receive
      * Reject same points on line_construct_pp()
      * Fix perpendicular operator when negative values are involved
      * Avoid division by zero on perpendicular operator
      * Fix intersection and distance operators when neither A nor B are 1
      * Return NULL for closest point when objects are parallel
      * Check whether closest point of line segments is the intersection point
      * Fix closest point of line segments being on the wrong segment
      
      Aside from handling those issues, the patch also aims to make operators
      more symmetric and less sen to precision loss.  The EPSILON interferes
      with even minor changes, but the least we can do is applying it to both
      sides of the operators equally.
      
      Author: Emre Hasegeli
      Reviewed-by: Tomas Vondra
      
      Discussion: https://www.postgresql.org/message-id/CAE2gYzxF7-5djV6-cEvqQu-fNsnt%3DEqbOURx7ZDg%2BVv6ZMTWbg%40mail.gmail.com
      2e2a392d
    • Michael Paquier's avatar
      Add basic regression tests for default monitoring roles · f535d5f0
      Michael Paquier authored
      The following default roles gain some coverage:
      - pg_read_all_stats
      - pg_read_all_settings
      
      Author: Alexandra Ryzhevich
      Discussion: https://postgr.es/m/CAOt4E5S5WJmDc9YpS1BfyAMQ5C1NEmiYynD6nUz42qVxphqkpA@mail.gmail.com
      f535d5f0
    • Michael Paquier's avatar
      Rework activation of commit timestamps during recovery · 8d28bf50
      Michael Paquier authored
      The activation and deactivation of commit timestamp tracking has not
      been handled consistently for a primary or standbys at recovery.  The
      facility can be activated at three different moments of recovery:
      - The beginning, where a primary would use the GUC value for the
      decision-making, and where a standby relies on the contents of the
      control file.
      - When replaying a XLOG_PARAMETER_CHANGE record at redo.
      - The end, where both primary and standby rely on the GUC value.
      
      Using the GUC value for a primary at the beginning of recovery causes
      problems with commit timestamp access when doing crash recovery.
      Particularly, when replaying transaction commits, it could be possible
      that an attempt to read commit timestamps is done for a transaction
      which committed at a moment when track_commit_timestamp was disabled.
      
      A test case is added to reproduce the failure.  The test works down to
      v11 as it takes advantage of transaction commits within procedures.
      
      Reported-by: Hailong Li
      Author: Masahiko Sawasa, Michael Paquier
      Reviewed-by: Kyotaro Horiguchi
      Discussion: https://postgr.es/m/11224478-a782-203b-1f17-e4797b39bdf0@qunar.com
      Backpatch-through: 9.5, where commit timestamps have been introduced.
      8d28bf50
  4. 25 Sep, 2018 11 commits
    • Andres Freund's avatar
      Remove absolete function TupleDescGetSlot(). · 10763358
      Andres Freund authored
      TupleDescGetSlot() was kept around for backward compatibility for
      user-written SRFs. With the TupleTableSlot abstraction work, that code
      will need to be version specific anyway, so there's no point in
      keeping the function around any longer.
      
      Author: Ashutosh Bapat
      Reviewed-By: Andres Freund
      Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
      10763358
    • Andres Freund's avatar
      Split ExecStoreTuple into ExecStoreHeapTuple and ExecStoreBufferHeapTuple. · 29c94e03
      Andres Freund authored
      Upcoming changes introduce further types of tuple table slots, in
      preparation of making table storage pluggable. New storage methods
      will have different representation of tuples, therefore the slot
      accessor should refer explicitly to heap tuples.
      
      Instead of just renaming the functions, split it into one function
      that accepts heap tuples not residing in buffers, and one accepting
      ones in buffers.  Previously one function was used for both, but that
      was a bit awkward already, and splitting will allow us to represent
      slot types for tuples in buffers and normal memory separately.
      
      This is split out from the patch introducing abstract slots, as this
      largely consists out of mechanical changes.
      
      Author: Ashutosh Bapat
      Reviewed-By: Andres Freund
      Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
      29c94e03
    • Andres Freund's avatar
      Remove function list from prologue of execTuples.c. · bbdfbb91
      Andres Freund authored
      That section is never in sync with the actual routines available and
      their functionality.
      
      Author: Ashutosh Bapat
      Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
      bbdfbb91
    • Andres Freund's avatar
      Change TupleTableSlot->tts_nvalid to type AttrNumber. · a598708f
      Andres Freund authored
      Previously it was an int / 4 bytes. The maximum number of attributes
      in a tuple is restricted by the maximum value Var->varattno, which is
      an AttrNumber/int16. Hence use the same data type for
      TupleTableSlot->tts_nvalid.
      
      Author: Ashutosh Bapat
      Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
      a598708f
    • Alvaro Herrera's avatar
      Remove obsolete comment · 5913b9bb
      Alvaro Herrera authored
      The documented shortcoming was actually fixed in 4c728f38
      so the comment is not true anymore.
      5913b9bb
    • Alvaro Herrera's avatar
      Remove fmgr.h inclusion from partition.h · 62e533d3
      Alvaro Herrera authored
      It's not needed anymore.
      62e533d3
    • Andres Freund's avatar
      Collect JIT instrumentation from workers. · 33001fd7
      Andres Freund authored
      Previously, when using parallel query, EXPLAIN (ANALYZE)'s JIT
      compilation timings did not include the overhead from doing so on the
      workers.  Fix that.
      
      We do so by simply aggregating the cost of doing JIT compilation on
      workers and the leader together. Arguably that's not quite accurate,
      because the total time spend doing so is spent in parallel - but it's
      hard to do much better.  For additional detail, when VERBOSE is
      specified, the stats for workers are displayed separately.
      
      Author: Amit Khandekar and Andres Freund
      Discussion: https://postgr.es/m/CAJ3gD9eLrz51RK_gTkod+71iDcjpB_N8eC6vU2AW-VicsAERpQ@mail.gmail.com
      Backpatch: 11-
      33001fd7
    • Tom Lane's avatar
      Make some fixes to allow building Postgres on macOS 10.14 ("Mojave"). · 5e221713
      Tom Lane authored
      Apple's latest rearrangements of the system-supplied headers have broken
      building of PL/Perl and PL/Tcl.  The only practical way to fix PL/Tcl is to
      start using the "-isysroot" compiler flag to point to SDK-supplied headers,
      as Apple expects.  We must also start distinguishing where to find Perl's
      headers from where to find its shared library; but that seems like good
      cleanup anyway.
      
      Extensions that formerly did something like -I$(perl_archlibexp)/CORE
      should now do -I$(perl_includedir)/CORE instead.  perl_archlibexp
      is still the place to look for libperl.so, though.
      
      If for some reason you don't like the default -isysroot setting, you can
      override that by setting PG_SYSROOT in configure's arguments.  I don't
      currently think people would need to do so, unless maybe for cross-version
      build purposes.
      
      In addition, teach configure where to find tclConfig.sh.  Our traditional
      method of searching $auto_path hasn't worked for the last couple of macOS
      releases, and it now seems clear that Apple's not going to change that.
      The workaround of manually specifying --with-tclconfig was annoying
      already, but Mojave's made it a lot more so because the sysroot path now
      has to be included as well.  Let's just wire the knowledge into configure
      instead.  To avoid breaking builds against non-default Tcl installations
      (e.g. MacPorts) wherein the $auto_path method probably still works,
      arrange to try the additional case only after all else has failed.
      
      Back-patch to all supported versions, since at least the buildfarm
      cares about that.  The changes are set up to not do anything on macOS
      releases that are old enough to not have functional sysroot trees.
      5e221713
    • Tom Lane's avatar
      Avoid unnecessary precision loss for pgbench's --rate target. · 5b7e0367
      Tom Lane authored
      It's fairly silly to truncate the throttle_delay to integer when the only
      math we ever do with it requires converting back to double.  Furthermore,
      given that people are starting to complain about restrictions like only
      supporting 1K client connections, I don't think we're very far away from
      situations where the precision loss matters.  As the code stood, for
      example, there's no difference between --rate 100001 and --rate 111111;
      both get converted to throttle_delay = 9.  Somebody trying to run 100
      threads and have each one dispatch around 1K TPS would find this lack of
      precision rather surprising, especially since the required per-thread
      delays are around 1ms, well within the timing precision of modern systems.
      5b7e0367
    • Thomas Munro's avatar
    • Michael Paquier's avatar
      Ignore publication tables when --no-publications is used · 08c9917e
      Michael Paquier authored
      96e1cb4c has added support for --no-publications in pg_dump, pg_dumpall
      and pg_restore, but forgot the fact that publication tables also need to
      be ignored when this option is used.
      
      Author: Gilles Darold
      Reviewed-by: Michael Paquier
      Discussion: https://postgr.es/m/3f48e812-b0fa-388e-2043-9a176bdee27e@dalibo.com
      Backpatch-through: 10, where publications have been added.
      08c9917e