Commit Graph

33686 Commits

Author SHA1 Message Date
Tom Lane 7ad1cd31bf Repair assorted issues in locale data extraction.
cache_locale_time (extraction of LC_TIME-related info) had never been
taught the lessons we previously learned about extraction of info related
to LC_MONETARY and LC_NUMERIC.  Specifically, commit 95a777c61 taught
PGLC_localeconv() that data coming out of localeconv() was in an encoding
determined by the relevant locale, but we didn't realize that there's a
similar issue with strftime().  And commit a4930e7ca hardened
PGLC_localeconv() against errors occurring partway through, but failed
to do likewise for cache_locale_time().  So, rearrange the latter
function to perform encoding conversion and not risk failure while
it's got the locales set to temporary values.

This time around I also changed PGLC_localeconv() to treat it as FATAL
if it can't restore the previous settings of the locale values.  There
is no reason (except possibly OOM) for that to fail, and proceeding with
the wrong locale values seems like a seriously bad idea --- especially
on Windows where we have to also temporarily change LC_CTYPE.  Also,
protect against the possibility that we can't identify the codeset
reported for LC_MONETARY or LC_NUMERIC; rather than just failing,
try to validate the data without conversion.

The user-visible symptom this fixes is that if LC_TIME is set to a locale
name that implies an encoding different from the database encoding,
non-ASCII localized day and month names would be retrieved in the wrong
encoding, leading to either unexpected encoding-conversion error reports
or wrong output from to_char().  The other possible failure modes are
unlikely enough that we've not seen reports of them, AFAIK.

The encoding conversion problems do not manifest on Windows, since
we'd already created special-case code to handle that issue there.

Per report from Juan José Santamaría Flecha.  Back-patch to all
supported versions.

Juan José Santamaría Flecha and Tom Lane

Discussion: https://postgr.es/m/CAC+AXB22So5aZm2vZe+MChYXec7gWfr-n-SK-iO091R0P_1Tew@mail.gmail.com
2019-04-23 18:51:30 -04:00
Tom Lane e0fb4c9d01 Remove useless comment.
Commit e439c6f0c removed IndexStmt.relationId, but not the comment
that had been added to explain it.  Said comment was therefore
very confusing.
2019-04-23 17:17:26 -04:00
Peter Geoghegan 9b10926263 Prevent O(N^2) unique index insertion edge case.
Commit dd299df8 made nbtree treat heap TID as a tiebreaker column,
establishing the principle that there is only one correct location (page
and page offset number) for every index tuple, no matter what.
Insertions of tuples into non-unique indexes proceed as if heap TID
(scan key's scantid) is just another user-attribute value, but
insertions into unique indexes are more delicate.  The TID value in
scantid must initially be omitted to ensure that the unique index
insertion visits every leaf page that duplicates could be on.  The
scantid is set once again after unique checking finishes successfully,
which can force _bt_findinsertloc() to step right one or more times, to
locate the leaf page that the new tuple must be inserted on.

Stepping right within _bt_findinsertloc() was assumed to occur no more
frequently than stepping right within _bt_check_unique(), but there was
one important case where that assumption was incorrect: inserting a
"duplicate" with NULL values.  Since _bt_check_unique() didn't do any
real work in this case, it wasn't appropriate for _bt_findinsertloc() to
behave as if it was finishing off a conventional unique insertion, where
any existing physical duplicate must be dead or recently dead.
_bt_findinsertloc() might have to grovel through a substantial portion
of all of the leaf pages in the index to insert a single tuple, even
when there were no dead tuples.

To fix, treat insertions of tuples with NULLs into a unique index as if
they were insertions into a non-unique index: never unset scantid before
calling _bt_search() to descend the tree, and bypass _bt_check_unique()
entirely.  _bt_check_unique() is no longer responsible for incoming
tuples with NULL values.

Discussion: https://postgr.es/m/CAH2-Wzm08nr+JPx4jMOa9CGqxWYDQ-_D4wtPBiKghXAUiUy-nQ@mail.gmail.com
2019-04-23 10:33:57 -07:00
Tom Lane f4a3fdfbdc Avoid order-of-execution problems with ALTER TABLE ADD PRIMARY KEY.
Up to now, DefineIndex() was responsible for adding attnotnull constraints
to the columns of a primary key, in any case where it hadn't been
convenient for transformIndexConstraint() to mark those columns as
is_not_null.  It (or rather its minion index_check_primary_key) did this
by executing an ALTER TABLE SET NOT NULL command for the target table.

The trouble with this solution is that if we're creating the index due
to ALTER TABLE ADD PRIMARY KEY, and the outer ALTER TABLE has additional
sub-commands, the inner ALTER TABLE's operations executed at the wrong
time with respect to the outer ALTER TABLE's operations.  In particular,
the inner ALTER would perform a validation scan at a point where the
table's storage might be inconsistent with its catalog entries.  (This is
on the hairy edge of being a security problem, but AFAICS it isn't one
because the inner scan would only be interested in the tuples' null
bitmaps.)  This can result in unexpected failures, such as the one seen
in bug #15580 from Allison Kaptur.

