Allow SCRAM authentication, when pg_hba.conf says 'md5'.
If a user has a SCRAM verifier in pg_authid.rolpassword, there's no reason
we cannot attempt to perform SCRAM authentication instead of MD5. The worst
that can happen is that the client doesn't support SCRAM, and the
authentication will fail. But previously, it would fail for sure, because
we would not even try. SCRAM is strictly more secure than MD5, so there's
no harm in trying it. This allows for a more graceful transition from MD5
passwords to SCRAM, as user passwords can be changed to SCRAM verifiers
incrementally, without changing pg_hba.conf.
Refactor the code in auth.c to support that better. Notably, we now have to
look up the user's pg_authid entry before sending the password challenge,
also when performing MD5 authentication. Also simplify the concept of a
"doomed" authentication. Previously, if a user had a password, but it had
expired, we still performed SCRAM authentication (but always returned error
at the end) using the salt and iteration count from the expired password.
Now we construct a fake salt, like we do when the user doesn't have a
password or doesn't exist at all. That simplifies get_role_password(), and
we can don't need to distinguish the "user has expired password", and
"user does not exist" cases in auth.c.
On second thoughts, also rename uaSASL to uaSCRAM. It refers to the
mechanism specified in pg_hba.conf, and while we use SASL for SCRAM
authentication at the protocol level, the mechanism should be called SCRAM,
not SASL. As a comparison, we have uaLDAP, even though it looks like the
plain 'password' authentication at the protocol level.
Discussion: https://www.postgresql.org/message-id/6425.1489506016@sss.pgh.pa.us Reviewed-by: Michael Paquier
Teodor Sigaev [Fri, 24 Mar 2017 10:53:40 +0000 (13:53 +0300)]
Fix backup canceling
Assert-enabled build crashes but without asserts it works by wrong way:
it may not reset forcing full page write and preventing from starting
exclusive backup with the same name as cancelled.
Patch replaces pair of booleans
nonexclusive_backup_running/exclusive_backup_running to single enum to
correctly describe backup state.
Backpatch to 9.6 where bug was introduced
Reported-by: David Steele
Authors: Michael Paquier, David Steele Reviewed-by: Anastasia Lubennikova
https://commitfest.postgresql.org/13/1068/
Robert Haas [Thu, 23 Mar 2017 20:10:43 +0000 (16:10 -0400)]
Fix enum definition.
Commit 249cf070e36721a65be74838c53acf8249faf935 assigned to one of
the labels in the middle the value that should have been assigned
to the first member of the enum. Rushabh's patch didn't have that
defect as submitted, but I managed to mess it up while editing.
Repair.
Peter Eisentraut [Thu, 23 Mar 2017 19:25:34 +0000 (15:25 -0400)]
ICU support
Add a column collprovider to pg_collation that determines which library
provides the collation data. The existing choices are default and libc,
and this adds an icu choice, which uses the ICU4C library.
The pg_locale_t type is changed to a union that contains the
provider-specific locale handles. Users of locale information are
changed to look into that struct for the appropriate handle to use.
Also add a collversion column that records the version of the collation
when it is created, and check at run time whether it is still the same.
This detects potentially incompatible library upgrades that can corrupt
indexes and other structures. This is currently only supported by
ICU-provided collations.
initdb initializes the default collation set as before from the `locale
-a` output but also adds all available ICU locales with a "-x-icu"
appended.
Currently, ICU-provided collations can only be explicitly named
collations. The global database locales are still always libc-provided.
ICU support is enabled by configure --with-icu.
Reviewed-by: Thomas Munro <thomas.munro@enterprisedb.com> Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Robert Haas [Thu, 23 Mar 2017 18:08:23 +0000 (14:08 -0400)]
Track the oldest XID that can be safely looked up in CLOG.
This provides infrastructure for looking up arbitrary, user-supplied
XIDs without a risk of scary-looking failures from within the clog
module. Normally, the oldest XID that can be safely looked up in CLOG
is the same as the oldest XID that can reused without causing
wraparound, and the latter is already tracked. However, while
truncation is in progress, the values are different, so we must
keep track of them separately.
Robert Haas [Thu, 23 Mar 2017 17:05:48 +0000 (13:05 -0400)]
Allow for parallel execution whenever ExecutorRun() is done only once.
Previously, it was unsafe to execute a plan in parallel if
ExecutorRun() might be called with a non-zero row count. However,
it's quite easy to fix things up so that we can support that case,
provided that it is known that we will never call ExecutorRun() a
second time for the same QueryDesc. Add infrastructure to signal
this, and cross-checks to make sure that a caller who claims this is
true doesn't later reneg.
While that pattern never happens with queries received directly from a
client -- there's no way to know whether multiple Execute messages
will be sent unless the first one requests all the rows -- it's pretty
common for queries originating from procedural languages, which often
limit the result to a single tuple or to a user-specified number of
tuples.
This commit doesn't actually enable parallelism in any additional
cases, because currently none of the places that would be able to
benefit from this infrastructure pass CURSOR_OPT_PARALLEL_OK in the
first place, but it makes it much more palatable to pass
CURSOR_OPT_PARALLEL_OK in places where we currently don't, because it
eliminates some cases where we'd end up having to run the parallel
plan serially.
Patch by me, based on some ideas from Rafia Sabih and corrected by
Rafia Sabih based on feedback from Dilip Kumar and myself.
Teodor Sigaev [Thu, 23 Mar 2017 16:38:47 +0000 (19:38 +0300)]
Reduce page locking in GIN vacuum
GIN vacuum during cleaning posting tree can lock this whole tree for a long
time with by holding LockBufferForCleanup() on root. Patch changes it with
two ways: first, cleanup lock will be taken only if there is an empty page
(which should be deleted) and, second, it tries to lock only subtree, not the
whole posting tree.
Author: Andrey Borodin with minor editorization by me Reviewed-by: Jeff Davis, me
https://commitfest.postgresql.org/13/896/
Simon Riggs [Thu, 23 Mar 2017 14:05:28 +0000 (14:05 +0000)]
Replication lag tracking for walsenders
Adds write_lag, flush_lag and replay_lag cols to pg_stat_replication.
Implements a lag tracker module that reports the lag times based upon
measurements of the time taken for recent WAL to be written, flushed and
replayed and for the sender to hear about it. These times
represent the commit lag that was (or would have been) introduced by each
synchronous commit level, if the remote server was configured as a
synchronous standby. For an asynchronous standby, the replay_lag column
approximates the delay before recent transactions became visible to queries.
If the standby server has entirely caught up with the sending server and
there is no more WAL activity, the most recently measured lag times will
continue to be displayed for a short time and then show NULL.
Physical replication lag tracking is automatic. Logical replication tracking
is possible but is the responsibility of the logical decoding plugin.
Tracking is a private module operating within each walsender individually,
with values reported to shared memory. Module not used outside of walsender.
Design and code is good enough now to commit - kudos to the author.
In many ways a difficult topic, with important and subtle behaviour so this
shoudl be expected to generate discussion and multiple open items: Test now!
Author: Thomas Munro, following designs by Fujii Masao and Simon Riggs
Review: Simon Riggs, Ian Barwick and Craig Ringer
Peter Eisentraut [Thu, 23 Mar 2017 12:36:36 +0000 (08:36 -0400)]
Logical replication support for initial data copy
Add functionality for a new subscription to copy the initial data in the
tables and then sync with the ongoing apply process.
For the copying, add a new internal COPY option to have the COPY source
data provided by a callback function. The initial data copy works on
the subscriber by receiving COPY data from the publisher and then
providing it locally into a COPY that writes to the destination table.
A WAL receiver can now execute full SQL commands. This is used here to
obtain information about tables and publications.
Several new options were added to CREATE and ALTER SUBSCRIPTION to
control whether and when initial table syncing happens.
Change pg_dump option --no-create-subscription-slots to
--no-subscription-connect and use the new CREATE SUBSCRIPTION
... NOCONNECT option for that.
Author: Petr Jelinek <petr.jelinek@2ndquadrant.com> Tested-by: Erik Rijkers <er@xs4all.nl>
Stephen Frost [Thu, 23 Mar 2017 03:44:58 +0000 (23:44 -0400)]
Expose waitforarchive option through pg_stop_backup()
Internally, we have supported the option to either wait for all of the
WAL associated with a backup to be archived, or to return immediately.
This option is useful to users of pg_stop_backup() as well, when they
are reading the stop backup record position and checking that the WAL
they need has been archived independently.
This patch adds an additional, optional, argument to pg_stop_backup()
which allows the user to indicate if they wish to wait for the WAL to be
archived or not. The default matches current behavior, which is to
wait.
Author: David Steele, with some minor changes, doc updates by me.
Reviewed by: Takayuki Tsunakawa, Fujii Masao
Discussion: https://postgr.es/m/758e3fd1-45b4-5e28-75cd-e9e7f93a4c02@pgmasters.net
Magnus Hagander [Wed, 22 Mar 2017 16:55:16 +0000 (17:55 +0100)]
Support multiple RADIUS servers
This changes all the RADIUS related parameters (radiusserver,
radiussecret, radiusport, radiusidentifier) to be plural and to accept a
comma separated list of servers, which will be tried in order.
Simon Riggs [Wed, 22 Mar 2017 16:37:28 +0000 (16:37 +0000)]
Assign AccessExclusiveLocks against subxacts in Hot Standby
Previously AELs were registered against the top-level xid, which could
cause locks to be held much longer than necessary in some cases during
Hot Standby replay. We now record locks directly against their appropriate
xids. Requires few code changes because original code allowed for this
situation but didn’t fully implement it.
Peter Eisentraut [Mon, 13 Feb 2017 21:50:29 +0000 (16:50 -0500)]
Change logical replication pg_hba.conf use
Logical replication no longer uses the "replication" keyword. It just
matches database entries in the normal way. The "replication" keyword
now only applies to physical replication.
Reviewed-by: Petr Jelinek <petr.jelinek@2ndquadrant.com>
Simon Riggs [Wed, 22 Mar 2017 13:09:36 +0000 (13:09 +0000)]
Improve performance of replay of AccessExclusiveLocks
A hot standby replica keeps a list of Access Exclusive locks for a top
level transaction. These locks are released when the top level transaction
ends. Searching of this list is O(N^2), and each transaction had to pay the
price of searching this list for locks, even if it didn't take any AE
locks itself.
This patch optimizes this case by having the master server track which
transactions took AE locks, and passes that along to the standby server in
the commit/abort record. This allows the standby to only try to release
locks for transactions which actually took any, avoiding the majority of
the performance issue.
Refactor MyXactAccessedTempRel into MyXactFlags to allow minimal additional
cruft with this.
Analysis and initial patch by David Rowley
Author: David Rowley and Simon Riggs
Simon Riggs [Wed, 22 Mar 2017 07:05:12 +0000 (07:05 +0000)]
Teach xlogreader to follow timeline switches
Uses page-based mechanism to ensure we’re using the correct timeline.
Tests are included to exercise the functionality using a cold disk-level copy
of the master that's started up as a replica with slots intact, but the
intended use of the functionality is with later features.
Craig Ringer, reviewed by Simon Riggs and Andres Freund
Peter Eisentraut [Wed, 22 Mar 2017 02:09:40 +0000 (22:09 -0400)]
doc: Update tool sets documentation for modern FreeBSD
For several operating systems, we give handy package manager one-liners
to install all the requirements for building our documentation. All
current production FreeBSD releases have a friendly new package manager
a bit like apt/yum, so give a one line command here. Also, add a brief
note about gmake vs make in the doc subdirectory.
Author: Thomas Munro <thomas.munro@enterprisedb.com>
Andrew Dunstan [Tue, 21 Mar 2017 13:12:46 +0000 (09:12 -0400)]
Use CallerFInfoFunctionCall with btree_gist for numeric types
None of the existing types actually need to use this mechanism, but this
will allow support for enum types which will need it. A separate patch
will adjust the varlena types support for consistency.
Simon Riggs [Tue, 21 Mar 2017 14:04:49 +0000 (14:04 +0000)]
Add a pg_recvlogical wrapper to PostgresNode
Allows testing of logical decoding using SQL interface and/or pg_recvlogical
Most logical decoding tests are in contrib/test_decoding. This module
is for work that doesn't fit well there, like where server restarts
are required.
Robert Haas [Tue, 21 Mar 2017 13:48:04 +0000 (09:48 -0400)]
Don't scan partitioned tables.
Partitioned tables do not contain any data; only their unpartitioned
descendents need to be scanned. However, the partitioned tables still
need to be locked, even though they're not scanned. To make that
work, Append and MergeAppend relations now need to carry a list of
(unscanned) partitioned relations that must be locked, and InitPlan
must lock all partitioned result relations.
Aside from the obvious advantage of avoiding some work at execution
time, this has two other advantages. First, it may improve the
planner's decision-making in some cases since the empty relation
might throw things off. Second, it paves the way to getting rid of
the storage for partitioned tables altogether.
Teodor Sigaev [Tue, 21 Mar 2017 13:23:10 +0000 (16:23 +0300)]
Fix support for some operators (&<, &>, $<|, |&>) in box operator class
of SP-GiST.
Bug exists since initial commit of box opclass for SP-GiST,
so backpath to 9.6
Author: Nikita Glukhov with minor editorization of tests by me Reviewed-by: Kyotaro Horiguchi, Anastasia Lubennikova
https://commitfest.postgresql.org/13/981/
Andrew Dunstan [Tue, 21 Mar 2017 12:57:46 +0000 (08:57 -0400)]
Add a direct function call mechanism using the caller's context.
The current DirectFunctionCall functions use NULL as the flinfo in
initializing the FunctionCallInfoData for the call. That means the
called function has no fn_mcxt or fn_extra to work with, and attempting
to do so will result in an access violation. These functions instead use
the provided flinfo, which will usually be the caller's own flinfo. The
caller needs to ensure that it doesn't use the fn_extra in way that is
incompatible with the way the called function will use it. The called
function should not rely on anything else in the provided context, as it
will be relevant to the caller, not the callee.
Andrew Dunstan [Mon, 20 Mar 2017 20:40:45 +0000 (16:40 -0400)]
Add IF NOT EXISTS for CREATE SERVER and CREATE USER MAPPING
There is still some inconsistency with the error messages surrounding
foreign servers. Some use the word "foreign" and some don't. My
inclination is to remove all such uses of "foreign" on the basis that
the CREATE/ALTER/DROP SERVER commands don't use the word. However, that
is left for another day. In this patch I have kept to the existing usage
in the affected commands, which omits "foreign".
Anastasia Lubennikova, reviewed by Arthur Zakirov and Ashtosh Bapat.
Andrew Dunstan [Mon, 20 Mar 2017 20:01:45 +0000 (16:01 -0400)]
Use a consistent error message style for user mappings.
User mappings are essentially anonymous, so messages referring to "user
mapping foo on server bar" are wrong, and inconsistent with other error
messages referring to user mappings. To be consistent with existing use,
use "user mapping for foo on server bar" instead.
I dropped the noise word "user" from the original suggestion to be
consistent with other uses.
Bump WAL version. The original patch to make hash indexes write-ahead
logged probably should have done this, and the single page vacuuming
patch probably should have done it again, but better late than never.
Tom Lane [Mon, 20 Mar 2017 16:51:45 +0000 (12:51 -0400)]
Add missed "make clean" logic in src/interfaces/libpq/Makefile.
Commit 818fd4a67 missed cleaning up the symlinks it added for various .c
files imported from src/port and src/common. Neatnik-ishly make the
file lists in the "clean" target look exactly like the earlier lists of
what to symlink in.
Tom Lane [Mon, 20 Mar 2017 16:30:08 +0000 (12:30 -0400)]
Improve regression test coverage for TID scanning.
TidScan plan nodes were not systematically tested before. These additions
raise the LOC coverage number for the basic regression tests from 52% to
92% in nodeTidscan.c, and from 60% to 93% in tidpath.c.
Teodor Sigaev [Mon, 20 Mar 2017 15:48:46 +0000 (18:48 +0300)]
Revert unintentional change in increasing usage count during pin of buffers,
this makes buffer access strategy have no effect.
Change was a part of commit 48354581a49c30f5757c203415aa8412d85b0f70 during 9.6
release cycle, so backpath to 9.6
Reported-by: Jim Nasby
Author: Alexander Korotkov Reviewed-by: Jim Nasby, Andres Freund
https://commitfest.postgresql.org/13/1029/
Stephen Frost [Sun, 19 Mar 2017 20:56:14 +0000 (16:56 -0400)]
pg_dump: Skip COLLATION-related regression tests
Not every platform supports non-default collations, as pointed out by
the buildfarm, so skip collation-related regression tests in pg_dump
when they aren't supported.
Tom Lane [Sat, 18 Mar 2017 21:49:06 +0000 (17:49 -0400)]
Fix REFRESH MATERIALIZED VIEW to report activity to the stats collector.
The non-concurrent code path for REFRESH MATERIALIZED VIEW failed to
report its updates to the stats collector. This is bad since it means
auto-analyze doesn't know there's any work to be done. Adjust it to
report the refresh as a table truncate followed by insertion of an
appropriate number of rows.
Since a matview could contain more than INT_MAX rows, change the
signature of pgstat_count_heap_insert() to accept an int64 rowcount.
(The accumulator it's adding into is already int64, but existing
callers could not insert more than a small number of rows at once,
so the argument had been declared just "int n".)
This is surely a bug fix, but changing pgstat_count_heap_insert()'s API
seems too risky for the back branches. Given the lack of previous
complaints, I'm not sure it's a big enough problem to justify a kluge
solution that would avoid that. So, no back-patch, at least for now.
Peter Eisentraut [Sat, 18 Mar 2017 18:17:28 +0000 (14:17 -0400)]
doc: Tweak reference page header style
Change the header style of references pages in HTML and PDF output to be
more like the old style under DSSSL. In particular, the page should
start with a header containing the command name, instead of just "Name".
Stephen Frost [Sat, 18 Mar 2017 17:18:24 +0000 (13:18 -0400)]
Improve pg_dump regression tests and code coverage
These improvements bring the lines-of-code coverage of pg_dump.c up to
87.7% (at least using LCOV 1.12, 1.11 seems to differ slightly). Nearly
every function is covered, three of the four which aren't are only
called when talking to older PG instances.
There is more which can, and should, be done here to improve the
coverage but it's past time to see what the buildfarm thinks of this.
What has been added:
- Coverage for many more command-line options
- Use command_fails_like instead of command_exit_is
- Operator classes, operator families
- Text search configuration, templates, parsers, dictionaries
- FDWs, servers, foreign tables
- Materialized views
- Improved Publications / Subscriptions test (though this needs work,
see PG10 open items and tests marked with XXX in 002_pg_dump.pl)
- Unlogged tables
- Partitioned tables
- Additional ACL testing for various object types
There is room for improvement, specifically:
- Various type-based option (alignment, storage, etc)
- Composite type collation
- Extra Procedural language functions (inline, validator)
- Different function options (SRF, Transform, config, security definer,
cost, leakproof)
- OpClass options (default, storage, order by, recheck)
- OpFamily options (order by, recheck)
- Aggregate functions (combinefunc, serialfunc, deserialfunc, etc)
- Text Search parser 'headline'
- Text Search template 'init'
- FDW options (handler, validator, options)
- Server options (type, version, options)
- User mapping options
- Default ACLs for sequences, types
- Security labels
- View circular dependencies (last function that needs coverage)
- Toast table autovacuum options
- Replica identity options
- Independent indexes (plus marking them as clustered on)
- Deferrable / initially deferred constraints
- Independent domain constraints
There's bits of extension pg_dump'ing also not covered, but those will
need to go into test_pg_dump (such as having a filter for config
tables).
Last, but not least, this approximately halves the number of tests run
with 'ok()' by removing the ok()-based checking of if all runs are
covered by each test. Instead, 002_pg_dump.pl will just exit out in
such a case (with a message in the log file). In general, when adding
tests, cover all runs unless there is a very good reason not to (such as
adding a 'catch-all' case). With these changes, the resulting output
and number of "tests" run is actually reduced.
Robert Haas [Sat, 18 Mar 2017 11:43:01 +0000 (07:43 -0400)]
Create and use wait events for read, write, and fsync operations.
Previous commits, notably 53be0b1add7064ca5db3cd884302dfc3268d884e and 6f3bd98ebfc008cbd676da777bb0b2376c4c4bfa, made it possible to see from
pg_stat_activity when a backend was stuck waiting for another backend,
but it's also fairly common for a backend to be stuck waiting for an
I/O. Add wait events for those operations, too.
Rushabh Lathia, with further hacking by me. Reviewed and tested by
Michael Paquier, Amit Kapila, Rajkumar Raghuwanshi, and Rahila Syed.
Tom Lane [Fri, 17 Mar 2017 18:58:06 +0000 (14:58 -0400)]
Fix WaitEventSetWait() to handle write-ready waits properly on Windows.
Windows apparently will not detect socket write-ready events unless a
preceding send attempt returned WSAEWOULDBLOCK. In many usage patterns
that's satisfied by the caller of WaitEvenSetWait(), but not always.
Apply the same solution that we already had in pgwin32_select(), namely to
perform a dummy WSASend() call with len=0. This will return WSAEWOULDBLOCK
if there's no buffer space (even though it could legitimately do nothing
and report success, which makes me a bit nervous about this solution;
but since it's been working fine in libpq, let's roll with it).
In passing, improve the comments about this in pgwin32_select(), and remove
duplicated code there.
Back-patch to 9.6 where WaitEventSetWait() was introduced. We might need
to back-patch something similar into predecessor code. But given the lack
of complaints so far, it's not clear that the case ever gets exercised
in the back branches, so I'm not going to expend effort on it right now.
This should resolve recurring failures on buildfarm member bowerbird,
which has been failing since 1e8a85009 went in.
Diagnosis and patch by Petr Jelinek, cosmetic adjustments by me.
Andrew Gierth [Fri, 17 Mar 2017 14:35:54 +0000 (14:35 +0000)]
Repair test for vacuum reltuples fix.
Concurrent auto-analyze could be holding a snapshot, affecting the
removal of deleted row versions. Remove the deletion to avoid this
happening. Per buildfarm.
In passing, make the test independent of assumptions of physical row
order, just out of sheer paranoia.
Robert Haas [Fri, 17 Mar 2017 13:46:58 +0000 (09:46 -0400)]
Rename "pg_clog" directory to "pg_xact".
Names containing the letters "log" sometimes confuse users into
believing that only non-critical data is present. It is hoped
this renaming will discourage ill-considered removals of transaction
status data.
Add TAP tests for password-based authentication methods.
Tests all combinations of users with MD5, plaintext and SCRAM verifiers
stored in pg_authid, with plain 'password', 'md5' and 'scram'
authentication methods.
Fix and simplify check for whether we're running as Windows service.
If the process token contains SECURITY_SERVICE_RID, but it has been
disabled by the SE_GROUP_USE_FOR_DENY_ONLY attribute, win32_is_service()
would incorrectly report that we're running as a service. That situation
arises, e.g. if postmaster is launched with a restricted security token,
with the "Log in as Service" privilege explicitly removed.
Replace the broken code with CheckProcessTokenMembership(), which does
this correctly. Also replace similar code in win32_is_admin(), even
though it got this right, for simplicity and consistency.
Per bug #13755, reported by Breen Hagan. Back-patch to all supported
versions. Patch by Takayuki Tsunakawa, reviewed by Michael Paquier.
Andrew Gierth [Thu, 16 Mar 2017 22:28:03 +0000 (22:28 +0000)]
Avoid having vacuum set reltuples to 0 on non-empty relations in the
presence of page pins, which leads to serious estimation errors in the
planner. This particularly affects small heavily-accessed tables,
especially where locking (e.g. from FK constraints) forces frequent
vacuums for mxid cleanup.
Fix by keeping separate track of pages whose live tuples were actually
counted vs. pages that were only scanned for freezing purposes. Thus,
reltuples can only be set to 0 if all pages of the relation were
actually counted.
Backpatch to all supported versions.
Per bug #14057 from Nicolas Baccelli, analyzed by me.
Robert Haas [Thu, 16 Mar 2017 19:05:02 +0000 (15:05 -0400)]
Add pg_ls_logdir() and pg_ls_waldir() functions.
These functions are intended to be used by monitoring tools, and,
unlike pg_ls_dir(), access to them can be granted to non-superusers,
so that those monitoring tools can observe the principle of least
privilege.
Dave Page, revised by me, and also reviewed a bit by Thomas Munro.
Robert Haas [Thu, 16 Mar 2017 17:34:59 +0000 (13:34 -0400)]
postgres_fdw: Push down FULL JOINs with restriction clauses.
The previous deparsing logic wasn't smart enough to produce subqueries
when deparsing; make it smart enough to do that. However, we only do
it that way when necessary, because it generates more complicated SQL
which will be harder for any humans reading the queries to understand.
Stephen Frost [Thu, 16 Mar 2017 04:13:37 +0000 (00:13 -0400)]
Be more careful about signed vs. unsigned char
The buildfarm has reminded me that not all systems consider char to be
signed and we need to be explicit. Adjust the various bits of mac8.c
for what we intend, mostly using casts to unsigned char as suggested by
Tom, and adjust the tests for valid input accordingly. Explicitly make
the hexlookup table signed as it's useful to use -1 there to indicate an
invalid value.
Stephen Frost [Thu, 16 Mar 2017 03:23:28 +0000 (23:23 -0400)]
Clean up overly paranoid checks in mac8.c
Andres' compiler points out, quite correctly, that there's no need for
some of the overly paranoid checks which were put into mac8.c. Remove
those, as they're useless, add some comments and make a few other minor
improvements- reduce the size of hexlookup by making it a char array
instead of an int array, and pass in the ptr location directly instead
of making hex2_to_uchar re-calculate the location based off the offset
every time.
Andres Freund [Thu, 16 Mar 2017 02:03:29 +0000 (19:03 -0700)]
Specify bindir in pg_isolation_regress_installcheck.
It appears dcae5faccab64776376d3 forgot to add it to
pg_isolation_regress_installcheck, while it was added to
pg_regress_installcheck. It seems to so far have escaped notice,
because buildfarm animals requiring it, didn't actually use
pg_isolation_regress_installcheck anywhere - that changed with 60f826c5e6244, triggering failures on narwhal and frogmouth.
I've decided to not, for now at least, backpatch this, because the
relevant invocations look quite different in the back branches. Seems
quite possible that we'll want to backport 60f826c5e6244 as a whole if
it proves stable.
Tom Lane [Wed, 15 Mar 2017 17:26:26 +0000 (13:26 -0400)]
Rewrite async-connection loop in libpqwalreceiver.c, once again.
The original coding in commit 1e8a85009 didn't use PQconnectPoll per
spec, and while the rewrite in e434ad39a is closer, it still doesn't
guarantee to wait until the socket is read-ready or write-ready (as
appropriate) before calling PQconnectPoll. It's not clear whether
that omission is causing the continuing failures on buildfarm member
bowerbird; but given the lack of other explanations meeting the
available facts, let's tighten that up and see what happens.
An independent issue in the same loop was that it had a race condition
whereby it could clear the process's latch without having serviced an
interrupt request, causing failure to respond to a cancel while waiting
for connection (the very problem 1e8a85009 was meant to fix).
projects / postgresql / log