Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 13 Sep, 2017 4 commits
  3. 12 Sep, 2017 8 commits
  5. 11 Sep, 2017 7 commits
  7. 10 Sep, 2017 3 commits
    • Tom Lane's avatar
      Quick-hack fix for foreign key cascade vs triggers with transition tables. · 3c435952
      Tom Lane authored 
      AFTER triggers using transition tables crashed if they were fired due
to a foreign key ON CASCADE update.  This is because ExecEndModifyTable
flushes the transition tables, on the assumption that any trigger that
could need them was already fired during ExecutorFinish.  Normally
that's true, because we don't allow transition-table-using triggers
to be deferred.  However, foreign key CASCADE updates force any
triggers on the referencing table to be deferred to the outer query
level, by means of the EXEC_FLAG_SKIP_TRIGGERS flag.  I don't recall
all the details of why it's like that and am pretty loath to redesign
it right now.  Instead, just teach ExecEndModifyTable to skip destroying
the TransitionCaptureState when that flag is set.  This will allow the
transition table data to survive until end of the current subtransaction.

This isn't a terribly satisfactory solution, because (1) we might be
leaking the transition tables for much longer than really necessary,
and (2) as things stand, an AFTER STATEMENT trigger will fire once per
RI updating query, ie once per row updated or deleted in the referenced
table.  I suspect that is not per SQL spec.  But redesigning this is a
research project that we're certainly not going to get done for v10.
So let's go with this hackish answer for now.

In passing, tweak AfterTriggerSaveEvent to not save the transition_capture
pointer into the event record for a deferrable trigger.  This is not
necessary to fix the current bug, but it avoids letting dangling pointers
to long-gone transition tables persist in the trigger event queue.  That's
at least a safety feature.  It might also allow merging shared trigger
states in more cases than before.

I added a regression test that demonstrates the crash on unpatched code,
and also exposes the behavior of firing the AFTER STATEMENT triggers
once per row update.

Per bug #14808 from Philippe Beaudoin.  Back-patch to v10.

Discussion: https://postgr.es/m/20170909064853.25630.12825@wrigleys.postgresql.org
      3c435952
    • Tom Lane's avatar
      Add a test harness for the red-black tree code. · 610bbdd8
      Tom Lane authored 
      This improves the regression tests' coverage of rbtree.c from pretty
awful (because some of the functions aren't used yet) to basically 100%.

Victor Drobny, reviewed by Aleksander Alekseev and myself

Discussion: https://postgr.es/m/c9d61310e16e75f8acaf6cb1c48b7b77@postgrespro.ru
      610bbdd8
    • Tom Lane's avatar
      Remove pre-order and post-order traversal logic for red-black trees. · f80e782a
      Tom Lane authored 
      This code isn't used, and there's no clear reason why anybody would ever
want to use it.  These traversal mechanisms don't yield a visitation order
that is semantically meaningful for any external purpose, nor are they
any faster or simpler than the left-to-right or right-to-left traversals.
(In fact, some rough testing suggests they are slower :-(.)  Moreover,
these mechanisms are impossible to test in any arm's-length fashion; doing
so requires knowledge of the red-black tree's internal implementation.
Hence, let's just jettison them.

Discussion: https://postgr.es/m/17735.1505003111@sss.pgh.pa.us
      f80e782a
  9. 09 Sep, 2017 2 commits
    • Peter Eisentraut's avatar
      pg_upgrade: Message style fixes · c824c7e2
      Peter Eisentraut authored
      c824c7e2
    • Tom Lane's avatar
      Fix failure-to-copy bug in commit 6f6b99d1. · fdf87ed4
      Tom Lane authored 
      The previous coding of get_qual_for_list() was careful to copy everything
it was using from the input data structure.  The new version missed
making a copy of pass-by-ref datum values that it's inserting into Consts.
This is not optional, however, as revealed by buildfarm failures on
machines running -DRELCACHE_FORCE_RELEASE: we're copying from a relcache
entry that could go away before the required lifespan of our output
expression.  I'm pretty sure -DCLOBBER_CACHE_ALWAYS machines won't like
this either, but none of them have reported in yet.
      fdf87ed4
  11. 08 Sep, 2017 14 commits
  13. 07 Sep, 2017 2 commits
    • Tom Lane's avatar
      Improve performance of get_actual_variable_range with recently-dead tuples. · 3ca930fc
      Tom Lane authored 
      In commit fccebe42, we hacked get_actual_variable_range() to scan the
index with SnapshotDirty, so that if there are many uncommitted tuples
at the end of the index range, it wouldn't laboriously scan through all
of them looking for a live value to return.  However, that didn't fix it
for the case of many recently-dead tuples at the end of the index;
SnapshotDirty recognizes those as committed dead and so we're back to
the same problem.

To improve the situation, invent a "SnapshotNonVacuumable" snapshot type
and use that instead.  The reason this helps is that, if the snapshot
rejects a given index entry, we know that the indexscan will mark that
index entry as killed.  This means the next get_actual_variable_range()
scan will proceed past that entry without visiting the heap, making the
scan a lot faster.  We may end up accepting a recently-dead tuple as
being the estimated extremal value, but that doesn't seem much worse than
the compromise we made before to accept not-yet-committed extremal values.

The cost of the scan is still proportional to the number of dead index
entries at the end of the range, so in the interval after a mass delete
but before VACUUM's cleaned up the mess, it's still possible for
get_actual_variable_range() to take a noticeable amount of time, if you've
got enough such dead entries.  But the constant factor is much much better
than before, since all we need to do with each index entry is test its
"killed" bit.

We chose to back-patch commit fccebe42 at the time, but I'm hesitant to
do so here, because this form of the problem seems to affect many fewer
people.  Also, even when it happens, it's less bad than the case fixed
by commit fccebe42 because we don't get the contention effects from
expensive TransactionIdIsInProgress tests.

Dmitriy Sarafannikov, reviewed by Andrey Borodin

Discussion: https://postgr.es/m/05C72CF7-B5F6-4DB9-8A09-5AC897653113@yandex.ru
      3ca930fc
    • Tom Lane's avatar
      Improve documentation about behavior of multi-statement Query messages. · b9764994
      Tom Lane authored 
      We've long done our best to sweep this topic under the rug, but in view
of recent work it seems like it's time to explain it more precisely.
Here's an initial cut at doing that.

Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1F6BE40D@G01JPEXMBYT05
      b9764994