To fix, let's remove the attempt to do SET NOT NULL from DefineIndex(),
reducing index_check_primary_key's role to verifying that the columns are
already not null.  (It shouldn't ever see such a case, but it seems wise
to keep the check for safety.)  Instead, make transformIndexConstraint()
generate ALTER TABLE SET NOT NULL subcommands to be executed ahead of
the ADD PRIMARY KEY operation in every case where it can't force the
column to be created already-not-null.  This requires only minor surgery
in parse_utilcmd.c, and it makes for a much more satisfying spec for
transformIndexConstraint(): it's no longer having to take it on faith
that someone else will handle addition of NOT NULL constraints.

To make that work, we have to move the execution of AT_SetNotNull into
an ALTER pass that executes ahead of AT_PASS_ADD_INDEX.  I moved it to
AT_PASS_COL_ATTRS, and put that after AT_PASS_ADD_COL to avoid failure
when the column is being added in the same command.  This incidentally
fixes a bug in the only previous usage of AT_PASS_COL_ATTRS, for
AT_SetIdentity: it didn't work either for a newly-added column.

Playing around with this exposed a separate bug in ALTER TABLE ONLY ...
ADD PRIMARY KEY for partitioned tables.  The intent of the ONLY modifier
in that context is to prevent doing anything that would require holding
lock for a long time --- but the implied SET NOT NULL would recurse to
the child partitions, and do an expensive validation scan for any child
where the column(s) were not already NOT NULL.  To fix that, invent a
new ALTER subcommand AT_CheckNotNull that just insists that a child
column be already NOT NULL, and apply that, not AT_SetNotNull, when
recursing to children in this scenario.  This results in a slightly laxer
definition of ALTER TABLE ONLY ... SET NOT NULL for partitioned tables,
too: that command will now work as long as all children are already NOT
NULL, whereas before it just threw up its hands if there were any
partitions.

In passing, clean up the API of generateClonedIndexStmt(): remove a
useless argument, ensure that the output argument is not left undefined,
update the header comment.

A small side effect of this change is that no-such-column errors in ALTER
TABLE ADD PRIMARY KEY now produce a different message that includes the
table name, because they are now detected by the SET NOT NULL step which
has historically worded its error that way.  That seems fine to me, so
I didn't make any effort to avoid the wording change.

The basic bug #15580 is of very long standing, and these other bugs
aren't new in v12 either.  However, this is a pretty significant change
in the way ALTER TABLE ADD PRIMARY KEY works.  On balance it seems best
not to back-patch, at least not till we get some more confidence that
this patch has no new bugs.

Patch by me, but thanks to Jie Zhang for a preliminary version.

Discussion: https://postgr.es/m/15580-d1a6de5a3d65da51@postgresql.org
Discussion: https://postgr.es/m/1396E95157071C4EBBA51892C5368521017F2E6E63@G08CNEXMBPEKD02.g08.fujitsu.local
2019-04-23 12:25:27 -04:00
Tom Lane c06e3550dc Don't request pretty-printed output from xmlNodeDump().
xml.c passed format = 1 to xmlNodeDump(), resulting in sometimes getting
extra whitespace (newlines + spaces) in the output.  We don't really want
that, first because whitespace might be semantically significant in some
XML uses, and second because it happens only very inconsistently.  Only
one case in our regression tests is affected.

This potentially affects the results of xpath() and the XMLTABLE construct,
when emitting nodeset values.

Note that the older code in contrib/xml2 doesn't do this; it seems
to have been an aboriginal bad decision in commit ea3b212fe.

While this definitely seems like a bug to me, the small number of
complaints to date argues against back-patching a behavioral change.
Hence, fix in HEAD only, at least for now.

Per report from Jean-Marc Voillequin.

Discussion: https://postgr.es/m/1EC8157EB499BF459A516ADCF135ADCE3A23A9CA@LON-WGMSX712.ad.moodys.net
2019-04-23 10:51:07 -04:00
Michael Paquier ccae190b91 Fix detection of passwords hashed with MD5 or SCRAM-SHA-256
This commit fixes a couple of issues related to the way password
verifiers hashed with MD5 or SCRAM-SHA-256 are detected, leading to
being able to store in catalogs passwords which do not follow the
supported hash formats:
- A MD5-hashed entry was checked based on if its header uses "md5" and
if the string length matches what is expected.  Unfortunately the code
never checked if the hash only used hexadecimal characters, as reported
by Tom Lane.
- A SCRAM-hashed entry was checked based on only its header, which
should be "SCRAM-SHA-256$", but it never checked for any fields
afterwards, as reported by Jonathan Katz.

