Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 11 Apr, 2019 7 commits
    • Tom Lane's avatar
      Split up a couple of long-running regression test scripts. · 385d396b
      Tom Lane authored 
      The point of this change is to increase the potential for parallelism
while running the core regression tests.  Most people these days are
using parallel testing modes on multi-core machines, so we might as
well try a bit harder to keep multiple cores busy.  Hence, a test that
runs much longer than others in its parallel group is a candidate to
be sub-divided.

In this patch, create_index.sql and join.sql are split up.
I haven't changed the content of the tests in any way, just
moved them.

I moved create_index.sql's SP-GiST-related tests into a new script
create_index_spgist, and moved its btree multilevel page deletion test
over to the existing script btree_index.  (btree_index is a more natural
home for that test, and it's shorter than others in its parallel group,
so this doesn't hurt total runtime of that group.)  There might be
room for more aggressive splitting of create_index, but this is enough
to improve matters considerably.

Likewise, I moved join.sql's "exercises for the hash join code" into
a new file join_hash.  Those exercises contributed three-quarters of
the script's runtime.  Which might well be excessive ... but for the
moment, I'm satisfied with shoving them into a different parallel
group, where they can share runtime with the roughly-equally-lengthy
gist test.

(Note for anybody following along at home: there are interesting
interactions between the runtimes of create_index and anything running
in parallel with it, because the tests of CREATE INDEX CONCURRENTLY
in that file will repeatedly block waiting for concurrent transactions
to commit.  As committed in this patch, create_index and
create_index_spgist have roughly equal runtimes, but that's mostly an
artifact of forced synchronization of the CONCURRENTLY tests; when run
serially, create_index is much faster.  A followup patch will reduce
the runtime of create_index_spgist and thereby also create_index.)

Discussion: https://postgr.es/m/735.1554935715@sss.pgh.pa.us
      385d396b
    • Tom Lane's avatar
      Move plpgsql error-trapping tests to a new module-specific test file. · 6726d8d4
      Tom Lane authored 
      The test for statement timeout has a 2-second timeout, which was only
moderately annoying when it was written, but nowadays it contributes
a pretty significant chunk of the elapsed time needed to run the core
regression tests on a fast machine.  We can improve this situation by
pushing the test into a plpgsql-specific test file instead of having
it in a core regression test.  That's a clean win when considering
just the core tests.  Even when considering check-world or a buildfarm
test run, we should come out ahead because the core tests get run
more times in those sequences.

Furthermore, since the plpgsql tests aren't currently parallelized,
it seems likely that the timing problems reflected in commit f1e671a0
(which increased that timeout from 1 sec to 2) will be much less severe
in this context.  Hence, let's try cutting the timeout back to 1 second
in hopes of a further win for check-world.  We can undo that if
buildfarm experience proves it to be a bad idea.

To give the new test file some modicum of intellectual coherency,
I moved the surrounding tests related to error-trapping along with
the statement timeout test proper.  Those other tests don't run long
enough to have any particular bearing on test-runtime considerations.
The tests are the same as before, except with minor adjustments to
not depend on an externally-created table.

Discussion: https://postgr.es/m/735.1554935715@sss.pgh.pa.us
      6726d8d4
    • Michael Meskes's avatar
      Fix off-by-one check that can lead to a memory overflow in ecpg. · ed16ba32
      Michael Meskes authored 
      Patch by Liu Huailing <liuhuailing@cn.fujitsu.com>
      ed16ba32
    • Tom Lane's avatar
      Remove duplicative polygon SP-GiST sequencing test. · 4aaa3b5c
      Tom Lane authored 
      Code coverage comparisons confirm that the tests using
quad_poly_tbl_ord_seq1/quad_poly_tbl_ord_idx1 hit no code
paths not also covered by the similar tests using
quad_poly_tbl_ord_seq2/quad_poly_tbl_ord_idx2.  Since these
test cases are pretty expensive, they need to contribute more
than zero benefit.

In passing, make quad_poly_tbl_ord_seq2 a temp table, since
there seems little reason to keep it around after the test.

Discussion: https://postgr.es/m/735.1554935715@sss.pgh.pa.us
      4aaa3b5c
    • Bruce Momjian's avatar
      doc: adjust libpq wording to be neither/nor · d4888a3f
      Bruce Momjian authored 
      Reported-by: postgresql@cohi.at

Discussion: https://postgr.es/m/155419437926.737.10876947446993402227@wrigleys.postgresql.org

Backpatch-through: 9.4
      d4888a3f
    • Tom Lane's avatar
      Remove redundant and ineffective test for btree insertion fast path. · f72d9a5e
      Tom Lane authored 
      indexing.sql's test for this feature was added along with the
feature in commit 2b272734.  However, shortly later that test was
rendered ineffective by commit 074251db, which limited when the
optimization would be applied, so that the test didn't test it.
Since then, commit dd299df8 added new tests (in btree_index.sql)
that actually do test the feature.  Code coverage comparisons
confirm that this test sequence adds no meaningful coverage, and
it's rather expensive, accounting for nearly half of the runtime
of indexing.sql according to my measurements.  So let's remove it.

Per advice from Peter Geoghegan.

Discussion: https://postgr.es/m/735.1554935715@sss.pgh.pa.us
      f72d9a5e
    • Alvaro Herrera's avatar
      Fix declaration after statement · 65d857d9
      Alvaro Herrera authored 
      This style is frowned upon.  I inadvertently introduced one in commit
fe0e0b4f.  (My compiler does not complain about it, even though
-Wdeclaration-after-statement is specified.  Weird.)

Author: Masahiko Sawada
      65d857d9
  3. 10 Apr, 2019 4 commits
  5. 09 Apr, 2019 9 commits
  7. 08 Apr, 2019 11 commits
  9. 07 Apr, 2019 5 commits
    • Tom Lane's avatar
      Avoid fetching past the end of the indoption array. · 80a96e06
      Tom Lane authored 
      pg_get_indexdef_worker carelessly fetched indoption entries even for
non-key index columns that don't have one.  99.999% of the time this
would be harmless, since the code wouldn't examine the value ... but
some fine day this will be a fetch off the end of memory, resulting
in SIGSEGV.

Detected through valgrind testing.  Odd that the buildfarm's valgrind
critters haven't noticed.
      80a96e06
    • Alvaro Herrera's avatar
      psql \dP: list partitioned tables and indexes · 1c5d9270
      Alvaro Herrera authored 
      The new command lists partitioned relations (tables and/or indexes),
possibly with their sizes, possibly including partitioned partitions;
their parents (if not top-level); if indexes show the tables they belong
to; and their descriptions.

While there are various possible improvements to this, having it in this
form is already a great improvement over not having any way to obtain
this report.

Author: Pavel Stěhule, with help from Mathias Brossard, Amit Langote and
	Justin Pryzby.
Reviewed-by: Amit Langote, Mathias Brossard, Melanie Plageman,
	Michaël Paquier, Álvaro Herrera
      1c5d9270
    • Tom Lane's avatar
      Clean up side-effects of commits ab5fcf2b et al. · 159970bc
      Tom Lane authored 
      Before those commits, partitioning-related code in the executor could
assume that ModifyTableState.resultRelInfo[] contains only leaf partitions.
However, now a fully-pruned update results in a dummy ModifyTable that
references the root partitioned table, and that breaks some stuff.

In v11, this led to an assertion or core dump in the tuple routing code.
Fix by disabling tuple routing, since we don't need that anyway.
(I chose to do that in HEAD as well for safety, even though the problem
doesn't manifest in HEAD as it stands.)

In v10, this confused ExecInitModifyTable's decision about whether it
needed to close the root table.  But we can get rid of that altogether
by being smarter about where to find the root table.

Note that since the referenced commits haven't shipped yet, this
isn't fixing any bug the field has seen.

Amit Langote, per a report from me

Discussion: https://postgr.es/m/20710.1554582479@sss.pgh.pa.us
      159970bc
    • Peter Eisentraut's avatar
      Report progress of REINDEX operations · 03f9e5cb
      Peter Eisentraut authored 
      This uses the same infrastructure that the CREATE INDEX progress
reporting uses.  Add a column to pg_stat_progress_create_index to
report the OID of the index being worked on.  This was not necessary
for CREATE INDEX, but it's useful for REINDEX.

Also edit the phase descriptions a bit to be more consistent with the
source code comments.

Discussion: https://www.postgresql.org/message-id/ef6a6757-c36a-9e81-123f-13b19e36b7d7%402ndquadrant.com
      03f9e5cb
    • Peter Eisentraut's avatar
      Cast pg_stat_progress_cluster.cluster_index_relid to oid · 106f2eb6
      Peter Eisentraut authored 
      It's tracked internally as bigint, but when presented to the user it
should be oid.
      106f2eb6
  11. 06 Apr, 2019 3 commits
  13. 05 Apr, 2019 1 commit
    • Tom Lane's avatar
      Use Append rather than MergeAppend for scanning ordered partitions. · 959d00e9
      Tom Lane authored 
      If we need ordered output from a scan of a partitioned table, but
the ordering matches the partition ordering, then we don't need to
use a MergeAppend to combine the pre-ordered per-partition scan
results: a plain Append will produce the same results.  This
both saves useless comparison work inside the MergeAppend proper,
and allows us to start returning tuples after istarting up just
the first child node not all of them.

However, all is not peaches and cream, because if some of the
child nodes have high startup costs then there will be big
discontinuities in the tuples-returned-versus-elapsed-time curve.
The planner's cost model cannot handle that (yet, anyway).
If we model the Append's startup cost as being just the first
child's startup cost, we may drastically underestimate the cost
of fetching slightly more tuples than are available from the first
child.  Since we've had bad experiences with over-optimistic choices
of "fast start" plans for ORDER BY LIMIT queries, that seems scary.
As a klugy workaround, set the startup cost estimate for an ordered
Append to be the sum of its children's startup costs (as MergeAppend
would).  This doesn't really describe reality, but it's less likely
to cause a bad plan choice than an underestimated startup cost would.
In practice, the cases where we really care about this optimization
will have child plans that are IndexScans with zero startup cost,
so that the overly conservative estimate is still just zero.

David Rowley, reviewed by Julien Rouhaud and Antonin Houska

Discussion: https://postgr.es/m/CAKJS1f-hAqhPLRk_RaSFTgYxd=Tz5hA7kQ2h4-DhJufQk8TGuw@mail.gmail.com
      959d00e9