Concurrent autovacuums running with the main regression test suite
could cause the tests with VACUUM (SKIP_LOCKED) to generate randomly
WARNING messages. For these tests, set client_min_messages to ERROR to
get rid of those random failures, as disabling autovacuum for the
relations operated would not completely close the failure window.
For isolation tests, disable autovacuum for the relations vacuumed with
SKIP_LOCKED. The tests are designed so as LOCK commands are taken
in a first session before running a concurrent VACUUM (SKIP_LOCKED) in a
second to generate WARNING messages, but a concurrent autovacuum could
cause the tests to be slower.
Reported-by: Tom Lane
Author: Michael Paquier
Reviewed-by: Andres Freund, Tom Lane
Discussion: https://postgr.es/m/25294.1573077278@sss.pgh.pa.us
Backpatch-through: 12
In detoast_attr_slice, VARSIZE_ANY was used to compute compressed length
of on-disk TOAST values. That's incorrect, because the varlena value may
be just a TOAST pointer, producing either bogus value or crashing.
This is likely why the code was crashing on big-endian machines before
540f31680913 replaced the VARSIZE with VARSIZE_ANY, which however only
masked the issue.
Reported-by: Rushabh Lathia
Discussion: https://postgr.es/m/CAL-OGkthU9Gs7TZchf5OWaL-Gsi=hXqufTxKv9qpNG73d5na_g@mail.gmail.com
When estimating number of distinct groups, we failed to ignore system
attributes when matching the group expressions to mvdistinct stats,
causing failures like
ERROR: negative bitmapset member not allowed
Fix that by simply skipping anything that is not a regular attribute.
Backpatch to PostgreSQL 10, where the extended stats were introduced.
Bug: #16111
Reported-by: Tuomas Leikola
Author: Tomas Vondra
Backpatch-through: 10
Discussion: https://postgr.es/m/16111-687799584c3a7e73@postgresql.org
Call ExecShutdownNode() after ExecutePlan()'s loop, rather than at each
break. We had forgotten to do that in one case. The omission caused
intermittent "temporary file leak" warnings from multi-batch parallel
hash joins with a LIMIT clause.
Back-patch to 11. Though the problem exists in theory in earlier
parallel query releases, nothing really depended on it.
Author: Kyotaro Horiguchi
Reviewed-by: Thomas Munro, Amit Kapila
Discussion: https://postgr.es/m/20191111.212418.2222262873417235945.horikyota.ntt%40gmail.com
reloptions.h includes since ba748f7 a set of macros to handle reloption
types in a way similar to how parseRelOptions() works. They have never
been used in the core code, and we have more simple methods now to parse
and fill in rd_options for a given relation depending on its relkind, so
remove this interface to simplify things.
Per discussion between Amit Langote, Álvaro Herrera and me.
Discussion: https://postgr.es/m/CA+HiwqE6zbNO92az6pp5GiTw4tr-9rfCE0t84whQSP+YwSKjMQ@mail.gmail.com
Partitioned tables do not have relation options yet, but, similarly to
what's done for views which have their own parsing table, it could make
sense to introduce new parameters for some of the existing default ones
like fillfactor, autovacuum, etc. Splitting things has the advantage to
make the information stored in rd_options include only the necessary
information, reducing the amount of memory used for a relcache entry
with partitioned tables if new reloptions are introduced at this level.
Author: Nikolay Shaplov
Reviewed-by: Amit Langote, Michael Paquier
Discussion: https://postgr.es/m/1627387.Qykg9O6zpu@x200m
At least buildfarm member florican doesn't use a material node above a
sort in the mark/restore case. As material is not intended to be
tested with that query, disallow.
Previously significant parts of tuplesort.c were untested. This
commit, while not testing every path, significantly increases
coverage. In particular, this adds tests for abbreviated key logic,
forward/backward scans & scrolling and mark/restore.
I tried to keep the table sizes reasonable, and stress the on-disk
paths by setting work_mem to low values for specific tests. The
buildfarm will tell whether more attention to test time is needed.
Author: Andres Freund
Discussion: https://postgr.es/m/20191013144153.ooxrfglvnaocsrx2@alap3.anarazel.de
We should throw an error for indeterminate collation, but bpcharne()
was missing that logic, resulting in a much less user-friendly error
(either an assertion failure or "cache lookup failed for collation 0").
Per report from Manuel Rigger. Back-patch to v12 where the mistake
came in, evidently in commit 5e1963fb7. (Before non-deterministic
collations, this function wasn't collation sensitive.)
Discussion: https://postgr.es/m/CA+u7OA4HOjtymxAbuGNh4-X_2R0Lw5n01tzvP8E5-i-2gQXYWA@mail.gmail.com
Initializing a pointer to "false" isn't per project style,
and reportedly some compilers warn about it (though I've
not seen any such warnings in the buildfarm).
Seems to have come in with commit ff11e7f4b, so back-patch
to v12 where that was added.
Didier Gautheron
Discussion: https://postgr.es/m/CAJRYxu+XQuM0qnSqt1Ujztu6fBPzMMAT3VEn6W32rgKG6A2Fsw@mail.gmail.com
This gets rid of our former behavior of forcibly downcasing
the postmaster's hostname list and truncating the elements to
NAMEDATALEN. In principle, DNS hostnames are case-insensitive
so the first behavior should be harmless, and server hostnames
are seldom long enough for the second behavior to be an issue.
But it's still dubious, and an easy fix is available: just use
SplitGUCList instead.
AFAICT, all other SplitIdentifierString calls in the backend are
OK: either the items actually are SQL identifiers, or they are
keywords that are short and case-insensitive.
Per thinking about bug #16106. While this has been wrong for
a very long time, the lack of field complaints means there's
little reason to back-patch.
Discussion: https://postgr.es/m/16106-7d319e4295d08e70@postgresql.org
Commit 6b76f1bb5 changed all the RADIUS auth parameters to be lists
rather than single values. But its use of SplitIdentifierString
to parse the list format was not very carefully thought through,
because that function thinks it's parsing SQL identifiers, which
means it will (a) downcase the strings and (b) truncate them to
be shorter than NAMEDATALEN. While downcasing should be harmless
for the server names and ports, it's just wrong for the shared
secrets, and probably for the NAS Identifier strings as well.
The truncation aspect is at least potentially a problem too,
though typical values for these parameters would fit in 63 bytes.
Fortunately, we now have a function SplitGUCList that is exactly
the same except for not doing the two unwanted things, so fixing
this is a trivial matter of calling that function instead.
While here, improve the documentation to show how to double-quote
the parameter values. I failed to resist the temptation to do
some copy-editing as well.
Report and patch from Marcos David (bug #16106); doc changes by me.
Back-patch to v10 where the aforesaid commit came in, since this is
arguably a regression from our previous behavior with RADIUS auth.
Discussion: https://postgr.es/m/16106-7d319e4295d08e70@postgresql.org
The TableFunc node (i.e., XMLTABLE) includes type and collation OIDs
that might not be referenced anywhere else in the expression tree,
so they need to be accounted for when extracting dependencies.
Fortunately, the practical effects of this are limited, since
(a) it's somewhat unlikely that people would be extracting
columns of non-builtin types from an XML document, and (b)
in many scenarios, the query would contain other references
to such types, or functions depending on them. However, it's
not hard to construct examples wherein the existing code lets
one drop a type used in XMLTABLE and thereby break a view.
This is evidently an original oversight in the XMLTABLE patch,
so back-patch to v10 where that came in.
Discussion: https://postgr.es/m/18427.1573508501@sss.pgh.pa.us
pg_upgrade needs to check whether certain non-upgradable data types
appear anywhere on-disk in the source cluster. It knew that it has
to check for these types being contained inside domains and composite
types; but it somehow overlooked that they could be contained in
arrays and ranges, too. Extend the existing recursive-containment
query to handle those cases.
We probably should have noticed this oversight while working on
commit 0ccfc2822 and follow-ups, but we failed to :-(. The whole
thing's possibly a bit overdesigned, since we don't really expect
that any of these types will appear on disk; but if we're going to
the effort of doing a recursive search then it's silly not to cover
all the possibilities.
While at it, refactor so that we have only one copy of the search
logic, not three-and-counting. Also, to keep the branches looking
more alike, back-patch the output wording change of commit 1634d3615.
Back-patch to all supported branches.
Discussion: https://postgr.es/m/31473.1573412838@sss.pgh.pa.us
This commit changes xact_desc() so that it reports the detail information about
PREPARE TRANSACTION record, like GID (global transaction identifier),
timestamp at prepare transaction, delete-on-abort/commit relations,
XID of subtransactions, and invalidation messages. These are helpful
when diagnosing 2PC-related troubles.
Author: Fujii Masao
Reviewed-by: Michael Paquier, Andrey Lepikhov, Kyotaro Horiguchi, Julien Rouhaud, Alvaro Herrera
Discussion: https://postgr.es/m/CAHGQGwEvhASad4JJnCv=0dW2TJypZgW_Vpb-oZik2a3utCqcrA@mail.gmail.com
postgres_fdw does not support two-phase transactions, so let's add a
small negative test case to check after it. Note that this is checked
using an end-of-xact callback to ensure a proper connection cleanup with
the foreign server, which is called before checking if a server is able
to handle 2PC with max_prepared_xacts, so this test does not need an
alternate output file.
Author: Gilles Darold
Discussion: https://postgr.es/m/20191108090507.GC1768@paquier.xyz
This new option terminates the other sessions connected to the target
database and then drop it. To terminate other sessions, the current user
must have desired permissions (same as pg_terminate_backend()). We don't
allow to terminate the sessions if prepared transactions, active logical
replication slots or subscriptions are present in the target database.
Author: Pavel Stehule with changes by me
Reviewed-by: Dilip Kumar, Vignesh C, Ibrar Ahmed, Anthony Nowocien,
Ryan Lambert and Amit Kapila
Discussion: https://postgr.es/m/CAP_rwwmLJJbn70vLOZFpxGw3XD7nLB_7+NKz46H5EOO2k5H7OQ@mail.gmail.com
It is pointless to show in those views auxiliary processes that don't
open network connections.
A small incompatibility is that anybody joining pg_stat_activity and
pg_stat_ssl/pg_stat_gssapi will have to use a left join if they want to
see such auxiliary processes.
Author: Euler Taveira
Discussion: https://postgr.es/m/20190904151535.GA29108@alvherre.pgsql
An upcoming patch that adds deduplication to the nbtree AM will rely on
_bt_keep_natts_fast() understanding that differences in TOAST input
state can never affect its answer. In particular, two opclass-equal
datums (with opclasses deemed safe for deduplication) should never be
treated as unequal by _bt_keep_natts_fast() due to TOAST input
differences.
This also seems like a good idea on general principle. nbtsplitloc.c
will now occasionally make better decisions about where to split a leaf
page. The behavior of _bt_keep_natts_fast() is now somewhat closer to
the behavior of _bt_keep_natts().
Discussion: https://postgr.es/m/CAH2-Wzn3Ee49Gmxb7V1VJ3-AC8fWn-Fr8pfWQebHe8rYRxt5OQ@mail.gmail.com
Bring datum_image_eq() in line with datumIsEqual() by adding support for
comparing cstring datums.
An upcoming patch that adds deduplication to the nbtree AM will use
datum_image_eq(). datum_image_eq() will need to work with all datatypes
that can be used as the storage type of a B-Tree index column, including
cstring. (cstring is used as the storage type for columns of type
"name" as a space-saving optimization.)
Discussion: https://postgr.es/m/CAH2-Wzn3Ee49Gmxb7V1VJ3-AC8fWn-Fr8pfWQebHe8rYRxt5OQ@mail.gmail.com
This completes the task begun in commit 1408d5d86, to synchronize
ECPG's exported definitions with the definition of bool used by
c.h (and, therefore, the one actually in use in the ECPG library).
On practically all modern platforms, ecpglib.h will now just
include <stdbool.h>, which should surprise nobody anymore.
That removes a header-inclusion-order hazard for ECPG clients,
who previously might get build failures or unexpected behavior
depending on whether they'd included <stdbool.h> themselves,
and if so, whether before or after ecpglib.h.
On platforms where sizeof(_Bool) is not 1 (only old PPC-based
Mac systems, as far as I know), things are still messy, as
inclusion of <stdbool.h> could still break ECPG client code.
There doesn't seem to be any clean fix for that, and given the
probably-negligible population of users who would care anymore,
it's not clear we should go far out of our way to cope with it.
This change at least fixes some header-inclusion-order hazards
for our own code, since c.h and ecpglib.h previously disagreed
on whether bool should be char or unsigned char.
To implement this with minimal invasion of ECPG client namespace,
move the choice of whether to rely on <stdbool.h> into configure,
and have it export a configuration symbol PG_USE_STDBOOL.
ecpglib.h no longer exports definitions for TRUE and FALSE,
only their lowercase brethren. We could undo that if we get
push-back about it.
Ideally we'd back-patch this as far as v11, which is where c.h
started to rely on <stdbool.h>. But the odds of creating problems
for formerly-working ECPG client code seem about as large as the
odds of fixing any non-working cases, so we'll just do this in HEAD.
Discussion: https://postgr.es/m/CAA4eK1LmaKO7Du9M9Lo=kxGU8sB6aL8fa3sF6z6d5yYYVe3BuQ@mail.gmail.com
Similar to commits 7e735035f2 and dddf4cdc33, this commit makes the order
of header file inclusion consistent for backend modules.
In the passing, removed a couple of duplicate inclusions.
Author: Vignesh C
Reviewed-by: Kuntal Ghosh and Amit Kapila
Discussion: https://postgr.es/m/CALDaNm2Sznv8RR6Ex-iJO6xAdsxgWhCoETkaYX=+9DW3q0QCfA@mail.gmail.com
The example of expansion of multiple views claimed that the resulting
subquery nest would not get fully flattened because of an aggregate
function. There's no aggregate in the example, though, only a user
defined function confusingly named MIN(). In a modern server, the
reason for the non-flattening is that MIN() is volatile, but I'm
unsure whether that was true back when this text was written.
Let's reduce the confusion level by using LEAST() instead (which
we didn't have at the time this example was created). And then
we can just say that the planner will flatten the sub-queries, so
the rewrite system doesn't have to.
Noted by Paul Jungwirth. This text is old enough to vote, so
back-patch to all supported branches.
Discussion: https://postgr.es/m/CA+renyXZFnmp9PcvX1EVR2dR=XG5e6E-AELr8AHCNZ8RYrpnPw@mail.gmail.com
Commits 4ea03f3f4 et al arranged to filter out row counts in parallel
plans, because those are dependent on the number of workers actually
obtained. Somehow I missed that the 'Rows Removed by Filter' counts
can also vary, so fix that too. Per buildfarm.
This seems worth a last-minute patch because unreliable regression
tests are a serious pain in the rear for packagers.
Like the previous patch, back-patch to v11 where this test was
introduced.
PredicateLockTuple() has a fast exit if tuple was written by the current
transaction, as in that case it already has a lock. This check can be
performed using TransactionIdIsCurrentTransactionId() instead of
SubTransGetTopmostTransaction(), to avoid any chance of having to hit the
disk.
Author: Ashwin Agrawal, based on a suggestion from Andres Freund
Reviewed-by: Thomas Munro
Discussion: https://postgr.es/m/CALfoeiv0k3hkEb3Oqk%3DziWqtyk2Jys1UOK5hwRBNeANT_yX%2Bng%40mail.gmail.com
If the passed in xid is the current top transaction, we can do a fast
check and exit early. This should work well for the current heap but
also works very well for proposed AMs that don't use a separate xid
for subtransactions.
Author: Ashwin Agrawal, based on a suggestion from Andres Freund
Reviewed-by: Thomas Munro
Discussion: https://postgr.es/m/CALfoeiv0k3hkEb3Oqk%3DziWqtyk2Jys1UOK5hwRBNeANT_yX%2Bng%40mail.gmail.com
The previous statement that using a passphrase disables the ability to
change the server's SSL configuration without a server restart was no
longer completely true since the introduction of
ssl_passphrase_command_supports_reload.
This happens when we add a replica identity column on a subscriber
that does not yet exist on the publisher, according to the mapping
maintained by the subscriber. Code that checks whether the target
relation on the subscriber is updatable would check the replica
identity attribute bitmap with a column number -1, which would result
in an error. To fix, skip such columns in the bitmap lookup and
consider the relation not updatable. The result is consistent with
the rule that the replica identity columns on the subscriber must be a
subset of those on the publisher, since if the column doesn't exist on
the publisher, the column set on the subscriber can't be a subset.
Reported-by: Tim Clarke <tim.clarke@minerva.info>
Analyzed-by: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Discussion: https://www.postgresql.org/message-id/flat/a9139c29-7ddd-973b-aa7f-71fed9c38d75%40minerva.info
The buildfarm has turned red after 1858b10 because VPATH builds need to
use "@abs_srcdir@" and not "@abs_builddir@" for paths coming directly
from the source tree. The input file of the new test got that right,
but not the output file.
Per complaints from several buildfarm animals, including desmoxytes and
culicidae. I have also reproduced the error by myself.
Add some support for automatically showing backtraces in certain error
situations in the server. Backtraces are shown on assertion failure;
also, a new setting backtrace_functions can be set to a list of C
function names, and all ereport()s and elog()s from the mentioned
functions will have backtraces generated. Finally, the function
errbacktrace() can be manually added to an ereport() call to generate a
backtrace for that call.
Authors: Peter Eisentraut, Álvaro Herrera
Discussion: https://postgr.es/m//5f48cb47-bf1e-05b6-7aae-3bf2cd01586d@2ndquadrant.com
Discussion: https://postgr.es/m/CAMsr+YGL+yfWE=JvbUbnpWtrRZNey7hJ07+zT4bYJdVp4Szdrg@mail.gmail.com
Currently, postgres_fdw does not support preparing a remote transaction
for two-phase commit even in the case where the remote transaction is
read-only, but the old error message appeared to imply that that was not
supported only if the remote transaction modified remote tables. Change
the message so as to include the case where the remote transaction is
read-only.
Also fix a comment above the message.
Also add a note about the lack of supporting PREPARE TRANSACTION to the
postgres_fdw documentation.
Reported-by: Gilles Darold
Author: Gilles Darold and Etsuro Fujita
Reviewed-by: Michael Paquier and Kyotaro Horiguchi
Backpatch-through: 9.4
Discussion: https://postgr.es/m/08600ed3-3084-be70-65ba-279ab19618a5%40darold.net
nbtree index builds once stashed the "minimum key" for a page, which was
used as the basis of the pivot tuple that gets placed in the next level
up (i.e. the tuple that stores the downlink to the page in question).
It doesn't quite work that way anymore, so the "minimum key" terminology
now seems misleading (these days the minimum key is actually a straight
copy of the high key from the left sibling, which is a distinct thing in
subtle but important ways). Rename this concept to "low key". This
name is a lot clearer given that there is now a sharp distinction
between pivot and non-pivot tuples. Also remove comments that describe
obsolete details about how the minimum key concept used to work.
Rather than generating the minus infinity item for the leftmost page on
a level by copying the new item and truncating that copy, simply
allocate a small buffer. The old approach confusingly created the
impression that the new item had some kind of significance. This was
another artifact of how things used to work before commits 8224de4f and
dd299df8.
Declaring this in the client-visible header ecpglib.h was a pretty
poor decision. It's not meant to be application-callable (and if
it was, putting it outside the extern "C" { ... } wrapper means
that C++ clients would fail to call it). And the declaration would
not even compile for a client, anyway, since it would not have the
macro pg_attribute_format_arg(). Fortunately, it seems that no
clients have tried to include this header with ENABLE_NLS defined,
or we'd have gotten complaints about that. But we have no business
putting such a restriction on client code.
Move the declaration to ecpglib_extern.h, since in fact nothing
outside src/interfaces/ecpg/ecpglib/ needs to call it.
The practical effect of this is just that clients can now safely
#include ecpglib.h while having ENABLE_NLS defined, but that seems
like enough of a reason to back-patch it.
Discussion: https://postgr.es/m/20590.1573069709@sss.pgh.pa.us
