1. 04 Dec, 2018 1 commit
    • Etsuro Fujita's avatar
      postgres_fdw: Improve cost and size estimation for aggregate pushdown. · f8f6e446
      Etsuro Fujita authored
      In commit 7012b132, which added aggregate
      pushdown to postgres_fdw, we didn't account for the evaluation cost and the
      selectivity of HAVING quals attached to ForeignPaths performing aggregate
      pushdown, as core had never accounted for that for AggPaths and GroupPaths.
      And we didn't set these values of the locally-checked quals (ie, fpinfo's
      local_conds_cost and local_conds_sel), which were initialized to zeros, but
      since estimate_path_cost_size factors in these to estimate the result size
      and the evaluation cost of such a ForeignPath when the use_remote_estimate
      option is enabled, this caused it to produce underestimated results in that
      case.
      
      By commit 7b6c0754 core was changed so that
      it accounts for the evaluation cost and the selectivity of HAVING quals in
      aggregation paths, so change the postgres_fdw's aggregate pushdown code as
      well as such.  This not only fixes the underestimation issue mentioned
      above, but improves the estimation using local statistics in that function
      when that option is disabled.
      
      This would be a bug fix rather than an improvement, but apply it to HEAD
      only to avoid destabilizing existing plan choices.
      
      Author: Etsuro Fujita
      Discussion: https://postgr.es/m/5BFD3EAD.2060301%40lab.ntt.co.jp
      f8f6e446
  2. 03 Dec, 2018 3 commits
    • Tom Lane's avatar
      Refactor documentation about privileges to centralize the info. · afc4a78a
      Tom Lane authored
      Expand section 5.6 "Privileges" to include the full definition of
      each privilege type, and an explanation of aclitem privilege displays,
      along with some helpful summary tables.  Most of this material came
      out of the GRANT reference page, although some of it is new.
      Adjust a bunch of links that were pointing to GRANT to point to 5.6.
      
      Fabien Coelho and Tom Lane, reviewed by Bradley DeJong
      
      Discussion: https://postgr.es/m/alpine.DEB.2.21.1807311735200.20743@lancre
      afc4a78a
    • Michael Paquier's avatar
      Add some missing schema qualifications · ee2b37ae
      Michael Paquier authored
      This does not improve the security and reliability of the touched areas,
      but it makes the style more consistent.
      
      Author: Michael Paquier
      Reviewed-by- Noah Misch
      Discussion: https://postgr.es/m/20180309075538.GD9376@paquier.xyz
      ee2b37ae
    • Michael Paquier's avatar
      Add PGXS options to control TAP and isolation tests, take two · d3c09b9b
      Michael Paquier authored
      The following options are added for extensions:
      - TAP_TESTS, to allow an extention to run TAP tests which are the ones
      present in t/*.pl.  A subset of tests can always be run with the
      existing PROVE_TESTS for developers.
      - ISOLATION, to define a list of isolation tests.
      - ISOLATION_OPTS, to pass custom options to isolation_tester.
      
      A couple of custom Makefile rules have been accumulated across the tree
      to cover the lack of facility in PGXS for a couple of releases when
      using those test suites, which are all now replaced with the new flags,
      without reducing the test coverage.  Note that tests of contrib/bloom/
      are not enabled yet, as those are proving unstable in the buildfarm.
      
      Author: Michael Paquier
      Reviewed-by: Adam Berlin, Álvaro Herrera, Tom Lane, Nikolay Shaplov,
      Arthur Zakirov
      Discussion: https://postgr.es/m/20180906014849.GG2726@paquier.xyz
      d3c09b9b
  3. 01 Dec, 2018 3 commits
    • Tom Lane's avatar
      Eliminate parallel-make hazard in ecpg/preproc. · 29180e5d
      Tom Lane authored
      Re-making ecpglib's typename.o is dangerous because another make thread
      could be doing that at the same time.  While we've not heard field
      complaints traceable to this, it seems inevitable that it'd bite someone
      eventually.  Instead, symlink typename.c into the preproc directory and
      recompile it there.  That file is small enough that compiling it twice
      isn't much of a penalty.  Furthermore, this way we get a .o file that's
      made without shlib CFLAGS, which seems cleaner.
      
      This requires adding more stuff to the module's -I list.  The MSVC
      aspect of that is untested, but I'm sure the buildfarm will tell me
      if I got it wrong.
      
      Per a suggestion from Peter Eisentraut.  Although this is theoretically
      a bug fix, the lack of field reports makes me feel we needn't back-patch.
      
      Discussion: https://postgr.es/m/31364.1543511708@sss.pgh.pa.us
      29180e5d
    • Tom Lane's avatar
      Rename ecpg's various "extern.h" files to have distinct names. · 3295f820
      Tom Lane authored
      This should reduce confusion, and in particular make it safe to
      copy typename.c into preproc/ and compile it there.
      
      This doesn't affect anything outside ecpg, and particularly not
      end users, because these files don't get installed; they just
      exist to share declarations among the .c files of each subdirectory.
      
      Discussion: https://postgr.es/m/31364.1543511708@sss.pgh.pa.us
      3295f820
    • Tom Lane's avatar
      Add a --socketdir option to pg_upgrade. · 2d34ad84
      Tom Lane authored
      This allows control of the directory in which the postmaster sockets
      are created for the temporary postmasters started by pg_upgrade.
      The default location remains the current working directory, which is
      typically fine, but if it is deeply nested then its pathname might
      be too long to be a socket name.
      
      In passing, clean up some messiness in pg_upgrade's option handling,
      particularly the confusing and undocumented way that configuration-only
      datadirs were handled.  And fix check_required_directory's substantially
      under-baked cleanup of directory pathnames.
      
      Daniel Gustafsson, reviewed by Hironobu Suzuki, some code cleanup by me
      
      Discussion: https://postgr.es/m/E72DD5C3-2268-48A5-A907-ED4B34BEC223@yesql.se
      2d34ad84
  4. 30 Nov, 2018 5 commits
  5. 29 Nov, 2018 9 commits
  6. 28 Nov, 2018 5 commits
    • Peter Geoghegan's avatar
      Have BufFileSize() ereport() on FileSize() failure. · 1a990b20
      Peter Geoghegan authored
      Move the responsibility for checking for and reporting a failure from
      the only current BufFileSize() caller, logtape.c, to BufFileSize()
      itself.  Code within buffile.c is generally responsible for interfacing
      with fd.c to report irrecoverable failures.  This seems like a
      convention that's worth sticking to.
      
      Reorganizing things this way makes it easy to make the error message
      raised in the event of BufFileSize() failure descriptive of the
      underlying problem.  We're now clear on the distinction between
      temporary file name and BufFile name, and can show errno, confident that
      its value actually relates to the error being reported.  In passing, an
      existing, similar buffile.c ereport() + errcode_for_file_access() site
      is changed to follow the same conventions.
      
      The API of the function BufFileSize() is changed by this commit, despite
      already being in a stable release (Postgres 11).  This seems acceptable,
      since the BufFileSize() ABI was changed by commit aa551830, which
      hasn't made it into a point release yet.  Besides, it's difficult to
      imagine a third party BufFileSize() caller not just raising an error
      anyway, since BufFile state should be considered corrupt when
      BufFileSize() fails.
      
      Per complaint from Tom Lane.
      
      Discussion: https://postgr.es/m/26974.1540826748@sss.pgh.pa.us
      Backpatch: 11-, where shared BufFiles were introduced.
      1a990b20
    • Peter Eisentraut's avatar
      Only allow one recovery target setting · f2cbffc7
      Peter Eisentraut authored
      The previous recovery.conf regime accepted multiple recovery_target*
      settings and used the last one.  This does not translate well to the
      general GUC system.  Specifically, under EXEC_BACKEND, the settings
      are written out not in any particular order, so the order in which
      they were originally set is not available to new processes.
      
      Rather than redesign the GUC system, it was decided to abandon the old
      behavior and only allow one recovery target setting.  A second setting
      will cause an error.  However, it is allowed to set the same parameter
      multiple times or unset a parameter and set a different one.
      
      Discussion: https://www.postgresql.org/message-id/flat/27802171543235530%40iva2-6ec8f0a6115e.qloud-c.yandex.net#701a59c837ad0bf8c244344aaf3ef5a4
      f2cbffc7
    • Bruce Momjian's avatar
      C comment: remove extra '*' · eae9143d
      Bruce Momjian authored
      Reported-by: Etsuro Fujita
      
      Discussion: https://postgr.es/m/5BFE34DE.1080404@lab.ntt.co.jp
      
      Author: Etsuro Fujita
      
      Backpatch-through: 10
      eae9143d
    • Thomas Munro's avatar
      Don't set PAM_RHOST for Unix sockets. · 0f9cdd7d
      Thomas Munro authored
      Since commit 2f1d2b7a we have set PAM_RHOST to "[local]" for Unix
      sockets.  This caused Linux PAM's libaudit integration to make DNS
      requests for that name.  It's not exactly clear what value PAM_RHOST
      should have in that case, but it seems clear that we shouldn't set it
      to an unresolvable name, so don't do that.
      
      Back-patch to 9.6.  Bug #15520.
      
      Author: Thomas Munro
      Reviewed-by: Peter Eisentraut
      Reported-by: Albert Schabhuetl
      Discussion: https://postgr.es/m/15520-4c266f986998e1c5%40postgresql.org
      0f9cdd7d
    • Tomas Vondra's avatar
      Do not decode TOAST data for table rewrites · f69c959d
      Tomas Vondra authored
      During table rewrites (VACUUM FULL and CLUSTER), the main heap is logged
      using XLOG / FPI records, and thus (correctly) ignored in decoding.
      But the associated TOAST table is WAL-logged as plain INSERT records,
      and so was logically decoded and passed to reorder buffer.
      
      That has severe consequences with TOAST tables of non-trivial size.
      Firstly, reorder buffer has to keep all those changes, possibly spilling
      them to a file, incurring I/O costs and disk space.
      
      Secondly, ReoderBufferCommit() was stashing all those TOAST chunks into
      a hash table, which got discarded only after processing the row from the
      main heap.  But as the main heap is not decoded for rewrites, this never
      happened, so all the TOAST data accumulated in memory, resulting either
      in excessive memory consumption or OOM.
      
      The fix is simple, as commit e9edc1ba already introduced infrastructure
      (namely HEAP_INSERT_NO_LOGICAL flag) to skip logical decoding of TOAST
      tables, but it only applied it to system tables.  So simply use it for
      all TOAST data in raw_heap_insert().
      
      That would however solve only the memory consumption issue - the TOAST
      changes would still be decoded and added to the reorder buffer, and
      spilled to disk (although without TOAST tuple data, so much smaller).
      But we can solve that by tweaking DecodeInsert() to just ignore such
      INSERT records altogether, using XLH_INSERT_CONTAINS_NEW_TUPLE flag,
      instead of skipping them later in ReorderBufferCommit().
      
      Review: Masahiko Sawada
      Discussion: https://www.postgresql.org/message-id/flat/1a17c643-e9af-3dba-486b-fbe31bc1823a%402ndquadrant.com
      Backpatch: 9.4-, where logical decoding was introduced
      f69c959d
  7. 27 Nov, 2018 8 commits
  8. 26 Nov, 2018 6 commits
    • Andres Freund's avatar
      Fix typo introduced in 578b2297. · 54bb22f6
      Andres Freund authored
      Author: Andreas Karlsson
      Discussion: https://postgr.es/m/0917c86f-e906-27c0-740e-abc581480823@proxel.se
      54bb22f6
    • Andres Freund's avatar
      Fix pg_upgrade for oid removal. · 12a53c73
      Andres Freund authored
      pg_upgrade previously copied pg_largeobject_metadata over from the old
      cluster. That doesn't work, because the table has oids before
      578b2297. I missed that.
      
      As most pieces of metadata for large objects already were dumped as
      DDL (except for comments overwritten by pg_upgrade, due to the copy of
      pg_largeobject_metadata) it seems reasonable to just also dump grants
      for large objects.  If we ever consider this a relevant performance
      problem, we'd need to fix the rest of the already emitted DDL
      too.
      
      There's still an open discussion about whether we'll want to force a
      specific ordering for the dumped objects, as currently
      pg_largeobjects_metadata potentially has a different ordering
      before/after pg_upgrade, which can make automated testing a bit
      harder.
      
      Reported-By: Andrew Dunstan
      Author: Andres Freund
      Discussion: https://postgr.es/m/91a8a980-41bc-412b-fba2-2ba71a141c2b@2ndQuadrant.com
      12a53c73
    • Tom Lane's avatar
      Fix translation of special characters in psql's LaTeX output modes. · 70d7e507
      Tom Lane authored
      latex_escaped_print() mistranslated \ and failed to provide any translation
      for # ^ and ~, all of which would typically lead to LaTeX document syntax
      errors.  In addition it didn't translate < > and |, which would typically
      render as unexpected characters.
      
      To some extent this represents shortcomings in ancient versions of LaTeX,
      which if memory serves had no easy way to render these control characters
      as ASCII text.  But that's been fixed for, um, decades.  In any case there
      is no value in emitting guaranteed-to-fail output for these characters.
      
      Noted while fooling with test cases added by commit 9a98984f.  Back-patch
      the code change to all supported versions.
      70d7e507
    • Tom Lane's avatar
      Avoid locale-dependent output in numericlocale check. · 95dcb8fc
      Tom Lane authored
      I'd forgotten that in the buildfarm, parts of the regression tests
      may run with psql exposed to a non-default LC_NUMERIC setting.
      Hence we can't assume that C locale prevails, nor is there any
      accessible way to force the setting for this single test step.
      Lobotomize the test case added by commit 9a98984f so that it covers as
      much as we can of print.c without having any locale-varying output.
      95dcb8fc
    • Alvaro Herrera's avatar
      Fix sample output for hash_metapage_info query · 67ed3b9d
      Alvaro Herrera authored
      One output column was duplicated.  Couldn't resist fixing the version
      number while at it.
      
      Reported-by: Gianni Ciolli
      67ed3b9d
    • Tom Lane's avatar
      Add CSV table output mode in psql. · aa2ba50c
      Tom Lane authored
      "\pset format csv", or --csv, selects comma-separated values table format.
      This is compliant with RFC 4180, except that we aren't too picky about
      whether the record separator is LF or CRLF; also, the user may choose a
      field separator other than comma.
      
      This output format is directly compatible with the server's COPY CSV
      format, and will also be useful as input to other programs.  It's
      considerably safer for that purpose than the old recommendation to
      use "unaligned" format, since the latter couldn't handle data
      containing the field separator character.
      
      Daniel Vérité, reviewed by Fabien Coelho and David Fetter, some
      tweaking by me
      
      Discussion: https://postgr.es/m/a8de371e-006f-4f92-ab72-2bbe3ee78f03@manitou-mail.org
      aa2ba50c