This allows the compiler / linker to mark affected pages as read-only.
There's a fair number of pre-requisite changes, to allow the const
properly be propagated. Most of consts were already required for
correctness anyway, just not represented on the type-level. Arguably
we could be more aggressive in using consts in related code, but..
This requires using a few of the types underlying typedefs that
removes pointers (e.g. const NameData *) as declaring the typedefed
type constant doesn't have the same meaning (it makes the variable
const, not what it points to).
Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de
Reporting only the stderr is unhelpful when the problem is that the
server output we're getting doesn't match what was expected. So we
should report the query output too; and just for good measure, let's
print the query we used and the output we expected.
Back-patch to 9.5 where poll_query_until was introduced.
Discussion: https://postgr.es/m/17913.1539634756@sss.pgh.pa.us
Commit b5febc1d1 added a contrib/btree_gist test case that has been
observed to fail in the buildfarm as a result of background auto-analyze
updating stats and changing the selected plan. Forestall that by
forcibly analyzing in foreground, instead. The new plan choice is
just as good for our purposes, since we really only care that an
index-only plan does not get selected.
Back-patch to 9.5, like the previous patch.
Discussion: https://postgr.es/m/14643.1539629304@sss.pgh.pa.us
localtime.c's "struct state" is a rather large object, ~23KB. We were
statically allocating one for gmtsub() to use to represent the GMT
timezone, even though that function is not at all heavily used and is
never reached in most backends. Let's malloc it on-demand, instead.
This does pose the question of how to handle a malloc failure, but
there's already a well-defined error report convention here, ie
set errno and return NULL.
We have but one caller of pg_gmtime in HEAD, and two in back branches,
neither of which were troubling to check for error. Make them do so.
The possible errors are sufficiently unlikely (out-of-range timestamp,
and now malloc failure) that I think elog() is adequate.
Back-patch to all supported branches to keep our copies of the IANA
timezone code in sync. This particular change is in a stanza that
already differs from upstream, so it's a wash for maintenance purposes
--- but only as long as we keep the branches the same.
Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de
This allows the compiler / linker to mark affected pages as read-only.
There's other cases, but they're a bit more invasive, and should go
through some review. These are easy.
They were found with
objdump -j .data -t src/backend/postgres|awk '{print $4, $5, $6}'|sort -r|less
Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de
There's several reasons for this change:
1) It reduces the total size of TupleTableSlot / reduces alignment
padding, making the commonly accessed members fit into a single
cacheline (but we currently do not force proper alignment, so
that's not yet guaranteed to be helpful)
2) Combining the booleans into a flag allows to combine read/writes
from memory.
3) With the upcoming slot abstraction changes, it allows to have core
and extended flags, in a memory efficient way.
Author: Ashutosh Bapat and Andres Freund
Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
heaptuple.c was never a particular good fit for slot_getattr(),
slot_getsomeattrs() and slot_getmissingattrs(), but in upcoming
changes slots will be made more abstract (allowing slots that contain
different types of tuples), making it clearly the wrong place.
Note that slot_deform_tuple() remains in it's current place, as it
clearly deals with a HeapTuple. getmissingattrs() also remains, but
it's less clear that that's correct - but execTuples.c wouldn't be the
right place.
Author: Ashutosh Bapat.
Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
ProcessUtility can recurse, and indeed can be driven to infinite
recursion, so it ought to have a check_stack_depth() call. This
covers the reported bug (portal trying to execute itself) and a bunch
of other cases that could perhaps arise somewhere.
Per bug #15428 from Malthe Borch. Back-patch to all supported branches.
Discussion: https://postgr.es/m/15428-b3c2915ec470b033@postgresql.org
When an error occurs during a benchmark run, exit with a nonzero exit
code and write a message at the end. Previously, it would just print
the error message when it happened but then proceed to print the run
summary normally and exit with status 0. To still allow
distinguishing setup from run-time errors, we use exit status 2 for
the new state, whereas existing errors during pgbench initialization
use exit status 1.
Reviewed-by: Fabien COELHO <coelho@cri.ensmp.fr>
With the PostgreSQL 11 release notes acknowledgments list, FOP reported
WARNING: Glyph "?" (0x144, nacute) not available in font "Times-Roman".
WARNING: Glyph "?" (0x15e, Scedilla) not available in font "Times-Roman".
WARNING: Glyph "?" (0x15f, scedilla) not available in font "Times-Roman".
WARNING: Glyph "?" (0x131, dotlessi) not available in font "Times-Roman".
This is because we have some new contributors whose names use letters
that we haven't used before, and apparently FOP can't handle them out
of the box.
For now, just fix this by "unaccenting" those names. In the future,
maybe this can be fixed better with a different font configuration.
There is also another warning
WARNING: Glyph "?" (0x3c0, pi) not available in font "Times-Roman".
but that existed in previous releases and is not touched here.
These test cases could be adversely affected by an upcoming change
to allow pullup of FROM-less subqueries. Tweak them to ensure that
they'll continue to test what they did before.
Discussion: https://postgr.es/m/5395.1539275668@sss.pgh.pa.us
This prevents failures in cases where we pull up a constant or var-free
expression from a subquery and put it into a full join's qual. That can
result in not recognizing the qual as containing a mergejoin-able or
hashjoin-able condition. A PHV prevents the problem because it is still
recognized as belonging to the side of the join the subquery is in.
I'm not very sure about the net effect of this change on plan quality.
In "typical" cases where the join keys are Vars, nothing changes.
In an affected case, the PHV-wrapped expression is less likely to be seen
as equal to PHV-less instances below the join, but more likely to be seen
as equal to similar expressions above the join, so it may end up being a
wash. In the one existing case where there's any visible change in a
regression-test plan, it amounts to referencing a lower computation of a
COALESCE result instead of recomputing it, which seems like a win.
Given my uncertainty about that and the lack of field complaints,
no back-patch, even though this is a very ancient problem.
Discussion: https://postgr.es/m/32090.1539378124@sss.pgh.pa.us
With the removal of the old abstime type, there are no longer any cases
in this test where we need to use the weaker castcontext-ignoring form
of binary coercibility check. (The other major source of such headaches,
apparently-incompatible hash functions, is now hashvalidate()'s problem
not this test script's problem.) Hence, just use binary_coercible()
everywhere, and remove the comments explaining why we don't do so ---
which were broken anyway by cda6a8d01.
I left physically_coercible() in place but renamed it to better
match what it's actually testing, and added some comments.
Also, in test queries that have an assumption about the maximum number
of function arguments they need to handle, add a clause to make them fail
if someday there's a relevant function with more arguments. Otherwise
we're likely not to notice that we need to extend the queries.
Discussion: https://postgr.es/m/27637.1539388060@sss.pgh.pa.us
On a primary, sets of XLOG_RUNNING_XACTS records are generated on a
periodic basis to allow recovery to build the initial state of
transactions for a hot standby. The set of transaction IDs is created
by scanning all the entries in ProcArray. However it happens that its
logic never counted on the fact that two-phase transactions finishing to
prepare can put ProcArray in a state where there are two entries with
the same transaction ID, one for the initial transaction which gets
cleared when prepare finishes, and a second, dummy, entry to track that
the transaction is still running after prepare finishes. This way
ensures a continuous presence of the transaction so as callers of for
example TransactionIdIsInProgress() are always able to see it as alive.
So, if a XLOG_RUNNING_XACTS takes a standby snapshot while a two-phase
transaction finishes to prepare, the record can finish with duplicated
XIDs, which is a state expected by design. If this record gets applied
on a standby to initial its recovery state, then it would simply fail,
so the odds of facing this failure are very low in practice. It would
be tempting to change the generation of XLOG_RUNNING_XACTS so as
duplicates are removed on the source, but this requires to hold on
ProcArrayLock for longer and this would impact all workloads,
particularly those using heavily two-phase transactions.
XLOG_RUNNING_XACTS is also actually used only to initialize the standby
state at recovery, so instead the solution is taken to discard
duplicates when applying the initial snapshot.
Diagnosed-by: Konstantin Knizhnik
Author: Michael Paquier
Discussion: https://postgr.es/m/0c96b653-4696-d4b4-6b5d-78143175d113@postgrespro.ru
Backpatch-through: 9.3
Set the release date. Do a bunch of copy-editing and markup improvement,
rearrange some stuff into what seemed a more sensible order, move some
things that did not seem to be in the right section.
Removing the separate Windows expected-files in commit f1885386f
turns out to have been too optimistic: on most (but not all!) of our
Windows buildfarm members, the tests still print floats with three
exponent digits, because they're invoking the native printf()
not snprintf.c.
But rather than put back the extra expected-files, let's hack
the three tests in question so that they adjust float formatting
the same way snprintf.c does.
Discussion: https://postgr.es/m/18890.1539374107@sss.pgh.pa.us
We can allow this macro to accept either abbreviated or non-abbreviated
allocation parameters by making use of __VA_ARGS__. As noted by Andres
Freund, it's unlikely that any compiler would have __builtin_constant_p
but not __VA_ARGS__, so this gives up little or no error checking, and
it avoids a minor but annoying API break for extensions.
With this change, there is no reason for anybody to call
AllocSetContextCreateExtended directly, so in HEAD I renamed it to
AllocSetContextCreateInternal. It's probably too late for an ABI
break like that in 11, though.
Discussion: https://postgr.es/m/20181012170355.bhxi273skjt6sag4@alap3.anarazel.de
There was no code to handle foreign key constraints on partitioned
tables in the case of ALTER TABLE DETACH; and if you happened to ATTACH
a partition that already had an equivalent constraint, that one was
ignored and a new constraint was created. Adding this to the fact that
foreign key cloning reuses the constraint name on the partition instead
of generating a new name (as it probably should, to cater to SQL
standard rules about constraint naming within schemas), the result was a
pretty poor user experience -- the most visible failure was that just
detaching a partition and re-attaching it failed with an error such as
ERROR: duplicate key value violates unique constraint "pg_constraint_conrelid_contypid_conname_index"
DETAIL: Key (conrelid, contypid, conname)=(26702, 0, test_result_asset_id_fkey) already exists.
because it would try to create an identically-named constraint in the
partition. To make matters worse, if you tried to drop the constraint
in the now-independent partition, that would fail because the constraint
was still seen as dependent on the constraint in its former parent
partitioned table:
ERROR: cannot drop inherited constraint "test_result_asset_id_fkey" of relation "test_result_cbsystem_0001_0050_monthly_2018_09"
This fix attacks the problem from two angles: first, when the partition
is detached, the constraint is also marked as independent, so the drop
now works. Second, when the partition is re-attached, we scan existing
constraints searching for one matching the FK in the parent, and if one
exists, we link that one to the parent constraint. So we don't end up
with a duplicate -- and better yet, we don't need to scan the referenced
table to verify that the constraint holds.
To implement this I made a small change to previously planner-only
struct ForeignKeyCacheInfo to contain the constraint OID; also relcache
now maintains the list of FKs for partitioned tables too.
Backpatch to 11.
Reported-by: Michael Vitale (bug #15425)
Discussion: https://postgr.es/m/15425-2dbc9d2aa999f816@postgresql.org
Windows, alone among our supported platforms, likes to emit three-digit
exponent fields even when two digits would do. Adjust such results to
look like the way everyone else does it. Eliminate a bunch of variant
expected-output files that were needed only because of this quirk.
Discussion: https://postgr.es/m/2934.1539122454@sss.pgh.pa.us
All options available in the utility get coverage:
- Tests with disabled page checksums.
- Tests with enabled test checksums.
- Emulation of corruption and broken checksums with a full scan and
single relfilenode scan.
This patch has been contributed mainly by Michael Banck and Magnus
Hagander with things presented on various threads, and I have gathered
all the contents into a single patch.
Author: Michael Banck, Magnus Hagander, Michael Paquier
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/20181005012645.GE1629@paquier.xyz
Repeatedly rewriting a mapped catalog table with VACUUM FULL or
CLUSTER could cause logical decoding to fail with:
ERROR, "could not map filenode \"%s\" to relation OID"
To trigger the problem the rewritten catalog had to have live tuples
with toasted columns.
The problem was triggered as during catalog table rewrites the
heap_insert() check that prevents logical decoding information to be
emitted for system catalogs, failed to treat the new heap's toast table
as a system catalog (because the new heap is not recognized as a
catalog table via RelationIsLogicallyLogged()). The relmapper, in
contrast to the normal catalog contents, does not contain historical
information. After a single rewrite of a mapped table the new relation
is known to the relmapper, but if the table is rewritten twice before
logical decoding occurs, the relfilenode cannot be mapped to a
relation anymore. Which then leads us to error out. This only
happens for toast tables, because the main table contents aren't
re-inserted with heap_insert().
The fix is simple, add a new heap_insert() flag that prevents logical
decoding information from being emitted, and accept during decoding
that there might not be tuple data for toast tables.
Unfortunately that does not fix pre-existing logical decoding
errors. Doing so would require not throwing an error when a filenode
cannot be mapped to a relation during decoding, and that seems too
likely to hide bugs. If it's crucial to fix decoding for an existing
slot, temporarily changing the ERROR in ReorderBufferCommit() to a
WARNING appears to be the best fix.
Author: Andres Freund
Discussion: https://postgr.es/m/20180914021046.oi7dm4ra3ot2g2kt@alap3.anarazel.de
Backpatch: 9.4-, where logical decoding was introduced
Without that the tests fail when forced to be run against a cluster
with synchronous_commit = off (as the WAL might not yet be flushed to
disk by the point logical decoding gets called, and thus the expected
output breaks). Most tests already do that, add it to a few newer tests.
Author: Andres Freund
It was previously a string setting that was converted into an enum by
custom code, but using the GUC enum facility seems much simpler and
doesn't change any functionality, except that
set transaction_isolation='default';
no longer works, but that was never documented and doesn't work with
any other transaction characteristics. (Note that this is not the
same as RESET or SET TO DEFAULT, which still work.)
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/457db615-e84c-4838-310e-43841eb806e5@iki.fi
This code used elog where it really ought to use ereport, mainly so that
it can report a SQLSTATE different from ERRCODE_INTERNAL_ERROR. There
were some other random deviations from typical error report practice too.
In addition, we can make some cleanups that were impractical six months
ago:
* Use one variadic macro, instead of several with different numbers
of arguments, reducing the temptation to force-fit messages into
particular numbers of arguments;
* Use %m, even in the frontend case, simplifying the code.
Discussion: https://postgr.es/m/6025.1527351693@sss.pgh.pa.us
numerology_1.out and float8-small-is-zero_1.out differ from their
base files only in showing plain zero rather than minus zero for
some results. I believe that in the wake of commit 6eb3eb577,
we will print minus zero as such on all IEEE-float platforms
(and non-IEEE floats are going to cause many more regression diffs
than this, anyway). Hence we should be able to remove these and
eliminate a bit of maintenance pain. Let's see if the buildfarm
agrees.
Discussion: https://postgr.es/m/29037.1539021687@sss.pgh.pa.us
NetBSD-current generates a large number of warnings about "%m" not
being appropriate to use with *printf functions. While that's true
for their native printf, it's surely not true for snprintf.c, so I
think they have misunderstood gcc's definition of the "gnu_printf"
archetype. Nonetheless, choosing "__syslog__" instead silences the
warnings; so teach configure about that.
Since this is only a cosmetic warning issue (and anyway it depends
on previous hacking to be self-consistent), no back-patch.
Discussion: https://postgr.es/m/16785.1539046036@sss.pgh.pa.us
This function lists the contents of the WAL archive status directory,
and is intended to be used by monitoring tools. Unlike pg_ls_dir(),
access to it can be granted to non-superusers so that those monitoring
tools can observe the principle of least privilege. Access is also
given by default to members of pg_monitor.
Author: Christoph Moench-Tegeder
Reviewed-by: Aya Iwata
Discussion: https://postgr.es/m/20180930205920.GA64534@elch.exwg.net
The idea here is that patches that add items to these lists will often
be easier to rebase over other additions to the same lists, because
they won't be trying to touch the very same line of configure.in.
There will still be merge conflicts in the configure script, but that
can be fixed just by re-running autoconf (or by leaving configure out
of the submitted patch to begin with ...)
Implementation note: use of m4_normalize() is necessary to get rid of
the newlines, else incorrect shell syntax will be emitted. But with
that hack, the generated configure script is identical to what it
was before.
Discussion: https://postgr.es/m/19344.1539050134@sss.pgh.pa.us
Originally committed as 15bc038f (plus some follow-ups), this was
reverted in 28e07270 due to a problem discovered in parallel
workers. This new version corrects that problem by sending the
list of uncommitted enum values to parallel workers.
Here follows the original commit message describing the change:
To prevent possibly breaking indexes on enum columns, we must keep
uncommitted enum values from getting stored in tables, unless we
can be sure that any such column is new in the current transaction.
Formerly, we enforced this by disallowing ALTER TYPE ... ADD VALUE
from being executed at all in a transaction block, unless the target
enum type had been created in the current transaction. This patch
removes that restriction, and instead insists that an uncommitted enum
value can't be referenced unless it belongs to an enum type created
in the same transaction as the value. Per discussion, this should be
a bit less onerous. It does require each function that could possibly
return a new enum value to SQL operations to check this restriction,
but there aren't so many of those that this seems unmaintainable.
Author: Andrew Dunstan and Tom Lane, with parallel query fix by Thomas Munro
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CAEepm%3D0Ei7g6PaNTbcmAh9tCRahQrk%3Dr5ZWLD-jr7hXweYX3yg%40mail.gmail.com
Discussion: https://postgr.es/m/4075.1459088427%40sss.pgh.pa.us
A warning on a NetBSD box revealed to me that pg_waldump/compat.c
is using vprintf(), which snprintf.c did not provide coverage for.
This is not good if we want to have uniform *printf behavior, and
it's pretty silly to omit when it's a one-line function.
I also noted that snprintf.c has pg_vsprintf() but for some reason
it was not exposed to the outside world, creating another way in
which code might accidentally invoke the platform *printf family.
Let's just make sure that we replace all eight of the POSIX-standard
printf family.
Also, upgrade plperl.h and plpython.h to make sure that they do
their undefine/redefine rain dance for all eight, not some random
maybe-sufficient subset thereof.
Up to now, float4out/float8out handled NaN and Infinity cases explicitly,
and invoked psprintf only for ordinary float values. This was done because
platform implementations of snprintf produce varying representations of
these special cases. But now that we use snprintf.c always, it's better
to give it the responsibility to produce a uniform representation of
these cases, so that we have uniformity across the board not only in
float4out/float8out. Hence, move that work into fmtfloat().
Also, teach fmtfloat() to recognize IEEE minus zero and handle it
correctly. The previous coding worked only accidentally, and would
fail for e.g. "%+f" format (it'd print "+-0.00000"). Now that we're
using snprintf.c everywhere, it's not acceptable for it to do weird
things in corner cases. (This incidentally avoids a portability
problem we've seen on some really ancient platforms, that native
sprintf does the wrong thing with minus zero.)
Also, introduce a new entry point in snprintf.c to allow float[48]out
to bypass the work of interpreting a well-known format spec, as well
as bypassing the overhead of the psprintf layer. I modeled this API
loosely on strfromd(). In my testing, this brings float[48]out back
to approximately the same speed they had when using native snprintf,
fixing one of the main performance issues caused by using snprintf.c.
(There is some talk of more aggressive work to improve the speed of
floating-point output conversion, but these changes seem to provide
a better starting point for such work anyway.)
Getting rid of the previous ad-hoc hack for Infinity/NaN in fmtfloat()
allows removing <ctype.h> from snprintf.c's #includes. I also removed
a few other #includes that I think are historical, though the buildfarm
may expose that as wrong.
Discussion: https://postgr.es/m/13178.1538794717@sss.pgh.pa.us
If there are many ExecRowMark structs, we spent O(N^2) time in
ExecFindRowMark during executor startup. Once upon a time this was
not of great concern, but the addition of native partitioning has
squeezed out enough other costs that this can become the dominant
overhead in some use-cases for tables with many partitions.
To fix, simply replace that List data structure with an array.
This adds a little bit of cost to execCurrentOf(), but not much,
and anyway that code path is neither of large importance nor very
efficient now. If we ever decide it is a bottleneck, constructing a
hash table for lookup-by-tableoid would likely be the thing to do.
Per complaint from Amit Langote, though this is different from
his fix proposal.
Discussion: https://postgr.es/m/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp
gcc 6.3 does not whine about this mistake I made in 39808e8868 but
evidently lots of other compilers do, according to Michael Paquier,
Peter Eisentraut, Arthur Zakirov, Tomas Vondra.
Discussion: too many to list
Error messages for creating a foreign key on a partitioned table using
ONLY or NOT VALID were wrong in mentioning the objects they worked on.
This commit adds on the way some regression tests missing for those
cases.
Author: Laurenz Albe
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/c11c05810a9ed65e9b2c817a9ef442275a32fe80.camel@cybertec.at
Andres Freund
Tom Lane
Thomas Munro
Peter Eisentraut
Alexander Korotkov
Michael Paquier
Alvaro Herrera
Greg Stark