Backpatch down to v10, which is where SCRAM has been introduced, and
where password verifiers in plain format have been removed.

Author: Jonathan Katz
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/016deb6b-1f0a-8e9f-1833-a8675b170aa9@postgresql.org
Backpatch-through: 10
2019-04-23 15:43:21 +09:00
Andres Freund b5f58cf213 Convert gist to compute page level xid horizon on primary.
Due to parallel development, gist added the missing conflict
information in c952eae52a, while 558a9165e0 moved that computation
to the primary for the index types that already had it.  Thus adapt
gist to also compute on the primary, using
index_compute_xid_horizon_for_tuples() instead of its own copy of the
logic.

This also adds pg_waldump support for XLOG_GIST_DELETE records, which
previously was not properly present.

Bumps WAL version.

Author: Andres Freund
Discussion: https://postgr.es/m/20190406050243.bszosdg4buvabfrt@alap3.anarazel.de
2019-04-22 14:28:30 -07:00
Tomas Vondra d08c44f7a4 Fix mvdistinct and dependencies size calculations
The formulas used to calculate size while (de)serializing mvndistinct
and functional dependencies were based on offset() of the structs. But
that is incorrect, because the structures are not copied directly, we
we copy the individual fields directly.

At the moment this works fine, because there is no alignment padding
on any platform we support. But it might break if we ever added some
fields into any of the structs, for example. It's also confusing.

Fixed by reworking the macros to directly sum sizes of serialized
fields. The macros are now useful only for serialiation, so there is
no point in keeping them in the public header file. So make them
private by moving them to the .c files.

Also adds a couple more asserts to check the serialization, and fixes
an incorrect allocation of MVDependency instead of (MVDependency *).

Reported-By: Tom Lane
Discussion: https://postgr.es/m/29785.1555365602@sss.pgh.pa.us
2019-04-21 20:23:34 +02:00
Stephen Frost eb882a1b71 GSSAPI: Improve documentation and tests
The GSSAPI encryption patch neglected to update the protocol
documentation to describe how to set up a GSSAPI encrypted connection
from a client to the server, so fix that by adding the appropriate
documentation to protocol.sgml.

The tests added for encryption support were overly long and couldn't be
run in parallel due to race conditions; this was largely because each
test was setting up its own KDC to perform the tests.  Instead, merge
the authentication tests and the encryption tests into the original
test, where we only create one KDC to run the tests with.  Also, have
the tests check what the server's opinion is of the connection and if it
was GSS authenticated or encrypted using the pg_stat_gssapi view.

In passing, fix the libpq label for GSSENC-Mode to be consistent with
the "PGGSSENCMODE" environment variable.

Missing protocol documentation pointed out by Michael Paquier.
Issues with the tests pointed out by Tom Lane and Peter Eisentraut.

Refactored tests and added documentation by me.

Reviewed by Robbie Harwood (protocol documentation) and Michael Paquier
(rework of the tests).
2019-04-19 21:22:22 -04:00
Andres Freund b8b94ea129 Fix slot type issue for fuzzy distance index scan over out-of-core table AM.
For amcanreorderby scans the nodeIndexscan.c's reorder queue holds
heap tuples, but the underlying table likely does not. Before this fix
we'd return different types of slots, depending on whether the tuple
came from the reorder queue, or from the index + table.

While that could be fixed by signalling that the node doesn't return a
fixed type of slot, it seems better to instead remove the separate
slot for the reorder queue, and use ExecForceStoreHeapTuple() to store
tuples from the queue. It's not particularly common to need
reordering, after all.

This reverts most of the iss_ReorderQueueSlot related changes to
nodeIndexscan.c made in 1a0586de36, except that now
ExecForceStoreHeapTuple() is used instead of ExecStoreHeapTuple().

Noticed when testing zheap against the in-core version of tableam.

Author: Andres Freund
2019-04-19 11:42:37 -07:00
Andres Freund 88e6ad3054 Fix two memory leaks around force-storing tuples in slots.
As reported by Tom, when ExecStoreMinimalTuple() had to perform a
conversion to store the minimal tuple in the slot, it forgot to
respect the shouldFree flag, and leaked the tuple into the current
memory context if true.  Fix that by freeing the tuple in that case.

Looking at the relevant code made me (Andres) realize that not having
the shouldFree parameter to ExecForceStoreHeapTuple() was a bad
idea. Some callers had to locally implement the necessary logic, and
in one case it was missing, creating a potential per-group leak in
non-hashed aggregation.

The choice to not free the tuple in ExecComputeStoredGenerated() is
not pretty, but not introduced by this commit - I'll start a separate
discussion about it.

