1. 31 Mar, 2021 22 commits
    • Alvaro Herrera's avatar
      Disable force_parallel_mode in libpq_pipeline · a6d3dea8
      Alvaro Herrera authored
      Some buildfarm animals with force_parallel_mode=regress were failing
      this test because the error is reported in a parallel worker quicker
      than the rows that succeed.
      
      Take the opportunity to move the SET of lc_messages out of the traced
      section, because it's not very interesting.
      Diagnosed-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
      Discussion: https://postgr.es/m/3304521.1617221724@sss.pgh.pa.us
      a6d3dea8
    • Tom Lane's avatar
      Fix unportable use of isprint(). · 9e20406d
      Tom Lane authored
      We must cast the arguments of <ctype.h> functions to unsigned
      char to avoid problems where char is signed.
      
      Speaking of which, considering that this *is* a <ctype.h> function,
      it's rather remarkable that we aren't seeing more complaints about
      not having included that header.
      
      Per buildfarm.
      9e20406d
    • Tom Lane's avatar
      Fix portability and safety issues in pqTraceFormatTimestamp. · f1be740a
      Tom Lane authored
      Remove confusion between time_t and pg_time_t; neither
      gettimeofday() nor localtime() deal in the latter.
      libpq indeed has no business using <pgtime.h> at all.
      
      Use snprintf not sprintf, to ensure we can't overrun the
      supplied buffer.  (Unlikely, but let's be safe.)
      
      Per buildfarm.
      f1be740a
    • Tom Lane's avatar
      Silence compiler warning in non-assert builds. · 8998e3ca
      Tom Lane authored
      Per buildfarm.
      8998e3ca
    • Tom Lane's avatar
      Don't prematurely cram a value into a short int. · c545e952
      Tom Lane authored
      Since a4d75c86, some buildfarm members have been warning that
      		Assert(attnum <= MaxAttrNumber);
      is useless if attnum is an AttrNumber.  I'm not certain how plausible
      it is that the value coming out of the bitmap could actually exceed
      MaxAttrNumber, but we seem to have thought that that was possible back
      in 7300a699.  Revert the intermediate variable to int so that we have
      the same overflow protection as before.
      c545e952
    • Stephen Frost's avatar
      Add a docs section for obsoleted and renamed functions and settings · 3b0c647b
      Stephen Frost authored
      The new appendix groups information on renamed or removed settings,
      commands, etc into an out-of-the-way part of the docs.
      
      The original id elements are retained in each subsection to ensure that
      the same filenames are produced for HTML docs. This prevents /current/
      links on the web from breaking, and allows users of the web docs
      to follow links from old version pages to info on the changes in the
      new version. Prior to this change, a link to /current/ for renamed
      sections like the recovery.conf docs would just 404. Similarly if
      someone searched for recovery.conf they would find the pg11 docs,
      but there would be no /12/ or /current/ link, so they couldn't easily
      find out that it was removed in pg12 or how to adapt.
      
      Index entries are also added so that there's a breadcrumb trail for
      users to follow when they know the old name, but not what we changed it
      to. So a user who is trying to find out how to set standby_mode in
      PostgreSQL 12+, or where pg_resetxlog went, now has more chance of
      finding that information.
      
      Craig Ringer and Stephen Frost
      Reviewed-by: Euler Taveira
      Discussion: https://postgr.es/m/CAGRY4nzPNOyYQ_1-pWYToUVqQ0ThqP5jdURnJMZPm539fdizOg%40mail.gmail.com
      Backpatch-through: 10
      3b0c647b
    • Tom Lane's avatar
      Suppress compiler warning in libpq_pipeline.c. · 522d1a89
      Tom Lane authored
      Some compilers seem to be concerned about the possibility that
      recv_step is not any of the defined enum values.  Silence
      warnings about uninitialized cmdtag in a different way than
      I did in 9fb9691a.
      522d1a89
    • Tom Lane's avatar
      Improve style of some replication-related error messages. · 6197db53
      Tom Lane authored
      Put the remote end's error message into the primary error string,
      instead of relegating it to errdetail().  Although this could end up
      being awkward if the remote sends us a really long error message,
      it seems more in keeping with our message style guidelines, and more
      helpful in situations where the errdetail could get dropped.
      
      Peter Smith
      
      Discussion: https://postgr.es/m/CAHut+Ps-Qv2yQceCwobQDP0aJOkfDzRFrOaR6+2Op2K=WHGeWg@mail.gmail.com
      6197db53
    • Alvaro Herrera's avatar
      Fix some libpq_pipeline test problems · db973ffb
      Alvaro Herrera authored
      Test pipeline_abort was not checking that it got the rows it expected in
      one mode; make it do so.  This doesn't fix the actual problem (no idea
      what that is, yet) but at least it should make it more obvious rather
      than being visible only as a difference in the trace output.
      
      While at it, fix other infelicities in the test:
      
      * I reversed the order of result vs. expected in like().
      
      * The output traces from -t are being put in the log dir, which means
      the buildfarm script uselessly captures them.  Put them in a separate
      dir tmp_check/traces instead, to avoid cluttering the buildfarm results.
      
      * Test pipelined_insert was using too large a row count.  Reduce that a
      tad and add a filler column to make each insert a little bulkier, while
      still keeping enough that a buffer is filled and we have to switch mode.
      db973ffb
    • Joe Conway's avatar
      Fix has_column_privilege function corner case · b12bd486
      Joe Conway authored
      According to the comments, when an invalid or dropped column oid is passed
      to has_column_privilege(), the intention has always been to return NULL.
      However, when the caller had table level privilege the invalid/missing
      column was never discovered, because table permissions were checked first.
      
      Fix that by introducing extended versions of pg_attribute_acl(check|mask)
      and pg_class_acl(check|mask) which take a new argument, is_missing. When
      is_missing is NULL, the old behavior is preserved. But when is_missing is
      passed by the caller, no ERROR is thrown for dropped or missing
      columns/relations, and is_missing is flipped to true. This in turn allows
      has_column_privilege to check for column privileges first, providing the
      desired semantics.
      
      Not backpatched since it is a user visible behavioral change with no previous
      complaints, and the fix is a bit on the invasive side.
      
      Author: Joe Conway
      Reviewed-By: Tom Lane
      Reported by: Ian Barwick
      Discussion: https://postgr.es/m/flat/9b5f4311-157b-4164-7fe7-077b4fe8ed84%40joeconway.com
      b12bd486
    • Tom Lane's avatar
      Rework planning and execution of UPDATE and DELETE. · 86dc9005
      Tom Lane authored
      This patch makes two closely related sets of changes:
      
      1. For UPDATE, the subplan of the ModifyTable node now only delivers
      the new values of the changed columns (i.e., the expressions computed
      in the query's SET clause) plus row identity information such as CTID.
      ModifyTable must re-fetch the original tuple to merge in the old
      values of any unchanged columns.  The core advantage of this is that
      the changed columns are uniform across all tables of an inherited or
      partitioned target relation, whereas the other columns might not be.
      A secondary advantage, when the UPDATE involves joins, is that less
      data needs to pass through the plan tree.  The disadvantage of course
      is an extra fetch of each tuple to be updated.  However, that seems to
      be very nearly free in context; even worst-case tests don't show it to
      add more than a couple percent to the total query cost.  At some point
      it might be interesting to combine the re-fetch with the tuple access
      that ModifyTable must do anyway to mark the old tuple dead; but that
      would require a good deal of refactoring and it seems it wouldn't buy
      all that much, so this patch doesn't attempt it.
      
      2. For inherited UPDATE/DELETE, instead of generating a separate
      subplan for each target relation, we now generate a single subplan
      that is just exactly like a SELECT's plan, then stick ModifyTable
      on top of that.  To let ModifyTable know which target relation a
      given incoming row refers to, a tableoid junk column is added to
      the row identity information.  This gets rid of the horrid hack
      that was inheritance_planner(), eliminating O(N^2) planning cost
      and memory consumption in cases where there were many unprunable
      target relations.
      
      Point 2 of course requires point 1, so that there is a uniform
      definition of the non-junk columns to be returned by the subplan.
      We can't insist on uniform definition of the row identity junk
      columns however, if we want to keep the ability to have both
      plain and foreign tables in a partitioning hierarchy.  Since
      it wouldn't scale very far to have every child table have its
      own row identity column, this patch includes provisions to merge
      similar row identity columns into one column of the subplan result.
      In particular, we can merge the whole-row Vars typically used as
      row identity by FDWs into one column by pretending they are type
      RECORD.  (It's still okay for the actual composite Datums to be
      labeled with the table's rowtype OID, though.)
      
      There is more that can be done to file down residual inefficiencies
      in this patch, but it seems to be committable now.
      
      FDW authors should note several API changes:
      
      * The argument list for AddForeignUpdateTargets() has changed, and so
      has the method it must use for adding junk columns to the query.  Call
      add_row_identity_var() instead of manipulating the parse tree directly.
      You might want to reconsider exactly what you're adding, too.
      
      * PlanDirectModify() must now work a little harder to find the
      ForeignScan plan node; if the foreign table is part of a partitioning
      hierarchy then the ForeignScan might not be the direct child of
      ModifyTable.  See postgres_fdw for sample code.
      
      * To check whether a relation is a target relation, it's no
      longer sufficient to compare its relid to root->parse->resultRelation.
      Instead, check it against all_result_relids or leaf_result_relids,
      as appropriate.
      
      Amit Langote and Tom Lane
      
      Discussion: https://postgr.es/m/CA+HiwqHpHdqdDn48yCEhynnniahH78rwcrv1rEX65-fsZGBOLQ@mail.gmail.com
      86dc9005
    • Peter Eisentraut's avatar
      Allow an alias to be attached to a JOIN ... USING · 055fee7e
      Peter Eisentraut authored
      This allows something like
      
          SELECT ... FROM t1 JOIN t2 USING (a, b, c) AS x
      
      where x has the columns a, b, c and unlike a regular alias it does not
      hide the range variables of the tables being joined t1 and t2.
      
      Per SQL:2016 feature F404 "Range variable for common column names".
      Reviewed-by: default avatarVik Fearing <vik.fearing@2ndquadrant.com>
      Reviewed-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
      Discussion: https://www.postgresql.org/message-id/flat/454638cf-d563-ab76-a585-2564428062af@2ndquadrant.com
      055fee7e
    • Etsuro Fujita's avatar
      Add support for asynchronous execution. · 27e1f145
      Etsuro Fujita authored
      This implements asynchronous execution, which runs multiple parts of a
      non-parallel-aware Append concurrently rather than serially to improve
      performance when possible.  Currently, the only node type that can be
      run concurrently is a ForeignScan that is an immediate child of such an
      Append.  In the case where such ForeignScans access data on different
      remote servers, this would run those ForeignScans concurrently, and
      overlap the remote operations to be performed simultaneously, so it'll
      improve the performance especially when the operations involve
      time-consuming ones such as remote join and remote aggregation.
      
      We may extend this to other node types such as joins or aggregates over
      ForeignScans in the future.
      
      This also adds the support for postgres_fdw, which is enabled by the
      table-level/server-level option "async_capable".  The default is false.
      
      Robert Haas, Kyotaro Horiguchi, Thomas Munro, and myself.  This commit
      is mostly based on the patch proposed by Robert Haas, but also uses
      stuff from the patch proposed by Kyotaro Horiguchi and from the patch
      proposed by Thomas Munro.  Reviewed by Kyotaro Horiguchi, Konstantin
      Knizhnik, Andrey Lepikhov, Movead Li, Thomas Munro, Justin Pryzby, and
      others.
      
      Discussion: https://postgr.es/m/CA%2BTgmoaXQEt4tZ03FtQhnzeDEMzBck%2BLrni0UWHVVgOTnA6C1w%40mail.gmail.com
      Discussion: https://postgr.es/m/CA%2BhUKGLBRyu0rHrDCMC4%3DRn3252gogyp1SjOgG8SEKKZv%3DFwfQ%40mail.gmail.com
      Discussion: https://postgr.es/m/20200228.170650.667613673625155850.horikyota.ntt%40gmail.com
      27e1f145
    • Peter Eisentraut's avatar
      Add p_names field to ParseNamespaceItem · 66392d39
      Peter Eisentraut authored
      ParseNamespaceItem had a wired-in assumption that p_rte->eref
      describes the table and column aliases exposed by the nsitem.  This
      relaxes this by creating a separate p_names field in an nsitem.  This
      is mainly preparation for a patch for JOIN USING aliases, but it saves
      one indirection in common code paths, so it's possibly a win on its
      own.
      
      Author: Tom Lane <tgl@sss.pgh.pa.us>
      Discussion: https://www.postgresql.org/message-id/785329.1616455091@sss.pgh.pa.us
      66392d39
    • Peter Eisentraut's avatar
      Add errhint_plural() function and make use of it · 91c5a8ca
      Peter Eisentraut authored
      Similar to existing errmsg_plural() and errdetail_plural().  Some
      errhint() calls hadn't received the proper plural treatment yet.
      91c5a8ca
    • Peter Eisentraut's avatar
      doc: Remove Cyrillic from unistr example · 287d2a97
      Peter Eisentraut authored
      Not supported by PDF build right now, so let's do without it.
      287d2a97
    • Amit Kapila's avatar
      Remove extra semicolon in postgres_fdw tests. · 13cb5bd8
      Amit Kapila authored
      Author: Suraj Kharage
      Reviewed-by: Bharath Rupireddy, Vignesh C
      Discussion: https://postgr.es/m/CAF1DzPWRfxUeH-wShz7P_pK5Tx6M_nEK+TkS8gn5ngvg07Q5=g@mail.gmail.com
      13cb5bd8
    • Amit Kapila's avatar
      Doc: Use consistent terminology for tablesync slots. · 9f456317
      Amit Kapila authored
      At some places in the docs, we refer to them as tablesync slots and at other
      places as table synchronization slots. For consistency, we refer to them as
      table synchronization slots at all places.
      
      Author: Peter Smith
      Reviewed-by: Amit Kapila
      Discussion: https://postgr.es/m/CAHut+PvzYNKCeZ=kKBDkh3dw-r=2D3fk=nNc9SXSW=CZGk69xg@mail.gmail.com
      9f456317
    • Noah Misch's avatar
      Accept slightly-filled pages for tuples larger than fillfactor. · 0ff8bbde
      Noah Misch authored
      We always inserted a larger-than-fillfactor tuple into a newly-extended
      page, even when existing pages were empty or contained nothing but an
      unused line pointer.  This was unnecessary relation extension.  Start
      tolerating page usage up to 1/8 the maximum space that could be taken up
      by line pointers.  This is somewhat arbitrary, but it should allow more
      cases to reuse pages.  This has no effect on tables with fillfactor=100
      (the default).
      
      John Naylor and Floris van Nee.  Reviewed by Matthias van de Meent.
      Reported by Floris van Nee.
      
      Discussion: https://postgr.es/m/6e263217180649339720afe2176c50aa@opammb0562.comp.optiver.com
      0ff8bbde
    • Michael Paquier's avatar
      Fix comment in parsenodes.h · 7ef64e7e
      Michael Paquier authored
      CreateStmt->inhRelations is a list of RangeVars, but a comment was
      incorrect about that.
      
      Author: Julien Rouhaud
      Discussion: https://postgr.es/m/20210330123015.yzekhz5sweqbgxdr@nol
      7ef64e7e
    • Michael Paquier's avatar
      Add support for --extension in pg_dump · 6568cef2
      Michael Paquier authored
      When specified, only extensions matching the given pattern are included
      in dumps.  Similarly to --table and --schema, when --strict-names is
      used,  a perfect match is required.  Also, like the two other options,
      this new option offers no guarantee that dependent objects have been
      dumped, so a restore may fail on a clean database.
      
      Tests are added in test_pg_dump/, checking after a set of positive and
      negative cases, with or without an extension's contents added to the
      dump generated.
      
      Author: Guillaume Lelarge
      Reviewed-by: David Fetter, Tom Lane, Michael Paquier, Asif Rehman,
      Julien Rouhaud
      Discussion: https://postgr.es/m/CAECtzeXOt4cnMU5+XMZzxBPJ_wu76pNy6HZKPRBL-j7yj1E4+g@mail.gmail.com
      6568cef2
    • Tom Lane's avatar
      Remove small inefficiency in ExecARDeleteTriggers/ExecARUpdateTriggers. · 65158f49
      Tom Lane authored
      Whilst poking at nodeModifyTable.c, I chanced to notice that while
      its calls to ExecBR*Triggers and ExecIR*Triggers are protected by
      tests to see if there are any relevant triggers to fire, its calls
      to ExecAR*Triggers are not; the latter functions do the equivalent
      tests themselves.  This seems possibly reasonable given the more
      complex conditions involved, but what's less reasonable is that
      the ExecAR* functions aren't careful to do no work when there is
      no work to be done.  ExecARInsertTriggers gets this right, but the
      other two will both force creation of a slot that the query may
      have no use for.  ExecARUpdateTriggers additionally performed a
      usually-useless ExecClearTuple() on that slot.  This is probably
      all pretty microscopic in real workloads, but a cycle shaved is a
      cycle earned.
      65158f49
  2. 30 Mar, 2021 13 commits
  3. 29 Mar, 2021 5 commits
    • Alvaro Herrera's avatar
      psql: call clearerr() just before printing · 8d645a11
      Alvaro Herrera authored
      We were never doing clearerr() on the output stream, which results in a
      message being printed after each result once an EOF is seen:
      
      could not print result table: Success
      
      This message was added by commit b0343699 (in the pg13 era); before
      that, the error indicator would never be examined.  So backpatch only
      that far back, even though the actual bug (to wit: the fact that the
      error indicator is never cleared) is older.
      8d645a11
    • David Rowley's avatar
      Adjust design of per-worker parallel seqscan data struct · af527705
      David Rowley authored
      The design of the data structures which allow storage of the per-worker
      memory during parallel seq scans were not ideal. The work done in
      56788d21 required an additional data structure to allow workers to
      remember the range of pages that had been allocated to them for
      processing during a parallel seqscan.  That commit added a void pointer
      field to TableScanDescData to allow heapam to store the per-worker
      allocation information.  However putting the field there made very little
      sense given that we have AM specific structs for that, e.g.
      HeapScanDescData.
      
      Here we remove the void pointer field from TableScanDescData and add a
      dedicated field for this purpose to HeapScanDescData.
      
      Previously we also allocated memory for this parallel per-worker data for
      all scans, regardless if it was a parallel scan or not.  This was just a
      wasted allocation for non-parallel scans, so here we make the allocation
      conditional on the scan being parallel.
      
      Also, add previously missing pfree() to free the per-worker data in
      heap_endscan().
      
      Reported-by: Andres Freund
      Reviewed-by: Andres Freund
      Discussion: https://postgr.es/m/20210317023101.anvejcfotwka6gaa@alap3.anarazel.de
      af527705
    • Andrew Dunstan's avatar
      Allow matching the DN of a client certificate for authentication · 6d7a6fea
      Andrew Dunstan authored
      Currently we only recognize the Common Name (CN) of a certificate's
      subject to be matched against the user name. Thus certificates with
      subjects '/OU=eng/CN=fred' and '/OU=sales/CN=fred' will have the same
      connection rights. This patch provides an option to match the whole
      Distinguished Name (DN) instead of just the CN. On any hba line using
      client certificate identity, there is an option 'clientname' which can
      have values of 'DN' or 'CN'. The default is 'CN', the current procedure.
      
      The DN is matched against the RFC2253 formatted DN, which looks like
      'CN=fred,OU=eng'.
      
      This facility of probably best used in conjunction with an ident map.
      
      Discussion: https://postgr.es/m/92e70110-9273-d93c-5913-0bccb6562740@dunslane.net
      
      Reviewed-By: Michael Paquier, Daniel Gustafsson, Jacob Champion
      6d7a6fea
    • Peter Eisentraut's avatar
      Clean up date_part tests a bit · efcc7572
      Peter Eisentraut authored
      Some tests for timestamp and timestamptz were in the date.sql test
      file.  Move them to their appropriate files, or drop tests cases that
      were already present there.
      efcc7572
    • Peter Eisentraut's avatar
      Add unistr function · f37fec83
      Peter Eisentraut authored
      This allows decoding a string with Unicode escape sequences.  It is
      similar to Unicode escape strings, but offers some more flexibility.
      
      Author: Pavel Stehule <pavel.stehule@gmail.com>
      Reviewed-by: default avatarAsif Rehman <asifr.rehman@gmail.com>
      Discussion: https://www.postgresql.org/message-id/flat/CAFj8pRA5GnKT+gDVwbVRH2ep451H_myBt+NTz8RkYUARE9+qOQ@mail.gmail.com
      f37fec83