1. 04 Feb, 2019 4 commits
    • Andrew Gierth's avatar
      Move port-specific parts of with_temp_install to port makefile. · 54f5f887
      Andrew Gierth authored
      Rather than define ld_library_path_ver with a big nested $(if), just
      put the overriding values in the makefiles for the relevant ports.
      
      Also add a variable for port makefiles to append their own stuff to
      with_temp_install, and use it to set LD_LIBRARY_PATH_RPATH=1 on
      FreeBSD which is needed to make LD_LIBRARY_PATH override DT_RPATH
      if DT_RUNPATH is not set (which seems to depend in unpredictable ways
      on the choice of compiler, at least on my system).
      
      Backpatch for the benefit of anyone doing regression tests on FreeBSD.
      (For other platforms there should be no functional change.)
      54f5f887
    • Amit Kapila's avatar
      Make FSM test portable. · 08ecdfe7
      Amit Kapila authored
      In b0eaa4c5, we allow FSM to be created only after 4 pages.  One of the
      tests check the FSM contents and to do that it populates many tuples in
      the relation.  The FSM contents depend on the availability of freespace in
      the page and it could vary because of the alignment of tuples.
      
      This commit removes the dependency on FSM contents.
      
      Author: Amit Kapila
      Discussion: https://postgr.es/m/CAA4eK1KADF6K1bagr0--mGv3dMcZ%3DH_Z-Qtvdfbp5PjaC6PJJA%40mail.gmail.com
      08ecdfe7
    • Amit Kapila's avatar
      Avoid creation of the free space map for small heap relations, take 2. · b0eaa4c5
      Amit Kapila authored
      Previously, all heaps had FSMs. For very small tables, this means that the
      FSM took up more space than the heap did. This is wasteful, so now we
      refrain from creating the FSM for heaps with 4 pages or fewer. If the last
      known target block has insufficient space, we still try to insert into some
      other page before giving up and extending the relation, since doing
      otherwise leads to table bloat. Testing showed that trying every page
      penalized performance slightly, so we compromise and try every other page.
      This way, we visit at most two pages. Any pages with wasted free space
      become visible at next relation extension, so we still control table bloat.
      As a bonus, directly attempting one or two pages can even be faster than
      consulting the FSM would have been.
      
      Once the FSM is created for a heap we don't remove it even if somebody
      deletes all the rows from the corresponding relation.  We don't think it is
      a useful optimization as it is quite likely that relation will again grow
      to the same size.
      
      Author: John Naylor, Amit Kapila
      Reviewed-by: Amit Kapila
      Tested-by: Mithun C Y
      Discussion: https://www.postgresql.org/message-id/CAJVSVGWvB13PzpbLEecFuGFc5V2fsO736BsdTakPiPAcdMM5tQ@mail.gmail.com
      b0eaa4c5
    • Michael Paquier's avatar
      Clarify behavior of initdb's --allow-group-access on Windows in docs · be12aa47
      Michael Paquier authored
      The option is ignored on Windows, and GUC data_directory_mode already
      mentioned that within its description in the documentation.
      
      Author: Michael Paquier
      Reported-by: Haribabu Kommi, David Steele
      Discussion: https://postgr.es/m/CAJrrPGefxTG43yk6BrOC7ZcMnCTccG9+inCSncvyys_t8Ev9cQ@mail.gmail.com
      Backpatch-through: 11
      be12aa47
  2. 03 Feb, 2019 3 commits
    • Thomas Munro's avatar
      Add shared_memory_type GUC. · f1bebef6
      Thomas Munro authored
      Since 9.3 we have used anonymous shared mmap for our main shared memory
      region, except in EXEC_BACKEND builds.  Provide a GUC so that users
      can opt for System V shared memory once again, like in 9.2 and earlier.
      
      A later patch proposes to add huge/large page support for AIX, which
      requires System V shared memory and provided the motivation to revive
      this possibility.  It may also be useful on some BSDs.
      
      Author: Andres Freund (revived and documented by Thomas Munro)
      Discussion: https://postgr.es/m/HE1PR0202MB28126DB4E0B6621CC6A1A91286D90%40HE1PR0202MB2812.eurprd02.prod.outlook.com
      Discussion: https://postgr.es/m/2AE143D2-87D3-4AD1-AC78-CE2258230C05%40FreeBSD.org
      f1bebef6
    • Andres Freund's avatar
      Move page initialization from RelationAddExtraBlocks() to use, take 2. · 0d1fe9f7
      Andres Freund authored
      Previously we initialized pages when bulk extending in
      RelationAddExtraBlocks(). That has a major disadvantage: It ties
      RelationAddExtraBlocks() to heap, as other types of storage are likely
      to need different amounts of special space, have different amount of
      free space (previously determined by PageGetHeapFreeSpace()).
      
      That we're relying on initializing pages, but not WAL logging the
      initialization, also means the risk for getting
      "WARNING:  relation \"%s\" page %u is uninitialized --- fixing"
      style warnings in vacuums after crashes/immediate shutdowns, is
      considerably higher. The warning sounds much more serious than what
      they are.
      
      Fix those two issues together by not initializing pages in
      RelationAddExtraPages() (but continue to do so in
      RelationGetBufferForTuple(), which is linked much more closely to
      heap), and accepting uninitialized pages as normal in
      vacuumlazy.c. When vacuumlazy encounters an empty page it now adds it
      to the FSM, but does nothing else.  We chose to not issue a debug
      message, much less a warning in that case - it seems rarely useful,
      and quite likely to scare people unnecessarily.
      
      For now empty pages aren't added to the VM, because standbys would not
      re-discover such pages after a promotion. In contrast to other sources
      for empty pages, there's no corresponding WAL records triggering FSM
      updates during replay.
      
      Previously when extending the relation, there was a moment between
      extending the relation, and acquiring an exclusive lock on the new
      page, in which another backend could lock the page. To avoid new
      content being put on that new page, vacuumlazy needed to acquire the
      extension lock for a brief moment when encountering a new page. A
      second corner case, only working somewhat by accident, was that
      RelationGetBufferForTuple() sometimes checks the last page in a
      relation for free space, without consulting the FSM; that only worked
      because PageGetHeapFreeSpace() interprets the zero page header in a
      new page as no free space.  The lack of handling this properly
      required reverting the previous attempt in 68420054.
      
      This issue can be solved by using RBM_ZERO_AND_LOCK when extending the
      relation, thereby avoiding this window. There's some added complexity
      when RelationGetBufferForTuple() is called with another buffer (for
      updates), to avoid deadlocks, but that's rarely hit at runtime.
      
      Author: Andres Freund
      Reviewed-By: Tom Lane
      Discussion: https://postgr.es/m/20181219083945.6khtgm36mivonhva@alap3.anarazel.de
      0d1fe9f7
    • Michael Paquier's avatar
      Add PG_CFLAGS, PG_CXXFLAGS, and PG_LDFLAGS variables to PGXS · ac3a9afd
      Michael Paquier authored
      Add PG_CFLAGS, PG_CXXFLAGS, and PG_LDFLAGS variables to pgxs.mk which
      will be appended or prepended to the corresponding make variables.
      Notably, there was previously no way to pass custom CXXFLAGS to third
      party extension module builds, COPT and PROFILE supporting only CFLAGS
      and LDFLAGS.
      
      Backpatch all the way down to ease integration with existing
      extensions.
      
      Author: Christoph Berg
      Reviewed-by: Andres Freund, Tom Lane, Michael Paquier
      Discussion: https://postgr.es/m/20181113104005.GA32154@msg.credativ.de
      Backpatch-through: 9.4
      ac3a9afd
  3. 02 Feb, 2019 2 commits
    • Amit Kapila's avatar
      Avoid possible deadlock while locking multiple heap pages. · 0b8bdb3c
      Amit Kapila authored
      To avoid deadlock, backend acquires a lock on heap pages in block
      number order.  In certain cases, lock on heap pages is dropped and
      reacquired.  In this case, the locks are dropped for reading in
      corresponding VM page/s. The issue is we re-acquire locks in bufferId
      order whereas the intention was to acquire in blockid order.
      
      This commit ensures that we will always acquire locks on heap pages in
      blockid order.
      
      Reported-by: Nishant Fnu
      Author: Nishant Fnu
      Reviewed-by: Amit Kapila and Robert Haas
      Backpatch-through: 9.4
      Discussion: https://postgr.es/m/5883C831-2ED1-47C8-BFAC-2D5BAE5A8CAE@amazon.com
      0b8bdb3c
    • Michael Paquier's avatar
      Improve installation instructions with pg_ctl in documentation · 3e938a83
      Michael Paquier authored
      The documentation includes sections to be able to initialize and start
      Postgres via a couple of commands.  Some of its recommendations involve
      using directly "postgres", which is inconsistent with the recommendation
      given by initdb.  At the same time make some other command calls more
      consistent with the rest, by using an absolute path when creating a
      database.
      
      Author: Andreas Scherbaum
      Reviewed-by: Michael Banck, Ryan Lambert
      3e938a83
  4. 01 Feb, 2019 6 commits
  5. 31 Jan, 2019 3 commits
  6. 30 Jan, 2019 7 commits
  7. 29 Jan, 2019 11 commits
    • Tom Lane's avatar
      Rename nodes/relation.h to nodes/pathnodes.h. · fa2cf164
      Tom Lane authored
      The old name of this file was never a very good indication of what it
      was for.  Now that there's also access/relation.h, we have a potential
      confusion hazard as well, so let's rename it to something more apropos.
      Per discussion, "pathnodes.h" is reasonable, since a good fraction of
      the file is Path node definitions.
      
      While at it, tweak a couple of other headers that were gratuitously
      importing relation.h into modules that don't need it.
      
      Discussion: https://postgr.es/m/7719.1548688728@sss.pgh.pa.us
      fa2cf164
    • Tom Lane's avatar
      Refactor planner's header files. · f09346a9
      Tom Lane authored
      Create a new header optimizer/optimizer.h, which exposes just the
      planner functions that can be used "at arm's length", without need
      to access Paths or the other planner-internal data structures defined
      in nodes/relation.h.  This is intended to provide the whole planner
      API seen by most of the rest of the system; although FDWs still need
      to use additional stuff, and more thought is also needed about just
      what selfuncs.c should rely on.
      
      The main point of doing this now is to limit the amount of new
      #include baggage that will be needed by "planner support functions",
      which I expect to introduce later, and which will be in relevant
      datatype modules rather than anywhere near the planner.
      
      This commit just moves relevant declarations into optimizer.h from
      other header files (a couple of which go away because everything
      got moved), and adjusts #include lists to match.  There's further
      cleanup that could be done if we want to decide that some stuff
      being exposed by optimizer.h doesn't belong in the planner at all,
      but I'll leave that for another day.
      
      Discussion: https://postgr.es/m/11460.1548706639@sss.pgh.pa.us
      f09346a9
    • Tom Lane's avatar
      Make some small planner API cleanups. · a1b8c41e
      Tom Lane authored
      Move a few very simple node-creation and node-type-testing functions
      from the planner's clauses.c to nodes/makefuncs and nodes/nodeFuncs.
      There's nothing planner-specific about them, as evidenced by the
      number of other places that were using them.
      
      While at it, rename and_clause() etc to is_andclause() etc, to clarify
      that they are node-type-testing functions not node-creation functions.
      And use "static inline" implementations for the shortest ones.
      
      Also, modify flatten_join_alias_vars() and some subsidiary functions
      to take a Query not a PlannerInfo to define the join structure that
      Vars should be translated according to.  They were only using the
      "parse" field of the PlannerInfo anyway, so this just requires removing
      one level of indirection.  The advantage is that now parse_agg.c can
      use flatten_join_alias_vars() without the horrid kluge of creating an
      incomplete PlannerInfo, which will allow that file to be decoupled from
      relation.h in a subsequent patch.
      
      Discussion: https://postgr.es/m/11460.1548706639@sss.pgh.pa.us
      a1b8c41e
    • Peter Eisentraut's avatar
      Fix pg_stat_ssl.clientdn · e77cfa54
      Peter Eisentraut authored
      Return null if there is no client certificate.  This is how it has
      always been documented, but in reality it returned an empty string.
      Reviewed-by: default avatarKyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
      Discussion: https://www.postgresql.org/message-id/flat/398754d8-6bb5-c5cf-e7b8-22e5f0983caf@2ndquadrant.com/
      e77cfa54
    • Peter Eisentraut's avatar
    • Peter Eisentraut's avatar
      Make SSL tests more robust · bdd6e9ba
      Peter Eisentraut authored
      Someone running these test could have key or certificate files in
      their ~/.postgresql/, which would interfere with the tests.  The way
      to override that is to specify sslcert=invalid and/or
      sslrootcert=invalid if no actual certificate is used for a particular
      test.  Document that and fix up one test that had a risk of failing in
      these circumstances.
      
      Discussion: https://www.postgresql.org/message-id/flat/398754d8-6bb5-c5cf-e7b8-22e5f0983caf@2ndquadrant.com/
      bdd6e9ba
    • Magnus Hagander's avatar
      Improve wording about WAL files in tar mode of pg_basebackup · 9745b528
      Magnus Hagander authored
      Author: Alex Kliukin
      Reviewed-By: Michael Paquier, Magnus Hagander
      9745b528
    • Etsuro Fujita's avatar
      postgres_fdw: Fix test for cached costs in estimate_path_cost_size(). · 449d0a85
      Etsuro Fujita authored
      estimate_path_cost_size() failed to re-use cached costs when the cached
      startup/total cost was 0, so it calculated the costs redundantly.
      
      This is an oversight in commit aa09cd24; but apply the patch to HEAD
      only because there are no reports of actual trouble from that.
      
      Author: Etsuro Fujita
      Discussion: https://postgr.es/m/5C4AF3F3.4060409%40lab.ntt.co.jp
      449d0a85
    • Michael Paquier's avatar
      Use catalog query to discover tables to process in vacuumdb · e0c2933a
      Michael Paquier authored
      vacuumdb would use a catalog query only when the command caller does not
      define a list of tables.  Switching to a catalog table represents two
      advantages:
      - Relation existence check can happen before running any VACUUM or
      ANALYZE query.  Before this change, if multiple relations are defined
      using --table, the utility would fail only after processing the
      firstly-defined ones, which may be a long some depending on the size of
      the relation.  This adds checks for the relation names, and does
      nothing, at least yet, for the attribute names.
      - More filtering options can become available for the utility user.
      These options, which may be introduced later on, are based on the
      relation size or the relation age, and need to be made available even if
      the user does not list any specific table with --table.
      
      Author: Nathan Bossart
      Reviewed-by: Michael Paquier, Masahiko Sawada
      Discussion: https://postgr.es/m/FFE5373C-E26A-495B-B5C8-911EC4A41C5E@amazon.com
      e0c2933a
    • Andres Freund's avatar
      Fix LLVM related headers to compile standalone (to fix cpluspluscheck). · da05eb51
      Andres Freund authored
      Previously llvmjit.h #error'ed when USE_LLVM was not defined, to
      prevent it from being included from code not having #ifdef USE_LLVM
      guards - but that's not actually that useful after, during the
      development of JIT support, LLVM related code was moved into a
      separately compiled .so.  Having that #error means cpluspluscheck
      doesn't work when llvm support isn't enabled, which isn't great.
      
      Similarly add USE_LLVM guards to llvmjit_emit.h, and additionally make
      sure it compiles standalone.
      
      Per complaint from Tom Lane.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/19808.1548692361@sss.pgh.pa.us
      Backpatch: 11, where JIT support was added
      da05eb51
    • Andres Freund's avatar
      Revert "Move page initialization from RelationAddExtraBlocks() to use." · 68420054
      Andres Freund authored
      This reverts commit fc02e672 and
      e6799d5a.
      
      Parts of the buildfarm error out with
      ERROR: page %u of relation "%s" should be empty but is not
      errors, and so far I/we do not know why. fc02e672 didn't fix the
      issue.  As I cannot reproduce the issue locally, it seems best to get
      the buildfarm green again, and reproduce the issue without time
      pressure.
      68420054
  8. 28 Jan, 2019 4 commits
    • Andres Freund's avatar
      Fix race condition between relation extension and vacuum. · fc02e672
      Andres Freund authored
      In e6799d5a I removed vacuumlazy.c trickery around re-checking
      whether a page is actually empty after acquiring an extension lock on
      the relation, because the page is not PageInit()ed anymore, and
      entries in the FSM ought not to lead to user-visible errors.
      
      As reported by various buildfarm animals that is not correct, given
      the way to code currently stands: If vacuum processes a page that's
      just been newly added by either RelationGetBufferForTuple() or
      RelationAddExtraBlocks(), it could add that page to the FSM and it
      could be reused by other backends, before those two functions check
      whether the newly added page is actually new.  That's a relatively
      narrow race, but several buildfarm machines appear to be able to hit
      it.
      
      While it seems wrong that the FSM, given it's lack of durability and
      approximative nature, can trigger errors like this, that seems better
      fixed in a separate commit. Especially given that a good portion of
      the buildfarm is red, and this is just re-introducing logic that
      existed a few hours ago.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20190128222259.zhi7ovzgtkft6em6@alap3.anarazel.de
      fc02e672
    • Tomas Vondra's avatar
      Separate per-batch and per-tuple memory contexts in COPY · 36a1281f
      Tomas Vondra authored
      In batching mode, COPY was using the same (per-tuple) memory context for
      allocations with longer lifetime. This was confusing but harmless, until
      commit 31f38174 added COPY FROM ... WHERE feature, introducing a risk
      of memory leak.
      
      The "per-tuple" memory context was reset only when starting new batch,
      but as the rows may be filtered out by the WHERE clauses, that may not
      happen at all.  The WHERE clause however has to be evaluated for all
      rows, before filtering them out.
      
      This commit separates the per-tuple and per-batch contexts, removing the
      ambiguity.  Expressions (both defaults and WHERE clause) are evaluated
      in the per-tuple context, while tuples are formed in the batch context.
      This allows resetting the contexts at appropriate times.
      
      The main complexity is related to partitioning, in which case we need to
      reset the batch context after forming the tuple (which happens before
      routing to leaf partition).  Instead of switching between two contexts
      as before, we simply copy the last tuple aside, reset the context and
      then copy the tuple back.  The performance impact is negligible, and
      juggling with two contexts is not free either.
      
      Discussion: https://www.postgresql.org/message-id/flat/CALAY4q_DdpWDuB5-Zyi-oTtO2uSk8pmy+dupiRe3AvAc++1imA@mail.gmail.com
      36a1281f
    • Tom Lane's avatar
      In the planner, replace an empty FROM clause with a dummy RTE. · 4be058fe
      Tom Lane authored
      The fact that "SELECT expression" has no base relations has long been a
      thorn in the side of the planner.  It makes it hard to flatten a sub-query
      that looks like that, or is a trivial VALUES() item, because the planner
      generally uses relid sets to identify sub-relations, and such a sub-query
      would have an empty relid set if we flattened it.  prepjointree.c contains
      some baroque logic that works around this in certain special cases --- but
      there is a much better answer.  We can replace an empty FROM clause with a
      dummy RTE that acts like a table of one row and no columns, and then there
      are no such corner cases to worry about.  Instead we need some logic to
      get rid of useless dummy RTEs, but that's simpler and covers more cases
      than what was there before.
      
      For really trivial cases, where the query is just "SELECT expression" and
      nothing else, there's a hazard that adding the extra RTE makes for a
      noticeable slowdown; even though it's not much processing, there's not
      that much for the planner to do overall.  However testing says that the
      penalty is very small, close to the noise level.  In more complex queries,
      this is able to find optimizations that we could not find before.
      
      The new RTE type is called RTE_RESULT, since the "scan" plan type it
      gives rise to is a Result node (the same plan we produced for a "SELECT
      expression" query before).  To avoid confusion, rename the old ResultPath
      path type to GroupResultPath, reflecting that it's only used in degenerate
      grouping cases where we know the query produces just one grouped row.
      (It wouldn't work to unify the two cases, because there are different
      rules about where the associated quals live during query_planner.)
      
      Note: although this touches readfuncs.c, I don't think a catversion
      bump is required, because the added case can't occur in stored rules,
      only plans.
      
      Patch by me, reviewed by David Rowley and Mark Dilger
      
      Discussion: https://postgr.es/m/15944.1521127664@sss.pgh.pa.us
      4be058fe
    • Andres Freund's avatar
      Install JIT related headers. · 5c118675
      Andres Freund authored
      There's no reason not to install these, and jit.h can be useful for
      users of e.g. planner hooks.
      
      Author: Donald Dong
      Reviewed-By: Andres Freund
      Discussion: https://postgr.es/m/296D405F-7F95-49F1-B565-389D6AA78505@csumb.edu
      Backpatch: 11-, where JIT compilation was introduced
      5c118675