1. 07 Oct, 2020 2 commits
    • 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
  2. 06 Oct, 2020 9 commits
  3. 05 Oct, 2020 8 commits
    • Bruce Momjian's avatar
      doc: show functions returning record types and use of ROWS FROM · dd0a64ed
      Bruce Momjian authored
      Previously it was unclear exactly how ROWS FROM behaved and how to cast
      the data types of columns returned by FROM functions.  Also document
      that only non-OUT record functions can have their columns cast to data
      types.
      
      Reported-by: guyren@gmail.com
      
      Discussion: https://postgr.es/m/158638264419.662.2482095087061084020@wrigleys.postgresql.org
      
      Backpatch-through: 9.5
      dd0a64ed
    • Bruce Momjian's avatar
      Overhaul pg_hba.conf clientcert's API · 253f1025
      Bruce Momjian authored
      Since PG 12, clientcert no longer supported only on/off, so remove 1/0
      as possible values, and instead support only the text strings
      'verify-ca' and 'verify-full'.
      
      Remove support for 'no-verify' since that is possible by just not
      specifying clientcert.
      
      Also, throw an error if 'verify-ca' is used and 'cert' authentication is
      used, since cert authentication requires verify-full.
      
      Also improve the docs.
      
      THIS IS A BACKWARD INCOMPATIBLE API CHANGE.
      
      Reported-by: Kyotaro Horiguchi
      
      Discussion: https://postgr.es/m/20200716.093012.1627751694396009053.horikyota.ntt@gmail.com
      
      Author: Kyotaro Horiguchi
      
      Backpatch-through: master
      253f1025
    • Tom Lane's avatar
      Include the process PID in assertion-failure messages. · 18c170a0
      Tom Lane authored
      This should help to identify what happened when studying the postmaster
      log after-the-fact.
      
      While here, clean up some old comments in the same function.
      
      Discussion: https://postgr.es/m/1568983.1601845687@sss.pgh.pa.us
      18c170a0
    • Tom Lane's avatar
      Fix two latent(?) bugs in equivclass.c. · 53c6daff
      Tom Lane authored
      get_eclass_for_sort_expr() computes expr_relids and nullable_relids
      early on, even though they won't be needed unless we make a new
      EquivalenceClass, which we often don't.  Aside from the probably-minor
      inefficiency, there's a memory management problem: these bitmapsets will
      be built in the caller's context, leading to dangling pointers if that
      is shorter-lived than root->planner_cxt.  This would be a live bug if
      get_eclass_for_sort_expr() could be called with create_it = true during
      GEQO join planning.  So far as I can find, the core code never does
      that, but it's hard to be sure that no extensions do, especially since
      the comments make it clear that that's supposed to be a supported case.
      Fix by not computing these values until we've switched into planner_cxt
      to build the new EquivalenceClass.
      
      generate_join_implied_equalities() uses inner_rel->relids to look up
      relevant eclasses, but it ought to be using nominal_inner_relids.
      This is presently harmless because a child RelOptInfo will always have
      exactly the same eclass_indexes as its topmost parent; but that might
      not be true forever, and anyway it makes the code confusing.
      
      The first of these is old (introduced by me in f3b3b8d5), so back-patch
      to all supported branches.  The second only dates to v13, but we might
      as well back-patch it to keep the code looking similar across branches.
      
      Discussion: https://postgr.es/m/1508010.1601832581@sss.pgh.pa.us
      53c6daff
    • Tom Lane's avatar
      Doc: fix parameter names in the docs of a couple of functions. · 9cc3d614
      Tom Lane authored
      The descriptions of make_interval() and pg_options_to_table()
      were randomly different from the reality embedded in pg_proc.
      
      (These are not all the discrepancies I found in a quick search,
      but the others perhaps require more discussion, since there's
      at least a case to be made for changing pg_proc not the docs.)
      
      make_interval issue noted by Thomas Kellerer.
      
      Discussion: https://postgr.es/m/7b154ef0-9f22-90b9-7734-4bf23686695b@gmx.net
      9cc3d614
    • Peter Eisentraut's avatar
      Support for OUT parameters in procedures · 2453ea14
      Peter Eisentraut authored
      Unlike for functions, OUT parameters for procedures are part of the
      signature.  Therefore, they have to be listed in pg_proc.proargtypes
      as well as mentioned in ALTER PROCEDURE and DROP PROCEDURE.
      Reviewed-by: default avatarAndrew Dunstan <andrew.dunstan@2ndquadrant.com>
      Reviewed-by: default avatarPavel Stehule <pavel.stehule@gmail.com>
      Discussion: https://www.postgresql.org/message-id/flat/2b8490fe-51af-e671-c504-47359dc453c5@2ndquadrant.com
      2453ea14
    • Tom Lane's avatar
      Improve stability of identity.sql regression test. · e8997420
      Tom Lane authored
      I noticed while trying to run the regression tests under a low
      geqo_threshold that one query on information_schema.columns had
      unstable (as in, variable from one run to the next) output order.
      This is pretty unsurprising given the complexity of the underlying
      plan.  Interestingly, of this test's three nigh-identical queries on
      information_schema.columns, the other two already had ORDER BY clauses
      guaranteeing stable output.  Let's make this one look the same.
      
      Back-patch to v10 where this test was added.  We've not heard field
      reports of the test failing, but this experience shows that it can
      happen when testing under even slightly unusual conditions.
      e8997420
    • Michael Paquier's avatar
      Fix handling of redundant options with COPY for "freeze" and "header" · 10c5291c
      Michael Paquier authored
      The handling of those options was inconsistent, as the processing used
      directly the value assigned to the option to check if it was redundant,
      leading to patterns like this one to succeed (note that false is
      specified first):
      COPY hoge to '/path/to/file/' (header off, header on);
      
      And the opposite would fail correctly (note that true is first here):
      COPY hoge to '/path/to/file/' (header on, header off);
      
      While on it, add some tests to check for all redundant patterns with the
      options of COPY.  I have gone through the code and did not notice
      similar mistakes for other commands.
      
      "header" got it wrong since b63990c6, and "freeze" was wrong from the
      start as of 8de72b66.  No backpatch is done per the lack of complaints.
      
      Reported-by: Rémi Lapeyre
      Discussion: https://postgr.es/m/20200929072433.GA15570@paquier.xyz
      Discussion: https://postgr.es/m/0B55BD07-83E4-439F-AACC-FA2D7CF50532@lenstra.fr
      10c5291c
  4. 04 Oct, 2020 1 commit
    • Tom Lane's avatar
      Make postgres.bki use the same literal-string syntax as postgresql.conf. · 97b61448
      Tom Lane authored
      The BKI file's string quoting conventions were previously quite weird,
      perhaps as a result of repurposing a function built to scan
      single-quoted strings to scan double-quoted ones.  Change to use the
      same rules as we use in GUC files, allowing some simplifications in
      genbki.pl and initdb.c.
      
      While at it, completely remove the backend's scanstr() function, which
      was essentially a duplicate of the string dequoting code in guc-file.l.
      Instead export that one (under a less generic name than it had) and let
      bootscanner.l use it.  Now we can clarify that scansup.c exists only to
      support the main lexer. We could alternatively have removed GUC_scanstr,
      but this way seems better since the previous arrangement could mislead
      a reader into thinking that scanstr() had something to do with the main
      lexer's handling of string literals.  Maybe it did once, but if so it
      was a long time ago.
      
      This patch does not bump catversion, since the initially-installed
      catalog contents don't change.  Note however that successful initdb
      after applying this patch will require up-to-date postgres.bki as well
      as postgres and initdb executables.
      
      In passing, remove a bunch of very-long-obsolete #include's in
      bootparse.y and bootscanner.l.
      
      John Naylor
      
      Discussion: https://postgr.es/m/CACPNZCtDpd18T0KATTmCggO2GdVC4ow86ypiq5ENff1VnauL8g@mail.gmail.com
      97b61448
  5. 03 Oct, 2020 3 commits
  6. 02 Oct, 2020 4 commits
  7. 01 Oct, 2020 5 commits
  8. 30 Sep, 2020 6 commits
    • Alvaro Herrera's avatar
      Reword partitioning error message · 9fc21227
      Alvaro Herrera authored
      The error message about columns in the primary key not including all of
      the partition key was unclear; reword it.
      
      Backpatch all the way to pg11, where it appeared.
      Reported-by: default avatarNagaraj Raj <nagaraj.sf@yahoo.com>
      Discussion: https://postgr.es/m/64062533.78364.1601415362244@mail.yahoo.com
      9fc21227
    • Tom Lane's avatar
      Fix handling of BC years in to_date/to_timestamp. · 489c9c34
      Tom Lane authored
      Previously, a conversion such as
      	to_date('-44-02-01','YYYY-MM-DD')
      would result in '0045-02-01 BC', as the code attempted to interpret
      the negative year as BC, but failed to apply the correction needed
      for our internal handling of BC years.  Fix the off-by-one problem.
      
      Also, arrange for the combination of a negative year and an
      explicit "BC" marker to cancel out and produce AD.  This is how
      the negative-century case works, so it seems sane to do likewise.
      
      Continue to read "year 0000" as 1 BC.  Oracle would throw an error,
      but we've accepted that case for a long time so I'm hesitant to
      change it in a back-patch.
      
      Per bug #16419 from Saeed Hubaishan.  Back-patch to all supported
      branches.
      
      Dar Alathar-Yemen and Tom Lane
      
      Discussion: https://postgr.es/m/16419-d8d9db0a7553f01b@postgresql.org
      489c9c34
    • Heikki Linnakangas's avatar
    • Peter Eisentraut's avatar
      Fix XML id to match GUC name · 300b6984
      Peter Eisentraut authored
      For some reason, the id of the description of
      max_parallel_maintenance_workers has been
      guc-max-parallel-workers-maintenance since the beginning.  Flip that
      around to make it consistent.
      300b6984
    • Tom Lane's avatar
      Remove obsolete replication settings within TAP tests. · 151c0c5f
      Tom Lane authored
      PostgresNode.pm set "max_wal_senders = 5" for replication testing,
      but this seems to be slightly too low for our current test suite.
      Slower buildfarm members frequently report "number of requested standby
      connections exceeds max_wal_senders" failures, due to old walsenders
      not exiting instantaneously.  Usually, the test does not fail overall
      because of automatic walreceiver restart, but sometimes the failure
      becomes visible; and in any case such retries slow down the test.
      
      That value came in with commit 89ac7004, but was soon obsoleted by
      f6d6d292, which raised the built-in default from zero to 10; so that
      PostgresNode.pm is actually setting it to less than the conservative
      built-in default.  That seems pretty pointless, so let's remove the
      special setting and let the default prevail, in hopes of making
      the TAP tests more robust.
      
      Likewise, the setting "max_replication_slots = 5" is obsolete and
      can be removed.
      
      While here, reverse-engineer a comment about why we're choosing
      less-than-default values for some other settings.
      
      (Note: before v12, max_wal_senders counted against max_connections
      so that the latter setting also needs some fiddling with.)
      
      Back-patch to v10 where the subscription tests were added.
      It's likely that the older branches aren't pushing the boundaries
      of max_wal_senders, but I'm disinclined to spend time trying to
      figure out exactly when it started to be a problem.
      
      Discussion: https://postgr.es/m/723911.1601417626@sss.pgh.pa.us
      151c0c5f
    • David Rowley's avatar
      Doc: Improve clarity on partitioned table limitations · 2b888647
      David Rowley authored
      Explicitly mention that primary key constraints are also included in the
      limitation that the constraint columns must be a superset of the partition key
      columns.
      
      Wording suggestion from Tom Lane.
      
      Discussion: https://postgr.es/m/64062533.78364.1601415362244@mail.yahoo.com
      Backpatch-through: 11, where unique constraints on partitioned tables were added
      2b888647
  9. 29 Sep, 2020 2 commits
    • Tom Lane's avatar
      Fix make_timestamp[tz] to accept negative years as meaning BC. · a094c8ff
      Tom Lane authored
      Previously we threw an error.  But make_date already allowed the case,
      so it is inconsistent as well as unhelpful for make_timestamp not to.
      
      Both functions continue to reject year zero.
      
      Code and test fixes by Peter Eisentraut, doc changes by me
      
      Discussion: https://postgr.es/m/13c0992c-f15a-a0ca-d839-91d3efd965d9@2ndquadrant.com
      a094c8ff
    • Tom Lane's avatar
      Fix memory leak in plpgsql's CALL processing. · a6b1f536
      Tom Lane authored
      When executing a CALL or DO in a non-atomic context (i.e., not inside
      a function or query), plpgsql creates a new plan each time through,
      as a rather hacky solution to some resource management issues.  But
      it failed to free this plan until exit of the current procedure or DO
      block, resulting in serious memory bloat in procedures that called
      other procedures many times.  Fix by remembering to free the plan,
      and by being more honest about restoring the previous state (otherwise,
      recursive procedure calls have a problem).
      
      There was also a smaller leak associated with recalculation of the
      "target" list of output variables.  Fix that by using the statement-
      lifespan context to hold non-permanent values.
      
      Back-patch to v11 where procedures were introduced.
      
      Pavel Stehule and Tom Lane
      
      Discussion: https://postgr.es/m/CAFj8pRDiiU1dqym+_P4_GuTWm76knJu7z9opWayBJTC0nQGUUA@mail.gmail.com
      a6b1f536