Tablespaces can be relocated in plain backup mode by specifying one or
more -T olddir=newdir options.
Author: Steeve Lennmark <steevel@handeldsbanken.se>
Reviewed-by: Peter Eisentraut <peter_e@gmx.net>
Coverity identified a number of places in which it couldn't prove that a
string being copied into a fixed-size buffer would fit. We believe that
most, perhaps all of these are in fact safe, or are copying data that is
coming from a trusted source so that any overrun is not really a security
issue. Nonetheless it seems prudent to forestall any risk by using
strlcpy() and similar functions.
Fixes by Peter Eisentraut and Jozef Mlich based on Coverity reports.
In addition, fix a potential null-pointer-dereference crash in
contrib/chkpass. The crypt(3) function is defined to return NULL on
failure, but chkpass.c didn't check for that before using the result.
The main practical case in which this could be an issue is if libc is
configured to refuse to execute unapproved hashing algorithms (e.g.,
"FIPS mode"). This ideally should've been a separate commit, but
since it touches code adjacent to one of the buffer overrun changes,
I included it in this commit to avoid last-minute merge issues.
This issue was reported by Honza Horak.
Security: CVE-2014-0065 for buffer overruns, CVE-2014-0066 for crypt()
We used to have externs for getopt() and its API variables scattered
all over the place. Now that we find we're going to need to tweak the
variable declarations for Cygwin, it seems like a good idea to have
just one place to tweak.
In this commit, the variables are declared "#ifndef HAVE_GETOPT_H".
That may or may not work everywhere, but we'll soon find out.
Andres Freund
Adjust handleCopyOut() to stop trying to write data once it's failed
one time. For typical cases such as out-of-disk-space or broken-pipe,
additional attempts aren't going to do anything but waste time, and
in any case clean truncation of the output seems like a better behavior
than randomly dropping blocks in the middle.
Also remove dubious (and misleadingly documented) attempt to force our way
out of COPY_OUT state if libpq didn't do that. If we did have a situation
like that, it'd be a bug in libpq and would be better fixed there, IMO.
We can hope that commit fa4440f516 took care
of any such problems, anyway.
Also fix longstanding bug in handleCopyIn(): PQputCopyEnd() only supports
a non-null errormsg parameter in protocol version 3, and will actively
fail if one is passed in version 2. This would've made our attempts
to get out of COPY_IN state after a failure into infinite loops when
talking to pre-7.4 servers.
Back-patch the COPY_OUT state change business back to 9.2 where it was
introduced, and the other two fixes into all supported branches.
If an error occurs in the foreground (backup) process of pg_basebackup,
and we exit in a controlled way, the background process (streaming
xlog process) would stay around and keep streaming.
This results in spurious empty lines in the server log. Instead, add
the newlines only when printing out the --echo output. In some cases,
this was already done, leading to two newlines being printed. Clean
that up as well.
From: Fabrízio de Royes Mello <fabriziomello@gmail.com>
Make ftello error-checking consistent to all calls and remove a
bit of ftello-related code which has been #if 0'd out since 2001.
Note that we are not concerned with the ftello() call under
snprintf() failing as it is just building a string to call
exit_horribly() with; printing -1 in such a case is fine.
Rather than reset errno (or just hope that its cleared already),
check just the result of the ftello for < 0 to determine if there
was an issue.
Oversight by me, pointed out by Tom.
This prevents pg_basebackup from generating excessive output when
dumping large clusters. The status is now updated once / second,
still making it possible to see that there is progress happening,
but limiting the total bandwidth.
Mika Eloranta, reviewed by Sawada Masahiko and Oskari Saarenmaa
When using verbose mode for pg_basebackup, in tar format sent to
stdout, we'd print an unitialized buffer as the filename.
Reported by Pontus Lundkvist
Improve pg_dump by checking results on various fgetc() calls which
previously were unchecked, ditto for ftello. Also clean up a couple
of very minor memory leaks by waiting to allocate structures until
after the initial check(s).
Issues spotted by Coverity.
Commit a5ff502fce was a brick shy of a load
in the backend lexer too, not just psql. Per further testing of bug #9068.
In passing, improve related comments.
Replication slots are a crash-safe data structure which can be created
on either a master or a standby to prevent premature removal of
write-ahead log segments needed by a standby, as well as (with
hot_standby_feedback=on) pruning of tuples whose removal would cause
replication conflicts. Slots have some advantages over existing
techniques, as explained in the documentation.
In a few places, we refer to the type of replication slots introduced
by this patch as "physical" slots, because forthcoming patches for
logical decoding will also have slots, but with somewhat different
properties.
Andres Freund and Robert Haas
Various places were supposing that errno could be expected to hold still
within an ereport() nest or similar contexts. This isn't true necessarily,
though in some cases it accidentally failed to fail depending on how the
compiler chanced to order the subexpressions. This class of thinko
explains recent reports of odd failures on clang-built versions, typically
missing or inappropriate HINT fields in messages.
Problem identified by Christian Kruse, who also submitted the patch this
commit is based on. (I fixed a few issues in his patch and found a couple
of additional places with the same disease.)
Back-patch as appropriate to all supported branches.
Move allocation to after we check the remote server version, to avoid
a possible, very minor, memory leak. This makes us more consistent
throughout as most places in pg_dump are done in the same way (due, in
part, to previous fixes like this).
Spotted by the Coverity scanner.
During parallel pg_dump, a worker process closing the connection caused
a minor memory leak (particularly minor as we are likely about to exit
anyway). Instead, free the memory in this case prior to returning NULL
to indicate connection closed.
Spotting by the Coverity scanner.
Back patch to 9.3 where this was introduced.
There was a bug in the psql's meta command \conninfo. When the
IP address was specified in the hostaddr and psql used it to create
a connection (i.e., psql -d "hostaddr=xxx"), \conninfo could not
display that address. This is because \conninfo got the connection
information only from PQhost() which could not return hostaddr.
This patch adds PQhostaddr(), and changes \conninfo so that it
can display not only the host name that PQhost() returns but also
the IP address which PQhostaddr() returns.
The bug has existed since 9.1 where \conninfo was introduced.
But it's too late to add new libpq function into the released versions,
so no backpatch.
krb5 has been deprecated since 8.3, and the recommended way to do
Kerberos authentication is using the GSSAPI authentication method
(which is still fully supported).
libpq retains the ability to identify krb5 authentication, but only
gives an error message about it being unsupported. Since all authentication
is initiated from the backend, there is no need to keep it at all
in the backend.
The psql Makefile was not creating $(datadir) before installing
psqlrc.sample there.
In most cases, the directory would be created in some other way, but for
the documented from-source client-only installation procedure, it could
fail.
Reported-by: Mike Blackwell <mike.blackwell@rrd.com>
The + modifier of \do didn't use to do anything, but now it adds an oprcode
column. This is useful both as an additional form of documentation of what
the operator does, and to save a step when finding out properties of the
underlying function.
Marko Tiikkaja, reviewed by Rushabh Lathia, adjusted a bit by me
Primarily, explain where to find the system-wide psqlrc file, per recent
gripe from John Sutton. Do some general wordsmithing and improve the
markup, too.
Also adjust psqlrc.sample so its comments about file location are somewhat
trustworthy. (Not sure why we bother with this file when it's empty,
but whatever.)
Back-patch to 9.2 where the startup file naming scheme was last changed.
Several previous commits have added columns to various \d queries without
updating their translate_columns[] arrays, leading to potentially incorrect
translations in NLS-enabled builds. Offenders include commit 893686762
(added prosecdef to \df+), c9ac00e6e (added description to \dc+) and
3b17efdfd (added description to \dC+). Fix those cases back to 9.3 or
9.2 as appropriate.
Since this is evidently more easily missed than one would like, in HEAD
also add an Assert that the supplied array is long enough. This requires
an API change for printQuery(), so it seems inappropriate for back
branches, but presumably all future changes will be tested in HEAD anyway.
In HEAD and 9.3, also clean up a whole lot of sloppiness in the emitted
SQL for \dy (event triggers): lack of translatability due to failing to
pass words-to-be-translated through gettext_noop(), inadequate schema
qualification, and sloppy formatting resulting in unnecessarily ugly
-E output.
Peter Eisentraut and Tom Lane, per bug #8702 from Sergey Burladyan
CREATE EVENT TRIGGER forgot to mark the event trigger as a member of its
extension, and pg_dump didn't pay any attention anyway when deciding
whether to dump the event trigger. Per report from Moshe Jacobson.
Given the obvious lack of testing here, it's rather astonishing that
ALTER EXTENSION ADD/DROP EVENT TRIGGER work, but they seem to.
There was an apparent attempt to limit the target database for
pg_restore to version 7.1.0 or later. Due to a leading zero this
was interpreted as an octal number, which allowed targets with
version numbers down to 2.87.36. The lowest actual release above
that was 6.0.0, so that was effectively the limit.
Since the success of the restore attempt will depend primarily on
on what statements were generated by the dump run, we don't want
pg_restore trying to guess whether a given target should be allowed
based on version number. Allow a connection to any version. Since
it is very unlikely that anyone would be using a recent version of
pg_restore to restore to a pre-6.0 database, this has little to no
practical impact, but it makes the code less confusing to read.
Issue reported and initial patch suggestion from Joel Jacobson
based on an article by Andrey Karpov reporting on issues found by
PVS-Studio static code analyzer. Final patch based on analysis by
Tom Lane. Back-patch to all supported branches.
This patch introduces generic support for ordered-set and hypothetical-set
aggregate functions, as well as implementations of the instances defined in
SQL:2008 (percentile_cont(), percentile_disc(), rank(), dense_rank(),
percent_rank(), cume_dist()). We also added mode() though it is not in the
spec, as well as versions of percentile_cont() and percentile_disc() that
can compute multiple percentile values in one pass over the data.
Unlike the original submission, this patch puts full control of the sorting
process in the hands of the aggregate's support functions. To allow the
support functions to find out how they're supposed to sort, a new API
function AggGetAggref() is added to nodeAgg.c. This allows retrieval of
the aggregate call's Aggref node, which may have other uses beyond the
immediate need. There is also support for ordered-set aggregates to
install cleanup callback functions, so that they can be sure that
infrastructure such as tuplesort objects gets cleaned up.
In passing, make some fixes in the recently-added support for variadic
aggregates, and make some editorial adjustments in the recent FILTER
additions for aggregates. Also, simplify use of IsBinaryCoercible() by
allowing it to succeed whenever the target type is ANY or ANYELEMENT.
It was inconsistent that it dealt with other polymorphic target types
but not these.
Atri Sharma and Andrew Gierth; reviewed by Pavel Stehule and Vik Fearing,
and rather heavily editorialized upon by Tom Lane
Previously, lookups of non-existent user names could return "Success";
it will now return "User does not exist" by resetting errno. This also
centralizes the user name lookup code in libpgport.
Report and analysis by Nicolas Marchildon; patch by me
WAL records of hint bit updates is useful to tools that want to examine
which pages have been modified. In particular, this is required to make
the pg_rewind tool safe (without checksums).
This can also be used to test how much extra WAL-logging would occur if
you enabled checksums, without actually enabling them (which you can't
currently do without re-initdb'ing).
Sawada Masahiko, docs by Samrat Revagade. Reviewed by Dilip Kumar, with
further changes by me.
When wal_level=logical, we'll log columns from the old tuple as
configured by the REPLICA IDENTITY facility added in commit
07cacba983. This makes it possible
a properly-configured logical replication solution to correctly
follow table updates even if they change the chosen key columns,
or, with REPLICA IDENTITY FULL, even if the table has no key at
all. Note that updates which do not modify the replica identity
column won't log anything extra, making the choice of a good key
(i.e. one that will rarely be changed) important to performance
when wal_level=logical is configured.
Each insert, update, or delete to a catalog table will also log
the CMIN and/or CMAX values of stamped by the current transaction.
This is necessary because logical decoding will require access to
historical snapshots of the catalog in order to decode some data
types, and the CMIN/CMAX values that we may need in order to judge
row visibility may have been overwritten by the time we need them.
Andres Freund, reviewed in various versions by myself, Heikki
Linnakangas, KONDO Mitsumasa, and many others.
pg_dumpall's charter is to be able to recreate a database cluster's
contents in a virgin installation, but it was failing to honor that
contract if the cluster had any ALTER DATABASE SET
default_transaction_read_only settings. By including a SET command
for the connection for each connection opened by pg_dumpall output,
errors are avoided and the source cluster is successfully
recreated.
There was discussion of whether to also set this for the connection
applying pg_dump output, but it was felt that it was both less
appropriate in that context, and far easier to work around.
Backpatch to all supported branches.
Ensure that the invocation command for postgres or pg_ctl runservice
double-quotes the executable's pathname; failure to do this leads to
trouble when the path contains spaces.
Also, ensure that the path ends in ".exe" in both cases and uses
backslashes rather than slashes as directory separators. The latter issue
is reported to confuse some third-party tools such as Symantec Backup Exec.
Also, rewrite the function to avoid buffer overrun issues by using a
PQExpBuffer instead of a fixed-size static buffer. Combinations of
very long executable pathnames and very long data directory pathnames
could have caused trouble before, for example.
Back-patch to all active branches, since this code has been like this
for a long while.
Naoya Anzai and Tom Lane, reviewed by Rajeev Rastogi
Previously, -d option for pg_isready was broken. When the name of the
database was specified by -d option, pg_isready failed with an error.
When the conninfo specified by -d option contained the setting of the
host name but not Numeric IP address (i.e., hostaddr), pg_isready
displayed wrong connection message. -d option could not handle a valid
URI prefix at all. This commit fixes these bugs of pg_isready.
Backpatch to 9.3, where pg_isready was introduced.
Per report from Josh Berkus and Robert Haas.
Original patch by Fabrízio de Royes Mello, heavily modified by me.
Formerly the planner had a hard-wired rule of thumb for guessing the amount
of space consumed by an aggregate function's transition state data. This
estimate is critical to deciding whether it's OK to use hash aggregation,
and in many situations the built-in estimate isn't very good. This patch
adds a column to pg_aggregate wherein a per-aggregate estimate can be
provided, overriding the planner's default, and infrastructure for setting
the column via CREATE AGGREGATE.
It may be that additional smarts will be required in future, perhaps even
a per-aggregate estimation function. But this is already a step forward.
This is extracted from a larger patch to improve the performance of numeric
and int8 aggregates. I (tgl) thought it was worth reviewing and committing
this infrastructure separately. In this commit, all built-in aggregates
are given aggtransspace = 0, so no behavior should change.
Hadi Moshayedi, reviewed by Pavel Stehule and Tomas Vondra
In --inserts and especially --column-inserts mode, we can get a useful
speedup by generating the common prefix of all a table's INSERT commands
just once, and then printing the prebuilt string for each row. This avoids
multiple invocations of fmtId() and other minor fooling around.
David Rowley
The previous coding was fairly unreadable and drew double-free warnings
from clang. I believe the double free was actually not reachable, because
PQconnectionNeedsPassword is coded to not return true if a password was
provided, so that the loop can't iterate more than twice. Nonetheless
it seems worth rewriting. No back-patch since this is just cosmetic.
pg_index.indisreplident had at one time in its development been called
indisidentity. describe.c got missed when it was renamed.
Bug introduced in commit 07cacba983.
Andres Freund
This is a similar fix as c6ec8793aa
9.2. This should never happen in 9.3 and newer since the special case
cannot happen there, but this patch synchronizes up the code so there
is no confusion on why they're different. An empty block is as harmless
in 9.3 as it was in 9.2, and can safely be ignored.
Pending patches for logical replication will use this to determine
which columns of a tuple ought to be considered as its candidate key.
Andres Freund, with minor, mostly cosmetic adjustments by me
The previous commit was intended to make psql show the full path name when
doing a \s (history save), but it was very badly implemented and would show
confusing if not outright wrong information in many situations; for
instance if the path name given to \s is absolute, or if \cd commands
involving relative paths have been issued. Consensus seems to be that
we don't especially need this functionality in \s, and certainly not in \s
alone. So revert rather than trying to fix it up. Per gripe from
Ian Barwick.
Although the bogus behavior exists in all supported versions, I'm not
back-patching, because the work created for translators (by change of
a translatable message) would probably outweigh the value of what is
after all a mostly-cosmetic change.
The parsing of WAL filenames of segments larger than > 255 was broken,
making pg_receivexlog unable to restart streaming after stopping it.
The bug was introduced by the changes in 9.3 to represent WAL segment number
as a 64-bit integer instead of two ints, log and seg. To fix, replace the
plain sscanf call with XLogFromFileName macro, which does the conversion
from log+seg to a 64-bit integer correcly.
Reported by Mika Eloranta.
This prevents the recently-added probe for shm_open() from crashing
on platforms that are impolite enough to deliver a signal rather than
returning ENOSYS for an unimplemented kernel call. At least on the
one known example (HPUX 10.20), ignoring SIGSYS does result in the
desired behavior of getting an ENOSYS error return instead.
Per discussion, we might later wish to do this in the backend as well,
but for now it seems sufficient to do it in initdb.
When we are using a C99-compliant vsnprintf implementation (which should be
most places, these days) it is worth the trouble to make use of its report
of how large the buffer needs to be to succeed. This patch adjusts
stringinfo.c and some miscellaneous usages in pg_dump to do that, relying
on the logic recently added in libpgcommon's psprintf.c. Since these
places want to know the number of bytes written once we succeed, modify the
API of pvsnprintf() to report that.
There remains near-duplicate logic in pqexpbuffer.c, but since that code
is in libpq, psprintf.c's approach of exit()-on-error isn't appropriate
for use there. Also note that I didn't bother touching the multitude
of places that call (v)snprintf without any attempt to provide a resizable
buffer.
Release-note-worthy incompatibility: the API of appendStringInfoVA()
changed. If there's any third-party code that's calling that directly,
it will need tweaking along the same lines as in this patch.
David Rowley and Tom Lane
Since an increasing number of views and foreign tables are now able
to be updated, complete with any table, view, or foreign table in
the relevant contexts. This avoids the need to use a complex
query that may be both confusing to end-users and nonperformant
to construct the list of possible completions.
Dean Rasheed, persuant to a complaint from Bernd Helme and a
suggestion from Peter Eisentraut
asprintf(), aside from not being particularly portable, has a fundamentally
badly-designed API; the psprintf() function that was added in passing in
the previous patch has a much better API choice. Moreover, the NetBSD
implementation that was borrowed for the previous patch doesn't work with
non-C99-compliant vsnprintf, which is something we still have to cope with
on some platforms; and it depends on va_copy which isn't all that portable
either. Get rid of that code in favor of an implementation similar to what
we've used for many years in stringinfo.c. Also, move it into libpgcommon
since it's not really libpgport material.
I think this patch will be enough to turn the buildfarm green again, but
there's still cosmetic work left to do, namely get rid of pg_asprintf()
in favor of using psprintf(). That will come in a followon patch.
Continuing 63f32f3416, libpgcommon should
depend on libpgport, but not vice versa. But wait_result_to_str() in
wait_error.c depends on pstrdup() in libpgcommon. So move exec.c and
wait_error.c from libpgport to libpgcommon. Also switch the link order
in the place that's actually used by the failing ecpg builds.
The function declarations have been left in port.h for now. That should
perhaps be separated sometime.
Now that msgfmt is run with -c by default, older versions of gettext are
complaining about the PO headers Last-Translator and Language-Team
still having their default values. Newer gettext versions fail to catch
this because of a bug (https://savannah.gnu.org/bugs/?40261), which is
why this hasn't been noticed before.
Copy updated versions of affected translation files from the
pgtranslations repository, were those files have been fixed.
Add asprintf(), pg_asprintf(), and psprintf() to simplify string
allocation and composition. Replacement implementations taken from
NetBSD.
Reviewed-by: Álvaro Herrera <alvherre@2ndquadrant.com>
Reviewed-by: Asif Naeem <anaeem.it@gmail.com>
If POSIX shared memory is supported and works, we prefer it.
Otherwise, we prefer System V, except on Windows, where we use
the implementation specific to that platform.
DISCARD ALL will now discard cached sequence information, as well.
Fabrízio de Royes Mello, reviewed by Zoltán Böszörményi, with some
further tweaks by me.
The cancel handler was uselessly set up even before the first connection
was opened. By setting it up afterwards, the user can use Ctrl+C to
abort psql if the initial connection attempt hangs.
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Reviewed-by: Ryan Kelly <rpkelly22@gmail.com>
When a timeline history file is fetched from server, it is initially created
with a temporary file name, and renamed to place. However, the temporary
file name was constructed using an uninitialized buffer. Usually that meant
that the file was created in current directory instead of the target, which
usually goes unnoticed, but if the target is on a different filesystem than
the current dir, the rename() would fail. Fix that.
The second issue is that pg_receivexlog would not take .partial files into
account when determining when scanning the target directory for existing
WAL files. If the timeline has switched in the server several times in the
last WAL segment, and pg_receivexlog is restarted, it would choose a too
old starting point. That's not a problem as long as the old WAL segment
exists in the server and can be streamed over, but will cause a failure if
it's not.
Backpatch to 9.3, where this timeline handling code was written.
Analysed by Andrew Gierth, bug #8453, based on a bug report on IRC.
There's no inherent reason why an aggregate function can't be variadic
(even VARIADIC ANY) if its transition function can handle the case.
Indeed, this patch to add the feature touches none of the planner or
executor, and little of the parser; the main missing stuff was DDL and
pg_dump support.
It is true that variadic aggregates can create the same sort of ambiguity
about parameters versus ORDER BY keys that was complained of when we
(briefly) had both one- and two-argument forms of string_agg(). However,
the policy formed in response to that discussion only said that we'd not
create any built-in aggregates with varying numbers of arguments, not that
we shouldn't allow users to do it. So the logical extension of that is
we can allow users to make variadic aggregates as long as we're wary about
shipping any such in core.
In passing, this patch allows aggregate function arguments to be named, to
the extent of remembering the names in pg_proc and dumping them in pg_dump.
You can't yet call an aggregate using named-parameter notation. That seems
like a likely future extension, but it'll take some work, and it's not what
this patch is really about. Likewise, there's still some work needed to
make window functions handle VARIADIC fully, but I left that for another
day.
initdb forced because of new aggvariadic field in Aggref parse nodes.
We already did this for -t (--table) in 9.3, but missed the other similar
options. For consistency, allow all of them to be specified multiple times.
Unfortunately it's too late to sneak this into 9.3, so commit to master
only.
This keeps the usual trigger file name unchanged from 9.2, avoiding nasty
issues if you use a pre-9.3 pg_ctl binary with a 9.3 server or vice versa.
The fallback behavior of creating a full checkpoint before starting up is now
triggered by a file called "fallback_promote". That can be useful for
debugging purposes, but we don't expect any users to have to resort to that
and we might want to remove that in the future, which is why the fallback
mechanism is undocumented.
For simple views which are automatically updatable, this patch allows
the user to specify what level of checking should be done on records
being inserted or updated. For 'LOCAL CHECK', new tuples are validated
against the conditionals of the view they are being inserted into, while
for 'CASCADED CHECK' the new tuples are validated against the
conditionals for all views involved (from the top down).
This option is part of the SQL specification.
Dean Rasheed, reviewed by Pavel Stehule
This allows reads to continue without any blocking while a REFRESH
runs. The new data appears atomically as part of transaction
commit.
Review questioned the Assert that a matview was not a system
relation. This will be addressed separately.
Reviewed by Hitoshi Harada, Robert Haas, Andres Freund.
Merged after review with security patch f3ab5d4.
In pg_basebackup.c:reached_end_position(), we're reading from an
internal pipe with our own background process but we're possibly
reading more bytes than will actually fit into our buffer due to
an off-by-one error. As we're reading from an internal pipe
there's no real risk here, but it's good form to not depend on
such convenient arrangements.
Bug spotted by the Coverity scanner.
Back-patch to 9.2 where this showed up.
In pg_dump.c:getEventTriggers, check what major version we are on
before calling createPQExpBuffer() to avoid leaking that bit of
memory.
Leak discovered by the Coverity scanner.
Back-patch to 9.3 where support for dumping event triggers was
added.
When creating the symlink for the xlog directory, free the string
which stores the link location. Not really an issue but it doesn't
hurt to be good about this- prior cleanups have fixed similar
issues.
Leak found by the Coverity scanner.
Not back-patching as I don't see it being worth the code churn.
In receivelog.c:writeTimeLineHistoryFile(), we were not properly
closing the open'd file descriptor in error cases. While this
wouldn't matter much if we were about to exit due to such an
error, that's not the case with pg_receivexlog as it can be a
long-running process and these errors are non-fatal.
This resource leak was found by the Coverity scanner.
Back-patch to 9.3 where this issue first appeared.
In streamutil.c:GetConnection(), upgrade failure to parse the
connection string to an exit(1) instead of simply returning NULL.
Most callers already immediately exited, but pg_receivexlog would
loop on this case, continually trying to re-parse the connection
string (which can't be changed after pg_receivexlog has started).
GetConnection() was already expected to exit(1) in some cases
(eg: failure to allocate memory or if unable to determine the
integer_datetimes flag), so this change shouldn't surprise anyone.
Began looking at this due to the Coverity scanner complaining that
we were leaking err_msg in this case- no longer an issue since we
just exit(1) immediately.
The command strings read by the child processes during parallel
pg_dump, after being read and handled, were not being free'd.
This patch corrects this relatively minor memory leak.
Leak found by the Coverity scanner.
Back patch to 9.3 where parallel pg_dump was introduced.
In 9.3, there's no particular limit on the number of bgworkers;
instead, we just count up the number that are actually registered,
and use that to set MaxBackends. However, that approach causes
problems for Hot Standby, which needs both MaxBackends and the
size of the lock table to be the same on the standby as on the
master, yet it may not be desirable to run the same bgworkers in
both places. 9.3 handles that by failing to notice the problem,
which will probably work fine in nearly all cases anyway, but is
not theoretically sound.
A further problem with simply counting the number of registered
workers is that new workers can't be registered without a
postmaster restart. This is inconvenient for administrators,
since bouncing the postmaster causes an interruption of service.
Moreover, there are a number of applications for background
processes where, by necessity, the background process must be
started on the fly (e.g. parallel query). While this patch
doesn't actually make it possible to register new background
workers after startup time, it's a necessary prerequisite.
Patch by me. Review by Michael Paquier.
Treat TOAST index just the same as normal one and get the OID
of TOAST index from pg_index but not pg_class.reltoastidxid.
This change allows us to handle multiple TOAST indexes, and
which is required infrastructure for upcoming
REINDEX CONCURRENTLY feature.
Patch by Michael Paquier, reviewed by Andres Freund and me.
SnapshotNow scans have the undesirable property that, in the face of
concurrent updates, the scan can fail to see either the old or the new
versions of the row. In many cases, we work around this by requiring
DDL operations to hold AccessExclusiveLock on the object being
modified; in some cases, the existing locking is inadequate and random
failures occur as a result. This commit doesn't change anything
related to locking, but will hopefully pave the way to allowing lock
strength reductions in the future.
The major issue has held us back from making this change in the past
is that taking an MVCC snapshot is significantly more expensive than
using a static special snapshot such as SnapshotNow. However, testing
of various worst-case scenarios reveals that this problem is not
severe except under fairly extreme workloads. To mitigate those
problems, we avoid retaking the MVCC snapshot for each new scan;
instead, we take a new snapshot only when invalidation messages have
been processed. The catcache machinery already requires that
invalidation messages be sent before releasing the related heavyweight
lock; else other backends might rely on locally-cached data rather
than scanning the catalog at all. Thus, making snapshot reuse
dependent on the same guarantees shouldn't break anything that wasn't
already subtly broken.
Patch by me. Review by Michael Paquier and Andres Freund.
When there's a comment on an index that was created with UNIQUE or PRIMARY
KEY constraint syntax, we need to label the comment as depending on the
constraint not the index, since only the constraint object actually appears
in the dump. This incorrect dependency can lead to parallel pg_restore
trying to restore the comment before the index has been created, per bug
#8257 from Lloyd Albin.
This patch fixes pg_dump to produce the right dependency in dumps made
in the future. Usually we also try to hack pg_restore to work around
bogus dependencies, so that existing (wrong) dumps can still be restored in
parallel mode; but that doesn't seem practical here since there's no easy
way to relate the constraint dump entry to the comment after the fact.
Andres Freund
In binary upgrade mode, we need to recreate and then drop dropped
columns so that all the columns get the right attribute number. This is
true for foreign tables as well as for native tables. For foreign
tables we have been getting the first part right but not the second,
leading to bogus columns in the upgraded database. Fix this all the way
back to 9.1, where foreign tables were introduced.
pg_isready displays the host name and the port number that it uses to connect
to the server. So far, pg_isready didn't use the conninfo specified in -d option
for calculating those host name and port number. This can lead to wrong display
to a user. This commit changes pg_isready so that it uses the conninfo for that
calculation.
Original patch by Phil Sorber, modified by me.
getSchemaData() must identify extension member objects and mark them
as not to be dumped. This must happen after reading all objects that can be
direct members of extensions, but before we begin to process table subsidiary
objects. Both rules and event triggers were wrong in this regard.
Backport rules portion of patch to 9.1 -- event triggers do not exist prior to 9.3.
Suggested fix by Tom Lane, initial complaint and patch by me.
In the primary_conninfo line that "pg_basebackup -R" generates, single
quotes in parameter values need to be escaped into \\'; the libpq parser
requires the quotes to be escaped into \', and recovery.conf parser requires
the \ to be escaped into \\.
Also, don't quote parameter values unnecessarily, to make the connection
string prettier. Most options in a libpq connection string don't need
quoting.
Reported by Hari Babu, closer analysis by Zoltan Boszormenyi, although I
didn't use his patch.
If a standby server has a cascading standby server connected to it, it's
possible that WAL has already been sent up to the next WAL page boundary,
splitting a WAL record in the middle, when the first standby server is
promoted. Don't throw an assertion failure or error in walsender if that
happens.
Also, fix a variant of the same bug in pg_receivexlog: if it had already
received WAL on previous timeline up to a segment boundary, when the
upstream standby server is promoted so that the timeline switch record falls
on the previous segment, pg_receivexlog would miss the segment containing
the timeline switch. To fix that, have walsender send the position of the
timeline switch at end-of-streaming, in addition to the next timeline's ID.
It was previously assumed that the switch happened exactly where the
streaming stopped.
Note: this is an incompatible change in the streaming protocol. You might
get an error if you try to stream over timeline switches, if the client is
running 9.3beta1 and the server is more recent. It should be fine after a
reconnect, however.
Reported by Fujii Masao.
Previously this state was represented by whether the view's disk file had
zero or nonzero size, which is problematic for numerous reasons, since it's
breaking a fundamental assumption about heap storage. This was done to
allow unlogged matviews to revert to unpopulated status after a crash
despite our lack of any ability to update catalog entries post-crash.
However, this poses enough risk of future problems that it seems better to
not support unlogged matviews until we can find another way. Accordingly,
revert that choice as well as a number of existing kluges forced by it
in favor of creating a pg_class.relispopulated flag column.
Very old versions of msgfmt choke on these specific messages, for reasons
that are unclear at the moment. Remove them so that we can ship a beta
release and not get complaints from testers (these messages will just go
untranslated, instead, and we're hardly at 100% coverage anyway).
Peter Eisentraut will look for a better fix later.
Print the command tag if we get PGRES_COMMAND_OK, and throw an error for
other cases. Per gripe from Michael Paquier.
In passing, add an fflush(), just to be real sure the output appears
before we sleep.
Previously, libpq and the backend had opposite ideas about whether
it was necessary for the client to send a CopyDone message after
receiving an ErrorResponse, making it impossible to cleanly exit
COPY BOTH mode. Fix libpq so that works correctly, adopting the
backend's notion that an ErrorResponse kills the copy in both
directions.
Adjust receivelog.c to avoid a degradation in the quality of the
resulting error messages. libpqwalreceiver.c is already doing
the right thing, so no adjustment needed there.
Add an explicit statement to the documentation explaining how
this part of the protocol is supposed to work, in the hopes of
avoiding future confusion in this area.
Since the consequences of all this confusion are very limited,
especially in the back-branches where no client ever attempts
to exit COPY BOTH mode without closing the connection entirely,
no back-patch.
Bruce Momjian
Peter Eisentraut
Tom Lane
Magnus Hagander
Stephen Frost
Fujii Masao
Robert Haas
Andrew Dunstan
Noah Misch
Heikki Linnakangas
Kevin Grittner
Tatsuo Ishii
Alvaro Herrera
Joe Conway
Simon Riggs