Our initial work with int128 neglected alignment considerations, an
oversight that came back to bite us in bug #14897 from Vincent Lachenal.
It is unsurprising that int128 might have a 16-byte alignment requirement;
what's slightly more surprising is that even notoriously lax Intel chips
sometimes enforce that.
Raising MAXALIGN seems out of the question: the costs in wasted disk and
memory space would be significant, and there would also be an on-disk
compatibility break. Nor does it seem very practical to try to allow some
data structures to have more-than-MAXALIGN alignment requirement, as we'd
have to push knowledge of that throughout various code that copies data
structures around.
The only way out of the box is to make type int128 conform to the system's
alignment assumptions. Fortunately, gcc supports that via its
__attribute__(aligned()) pragma; and since we don't currently support
int128 on non-gcc-workalike compilers, we shouldn't be losing any platform
support this way.
Although we could have just done pg_attribute_aligned(MAXIMUM_ALIGNOF) and
called it a day, I did a little bit of extra work to make the code more
portable than that: it will also support int128 on compilers without
__attribute__(aligned()), if the native alignment of their 128-bit-int
type is no more than that of int64.
Add a regression test case that exercises the one known instance of the
problem, in parallel aggregation over a bigint column.
This will need to be back-patched, along with the preparatory commit
91aec93e6. But let's see what the buildfarm makes of it first.
Discussion: https://postgr.es/m/20171110185747.31519.28038@wrigleys.postgresql.org
Generalize section 1 to handle stuff that is principally about the
compiler (not libraries), such as attributes, and collect stuff there
that had been dropped into various other parts of c.h. Also, push
all the gettext macros into section 8, so that section 0 is really
just inclusions rather than inclusions and random other stuff.
The primary goal here is to get pg_attribute_aligned() defined before
section 3, so that we can use it with int128. But this seems like good
cleanup anyway.
This patch just moves macro definitions around, and shouldn't result
in any changes in generated code. But I'll push it out separately
to see if the buildfarm agrees.
Discussion: https://postgr.es/m/20171110185747.31519.28038@wrigleys.postgresql.org
Commit 5ecc0d738 removed the hard-wired superuser checks in lo_import
and lo_export in favor of protecting them with SQL permissions, but
failed to adjust the documentation to match. Fix that, and add a
<caution> paragraph pointing out the nontrivial security hazards
involved with actually granting such permissions. (It's still better
than ALLOW_DANGEROUS_LO_FUNCTIONS, though.)
Also, commit ae20b23a9 caused large object read/write privilege to
be checked during lo_open() rather than in the actual read or write
calls. Document that.
Discussion: https://postgr.es/m/CAB7nPqRHmNOYbETnc_2EjsuzSM00Z+BWKv9sy6tnvSd5gWT_JA@mail.gmail.com
Instead of passing large swaths of boolean arguments, define some flags
that can be used in a bitmask. This makes it easier not only to figure
out what each call site is doing, but also to add some new flags.
The flags are split in two -- one set for index_create directly and
another for constraints. index_create() itself receives both, and then
passes down the latter to index_constraint_create(), which can also be
called standalone.
Discussion: https://postgr.es/m/20171023151251.j75uoe27gajdjmlm@alvherre.pgsql
Reviewed-by: Simon Riggs
This feature caters to specialized use-cases such as running the normal
pgbench scenario with nonstandard indexes, or inserting other actions
between steps of the initialization sequence. The normal sequence of
initialization actions is broken down into half a dozen steps which can
be executed in a user-specified order, to the extent to which that's
sensible. The actions themselves aren't changed, except to make them
more robust against nonstandard uses:
* all four tables are now dropped in one DROP command, to reduce
assumptions about what foreign key relationships exist;
* all four tables are now truncated at the start of the data load
step, for consistency;
* the foreign key creation commands now specify constraint names, to
prevent accidentally creating duplicate constraints by executing the
'f' step twice.
Make some cosmetic adjustments in the messages emitted by pgbench
so that it's clear which steps are getting run, and so that the
messages agree with the documented names of the steps.
In passing, fix failure to enforce that the -v option is used only
in benchmarking mode.
Masahiko Sawada, reviewed by Fabien Coelho, editorialized a bit by me
Discussion: https://postgr.es/m/CAD21AoCsz0ZzfCFcxYZ+PUdpkDd5VsCSG0Pre_-K1EgokCDFYA@mail.gmail.com
Up until now, we only tracked the number of parameters, which was
sufficient to allocate an array of Datums of the appropriate size,
but not sufficient to, for example, know how to serialize a Datum
stored in one of those slots. An upcoming patch wants to do that,
so add this tracking to make it possible.
Patch by me, reviewed by Tom Lane and Amit Kapila.
Discussion: http://postgr.es/m/CA+TgmoYqpxDKn8koHdW8BEKk8FMUL0=e8m2Qe=M+r0UBjr3tuQ@mail.gmail.com
Apart from calling write_stderr() on failure, the handler depends on no
PostgreSQL facilities. We have experienced crashes before reaching the
former call site. Given such an early crash, this change cannot hurt
and may produce a helpful dump. Absent an early crash, this change has
no effect. Back-patch to 9.3 (all supported versions).
Takayuki Tsunakawa
Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1F80CD13@G01JPEXMBYT05
PostgreSQL running as a Windows service crashed upon calling
write_stderr() before MemoryContextInit(). This fix completes work
started in 5735efee15540765315aa8c1a230575e756037f7. Messages this
early contain only ASCII bytes; if we removed the CurrentMemoryContext
requirement, the ensuing conversions would have no effect. Back-patch
to 9.3 (all supported versions).
Takayuki Tsunakawa, reviewed by Michael Paquier.
Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1F80CC73@G01JPEXMBYT05
This suite had been a proper superset of the regular ecpg test suite,
but the three newest tests didn't reach it. To make this less likely to
recur, delete the extra schedule file and pass the TCP-specific test on
the command line. Back-patch to 9.3 (all supported versions).
Since commit 868898739a8da9ab74c105b8349b7b5c711f265a, it has assumed
"localhost" resolves to both ::1 and 127.0.0.1. We gain nothing from
that assumption, and it does not hold in a default installation of Red
Hat Enterprise Linux 5. Back-patch to 9.3 (all supported versions).
When a value contained an XML declaration naming some other encoding,
this function interpreted UTF8 bytes as the named encoding, yielding
mojibake. xml_parse() already has similar logic. This would be
necessary but not sufficient for non-UTF8 databases, so preserve
behavior there until the xpath facility can support such databases
comprehensively. Back-patch to 9.3 (all supported versions).
Pavel Stehule and Noah Misch
Discussion: https://postgr.es/m/CAFj8pRC-dM=tT=QkGi+Achkm+gwPmjyOayGuUfXVumCxkDgYWg@mail.gmail.com
An LDAP URL without a host name such as "ldap://" or without a base DN
such as "ldap://localhost" would cause a crash when reading pg_hba.conf.
If no binddn is configured, an error message might end up trying to print a
null pointer, which could crash on some platforms.
Author: Thomas Munro <thomas.munro@enterprisedb.com>
Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
Make bloom WAL test compare psql output text, not just result codes;
this was evidently the intent all along, but it was mis-coded.
In passing, make sure we will notice any failure in setup steps.
Alexander Korotkov, reviewed by Michael Paquier and Masahiko Sawada
Discussion: https://postgr.es/m/CAPpHfdtohPdQ9rc5mdWjxq+3VsBNw534KV_5O65dTQrSdVJNgw@mail.gmail.com
Hash partitioning is useful when you want to partition a growing data
set evenly. This can be useful to keep table sizes reasonable, which
makes maintenance operations such as VACUUM faster, or to enable
partition-wise join.
At present, we still depend on constraint exclusion for partitioning
pruning, and the shape of the partition constraints for hash
partitioning is such that that doesn't work. Work is underway to fix
that, which should both improve performance and make partitioning
pruning work with hash partitioning.
Amul Sul, reviewed and tested by Dilip Kumar, Ashutosh Bapat, Yugo
Nagata, Rajkumar Raghuwanshi, Jesper Pedersen, and by me. A few
final tweaks also by me.
Discussion: http://postgr.es/m/CAAJ_b96fhpJAP=ALbETmeLk1Uni_GFZD938zgenhF49qgDTjaQ@mail.gmail.com
Up to now, ACL checks for large objects happened at the level of
the SQL-callable functions, which led to CVE-2017-7548 because of a
missing check. Push them down to be enforced in inv_api.c as much
as possible, in hopes of preventing future bugs. This does have the
effect of moving read and write permission errors to happen at lo_open
time not loread or lowrite time, but that seems acceptable.
Michael Paquier and Tom Lane
Discussion: https://postgr.es/m/CAB7nPqRHmNOYbETnc_2EjsuzSM00Z+BWKv9sy6tnvSd5gWT_JA@mail.gmail.com
While it's generally unwise to give permissions on these functions to
anyone but a superuser, we've been moving away from hard-wired permission
checks inside functions in favor of using the SQL permission system to
control access. Bring lo_import() and lo_export() into compliance with
that approach.
In particular, this removes the manual configuration option
ALLOW_DANGEROUS_LO_FUNCTIONS. That dates back to 1999 (commit 4cd4a54c8);
it's unlikely anyone has used it in many years. Moreover, if you really
want such behavior, now you can get it with GRANT ... TO PUBLIC instead.
Michael Paquier
Discussion: https://postgr.es/m/CAB7nPqRHmNOYbETnc_2EjsuzSM00Z+BWKv9sy6tnvSd5gWT_JA@mail.gmail.com
Somebody messed up a refactoring here. As it stood, we'd check pg_ctl's
--version output twice for each cluster. Worse, the first check for the
new cluster's version happened before we'd done any validate_exec checks
there, breaking the check ordering the code intended.
A. Akenteva
Discussion: https://postgr.es/m/f9266a85d918a3cf3a386b5148aee666@postgrespro.ru
Upon further review, our Bonjour code doesn't actually work with the
Avahi not-too-compatible compatibility library. While you can get it
to work on non-macOS platforms if you link to Apple's own mDNSResponder
code, there don't seem to be many people who care about that. Leaving in
the AC_SEARCH_LIBS call seems more likely to encourage people to build
broken configurations than to do anything very useful.
Hence, remove the AC_SEARCH_LIBS call and put in a warning comment instead.
Discussion: https://postgr.es/m/2D8331C5-D64F-44C1-8717-63EDC6EAF7EB@brightforge.com
On macOS the relevant functions require no special library, but elsewhere
we need to pull in libdns_sd.
Back-patch to supported branches. No docs change since the docs do not
suggest that this is a Mac-only feature.
Luke Lonergan
Discussion: https://postgr.es/m/2D8331C5-D64F-44C1-8717-63EDC6EAF7EB@brightforge.com
The point of having separate ResourceOwnerEnlargeFoo and
ResourceOwnerRememberFoo functions is so that resource allocation
can happen in between. Doing it in some other order is just wrong.
OpenTemporaryFile() did open(), enlarge, remember, which would leak the
open file if the enlarge step ran out of memory. Because fd.c has its own
layer of resource-remembering, the consequences look like they'd be limited
to an intratransaction FD leak, but it's still not good.
IncrBufferRefCount() did enlarge, remember, incr-refcount, which would blow
up if the incr-refcount step ever failed. It was safe enough when written,
but since the introduction of PrivateRefCountHash, I think the assumption
that no error could happen there is pretty shaky.
The odds of real problems from either bug are probably small, but still,
back-patch to supported branches.
Thomas Munro and Tom Lane, per a comment from Andres Freund
The lower case spellings are C and C++ standard and are used in most
parts of the PostgreSQL sources. The upper case spellings are only used
in some files/modules. So standardize on the standard spellings.
The APIs for ICU, Perl, and Windows define their own TRUE and FALSE, so
those are left as is when using those APIs.
In code comments, we use the lower-case spelling for the C concepts and
keep the upper-case spelling for the SQL concepts.
Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
isdigit(), isspace(), etc are likely to give surprising results if passed a
signed char. We should always cast the argument to unsigned char to avoid
that. Error in commit 63d6b97fd, found by buildfarm member gaur.
Back-patch to 9.3, like that commit.
Previously server reserved WAL for last two checkpoints,
which used too much disk space for small servers.
Bumps PG_CONTROL_VERSION
Author: Simon Riggs <simon@2ndQuadrant.com>
Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
Add docs to explain this for other backup mechanisms
Author: David Steele <david@pgmasters.net>
Reviewed-by: Petr Jelinek <petr.jelinek@2ndQuadrant.com> et al
configure computed PG_VERSION_NUM incorrectly. (Coulda sworn I tested
that logic back when, but it had an obvious thinko.)
pg_upgrade had not been taught about the new dispensation with just
one part in the major version number.
Both things accidentally failed to fail with 10.0, but with 10.1 we
got the wrong results.
Per buildfarm.
The problem reported as CVE-2017-15098 was already resolved in HEAD by
commit 37a795a60, but let's add the relevant test cases anyway.
Michael Paquier and Tom Lane, per a report from David Rowley.
Security: CVE-2017-15098
By default, $PGUSER has permission to unlink $PGLOG. If $PGUSER
replaces $PGLOG with a symbolic link, the server will corrupt the
link-targeted file by appending log messages. Since these scripts open
$PGLOG as root, the attack works regardless of target file ownership.
"make install" does not install these scripts anywhere. Users having
manually installed them in the past should repeat that process to
acquire this fix. Most script users have $PGLOG writable to root only,
located in $PGDATA. Just before updating one of these scripts, such
users should rename $PGLOG to $PGLOG.old. The script will then recreate
$PGLOG with proper ownership.
Reviewed by Peter Eisentraut. Reported by Antoine Scemama.
Security: CVE-2017-12172
The update path of an INSERT ... ON CONFLICT DO UPDATE requires SELECT
permission on the columns of the arbiter index, but it failed to check
for that in the case of an arbiter specified by constraint name.
In addition, for a table with row level security enabled, it failed to
check updated rows against the table's SELECT policies when the update
path was taken (regardless of how the arbiter index was specified).
Backpatch to 9.5 where ON CONFLICT DO UPDATE and RLS were introduced.
Security: CVE-2017-15099
Makefile.global assigns this prerequisite to every target named "check",
but similar targets must mention it explicitly. Affected targets
failed, tested $PATH binaries, or tested a stale temporary installation.
The src/test/modules examples worked properly when called as "make -C
src/test/modules/$FOO check", but "make -j" allowed the test to start
before the temporary installation was in place. Back-patch to 9.5,
where commit dcae5faccab64776376d354decda0017c648bb53 introduced the
shared temp-install.
In the v10 branch, also back-patch the effects of 1ff01b390 and c29c57890
on these files, to reduce future maintenance issues. (I'd do it further
back, except that the 9.X branches differ anyway due to xlog-to-wal
link tag renaming.)
