Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 12 Apr, 2019 4 commits
  3. 11 Apr, 2019 9 commits
    • Tom Lane's avatar
      Re-order some regression test scripts for more parallelism. · 798070ec
      Tom Lane authored 
      Move the strings, numerology, insert, insert_conflict, select and
errors tests to be parts of nearby parallel groups, instead of
executing by themselves.  (Moving "select" required adjusting the
constraints test, which uses a table named "tmp" as select also
does.  There don't seem to be any other conflicts.)

Move psql and stats_ext to the next parallel group, where the rules
test also has a long runtime.  To make it safe to run stats_ext in
parallel with rules, I adjusted the latter to only dump views/rules
from the pg_catalog and public schemas, which was what it was doing
anyway.  stats_ext makes some views in a transient schema, which now
will not affect rules.

Reorder serial_schedule to match parallel_schedule.

Discussion: https://postgr.es/m/735.1554935715@sss.pgh.pa.us
      798070ec
    • Tom Lane's avatar
      Speed up sort-order-comparison tests in create_index_spgist. · 5874c705
      Tom Lane authored 
      This test script verifies that KNN searches of an SP-GiST index
produce the same sort order as a seqscan-and-sort.  The FULL JOINs
used for that are exceedingly slow, however.  Investigation shows
that the problem is that the initial join is on the rank() values,
and we have a lot of duplicates due to the data set containing 1000
duplicate points.  We're therefore going to produce 1000000 join
rows that have to be thrown away again by the join filter.

We can improve matters by using row_number() instead of rank(),
so that the initial join keys are unique.  The catch is that
that makes the results sensitive to the sorting of rows with
equal distances from the reference point.  That doesn't matter
for the actually-equal points, but as luck would have it, the
data set also contains two distinct points that have identical
distances to the origin.  So those two rows could legitimately
appear in either order, causing unwanted output from the check
queries.

However, it doesn't seem like it's the job of this test to
check whether the <-> operator correctly computes distances;
its charter is just to verify that SP-GiST emits the values
in distance order.  So we can dodge the indeterminacy problem
by having the check only compare row numbers and distances
not the actual point values.

This change reduces the run time of create_index_spgist by a good
three-quarters, on my machine, with ensuing beneficial effects on
the runtime of create_index (thanks to interactions with CREATE
INDEX CONCURRENTLY tests in the latter).  I see a net improvement
of more than 2X in the runtime of their parallel test group.

Discussion: https://postgr.es/m/735.1554935715@sss.pgh.pa.us
      5874c705
    • 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
  5. 10 Apr, 2019 4 commits
  7. 09 Apr, 2019 9 commits
  9. 08 Apr, 2019 11 commits
  11. 07 Apr, 2019 3 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