1. 22 Oct, 2020 3 commits
  2. 21 Oct, 2020 6 commits
    • Tom Lane's avatar
      Fix connection string handling in psql's \connect command. · 85c54287
      Tom Lane authored
      psql's \connect claims to be able to re-use previous connection
      parameters, but in fact it only re-uses the database name, user name,
      host name (and possibly hostaddr, depending on version), and port.
      This is problematic for assorted use cases.  Notably, pg_dump[all]
      emits "\connect databasename" commands which we would like to have
      re-use all other parameters.  If such a script is loaded in a psql run
      that initially had "-d connstring" with some non-default parameters,
      those other parameters would be lost, potentially causing connection
      failure.  (Thus, this is the same kind of bug addressed in commits
      a45bc8a4 and 8e5793ab, although the details are much different.)
      
      To fix, redesign do_connect() so that it pulls out all properties
      of the old PGconn using PQconninfo(), and then replaces individual
      properties in that array.  In the case where we don't wish to re-use
      anything, get libpq's default settings using PQconndefaults() and
      replace entries in that, so that we don't need different code paths
      for the two cases.
      
      This does result in an additional behavioral change for cases where
      the original connection parameters allowed multiple hosts, say
      "psql -h host1,host2", and the \connect request allows re-use of the
      host setting.  Because the previous coding relied on PQhost(), it
      would only permit reconnection to the same host originally selected.
      Although one can think of scenarios where that's a good thing, there
      are others where it is not.  Moreover, that behavior doesn't seem to
      meet the principle of least surprise, nor was it documented; nor is
      it even clear it was intended, since that coding long pre-dates the
      addition of multi-host support to libpq.  Hence, this patch is content
      to drop it and re-use the host list as given.
      
      Per Peter Eisentraut's comments on bug #16604.  Back-patch to all
      supported branches.
      
      Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
      85c54287
    • Alvaro Herrera's avatar
      Use fast checkpoint in PostgresNode::backup() · 831611b1
      Alvaro Herrera authored
      Should cause tests to be a bit faster
      831611b1
    • Tom Lane's avatar
      Remove the option to build thread_test.c outside configure. · 8a212118
      Tom Lane authored
      Theoretically one could go into src/test/thread and build/run this
      program there.  In practice, that hasn't worked since 96bf88d5,
      and probably much longer on some platforms (likely including just
      the sort of hoary leftovers where this test might be of interest).
      While it wouldn't be too hard to repair the breakage, the fact that
      nobody has noticed for two years shows that there is zero usefulness
      in maintaining this build pathway.  Let's get rid of it and decree
      that thread_test.c is *only* meant to be built/used in configure.
      
      Given that decision, it makes sense to put thread_test.c under config/
      and get rid of src/test/thread altogether, so that's what I did.
      
      In passing, update src/test/README, which had been ignored by some
      not-so-recent additions of subdirectories.
      
      Discussion: https://postgr.es/m/227659.1603041612@sss.pgh.pa.us
      8a212118
    • Peter Eisentraut's avatar
      Remove obsolete ifdefs · 555eb1a4
      Peter Eisentraut authored
      Commit 8dace66e added #ifdefs for a
      number of errno symbols because they were not present on Windows.
      Later, commit 125ad539 added
      replacement #defines for some of those symbols.  So some of the
      changes from the first commit are made dead code by the second commit
      and can now be removed.
      
      Discussion: https://www.postgresql.org/message-id/flat/6dee8574-b0ad-fc49-9c8c-2edc796f0033@2ndquadrant.com
      555eb1a4
    • Peter Eisentraut's avatar
      Fix -Wcast-function-type warnings on Windows/MinGW · 8a58347a
      Peter Eisentraut authored
      After de8feb1f, some warnings remained
      that were only visible when using GCC on Windows.  Fix those as well.
      
      Note that the ecpg test source files don't use the full pg_config.h,
      so we can't use pg_funcptr_t there but have to do it the long way.
      8a58347a
    • Michael Paquier's avatar
      Review format of code generated by PerfectHash.pm · 19ae53c9
      Michael Paquier authored
      80f8eb79 has added to the normalization quick check headers some code
      generated by PerfectHash.pm that is incompatible with the settings of
      gitattributes for this repository, as whitespaces followed a set of tabs
      for the first element of a line in the table.  Instead of adding a new
      exception to gitattributes, rework the format generated so as a right
      padding with spaces is used instead of a left padding.  This keeps the
      table generated in a readable shape with its set of columns, making
      unnecessary an update of gitattributes.
      
      Reported-by: Peter Eisentraut
      Author: John Naylor
      Discussion: https://postgr.es/m/d601b3b5-a3c7-5457-2f84-3d6513d690fc@2ndquadrant.com
      19ae53c9
  3. 20 Oct, 2020 2 commits
  4. 19 Oct, 2020 14 commits
    • Tom Lane's avatar
      Fix connection string handling in src/bin/scripts/ programs. · 8e5793ab
      Tom Lane authored
      When told to process all databases, clusterdb, reindexdb, and vacuumdb
      would reconnect by replacing their --maintenance-db parameter with the
      name of the target database.  If that parameter is a connstring (which
      has been allowed for a long time, though we failed to document that
      before this patch), we'd lose any other options it might specify, for
      example SSL or GSS parameters, possibly resulting in failure to connect.
      Thus, this is the same bug as commit a45bc8a4 fixed in pg_dump and
      pg_restore.  We can fix it in the same way, by using libpq's rules for
      handling multiple "dbname" parameters to add the target database name
      separately.  I chose to apply the same refactoring approach as in that
      patch, with a struct to handle the command line parameters that need to
      be passed through to connectDatabase.  (Maybe someday we can unify the
      very similar functions here and in pg_dump/pg_restore.)
      
      Per Peter Eisentraut's comments on bug #16604.  Back-patch to all
      supported branches.
      
      Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
      8e5793ab
    • Tom Lane's avatar
      Fix list-munging bug that broke SQL function result coercions. · c8ab9701
      Tom Lane authored
      Since commit 913bbd88, check_sql_fn_retval() can either insert type
      coercion steps in-line in the Query that produces the SQL function's
      results, or generate a new top-level Query to perform the coercions,
      if modifying the Query's output in-place wouldn't be safe.  However,
      it appears that the latter case has never actually worked, because
      the code tried to inject the new Query back into the query list it was
      passed ... which is not the list that will be used for later processing
      when we execute the SQL function "normally" (without inlining it).
      So we ended up with no coercion happening at run-time, leading to
      wrong results or crashes depending on the datatypes involved.
      
      While the regression tests look like they cover this area well enough,
      through a huge bit of bad luck all the test cases that exercise the
      separate-Query path were checking either inline-able cases (which
      accidentally didn't have the bug) or cases that are no-ops at runtime
      (e.g., varchar to text), so that the failure to perform the coercion
      wasn't obvious.  The fact that the cases that don't work weren't
      allowed at all before v13 probably contributed to not noticing the
      problem sooner, too.
      
      To fix, get rid of the separate "flat" list of Query nodes and instead
      pass the real two-level list that is going to be used later.  I chose
      to make the same change in check_sql_fn_statements(), although that has
      no actual bug, just so that we don't need that data structure at all.
      
      This is an API change, as evidenced by the adjustments needed to
      callers outside functions.c.  That's a bit scary to be doing in a
      released branch, but so far as I can tell from a quick search,
      there are no outside callers of these functions (and they are
      sufficiently specific to our semantics for SQL-language functions that
      it's not apparent why any extension would need to call them).  In any
      case, v13 already changed the API of check_sql_fn_retval() compared to
      prior branches.
      
      Per report from pinker.  Back-patch to v13 where this code came in.
      
      Discussion: https://postgr.es/m/1603050466566-0.post@n3.nabble.com
      c8ab9701
    • Heikki Linnakangas's avatar
      Misc documentation fixes. · c5f42daa
      Heikki Linnakangas authored
      - Misc grammar and punctuation fixes.
      
      - Stylistic cleanup: use spaces between function arguments and JSON fields
        in examples. For example "foo(a,b)" -> "foo(a, b)". Add semicolon after
        last END in a few PL/pgSQL examples that were missing them.
      
      - Make sentence that talked about "..." and ".." operators more clear,
        by avoiding to end the sentence with "..". That makes it look the same
        as "..."
      
      - Fix syntax description for HAVING: HAVING conditions cannot be repeated
      
      Patch by Justin Pryzby, per Yaroslav Schekin's report. Backpatch to all
      supported versions, to the extent that the patch applies easily.
      
      Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com
      c5f42daa
    • Heikki Linnakangas's avatar
      Fix TRUNCATE doc: ALTER SEQUENCE RESTART is now transactional. · 2a972e01
      Heikki Linnakangas authored
      ALTER SEQUENCE RESTART was made transactional in commit 3d79013b.
      Backpatch to v10, where that was introduced.
      
      Patch by Justin Pryzby, per Yaroslav Schekin's report.
      
      Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com
      2a972e01
    • Heikki Linnakangas's avatar
      Fix output of tsquery example in docs. · c0bc4c68
      Heikki Linnakangas authored
      The output for this query changed in commit 4e2477b7b8. Backport to 9.6
      like that commit.
      
      Patch by Justin Pryzby, per Yaroslav Schekin's report.
      
      Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com
      c0bc4c68
    • Heikki Linnakangas's avatar
      Fix doc for full text search distance operator. · 1a64c763
      Heikki Linnakangas authored
      Commit 028350f6 changed its behavior from "at most" to "exactly", but
      forgot to update the documentation. Backpatch to 9.6.
      
      Patch by Justin Pryzby, per Yaroslav Schekin's report.
      
      Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com
      1a64c763
    • Magnus Hagander's avatar
      Update link for pllua · b4d5b458
      Magnus Hagander authored
      Author: Daniel Gustafsson <daniel@yesql.se>
      Discussion: https://postgr.es/m/A05874AE-8771-4C61-A24E-0B6249B8F3C2@yesql.se
      b4d5b458
    • Heikki Linnakangas's avatar
      Remove PartitionRoutingInfo struct. · fb5883da
      Heikki Linnakangas authored
      The extra indirection neeeded to access its members via its enclosing
      ResultRelInfo seems pointless. Move all the fields from
      PartitionRoutingInfo to ResultRelInfo.
      
      Author: Amit Langote
      Reviewed-by: Alvaro Herrera
      Discussion: https://www.postgresql.org/message-id/CA%2BHiwqFViT47Zbr_ASBejiK7iDG8%3DQ1swQ-tjM6caRPQ67pT%3Dw%40mail.gmail.com
      fb5883da
    • Heikki Linnakangas's avatar
      Revise child-to-root tuple conversion map management. · 69735336
      Heikki Linnakangas authored
      Store the tuple conversion map to convert a tuple from a child table's
      format to the root format in a new ri_ChildToRootMap field in
      ResultRelInfo. It is initialized if transition tuple capture for FOR
      STATEMENT triggers or INSERT tuple routing on a partitioned table is
      needed. Previously, ModifyTable kept the maps in the per-subplan
      ModifyTableState->mt_per_subplan_tupconv_maps array, or when tuple
      routing was used, in
      ResultRelInfo->ri_Partitioninfo->pi_PartitionToRootMap. The new field
      replaces both of those.
      
      Now that the child-to-root tuple conversion map is always available in
      ResultRelInfo (when needed), remove the TransitionCaptureState.tcs_map
      field. The callers of Exec*Trigger() functions no longer need to set or
      save it, which is much less confusing and bug-prone. Also, as a future
      optimization, this will allow us to delay creating the map for a given
      result relation until the relation is actually processed during
      execution.
      
      Author: Amit Langote
      Discussion: https://www.postgresql.org/message-id/CA%2BHiwqHtCWLdK-LO%3DNEsvOdHx%2B7yv4mE_zYK0i3BH7dXb-wxog%40mail.gmail.com
      69735336
    • Heikki Linnakangas's avatar
      Clean up code to resolve the "root target relation" in nodeModifyTable.c · f49b85d7
      Heikki Linnakangas authored
      When executing DDL on a partitioned table or on a table with inheritance
      children, statement-level triggers must be fired against the table given
      in the original statement. The code to look that up was a bit messy and
      duplicative. Commit 501ed02c added a helper function,
      getASTriggerResultRelInfo() (later renamed to getTargetResultRelInfo())
      for it, but for some reason it was only used when firing AFTER STATEMENT
      triggers and the code to fire BEFORE STATEMENT triggers duplicated the
      logic.
      
      Determine the target relation in ExecInitModifyTable(), and set it always
      in ModifyTableState. Code that used to call getTargetResultRelInfo() can
      now use ModifyTableState->rootResultRelInfo directly.
      
      Discussion: https://www.postgresql.org/message-id/CA%2BHiwqFViT47Zbr_ASBejiK7iDG8%3DQ1swQ-tjM6caRPQ67pT%3Dw%40mail.gmail.com
      f49b85d7
    • Peter Eisentraut's avatar
      Avoid invalid alloc size error in shm_mq · 26ec6b59
      Peter Eisentraut authored
      In shm_mq_receive(), a huge payload could trigger an unjustified
      "invalid memory alloc request size" error due to the way the buffer
      size is increased.
      
      Add error checks (documenting the upper limit) and avoid the error by
      limiting the allocation size to MaxAllocSize.
      
      Author: Markus Wanner <markus.wanner@2ndquadrant.com>
      Discussion: https://www.postgresql.org/message-id/flat/3bb363e7-ac04-0ac4-9fe8-db1148755bfa%402ndquadrant.com
      26ec6b59
    • Peter Eisentraut's avatar
      Improve whitespace · afa0d53d
      Peter Eisentraut authored
      The SQL file was using a mix of tabs and spaces inconsistently.
      Convert all to spaces and adjust indentation accordingly.
      afa0d53d
    • Amit Kapila's avatar
      Change the docs for PARALLEL option of Vacuum. · 560d260d
      Amit Kapila authored
      The rules to choose the number of parallel workers to perform parallel
      vacuum operation were not clearly specified.
      
      Reported-by: Peter Eisentraut
      Author: Amit Kapila
      Backpatch-through: 13, where it was introduced
      Discussion: https://postgr.es/m/36aa8aea-61b7-eb3c-263b-648e0cb117b7@2ndquadrant.com
      560d260d
    • Michael Paquier's avatar
      Fix potential memory leak in pgcrypto · ca2a12c9
      Michael Paquier authored
      When allocating a EVP context, it would have been possible to leak some
      memory allocated directly by OpenSSL, that PostgreSQL lost track of if
      the initialization of the context allocated failed.  The cleanup can be
      done with EVP_MD_CTX_destroy().
      
      Note that EVP APIs exist since OpenSSL 0.9.7 and we have in the tree
      equivalent implementations for older versions since ce9b75db (code
      removed with 9b7cd59a as of 10~).  However, in 9.5 and 9.6, the existing
      code makes use of EVP_MD_CTX_destroy() and EVP_MD_CTX_create() without
      an equivalent implementation when building the tree with OpenSSL 0.9.6
      or older, meaning that this code is in reality broken with such versions
      since it got introduced in e2838c5.  As we have heard no complains about
      that, it does not seem worth bothering with in 9.5 and 9.6, so I have
      left that out for simplicity.
      
      Author: Michael Paquier
      Discussion: https://postgr.es/m/20201015072212.GC2305@paquier.xyz
      Backpatch-through: 9.5
      ca2a12c9
  5. 18 Oct, 2020 3 commits
    • David Rowley's avatar
      Prevent overly large and NaN row estimates in relations · a90c950f
      David Rowley authored
      Given a query with enough joins, it was possible that the query planner,
      after multiplying the row estimates with the join selectivity that the
      estimated number of rows would exceed the limits of the double data type
      and become infinite.
      
      To give an indication on how extreme a case is required to hit this, the
      particular example case reported required 379 joins to a table without any
      statistics, which resulted in the 1.0/DEFAULT_NUM_DISTINCT being used for
      the join selectivity.  This eventually caused the row estimates to go
      infinite and resulted in an assert failure in initial_cost_mergejoin()
      where the infinite row estimated was multiplied by an outerstartsel of 0.0
      resulting in NaN.  The failing assert verified that NaN <= Inf, which is
      false.
      
      To get around this we use clamp_row_est() to cap row estimates at a
      maximum of 1e100.  This value is thought to be low enough that costs
      derived from it would remain within the bounds of what the double type can
      represent.
      
      Aside from fixing the failing Assert, this also has the added benefit of
      making it so add_path() will still receive proper numerical values as
      costs which will allow it to make more sane choices when determining the
      cheaper path in extreme cases such as the one described above.
      
      Additionally, we also get rid of the isnan() checks in the join costing
      functions. The actual case which originally triggered those checks to be
      added in the first place never made it to the mailing lists.  It seems
      likely that the new code being added to clamp_row_est() will result in
      those becoming checks redundant, so just remove them.
      
      The fairly harmless assert failure problem does also exist in the
      backbranches, however, a more minimalistic fix will be applied there.
      
      Reported-by: Onder Kalaci
      Reviewed-by: Tom Lane
      Discussion: https://postgr.es/m/DM6PR21MB1211FF360183BCA901B27F04D80B0@DM6PR21MB1211.namprd21.prod.outlook.com
      a90c950f
    • Tom Lane's avatar
      Update the Winsock API version requested by libpq. · d5a9a661
      Tom Lane authored
      According to Microsoft's documentation, 2.2 has been the current
      version since Windows 98 or so.  Moreover, that's what the Postgres
      backend has been requesting since 2004 (cf commit 4cdf51e6).
      So there seems no reason for libpq to keep asking for 1.1.
      
      Bring thread_test along, too, so that we're uniformly asking for 2.2
      in all our WSAStartup calls.
      
      It's not clear whether there's any point in back-patching this,
      so for now I didn't.
      
      Discussion: https://postgr.es/m/132799.1602960277@sss.pgh.pa.us
      d5a9a661
    • Tom Lane's avatar
      In pg_restore's dump_lo_buf(), work a little harder on error handling. · 929c69aa
      Tom Lane authored
      Failure to write data to a large object during restore led to an ugly
      and uninformative error message.  To add insult to injury, it then
      fatal'd out, where other SQL-level errors usually result in pressing on.
      
      Report the underlying error condition, rather than just giving not-very-
      useful byte counts, and use warn_or_exit_horribly() so as to adhere to
      pg_restore's general policy about whether to continue or not.
      
      Also recognize that lo_write() returns int not size_t.
      
      Per report from Justin Pryzby, though I didn't use his patch.
      Given the lack of comparable complaints, I'm not sure this is
      worth back-patching.
      
      Discussion: https://postgr.es/m/20201018010232.GF9241@telsasoft.com
      929c69aa
  6. 17 Oct, 2020 4 commits
    • Tom Lane's avatar
      In libpq for Windows, call WSAStartup once and WSACleanup not at all. · 7d00a6b2
      Tom Lane authored
      The Windows documentation insists that every WSAStartup call should
      have a matching WSACleanup call.  However, if that ever had actual
      relevance, it wasn't in this century.  Every remotely-modern Windows
      kernel is capable of cleaning up when a process exits without doing
      that, and must be so to avoid resource leaks in case of a process
      crash.  Moreover, Postgres backends have done WSAStartup without
      WSACleanup since commit 4cdf51e6 in 2004, and we've never seen any
      indication of a problem with that.
      
      libpq's habit of doing WSAStartup during connection start and
      WSACleanup during shutdown is also rather inefficient, since a
      series of non-overlapping connection requests leads to repeated,
      quite expensive DLL unload/reload cycles.  We document a workaround
      for that (having the application call WSAStartup for itself), but
      that's just a kluge.  It's also worth noting that it's far from
      uncommon for applications to exit without doing PQfinish, and
      we've not heard reports of trouble from that either.
      
      However, the real reason for acting on this is that recent
      experiments by Alexander Lakhin suggest that calling WSACleanup
      during PQfinish might be triggering the symptom we occasionally see
      that a process using libpq fails to emit expected stdio output.
      
      Therefore, let's change libpq so that it calls WSAStartup only
      once per process, during the first connection attempt, and never
      calls WSACleanup at all.
      
      While at it, get rid of the only other WSACleanup call in our code
      tree, in pg_dump/parallel.c; that presumably is equally useless.
      
      If this proves to suppress the fairly-common ecpg test failures
      we see on Windows, I'll back-patch, but for now let's just do it
      in HEAD and see what happens.
      
      Discussion: https://postgr.es/m/ac976d8c-03df-d6b8-025c-15a2de8d9af1@postgrespro.ru
      7d00a6b2
    • Tom Lane's avatar
      Doc: caution against misuse of 'now' and related datetime literals. · 54084981
      Tom Lane authored
      Section 8.5.1.4, which defines these literals, made only a vague
      reference to the fact that they might be evaluated too soon to be
      safe in non-interactive contexts.  Provide a more explicit caution
      against misuse.  Also, generalize the wording in the related tip in
      section 9.9.4: while it clearly described this problem, it implied
      (or really, stated outright) that the problem only applies to table
      DEFAULT clauses.
      
      Per gripe from Tijs van Dam.  Back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/c2LuRv9BiRT3bqIo5mMQiVraEXey_25B4vUn0kDqVqilwOEu_iVF1tbtvLnyQK7yDG3PFaz_GxLLPil2SDkj1MCObNRVaac-7j1dVdFERk8=@thalex.com
      54084981
    • Tom Lane's avatar
      Update time zone data files to tzdata release 2020c. · c4a803ac
      Tom Lane authored
      DST law changes in Morocco, Canadian Yukon, Fiji, Macquarie Island,
      Casey Station (Antarctica).  Historical corrections for France,
      Hungary, Monaco.
      c4a803ac
    • Tom Lane's avatar
      Sync our copy of the timezone library with IANA release tzcode2020c. · ce0e97f8
      Tom Lane authored
      This changes zic's default output format from "-b fat" to "-b slim".
      We were already using "slim" in v13/HEAD, so those branches drop
      the explicit -b switch in the Makefiles.  Instead, add an explicit
      "-b fat" in v12 and before, so that we don't change the output file
      format in those branches.  (This is perhaps excessively conservative,
      but we decided not to do so in a1207910, and I'll stick with that.)
      
      Other non-cosmetic changes are to drop support for zic's long-obsolete
      "-y" switch, and to ensure that strftime() does not change errno
      unless it fails.
      
      As usual with tzcode changes, back-patch to all supported branches.
      ce0e97f8
  7. 16 Oct, 2020 5 commits
  8. 15 Oct, 2020 3 commits