Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 18 Dec, 2019 1 commit
  3. 17 Dec, 2019 14 commits
  5. 16 Dec, 2019 6 commits
  7. 15 Dec, 2019 1 commit
  9. 14 Dec, 2019 5 commits
    • Tom Lane's avatar
      Try to stabilize results of new tuplesort regression test. · baa32ce2
      Tom Lane authored 
      It appears that a concurrent autovacuum/autoanalyze run can cause
changes in the plans expected by this test.  To prevent that, change
the tables it uses to be temp tables --- there's no need for them
to be permanent, and this should save a few cycles too.

Discussion: https://postgr.es/m/3244.1576160824@sss.pgh.pa.us
      baa32ce2
    • Tom Lane's avatar
      Prevent overly-aggressive collapsing of joins to RTE_RESULT relations. · 6ea364e7
      Tom Lane authored 
      The RTE_RESULT simplification logic added by commit 4be058fe had a
flaw: it would collapse out a RTE_RESULT that is due to compute a
PlaceHolderVar, and reassign the PHV to the parent join level, even if
another input relation of the join contained a lateral reference to
the PHV.  That can't work because the PHV would be computed too late.
In practice it led to failures of internal sanity checks later in
planning (either assertion failures or errors such as "failed to
construct the join relation").

To fix, add code to check for the presence of such PHVs in relevant
portions of the query tree.  Notably, this required refactoring
range_table_walker so that a caller could ask to walk individual RTEs
not the whole list.  (It might be a good idea to refactor
range_table_mutator in the same way, if only to keep those functions
looking similar; but I didn't do so here as it wasn't necessary for
the bug fix.)

This exercise also taught me that find_dependent_phvs(), as it stood,
could only safely be used on the entire Query, not on subtrees.
Adjust its API to reflect that; which in passing allows it to have
a fast path for the common case of no PHVs anywhere.

Per report from Will Leinweber.  Back-patch to v12 where the bug
was introduced.

Discussion: https://postgr.es/m/CALLb-4xJMd4GZt2YCecMC95H-PafuWNKcmps4HLRx2NHNBfB4g@mail.gmail.com
      6ea364e7
    • Michael Paquier's avatar
      Fix memory leak when initializing DH parameters in backend · e0e569e1
      Michael Paquier authored 
      When loading DH parameters used for the generation of ephemeral DH keys
in the backend, the code has never bothered releasing the memory used
for the DH information loaded from a file or from libpq's default.  This
commit makes sure that the information is properly free()'d.

Note that as SSL parameters can be reloaded, this can cause an accumulation
of memory leaked.  As the leak is minor, no backpatch is done.

Reported-by: Dmitry Uspenskiy
Discussion: https://postgr.es/m/16160-18367e56e9a28264@postgresql.org
      e0e569e1
    • Thomas Munro's avatar
      Fix mdsyncfiletag(), take II. · 7c85be08
      Thomas Munro authored 
      The previous commit failed to consider that FileGetRawDesc() might
not return a valid fd, as discovered on the build farm.  Switch to
using the File interface only.

Back-patch to 12, like the previous commit.
      7c85be08
    • Thomas Munro's avatar
      Don't use _mdfd_getseg() in mdsyncfiletag(). · 7bb3102c
      Thomas Munro authored 
      _mdfd_getseg() opens all segments up to the requested one.  That
causes problems for mdsyncfiletag(), if mdunlinkfork() has
already unlinked other segment files.  Open the file we want
directly by name instead, if it's not already open.

The consequence of this bug was a rare panic in the checkpointer,
made more likely if you saturated the sync request queue so that
the SYNC_FORGET_REQUEST messages for a given relation were more
likely to be absorbed in separate cycles by the checkpointer.

Back-patch to 12.  Defect in commit 3eb77eba.

Author: Thomas Munro
Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20191119115759.GI30362%40telsasoft.com
      7bb3102c
  11. 13 Dec, 2019 2 commits
    • Heikki Linnakangas's avatar
      Fix crash when a page was split during GiST index creation. · a7ee7c85
      Heikki Linnakangas authored 
      The bug was similar to the one that was fixed in commit 22251686. When
we split page X and insert the downlink for the new page, the parent page
might also need to be split. When that happens, the downlink offset number
we remembered for X is no longer valid. We correctly called
gistFindCorrectParent() to re-find it, but gistFindCorrectParent() doesn't
do anything if the LSN of the page hasn't changed, and we stopped updating
LSNs during index build in commit 9155580f. The buggy codepath was taken
if the page was split into three or more pages, and inserting the downlink
caused the parent page to split. To fix, explicitly mark the downlink
offset number as invalid, to force gistFindCorrectParent() to re-find it.

Fixes bug #16134 reported by Alexander Lakhin, reported again as #16162 by
Andreas Kunert. Thanks to Jeff Janes, Tom Lane and Tomas Vondra for
debugging. Backpatch to v12, where we stopped WAL-logging during index
build.

Discussion: https://www.postgresql.org/message-id/16134-0423f729671dec64%40postgresql.org
Discussion: https://www.postgresql.org/message-id/16162-45d21b7b6c1a3105%40postgresql.org
      a7ee7c85
    • Tom Lane's avatar
      Modernize our readline API a tad. · 5e7bedc5
      Tom Lane authored 
      Prefer to call "rl_filename_completion_function" and
"rl_completion_matches", rather than using the names without the rl_
prefix.  This matches Readline's documentation, and makes our code
a little clearer about which names are external.  On platforms that
only have the un-prefixed names (just some very ancient versions of
libedit, AFAICT), reverse the direction of the compatibility macro
definitions to match.

Also, remove our extern declaration of "filename_completion_function";
whatever libedit versions may have failed to declare that are surely
dead and buried.

Discussion: https://postgr.es/m/23608.1576248145@sss.pgh.pa.us
      5e7bedc5
  13. 12 Dec, 2019 6 commits
  15. 11 Dec, 2019 5 commits
    • Tom Lane's avatar
      Remove unstable test case added in commit 5935917c. · 776a2c88
      Tom Lane authored 
      The buildfarm says this produces some unexpected output with
force_parallel_mode = regress.  There's probably a bug underneath
that, but for the moment just delete the test case to make the
buildfarm green again.

(I now notice that the case had also failed to get updated to follow
commit d52eaa09, which made plan_cache_mode = force_generic_plan
prevail throughout partition_prune.sql; it was thereby managing to
break a later test.  When/if we put this back in, *don't* include the
SET and RESET commands.)
      776a2c88
    • Tom Lane's avatar
      Allow executor startup pruning to prune all child nodes. · 5935917c
      Tom Lane authored 
      Previously, if the startup pruning logic proved that all child nodes
of an Append or MergeAppend could be pruned, we still kept one, just
to keep EXPLAIN from failing.  The previous commit removed the
ruleutils.c limitation that required this kluge, so drop it.  That
results in less-confusing EXPLAIN output, as per a complaint from
Yuzuko Hosoya.

David Rowley

Discussion: https://postgr.es/m/001001d4f44b$2a2cca50$7e865ef0$@lab.ntt.co.jp
      5935917c
    • Tom Lane's avatar
      Further adjust EXPLAIN's choices of table alias names. · 6ef77cf4
      Tom Lane authored 
      This patch causes EXPLAIN to always assign a separate table alias to the
parent RTE of an append relation (inheritance set); before, such RTEs
were ignored if not actually scanned by the plan.  Since the child RTEs
now always have that same alias to start with (cf. commit 55a1954d),
the net effect is that the parent RTE usually gets the alias used or
implied by the query text, and the children all get that alias with "_N"
appended.  (The exception to "usually" is if there are duplicate aliases
in different subtrees of the original query; then some of those original
RTEs will also have "_N" appended.)

This results in more uniform output for partitioned-table plans than
we had before: the partitioned table itself gets the original alias,
and all child tables have aliases with "_N", rather than the previous
behavior where one of the children would get an alias without "_N".

The reason for giving the parent RTE an alias, even if it isn't scanned
by the plan, is that we now use the parent's alias to qualify Vars that
refer to an appendrel output column and appear above the Append or
MergeAppend that computes the appendrel.  But below the append, Vars
refer to some one of the child relations, and are displayed that way.
This seems clearer than the old behavior where a Var that could carry
values from any child relation was displayed as if it referred to only
one of them.

While at it, change ruleutils.c so that the code paths used by EXPLAIN
deal in Plan trees not PlanState trees.  This effectively reverts a
decision made in commit 1cc29fe7, which seemed like a good idea at
the time to make ruleutils.c consistent with explain.c.  However,
it's problematic because we'd really like to allow executor startup
pruning to remove all the children of an append node when possible,
leaving no child PlanState to resolve Vars against.  (That's not done
here, but will be in the next patch.)  This requires different handling
of subplans and initplans than before, but is otherwise a pretty
straightforward change.

Discussion: https://postgr.es/m/001001d4f44b$2a2cca50$7e865ef0$@lab.ntt.co.jp
      6ef77cf4
    • Alvaro Herrera's avatar
      Emit parameter values during query bind/execute errors · ba79cb5d
      Alvaro Herrera authored 
      This makes such log entries more useful, since the cause of the error
can be dependent on the parameter values.

Author: Alexey Bashtanov, Álvaro Herrera
Discussion: https://postgr.es/m/0146a67b-a22a-0519-9082-bc29756b93a2@imap.cc
Reviewed-by: Peter Eisentraut, Andres Freund, Tom Lane
      ba79cb5d
    • Tom Lane's avatar
      Use only one thread to handle incoming signals on Windows. · 16114f2e
      Tom Lane authored 
      Since its inception, our Windows signal emulation code has worked by
running a main signal thread that just watches for incoming signal
requests, and then spawns a new thread to handle each such request.
That design is meant for servers in which requests can take substantial
effort to process, and it's worth parallelizing the handling of
requests.  But those assumptions are just bogus for our signal code.
It's not much more than pg_queue_signal(), which is cheap and can't
parallelize at all, plus we don't really expect lots of signals to
arrive at the same backend at once.  More importantly, this approach
creates failure modes that we could do without: either inability to
spawn a new thread or inability to create a new pipe handle will risk
loss of signals.  Hence, dispense with the separate per-signal threads
and just service each request in-line in the main signal thread.  This
should be a bit faster (for the normal case of one signal at a time)
as well as more robust.

Patch by me; thanks to Andrew Dunstan for testing and Amit Kapila
for review.

Discussion: https://postgr.es/m/4412.1575748586@sss.pgh.pa.us
      16114f2e