1. 20 Sep, 2022 4 commits
    • Tom Lane's avatar
      Disable -Wdeprecated-non-prototype in the back branches. · dcd7dbed
      Tom Lane authored
      There doesn't seem to be any good ABI-preserving way to silence
      clang 15's -Wdeprecated-non-prototype warnings about our tree-walk
      APIs.  While we've fixed it properly in HEAD, the only way to not
      see hundreds of these in the back branches is to disable the
      warnings.  We're not going to do anything about them, so we might
      as well disable them.
      
      I noticed that we also get some of these warnings about fmgr.c's
      support for V0 function call convention, in branches before v10
      where we removed that.  That's another area we aren't going to
      change, so turning off the warning seems fine for that too.
      
      Per project policy, this is a candidate for back-patching into
      out-of-support branches: it suppresses annoying compiler warnings
      but changes no behavior.  Hence, back-patch all the way to 9.2.
      
      Discussion: https://postgr.es/m/CA+hUKGKpHPDTv67Y+s6yiC8KH5OXeDg6a-twWo_xznKTcG0kSA@mail.gmail.com
      dcd7dbed
    • Tom Lane's avatar
      Suppress variable-set-but-not-used warnings from clang 15. · 2e124d85
      Tom Lane authored
      clang 15+ will issue a set-but-not-used warning when the only
      use of a variable is in autoincrements (e.g., "foo++;").
      That's perfectly sensible, but it detects a few more cases that
      we'd not noticed before.  Silence the warnings with our usual
      methods, such as PG_USED_FOR_ASSERTS_ONLY, or in one case by
      actually removing a useless variable.
      
      One thing that we can't nicely get rid of is that with %pure-parser,
      Bison emits "yynerrs" as a local variable that falls foul of this
      warning.  To silence those, I inserted "(void) yynerrs;" in the
      top-level productions of affected grammars.
      
      Per recently-established project policy, this is a candidate
      for back-patching into out-of-support branches: it suppresses
      annoying compiler warnings but changes no behavior.  Hence,
      back-patch to 9.5, which is as far as these patches go without
      issues.  (A preliminary check shows that the prior branches
      need some other set-but-not-used cleanups too, so I'll leave
      them for another day.)
      
      Discussion: https://postgr.es/m/514615.1663615243@sss.pgh.pa.us
      2e124d85
    • Michael Paquier's avatar
      doc: Fix parameter name for pg_create_logical_replication_slot() · 382cc680
      Michael Paquier authored
      The parameter controlling if two-phase transactions can be decoded was
      named "two_phase" in the documentation while its procedure defines
      "twophase".
      
      Author: Florin Irion
      Discussion: https://postgr.es/m/5eeabd10-1aff-ea61-f92d-9fa0d9a7e207@gmail.com
      Backpatch-through: 14
      382cc680
    • Michael Paquier's avatar
      Fix incorrect variable types for origin IDs in decode.c · e68fc64f
      Michael Paquier authored
      These variables used XLogRecPtr instead of RepOriginId.
      
      Author: Masahiko Sawada
      Discussion: https://postgr.es/m/CAD21AoBm-vNyBSXGp4bmJGvhr=S-EGc5q1dtV70cFTcJvLhC=Q@mail.gmail.com
      Backpatch-through: 14
      e68fc64f
  2. 19 Sep, 2022 1 commit
    • Tom Lane's avatar
      Future-proof the recursion inside ExecShutdownNode(). · 7394c763
      Tom Lane authored
      The API contract for planstate_tree_walker() callbacks is that they
      take a PlanState pointer and a context pointer.  Somebody figured
      they could save a couple lines of code by ignoring that, and passing
      ExecShutdownNode itself as the walker even though it has but one
      argument.  Somewhat remarkably, we've gotten away with that so far.
      However, it seems clear that the upcoming C2x standard means to
      forbid such cases, and compilers that actively break such code
      likely won't be far behind.  So spend the extra few lines of code
      to do it honestly with a separate walker function.
      
      In HEAD, we might as well go further and remove ExecShutdownNode's
      useless return value.  I left that as-is in back branches though,
      to forestall complaints about ABI breakage.
      
      Back-patch, with the thought that this might become of practical
      importance before our stable branches are all out of service.
      It doesn't seem to be fixing any live bug on any currently known
      platform, however.
      
      Discussion: https://postgr.es/m/208054.1663534665@sss.pgh.pa.us
      7394c763
  3. 17 Sep, 2022 2 commits
  4. 16 Sep, 2022 1 commit
    • Tom Lane's avatar
      Improve plpgsql's ability to handle arguments declared as RECORD. · 56d45fda
      Tom Lane authored
      Treat arguments declared as RECORD as if that were a polymorphic type
      (which it is, sort of), in that we substitute the actual argument type
      while forming the function cache lookup key.  This allows the specific
      composite type to be known in some cases where it was not before,
      at the cost of making a separate function cache entry for each named
      composite type that's passed to the function during a session.  The
      particular symptom discussed in bug #17610 could be solved in other
      more-efficient ways, but only at the cost of considerable development
      work, and there are other cases where we'd still fail without this.
      
      Per bug #17610 from Martin Jurča.  Back-patch to v11 where we first
      allowed plpgsql functions to be declared as taking type RECORD.
      
      Discussion: https://postgr.es/m/17610-fb1eef75bf6c2364@postgresql.org
      56d45fda
  5. 15 Sep, 2022 1 commit
    • Tom Lane's avatar
      Detect format-string mistakes in the libpq_pipeline test module. · bff7bc6c
      Tom Lane authored
      I happened to notice that libpq_pipeline's private implementation
      of pg_fatal lacked any pg_attribute_printf decoration.  Indeed,
      adding that turned up a mistake!  We'd likely never have noticed
      because the error exits in this code are unlikely to get hit,
      but still, it's a bug.
      
      We're so used to having the compiler check this stuff for us that
      a printf-like function without pg_attribute_printf is a land mine.
      I wonder if there is a way to detect such omissions.
      
      Back-patch to v14 where this code came in.
      bff7bc6c
  6. 14 Sep, 2022 3 commits
    • Etsuro Fujita's avatar
      postgres_fdw: Avoid 'variable not found in subplan target list' error. · b53d104a
      Etsuro Fujita authored
      The tlist of the EvalPlanQual outer plan for a ForeignScan node is
      adjusted to produce a tuple whose descriptor matches the scan tuple slot
      for the ForeignScan node.  But in the case where the outer plan contains
      an extra Sort node, if the new tlist contained columns required only for
      evaluating PlaceHolderVars or columns required only for evaluating local
      conditions, this would cause setrefs.c to fail with the error.
      
      The cause of this is that when creating the outer plan by injecting the
      Sort node into an alternative local join plan that could emit such extra
      columns as well, we fail to arrange for the outer plan to propagate them
      up through the Sort node, causing setrefs.c to fail to match up them in
      the new tlist to what is available from the outer plan.  Repair.
      
      Per report from Alexander Pyhalov.
      
      Richard Guo and Etsuro Fujita, reviewed by Alexander Pyhalov and Tom Lane.
      Backpatch to all supported versions.
      
      Discussion: http://postgr.es/m/cfb17bf6dfdf876467bd5ef533852d18%40postgrespro.ru
      b53d104a
    • Michael Paquier's avatar
      Fix incorrect value for "strategy" with deflateParams() in walmethods.c · 4b529f46
      Michael Paquier authored
      The zlib documentation mentions the values supported for the compression
      strategy, but this code has been using a hardcoded value of 0 rather
      than Z_DEFAULT_STRATEGY.  This commit adjusts the code to use
      Z_DEFAULT_STRATEGY.
      
      Backpatch down to where this code has been added to ease the backport of
      any future patch touching this area.
      
      Reported-by: Tom Lane
      Discussion: https://postgr.es/m/1400032.1662217889@sss.pgh.pa.us
      Backpatch-through: 10
      4b529f46
    • Peter Eisentraut's avatar
      Expand palloc/pg_malloc API for more type safety · b7f37af7
      Peter Eisentraut authored
      This adds additional variants of palloc, pg_malloc, etc. that
      encapsulate common usage patterns and provide more type safety.
      
      Specifically, this adds palloc_object(), palloc_array(), and
      repalloc_array(), which take the type name of the object to be
      allocated as its first argument and cast the return as a pointer to
      that type.  There are also palloc0_object() and palloc0_array()
      variants for initializing with zero, and pg_malloc_*() variants of all
      of the above.
      
      Inspired by the talloc library.
      
      This is backpatched from master so that future backpatchable code can
      make use of these APIs.  This patch by itself does not contain any
      users of these APIs.
      Reviewed-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
      Discussion: https://www.postgresql.org/message-id/flat/bb755632-2a43-d523-36f8-a1e7a389a907@enterprisedb.com
      b7f37af7
  7. 12 Sep, 2022 3 commits
  8. 09 Sep, 2022 2 commits
    • Tom Lane's avatar
      Fix possible omission of variable storage markers in ECPG. · be0b0528
      Tom Lane authored
      The ECPG preprocessor converted code such as
      
      static varchar str1[10], str2[20], str3[30];
      
      into
      
      static  struct varchar_1  { int len; char arr[ 10 ]; }  str1 ;
              struct varchar_2  { int len; char arr[ 20 ]; }  str2 ;
              struct varchar_3  { int len; char arr[ 30 ]; }  str3 ;
      
      thus losing the storage attribute for the later variables.
      Repeat the declaration for each such variable.
      
      (Note that this occurred only for variables declared "varchar"
      or "bytea", which may help explain how it escaped detection
      for so long.)
      
      Andrey Sokolov
      
      Discussion: https://postgr.es/m/942241662288242@mail.yandex.ru
      be0b0528
    • Tom Lane's avatar
      Reject bogus output from uuid_create(3). · e55ccb3b
      Tom Lane authored
      When using the BSD UUID functions, contrib/uuid-ossp expects
      uuid_create() to produce a version-1 UUID.  FreeBSD still does so,
      but in recent NetBSD releases that function produces a version-4
      (random) UUID instead.  That's not acceptable for our purposes:
      if the user wanted v4 she would have asked for v4, not v1.
      Hence, check the version digit and complain if it's not '1'.
      
      Also drop the documentation's claim that the NetBSD implementation
      is usable.  It might be, depending on which OS version you're using,
      but we're not going to get into that kind of detail.
      
      (Maybe someday we should ditch all these external libraries
      and just write our own UUID code, but today is not that day.)
      
      Nazir Bilal Yavuz, with cosmetic adjustments and docs by me.
      Backpatch to all supported versions.
      
      Discussion: https://postgr.es/m/3848059.1661038772@sss.pgh.pa.us
      Discussion: https://postgr.es/m/17358-89806e7420797025@postgresql.org
      e55ccb3b
  9. 08 Sep, 2022 1 commit
    • Alvaro Herrera's avatar
      Choose FK name correctly during partition attachment · 640c20d6
      Alvaro Herrera authored
      During ALTER TABLE ATTACH PARTITION, if the name of a parent's foreign
      key constraint is already used on the partition, the code tries to
      choose another one before the FK attributes list has been populated,
      so the resulting constraint name was "<relname>__fkey" instead of
      "<relname>_<attrs>_fkey".  Repair, and add a test case.
      
      Backpatch to 12.  In 11, the code to attach a partition was not smart
      enough to cope with conflicting constraint names, so the problem doesn't
      exist there.
      
      Author: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
      Discussion: https://postgr.es/m/20220901184156.738ebee5@karst
      640c20d6
  10. 05 Sep, 2022 1 commit
  11. 03 Sep, 2022 3 commits
  12. 02 Sep, 2022 1 commit
  13. 01 Sep, 2022 8 commits
  14. 31 Aug, 2022 5 commits
  15. 30 Aug, 2022 1 commit
    • Tom Lane's avatar
      On NetBSD, force dynamic symbol resolution at postmaster start. · 464db467
      Tom Lane authored
      The default of lazy symbol resolution means that when the postmaster
      first reaches the select() call in ServerLoop, it'll need to resolve
      the link to that libc entry point.  NetBSD's dynamic loader takes
      an internal lock while doing that, and if a signal interrupts the
      operation then there is a risk of self-deadlock should the signal
      handler do anything that requires that lock, as several of the
      postmaster signal handlers do.  The window for this is pretty narrow,
      and timing considerations make it unlikely that a signal would arrive
      right then anyway.  But it's semi-repeatable on slow single-CPU
      machines, and in principle the race could happen with any hardware.
      
      The least messy solution to this is to force binding of dynamic
      symbols at postmaster start, using the "-z now" linker option.
      While we're at it, also use "-z relro" so as to provide a small
      security gain.
      
      It's not entirely clear whether any other platforms share this
      issue, but for now we'll assume it's NetBSD-specific.  (We might
      later try to use "-z now" on more platforms for performance
      reasons, but that would not likely be something to back-patch.)
      
      Report and patch by me; the idea to fix it this way is from
      Andres Freund.
      
      Discussion: https://postgr.es/m/3384826.1661802235@sss.pgh.pa.us
      464db467
  16. 29 Aug, 2022 1 commit
    • Robert Haas's avatar
      Prevent WAL corruption after a standby promotion. · 0e54a5e2
      Robert Haas authored
      When a PostgreSQL instance performing archive recovery but not using
      standby mode is promoted, and the last WAL segment that it attempted
      to read ended in a partial record, the previous code would create
      invalid WAL on the new timeline. The WAL from the previously timeline
      would be copied to the new timeline up until the end of the last valid
      record, but instead of beginning to write WAL at immediately
      afterwards, the promoted server would write an overwrite contrecord at
      the beginning of the next segment. The end of the previous segment
      would be left as all-zeroes, resulting in failures if anything tried
      to read WAL from that file.
      
      The root of the issue is that ReadRecord() decides whether to set
      abortedRecPtr and missingContrecPtr based on the value of StandbyMode,
      but ReadRecord() switches to a new timeline based on the value of
      ArchiveRecoveryRequested. We shouldn't try to write an overwrite
      contrecord if we're switching to a new timeline, so change the test in
      ReadRecod() to check ArchiveRecoveryRequested instead.
      
      Code fix by Dilip Kumar. Comments by me incorporating suggested
      language from Álvaro Herrera. Further review from Kyotaro Horiguchi
      and Sami Imseih.
      
      Discussion: http://postgr.es/m/CAFiTN-t7umki=PK8dT1tcPV=mOUe2vNhHML6b3T7W7qqvvajjg@mail.gmail.com
      Discussion: http://postgr.es/m/FB0DEA0B-E14E-43A0-811F-C1AE93D00FF3%40amazon.com
      0e54a5e2
  17. 28 Aug, 2022 1 commit
  18. 27 Aug, 2022 1 commit