Reported-By: Tom Lane
Discussion: https://postgr.es/m/366.1555382816@sss.pgh.pa.us
2019-04-19 11:39:56 -07:00
Tom Lane 4d5840cea9 Fix problems with auto-held portals.
HoldPinnedPortals() did things in the wrong order: it must not mark
a portal autoHeld until it's been successfully held.  Otherwise,
a failure while persisting the portal results in a server crash
because we think the portal is in a good state when it's not.

Also add a check that portal->status is READY before attempting to
hold a pinned portal.  We have such a check before the only other
use of HoldPortal(), so it seems unwise not to check it here.

Lastly, rethink the responsibility for where to call HoldPinnedPortals.
The comment for it imagined that it was optional for any individual PL
to call it or not, but that cannot be the case: if some outer level of
procedure has a pinned portal, failing to persist it when an inner
procedure commits is going to be trouble.  Let's have SPI do it instead
of the individual PLs.  That's not a complete solution, since in theory
a PL might not be using SPI to perform commit/rollback, but such a PL
is going to have to be aware of lots of related requirements anyway.
(This change doesn't cause an API break for any external PLs that might
be calling HoldPinnedPortals per the old regime, because calling it
twice during a commit or rollback sequence won't hurt.)

Per bug #15703 from Julian Schauder.  Back-patch to v11 where this code
came in.

Discussion: https://postgr.es/m/15703-c12c5bc0ea34ba26@postgresql.org
2019-04-19 11:20:37 -04:00
Michael Paquier 148266fa35 Fix collection of typos and grammar mistakes in docs and comments
Author: Justin Pryzby
Discussion: https://postgr.es/m/20190330224333.GQ5815@telsasoft.com
2019-04-19 16:57:40 +09:00
Michael Paquier 5a9323eab6 Remove dependency to pageinspect in recovery tests
If contrib/pageinspect is not installed, this causes the test checking
the minimum recovery point to fail.  The point is that the dependency
with pageinspect is not really necessary as the test does also all
checks with an offline cluster by scanning directly the on-disk pages,
which is enough for the purpose of the test.

Per complaint from Tom Lane.

Author: Michael Paquier
Discussion: https://postgr.es/m/17806.1555566345@sss.pgh.pa.us
2019-04-19 15:51:23 +09:00
Andres Freund 75e03eabea Fix potential use-after-free for BEFORE UPDATE row triggers on non-core AMs.
When such a trigger returns the old row version, it naturally get
stored in the slot for the trigger result. When a table AMs doesn't
store HeapTuples internally, ExecBRUpdateTriggers() frees the old row
version passed to triggers - but before this fix it might still be
referenced by the slot holding the new tuple.

Noticed when running the out-of-core zheap AM against the in-core
version of tableam.

Author: Andres Freund
2019-04-18 17:53:54 -07:00
Peter Eisentraut bb385c4fb0 Fix handling of temp and unlogged tables in FOR ALL TABLES publications
If a FOR ALL TABLES publication exists, temporary and unlogged tables
are ignored for publishing changes.  But CheckCmdReplicaIdentity()
would still check in that case that such a table has a replica
identity set before accepting updates.  To fix, have
GetRelationPublicationActions() return that such a table publishes no
actions.

Discussion: https://www.postgresql.org/message-id/f3f151f7-c4dd-1646-b998-f60bd6217dd3@2ndquadrant.com
2019-04-18 08:55:55 +02:00
Andres Freund 4d01835927 pg_dump: Remove stray option parsing support for -o.
I (Andres) missed this in 578b229718, the removal of WITH OIDS
support.

Author: Daniel Verite
Discussion: https://postgr.es/m/f06e9735-3717-4904-8c95-47d0b9c3bb10@manitou-mail.org
2019-04-17 17:28:02 -07:00
Alvaro Herrera 421a2c4832 Tie loose ends in psql's new \dP command
* Remove one unnecessary pg_class join in SQL command.  Not needed,
  because we use a regclass cast instead.

* Doc: refer to "partitioned relations" rather than specifically tables,
  since indexes are also displayed.

* Rename "On table" column to "Table", for consistency with \di.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20190407212525.GB10080@telsasoft.com
2019-04-17 18:38:49 -04:00
Alvaro Herrera b036982db7 psql: display tablespace for partitioned indexes
Nothing was shown previously.
2019-04-17 18:17:43 -04:00
Bruce Momjian fb9c475597 postgresql.conf.sample: add proper defaults for include actions
Previously, include actions include_dir, include_if_exists, and include
listed commented-out values which were not the defaults, which is
inconsistent with other entries.  Instead, replace them with '', which
is the default value.

Reported-by: Emanuel Araújo

Discussion: https://postgr.es/m/CAMuTAkYMx6Q27wpELDR3_v9aG443y7ZjeXu15_+1nGUjhMWOJA@mail.gmail.com

Backpatch-through: 9.4
2019-04-17 18:12:10 -04:00
Tom Lane 1a75c1d0c5 Fix unportable code in pgbench.
The buildfarm points out that UINT64_FORMAT might not work with sscanf;
it's calibrated for our printf implementation, which might not agree
with the platform-supplied sscanf.  Fall back to just accepting an
unsigned long, which is already more than the documentation promises.

Oversight in e6c3ba7fb; back-patch to v11, as that was.
2019-04-17 17:30:29 -04:00
Tom Lane 8cde7f4948 Fix assorted minor bogosity in GSSAPI transport error messages.
I noted that some buildfarm members were complaining about %ld being
used to format values that are (probably) declared size_t.  Use %zu
instead, and insert a cast just in case some versions of the GSSAPI
API declare the length field differently.  While at it, clean up
gratuitous differences in wording of equivalent messages, show
the complained-of length in all relevant messages not just some,
include trailing newline where needed, adjust random deviations
from project-standard code layout and message style, etc.
2019-04-17 17:06:50 -04:00
Tom Lane b4f96d69ad Minor jsonpath fixes.
Restore missed "make clean" rule, fix misspelling.

John Naylor

Discussion: https://postgr.es/m/CACPNZCt5B8jDCCGQiFoSuqmg-za_NCy4QDioBTLaNRih9+-bXg@mail.gmail.com
2019-04-17 13:37:00 -04:00
Magnus Hagander 252b707bc4 Return NULL for checksum failures if checksums are not enabled
Returning 0 could falsely indicate that there is no problem. NULL
correctly indicates that there is no information about potential
problems.

Also return 0 as numbackends instead of NULL for shared objects (as no
connection can be made to a shared object only).

Author: Julien Rouhaud <rjuju123@gmail.com>
Reviewed-by: Robert Treat <rob@xzilla.net>
2019-04-17 13:51:48 +02:00
Michael Paquier 9010156445 Fix thinko introduced by 82a5649 in slot.c
When saving a replication slot, failing to close the temporary path used
to save the slot information is considered as a failure and reported as
such.  However the code forgot to leave immediately as other failure
paths do.

Noticed while looking up at this area of the code for another patch.
2019-04-17 10:01:22 +09:00
Michael Paquier 47ac2033d4 Simplify some ERROR paths clearing wait events and transient files
Transient files and wait events get normally cleaned up when seeing an
exception (be it in the context of a transaction for a backend or
another process like the checkpointer), hence there is little point in
complicating error code paths to do this work.  This shaves a bit of
code, and removes some extra handling with errno which needed to be
preserved during the cleanup steps done.

Reported-by: Masahiko Sawada
Author: Michael Paquier
Reviewed-by: Tom Lane, Masahiko Sawada
Discussion: https://postgr.es/m/CAD21AoDhHYVq5KkXfkaHhmjA-zJYj-e4teiRAJefvXuKJz1tKQ@mail.gmail.com
2019-04-17 09:51:45 +09:00
Michael Paquier a6dcf9df4d Rework handling of invalid indexes with REINDEX CONCURRENTLY
Per discussion with others, allowing REINDEX INDEX CONCURRENTLY to work
for invalid indexes when working directly on them can have a lot of
value to unlock situations with invalid indexes without having to use a
dance involving DROP INDEX followed by an extra CREATE INDEX
CONCURRENTLY (which would not work for indexes with constraint
dependency anyway).  This also does not create extra bloat on the
relation involved as this works on individual indexes, so let's enable
it.

Note that REINDEX TABLE CONCURRENTLY still bypasses invalid indexes as
we don't want to bloat the number of indexes defined on a relation in
the event of multiple and successive failures of REINDEX CONCURRENTLY.

More regression tests are added to cover those behaviors, using an
invalid index created with CREATE INDEX CONCURRENTLY.

Reported-by: Dagfinn Ilmari Mannsåker, Álvaro Herrera
Author: Michael Paquier
Reviewed-by: Peter Eisentraut, Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/20190411134947.GA22043@alvherre.pgsql
2019-04-17 09:33:51 +09:00
Michael Paquier 5ed4b123b6 Remove duplicate assignment when initializing logical decoder context
The private data in the WAL reader is already getting set when
allocating it.

Author: Antonin Houska
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/30563.1555329094@localhost
2019-04-16 15:08:38 +09:00
Noah Misch e12a472612 Don't write to stdin of a test process that could have already exited.
Instead, close that stdin.  Per buildfarm member conchuela.  Back-patch
to 9.6, where the test was introduced.

Discussion: https://postgr.es/m/26478.1555373328@sss.pgh.pa.us
2019-04-15 18:13:44 -07:00
Tom Lane dde7fb7836 Use [FLEXIBLE_ARRAY_MEMBER] not [1] in MultiSortSupportData.
This struct seems to have not gotten the word about preferred
coding style for variable-length arrays.
2019-04-15 19:32:44 -04:00
Tomas Vondra dbb984128e Convert pre-existing stats_ext tests to new style
The regression tests added in commit 7300a69950 test cardinality
estimates using a function that extracts the interesting pieces
from the EXPLAIN output, instead of testing the whole plan. That
seems both easier to understand and less fragile, so this applies
the same approach to pre-existing tests of ndistinct coefficients
and functional dependencies.

Discussion: https://postgr.es/m/dfdac334-9cf2-2597-fb27-f0fb3753f435@2ndquadrant.com
2019-04-16 00:02:22 +02:00
Tomas Vondra 3824ca30d1 Fix pg_mcv_list deserialization
The memcpy() was copying type OIDs in the wrong direction, so the
deserialized MCV list always had them as 0. This is mostly harmless
except when printing the data in pg_mcv_list_items(), in which case
it reported

    ERROR:  cache lookup failed for type 0

Also added a simple regression test for pg_mcv_list_items() function,
printing a single-item MCV list.

Reported-By: Dean Rasheed
Discussion: https://postgr.es/m/CAEZATCX6T0iDTTZrqyec4Cd6b4yuL7euu4=rQRXaVBAVrUi1Cg@mail.gmail.com
2019-04-16 00:01:39 +02:00
Tom Lane 4b40e44f07 Fix failure with textual partition hash keys.
Commit 5e1963fb7 overlooked two places in partbounds.c that now
need to pass a collation identifier to the hash functions for
a partition key column.

Amit Langote, per report from Jesper Pedersen

Discussion: https://postgr.es/m/a620f85a-42ab-e0f3-3337-b04b97e2e2f5@redhat.com
2019-04-15 16:47:09 -04:00
Tom Lane 47169c2550 Avoid possible regression test instability in timestamp.sql.
Concurrent autovacuum could result in a change in the order of the
live rows in timestamp_tbl.  While this would not happen with the
default autovacuum parameters, it's fairly easy to hit if
autovacuum_vacuum_threshold is made small enough to allow autovac
to decide to process this table.  That's a stumbling block for trying
to exercise autovacuum aggressively using the core regression tests.

To fix, replace an unqualified DELETE with a TRUNCATE.  There's a
similar DELETE just above (and no order-sensitive queries between),
so this doesn't lose any test coverage and might indeed be argued
to improve it.

Discussion: https://postgr.es/m/17428.1555348950@sss.pgh.pa.us
2019-04-15 16:20:01 -04:00
Alexander Korotkov 1e87198182 Fix division by zero in _bt_vacuum_needs_cleanup()
Checks inside _bt_vacuum_needs_cleanup() allow division by zero to happen when
metad->btm_last_cleanup_num_heap_tuples == 0.  This commit adjusts the
expression so that no division by zero might happen.

Reported-by: Piotr Stefaniak
Discussion: https://postgr.es/m/DB8PR03MB5931C41F7787A95313F08322F22A0%40DB8PR03MB5931.eurprd03.prod.outlook.com
Reviewed-by: Masahiko Sawada
Backpatch-through: 11
2019-04-15 20:20:43 +03:00
Etsuro Fujita 3a45321a49 Fix thinko in ExecCleanupTupleRouting().
Commit 3f2393edef changed ExecCleanupTupleRouting() so that it skipped
cleaning up subplan resultrels before calling EndForeignInsert(), but
that would cause an issue: when those resultrels were foreign tables,
the FDWs would fail to shut down.  Repair by skipping it after calling
EndForeignInsert() as before.

Author: Etsuro Fujita
Reviewed-by: David Rowley and Amit Langote
Discussion: https://postgr.es/m/5CAF3B8F.2090905@lab.ntt.co.jp
2019-04-15 19:01:09 +09:00
Peter Eisentraut abb9c63b2c Unbreak index optimization for LIKE on bytea
The same code is used to handle both text and bytea, but bytea is not
collation-aware, so we shouldn't call get_collation_isdeterministic()
in that case, since that will error out with an invalid collation.

Reported-by: Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/CAM2%2B6%3DWaf3qJ1%3DyVTUH8_yG-SC0xcBMY%2BSFLhvKKNnWNXSUDBw%40mail.gmail.com
2019-04-15 09:29:17 +02:00
Michael Paquier c34677fdaa Fix SHOW ALL command for non-superusers with replication connection
Since Postgres 10, SHOW commands can be triggered with replication
connections in a WAL sender context, however it missed that a
transaction context is needed for syscache lookups.  This commit makes
sure that the syscache lookups can happen correctly by setting a
transaction context when running SHOW commands in a WAL sender.

Superuser-only parameters can be displayed using SHOW commands not only
to superusers, but also to members of system role pg_read_all_settings,
which requires a syscache lookup to check if the connected role is a
member of this system role or not, or the instance crashes.  Superusers
do not need to check the syscache so it worked correctly in this case.

New tests are added to cover this issue.

Reported-by: Alexander Kukushkin
Author: Michael Paquier
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/15734-2daa8761eeed8e20@postgresql.org
Backpatch-through: 10
2019-04-15 12:34:32 +09:00
Noah Misch 4ab02e8156 Test both 0.0.0.0 and 127.0.0.x addresses to find a usable port.
Commit c098509927 changed
PostgresNode::get_new_node() to probe 0.0.0.0 instead of 127.0.0.1, but
the new test was less effective for Windows native Perl.  This increased
the failure rate of buildfarm members bowerbird and jacana.  Instead,
test 0.0.0.0 and concrete addresses.  This restores the old level of
defense, but the algorithm is still subject to its longstanding time of
check to time of use race condition.  Back-patch to 9.6, like the
previous change.

Discussion: https://postgr.es/m/GrdLgAdUK9FdyZg8VIcTDKVOkys122ZINEb3CjjoySfGj2KyPiMKTh1zqtRp0TAD7FJ27G-OBB3eplxIB5GhcQH5o8zzGZfp0MuJaXJxVxk=@yesql.se
2019-04-14 20:02:19 -07:00
Michael Paquier d9f543e9e9 Switch TAP tests of pg_rewind to use non-superuser role, take two
Up to now the tests of pg_rewind have been using a superuser for all its
tests (which is the default of many tests actually, and something that
ought to be reviewed) when involving an online source server, still it
is possible to use a non-superuser role to do that as long as this role
is granted permissions to execute all the source-side functions used for
the rewind.  This is possible since v11, and was already documented as
of bfc8068.

PostgresNode::init is extended so as callers of this routine can add
extra options to configure the authentication of a new node, which gets
used by this commit, and allows the tests to work properly on Windows
where SSPI is used.

This will allow to catch up easily any change in pg_rewind if the tool
begins to use more backend-side functions, so as the properties
introduced by v11 are kept.

Per suggestion from Peter Eisentraut.

Author: Michael Paquier
Reviewed-by: Magnus Hagander
Discussion: https://postgr.es/m/20190411041336.GM2728@paquier.xyz
2019-04-14 18:47:51 +09:00
Noah Misch 9daefff122 MSYS: Translate REGRESS_SHLIB to a Windows file name.
Per buildfarm member jacana.  Back-patch to v11; earlier branches skip
the affected test under msys.

Discussion: https://postgr.es/m/GrdLgAdUK9FdyZg8VIcTDKVOkys122ZINEb3CjjoySfGj2KyPiMKTh1zqtRp0TAD7FJ27G-OBB3eplxIB5GhcQH5o8zzGZfp0MuJaXJxVxk=@yesql.se
2019-04-14 00:42:34 -07:00
Noah Misch 947a35014f When Perl "kill(9, ...)" fails, try "pg_ctl kill".
Per buildfarm member jacana, the former fails under msys Perl 5.8.8.
Back-patch to 9.6, like the code in question.

Discussion: https://postgr.es/m/GrdLgAdUK9FdyZg8VIcTDKVOkys122ZINEb3CjjoySfGj2KyPiMKTh1zqtRp0TAD7FJ27G-OBB3eplxIB5GhcQH5o8zzGZfp0MuJaXJxVxk=@yesql.se
2019-04-13 11:09:27 -07:00
Tom Lane 5f1433ac5e Prevent memory leaks associated with relcache rd_partcheck structures.
The original coding of generate_partition_qual() just copied the list
of predicate expressions into the global CacheMemoryContext, making it
effectively impossible to clean up when the owning relcache entry is
destroyed --- the relevant code in RelationDestroyRelation() only managed
to free the topmost List header :-(.  This resulted in a session-lifespan
memory leak whenever a table partition's relcache entry is rebuilt.
Fortunately, that's not normally a large data structure, and rebuilds
shouldn't occur all that often in production situations; but this is
still a bug worth fixing back to v10 where the code was introduced.

To fix, put the cached expression tree into its own small memory context,
as we do with other complicated substructures of relcache entries.
Also, deal more honestly with the case that a partition has an empty
partcheck list; while that probably isn't a case that's very interesting
for production use, it's legal.

In passing, clarify comments about how partitioning-related relcache
data structures are managed, and add some Asserts that we're not leaking
old copies when we overwrite these data fields.

Amit Langote and Tom Lane

Discussion: https://postgr.es/m/7961.1552498252@sss.pgh.pa.us
2019-04-13 13:22:26 -04:00
Noah Misch c098509927 Consistently test for in-use shared memory.
postmaster startup scrutinizes any shared memory segment recorded in
postmaster.pid, exiting if that segment matches the current data
directory and has an attached process.  When the postmaster.pid file was
missing, a starting postmaster used weaker checks.  Change to use the
same checks in both scenarios.  This increases the chance of a startup
failure, in lieu of data corruption, if the DBA does "kill -9 `head -n1
postmaster.pid` && rm postmaster.pid && pg_ctl -w start".  A postmaster
will no longer stop if shmat() of an old segment fails with EACCES.  A
postmaster will no longer recycle segments pertaining to other data
directories.  That's good for production, but it's bad for integration
tests that crash a postmaster and immediately delete its data directory.
Such a test now leaks a segment indefinitely.  No "make check-world"
test does that.  win32_shmem.c already avoided all these problems.  In
9.6 and later, enhance PostgresNode to facilitate testing.  Back-patch
to 9.4 (all supported versions).

Reviewed (in earlier versions) by Daniel Gustafsson and Kyotaro HORIGUCHI.

Discussion: https://postgr.es/m/20190408064141.GA2016666@rfd.leadboat.com
2019-04-12 22:36:38 -07:00
Michael Paquier db8db624e8 Revert "Switch TAP tests of pg_rewind to use a role with minimal permissions"
This reverts commit d4e2a84, which added a new user with limited
permissions to run the TAP tests of pg_rewind.  Buildfarm machine
members on Windows jacana and bowerbird have been complaining about
that, the new role not being able to run the rewind because SSPI is not
configured to allow it.

Fixing the test requires passing down directly the new user to
pg_regress with --create-role so as SSPI can work properly.

Reported-by: Andrew Dunstan
Discussion: https://postgr.es/m/3cd43d33-f415-cc41-ade3-7230ab15b2c9@2ndQuadrant.com
2019-04-13 13:20:21 +09:00
Magnus Hagander 77bd49adba Show shared object statistics in pg_stat_database
This adds a row to the pg_stat_database view with datoid 0 and datname
NULL for those objects that are not in a database. This was added
particularly for checksums, but we were already tracking more satistics
for these objects, just not returning it.

Also add a checksum_last_failure column that holds the timestamptz of
the last checksum failure that occurred in a database (or in a
non-dataabase file), if any.

Author: Julien Rouhaud <rjuju123@gmail.com>
2019-04-12 14:04:50 +02:00
Peter Eisentraut ef6f30fe77 Fix REINDEX CONCURRENTLY of partitions
In case of a partition index, when swapping the old and new index, we
also need to attach the new index as a partition and detach the old
one.  Also, to handle partition indexes, we not only need to change
dependencies referencing the index, but also dependencies of the index
referencing something else.  The previous code did this only
specifically for a constraint, but we also need to do this for
partitioned indexes.  So instead write a generic function that does it
for all dependencies.

Author: Michael Paquier <michael@paquier.xyz>
Author: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Discussion: https://www.postgresql.org/message-id/flat/DF4PR8401MB11964EDB77C860078C343BEBEE5A0%40DF4PR8401MB1196.NAMPRD84.PROD.OUTLOOK.COM#154df1fedb735190a773481765f7b874
2019-04-12 08:36:05 +02:00
Thomas Munro f7feb020c3 Fix GetNewTransactionId()'s interaction with xidVacLimit.
Commit ad308058 switched to returning a FullTransactionId, but
failed to load the potentially updated value in the case where
xidVacLimit is reached and we release and reacquire the lock.
Repair, closing bug #15727.

While reviewing that commit, also fix the size computation used
by EstimateTransactionStateSize() and switch to the mul_size()
macro traditionally used in such expressions.

Author: Thomas Munro
Reported-by: Roman Zharkov
Discussion: https://postgr.es/m/15727-0be246e7d852d229%40postgresql.org
2019-04-12 16:47:50 +12:00
Michael Paquier d87ab88686 Fix typos in reloptions.c
Author: Kirk Jamison
Discussion: https://postgr.es/m/D09B13F772D2274BB348A310EE3027C6493463@g01jpexmbkw24
2019-04-12 12:56:38 +09:00
Michael Paquier d4e2a843e6 Switch TAP tests of pg_rewind to use a role with minimal permissions
Up to now the tests of pg_rewind have been using a superuser for all the
tests (which is the default of many tests actually, and something that
ought to be reviewed) when involving an online source server, still it
is possible to use a non-superuser role to do that as long as this role
is granted permissions to execute all the source-side functions used for
the rewind.  This is possible since v11, and was already documented as
of bfc8068.

This will allow to catch up easily any change in pg_rewind if the tool
begins to use more backend-side functions, so as the properties
introduced by v11 are kept.

Per suggestion from Peter Eisentraut.

Author: Michael Paquier
Reviewed-by: Magnus Hagander
Discussion: https://postgr.es/m/20190411041336.GM2728@paquier.xyz
2019-04-12 10:46:43 +09:00