Commit Graph

36457 Commits

Author SHA1 Message Date
Heikki Linnakangas
7ca32e255b Fix hot standby bug with GiST scans.
Don't reset the rightlink of a page when replaying a page update record.
This was a leftover from pre-hot standby days, when it was not possible to
have scans concurrent with WAL replay. Resetting the right-link was not
necessary back then either, but it was done for the sake of tidiness. But
with hot standby, it's wrong, because a concurrent scan might still need it.

Backpatch all versions with hot standby, 9.0 and above.
2014-04-08 14:51:40 +03:00
Heikki Linnakangas
38a2b95c34 Zero padding byte at end of GIN posting list.
This isn't strictly necessary, but helps debugging.
2014-04-07 19:49:03 +03:00
Robert Haas
f235db03ff Remove 'make clean' support for ipc_test.
I missed this in the previous commit; Tom Lane spotted my error.
2014-04-07 11:45:27 -04:00
Robert Haas
315772e4ec Assert that strong-lock count is >0 everywhere it's decremented.
The one existing assertion of this type has tripped a few times in the
buildfarm lately, but it's not clear whether the problem is really
originating there or whether it's leftovers from a trip through one
of the other two paths that lack a matching assertion.  So add one.

Since the same bug(s) most likely exist(s) in the back-branches also,
back-patch to 9.2, where the fast-path lock mechanism was added.
2014-04-07 10:59:42 -04:00
Robert Haas
b8a721149b Remove ipc_test.
This doesn't seem to be useful any more, and it's not really worth the
effort to keep updating it every time relevant dependencies or calling
signatures in the shared memory or semaphore code change.
2014-04-07 10:40:47 -04:00
Heikki Linnakangas
594bac4272 Fix WAL replay bug in the new GIN incomplete-split code.
Forgot to set the incomplete-split flag on the left page half, in redo of a
page split.

Spotted this by comparing the page contents on master and standby, after
inserting/applying each WAL record.
2014-04-07 14:37:30 +03:00
Simon Riggs
7d8f1de1bc Extra warnings and errors for PL/pgSQL
Infrastructure to allow
 plpgsql.extra_warnings
 plpgsql.extra_errors

Initial extra checks only for shadowed_variables

Marko Tiikkaja and Petr Jelinek
Reviewed by Simon Riggs and Pavel Stěhule
2014-04-06 12:21:51 -04:00
Simon Riggs
f14a6bbedb Isolation test files for ALTER TABLE patch 2014-04-06 11:44:24 -04:00
Simon Riggs
e5550d5fec Reduce lock levels of some ALTER TABLE cmds
VALIDATE CONSTRAINT

CLUSTER ON
SET WITHOUT CLUSTER

ALTER COLUMN SET STATISTICS
ALTER COLUMN SET ()
ALTER COLUMN RESET ()

All other sub-commands use AccessExclusiveLock

Simon Riggs and Noah Misch

Reviews by Robert Haas and Andres Freund
2014-04-06 11:13:43 -04:00
Tom Lane
80a5cf643a Improve contrib/pg_trgm's heuristics for regexp index searches.
When extracting trigrams from a regular expression for search of a GIN or
GIST trigram index, it's useful to penalize (preferentially discard)
trigrams that contain whitespace, since those are typically far more common
in the index than trigrams not containing whitespace.  Of course, this
should only be a preference not a hard rule, since we might otherwise end
up with no trigrams to search for.  The previous coding tended to produce
fairly inefficient trigram search sets for anchored regexp patterns, as
reported by Erik Rijkers.  This patch penalizes whitespace-containing
trigrams, and also reduces the target number of extracted trigrams, since
experience suggests that the original coding tended to select too many
trigrams to search for.

Alexander Korotkov, reviewed by Tom Lane
2014-04-05 20:48:47 -04:00
Tom Lane
5d8117e1f3 Block signals earlier during postmaster startup.
Formerly, we set up the postmaster's signal handling only when we were
about to start launching subprocesses.  This is a bad idea though, as
it means that for example a SIGINT arriving before that will kill the
postmaster instantly, perhaps leaving lockfiles, socket files, shared
memory, etc laying about.  We'd rather that such a signal caused orderly
postmaster termination including releasing of those resources.  A simple
fix is to move the PostmasterMain stanza that initializes signal handling
to an earlier point, before we've created any such resources.  Then, an
early-arriving signal will be blocked until we're ready to deal with it
in the usual way.  (The only part that really needs to be moved up is
blocking of signals, but it seems best to keep the signal handler
installation calls together with that; for one thing this ensures the
kernel won't drop any signals we wished to get.  The handlers won't get
invoked in any case until we unblock signals in ServerLoop.)

Per a report from MauMau.  He proposed changing the way "pg_ctl stop"
works to deal with this, but that'd just be masking one symptom not
fixing the core issue.

It's been like this since forever, so back-patch to all supported branches.
2014-04-05 18:16:08 -04:00
Heikki Linnakangas
ffbba6ee12 Fix another palloc in critical section.
Also add a regression test for a GIN index with enough items with the same
key, so that a GIN posting tree gets created. Apparently none of the
existing GIN tests were large enough for that.

This code is new, no backpatching required.
2014-04-05 22:15:58 +03:00
Tom Lane
6862ca6970 Fix processing of PGC_BACKEND GUC parameters on Windows.
EXEC_BACKEND builds (i.e., Windows) failed to absorb values of PGC_BACKEND
parameters if they'd been changed post-startup via the config file.  This
for example prevented log_connections from working if it were turned on
post-startup.  The mechanism for handling this case has always been a bit
of a kluge, and it wasn't revisited when we implemented EXEC_BACKEND.
While in a normal forking environment new backends will inherit the
postmaster's value of such settings, EXEC_BACKEND backends have to read
the settings from the CONFIG_EXEC_PARAMS file, and they were mistakenly
rejecting them.  So this case has always been broken in the Windows port;
so back-patch to all supported branches.

Amit Kapila
2014-04-05 12:41:25 -04:00
Tom Lane
44c5d387ea ecpg/ecpglib must build the src/port files it uses with -DFRONTEND.
Remarkably, this hasn't been noticed before, though it surely should
have been happening since around the fall of the Byzantine empire.
Commit 438b529604 changed path.c to depend on FRONTEND, and that exposed
the omission, per buildfarm reports.

I'm suspicious that some other subdirectories are missing this too,
but this one change is enough to make ecpg tests pass for me.
2014-04-05 02:20:25 -04:00
Tom Lane
abe075dfff Fix tablespace creation WAL replay to work on Windows.
The code segment that removes the old symlink (if present) wasn't clued
into the fact that on Windows, symlinks are junction points which have
to be removed with rmdir().

Backpatch to 9.0, where the failing code was introduced.

MauMau, reviewed by Muhammad Asif Naeem and Amit Kapila
2014-04-04 23:09:35 -04:00
Tom Lane
b203c57bb7 Allow "-C variable" and "--describe-config" even to root users.
There's no really compelling reason to refuse to do these read-only,
non-server-starting options as root, and there's at least one good
reason to allow -C: pg_ctl uses -C to find out the true data directory
location when pointed at a config-only directory.  On Windows, this is
done before dropping administrator privileges, which means that pg_ctl
fails for administrators if and only if a config-only layout is used.

Since the root-privilege check is done so early in startup, it's a bit
awkward to check for these switches.  Make the somewhat arbitrary
decision that we'll only skip the root check if -C is the first switch.
This is not just to make the code a bit simpler: it also guarantees that
we can't misinterpret a --boot mode switch.  (While AuxiliaryProcessMain
doesn't currently recognize any such switch, it might have one in the
future.)  This is no particular problem for pg_ctl, and since the whole
behavior is undocumented anyhow, it's not a documentation issue either.
(--describe-config only works as the first switch anyway, so this is
no restriction for that case either.)

Back-patch to 9.2 where pg_ctl first began to use -C.

MauMau, heavily edited by me
2014-04-04 22:03:35 -04:00
Tom Lane
2209c0f861 Preserve errno across free().
Dept. of second thoughts: free() isn't guaranteed not to change errno.
Make sure we report the right error if getcwd() fails.
2014-04-04 19:07:37 -04:00
Tom Lane
9aca512506 Make sure -D is an absolute path when starting server on Windows.
This is needed because Windows services may get started with a different
current directory than where pg_ctl is executed.  We want relative -D
paths to be interpreted relative to pg_ctl's CWD, similarly to what
happens on other platforms.

In support of this, move the backend's make_absolute_path() function
into src/port/path.c (where it probably should have been long since)
and get rid of the rather inferior version in pg_regress.

Kumar Rajeev Rastogi, reviewed by MauMau
2014-04-04 18:42:13 -04:00
Tom Lane
8120c7452a Fix bogus time printout in walreceiver's debug log messages.
The displayed sendtime and receipttime were always exactly equal, because
somebody forgot that timestamptz_to_str returns a static buffer (thereby
simplifying life for most callers, at the cost of complicating it for those
who need two results concurrently).  Apply the same pstrdup solution used
by the other call sites with this issue.  Back-patch to 9.2 where the
faulty code was introduced.  Per bug #9849 from Haruka Takatsuka, though
this is not exactly his patch.

Possibly we should change timestamptz_to_str's API, but I wouldn't want
to do so in the back branches.
2014-04-04 11:44:04 -04:00
Robert Haas
59202fae04 Fix some compiler warnings that clang emits with -pedantic.
Andres Freund
2014-04-04 11:29:50 -04:00
Heikki Linnakangas
b1236f4b7b Move multixid allocation out of critical section.
It can fail if you run out of memory.

This call was added in 9.3, so backpatch to 9.3 only.
2014-04-04 18:20:22 +03:00
Heikki Linnakangas
d9e7873bbb In checkpoint, move the check for in-progress xacts out of critical section.
GetVirtualXIDsDelayingChkpt calls palloc, which isn't safe in a critical
section. I thought I covered this case with the exemption for the
checkpointer, but CreateCheckPoint is also called from the startup process.
2014-04-04 17:31:22 +03:00
Heikki Linnakangas
4a170ee9e0 Add an Assertion that you don't palloc within a critical section.
This caught a bunch of cases doing that already, which I just fixed in
previous commit. This is the assertion itself.

Per Tom Lane's idea.
2014-04-04 14:28:54 +03:00
Heikki Linnakangas
877b088785 Avoid allocations in critical sections.
If a palloc in a critical section fails, it becomes a PANIC.
2014-04-04 13:35:44 +03:00
Tom Lane
c7b3539599 Fix non-equivalence of VARIADIC and non-VARIADIC function call formats.
For variadic functions (other than VARIADIC ANY), the syntaxes foo(x,y,...)
and foo(VARIADIC ARRAY[x,y,...]) should be considered equivalent, since the
former is converted to the latter at parse time.  They have indeed been
equivalent, in all releases before 9.3.  However, commit 75b39e790 made an
ill-considered decision to record which syntax had been used in FuncExpr
nodes, and then to make equal() test that in checking node equality ---
which caused the syntaxes to not be seen as equivalent by the planner.
This is the underlying cause of bug #9817 from Dmitry Ryabov.

It might seem that a quick fix would be to make equal() disregard
FuncExpr.funcvariadic, but the same commit made that untenable, because
the field actually *is* semantically significant for some VARIADIC ANY
functions.  This patch instead adopts the approach of redefining
funcvariadic (and aggvariadic, in HEAD) as meaning that the last argument
is a variadic array, whether it got that way by parser intervention or was
supplied explicitly by the user.  Therefore the value will always be true
for non-ANY variadic functions, restoring the principle of equivalence.
(However, the planner will continue to consider use of VARIADIC as a
meaningful difference for VARIADIC ANY functions, even though some such
functions might disregard it.)

In HEAD, this change lets us simplify the decompilation logic in
ruleutils.c, since the funcvariadic/aggvariadic flag tells directly whether
to print VARIADIC.  However, in 9.3 we have to continue to cope with
existing stored rules/views that might contain the previous definition.
Fortunately, this just means no change in ruleutils.c, since its existing
behavior effectively ignores funcvariadic for all cases other than VARIADIC
ANY functions.

In HEAD, bump catversion to reflect the fact that FuncExpr.funcvariadic
changed meanings; this is sort of pro forma, since I don't believe any
built-in views are affected.

Unfortunately, this patch doesn't magically fix everything for affected
9.3 users.  After installing 9.3.5, they might need to recreate their
rules/views/indexes containing variadic function calls in order to get
everything consistent with the new definition.  As in the cited bug,
the symptom of a problem would be failure to use a nominally matching
index that has a variadic function call in its definition.  We'll need
to mention this in the 9.3.5 release notes.
2014-04-03 22:02:24 -04:00
Tom Lane
741364bf5c Code review for commit d26888bc4d.
Mostly, copy-edit the comments; but also fix it to not reject domains over
arrays.
2014-04-03 16:57:45 -04:00
Tom Lane
42c6236f37 Fix documentation about joining pg_locks to other views.
The advice to join to pg_prepared_xacts via the transaction column was not
updated when the transaction column was replaced by virtualtransaction.
Since it's not quite obvious how to do that join, give an explicit example.
For consistency also give an example for the adjacent case of joining to
pg_stat_activity.  And link-ify the view references too, just because we
can.  Per bug #9840 from Alexey Bashtanov.

Michael Paquier and Tom Lane
2014-04-03 14:18:25 -04:00
Tom Lane
879808e519 Avoid promising that "ADD COLUMN ... DEFAULT NULL" is free.
The system realizes that DEFAULT NULL is dummy in simple cases, but not if
a cast function (such as a length coercion) needs to be applied.  It's
dubious that suppressing that function call would be appropriate, anyway.
For the moment, let's just adjust the docs to say that you should omit the
DEFAULT clause if you don't want a rewrite to happen.  Per gripe from Amit
Langote.
2014-04-03 12:38:00 -04:00
Heikki Linnakangas
04e298b826 Avoid palloc in critical section in GiST WAL-logging.
Memory allocation can fail if you run out of memory, and inside a critical
section that will lead to a PANIC. Use conservatively-sized arrays in stack
instead.

There was previously no explicit limit on the number of pages a GiST split
can produce, it was only limited by the number of LWLocks that can be held
simultaneously (100 at the moment). This patch adds an explicit limit of 75
pages. That should be plenty, a typical split shouldn't produce more than
2-3 page halves.

The bug has been there forever, but only backpatch down to 9.1. The code
was changed significantly in 9.1, and it doesn't seem worth the risk or
trouble to adapt this for 9.0 and 8.4.
2014-04-03 15:43:50 +03:00
Tom Lane
fc752505a9 Fix assorted issues in client host name lookup.
The code for matching clients to pg_hba.conf lines that specify host names
(instead of IP address ranges) failed to complain if reverse DNS lookup
failed; instead it silently didn't match, so that you might end up getting
a surprising "no pg_hba.conf entry for ..." error, as seen in bug #9518
from Mike Blackwell.  Since we don't want to make this a fatal error in
situations where pg_hba.conf contains a mixture of host names and IP
addresses (clients matching one of the numeric entries should not have to
have rDNS data), remember the lookup failure and mention it as DETAIL if
we get to "no pg_hba.conf entry".  Apply the same approach to forward-DNS
lookup failures, too, rather than treating them as immediate hard errors.

Along the way, fix a couple of bugs that prevented us from detecting an
rDNS lookup error reliably, and make sure that we make only one rDNS lookup
attempt; formerly, if the lookup attempt failed, the code would try again
for each host name entry in pg_hba.conf.  Since more or less the whole
point of this design is to ensure there's only one lookup attempt not one
per entry, the latter point represents a performance bug that seems
sufficient justification for back-patching.

Also, adjust src/port/getaddrinfo.c so that it plays as well as it can
with this code.  Which is not all that well, since it does not have actual
support for rDNS lookup, but at least it should return the expected (and
required by spec) error codes so that the main code correctly perceives the
lack of functionality as a lookup failure.  It's unlikely that PG is still
being used in production on any machines that require our getaddrinfo.c,
so I'm not excited about working harder than this.

To keep the code in the various branches similar, this includes
back-patching commits c424d0d105 and
1997f34db4 into 9.2 and earlier.

Back-patch to 9.1 where the facility for hostnames in pg_hba.conf was
introduced.
2014-04-02 17:11:24 -04:00
Tom Lane
f33a71a786 De-anonymize the union in JsonbValue.
Needed for strict C89 compliance.
2014-04-02 14:30:08 -04:00
Tom Lane
682c5bbec5 Fix bugs in manipulation of PgBackendStatus.st_clienthostname.
Initialization of this field was not being done according to the
st_changecount protocol (it has to be done within the changecount increment
range, not outside).  And the test to see if the value should be reported
as null was wrong.  Noted while perusing uses of Port.remote_hostname.

This was wrong from the introduction of this code (commit 4a25bc145),
so back-patch to 9.1.
2014-04-01 21:30:34 -04:00
Heikki Linnakangas
8bbbcb91ba Fix bug in the new GIN incomplete-split code.
Inserting a downlink to an internal page clears the incomplete-split flag
of the child's left sibling, so the left sibling's LSN also needs to be
updated and it needs to be marked dirty. The codepath for an insertion got
this right, but the case where the internal node is split because of
inserting the new downlink missed that.
2014-04-01 22:49:47 +03:00
Tom Lane
6eff0accfe Doc: improve discussion of reverse+forward host name lookup in pg_hba.conf.
Fix some grammatical issues and make it a bit more readable.
2014-04-01 15:20:38 -04:00
Heikki Linnakangas
cfe992e7eb Remove dead check for backup block, replace with Assert.
We don't use backup blocks with GIN vacuum records anymore, the page is
always recreated from scratch.
2014-04-01 21:16:10 +03:00
Heikki Linnakangas
954523cdfe Fix bug in the new B-tree incomplete-split code.
Inserting a downlink to an internal page clears the incomplete-split flag
of the child's left sibling, so the left sibling's LSN also needs to be
updated.
2014-04-01 19:19:47 +03:00
Heikki Linnakangas
f7534296b4 Move SizeOfHeapNewCid next to xl_heap_new_cid struct.
They belong together, but the xl_heap_rewrite_mapping struct was wedged
in between.
2014-04-01 16:23:16 +03:00
Heikki Linnakangas
554bb3beba Fix typo in comment.
Amit Langote
2014-04-01 09:27:37 +03:00
Robert Haas
4bc15a8bfb Mark FastPathStrongRelationLocks volatile.
Otherwise, the compiler might decide to move modifications to data
within this structure outside the enclosing SpinLockAcquire /
SpinLockRelease pair, leading to shared memory corruption.

This may or may not explain a recent lmgr-related buildfarm failure
on prairiedog, but it needs to be fixed either way.
2014-03-31 14:32:12 -04:00
Robert Haas
0f95b723eb test_decoding: Update .gitignore
Commit 7317d8d961 changed the set of
things that need to be ignored, but neglected to update .gitignore.
2014-03-31 14:18:35 -04:00
Robert Haas
066254cea1 Count buffers dirtied due to hints in pgBufferUsage.shared_blks_dirtied.
Previously, such buffers weren't counted, with the possible result that
EXPLAIN (BUFFERS) and pg_stat_statements would understate the true
number of blocks dirtied by an SQL statement.

Back-patch to 9.2, where this counter was introduced.

Amit Kapila
2014-03-31 13:06:26 -04:00
Robert Haas
3f0e4be453 Fix thinko in logical decoding code.
Andres Freund
2014-03-31 13:03:18 -04:00
Heikki Linnakangas
14d02f0bb3 Rewrite the way GIN posting lists are packed on a page, to reduce WAL volume.
Inserting (in retail) into the new 9.4 format GIN posting tree created much
larger WAL records than in 9.3. The previous strategy to WAL logging was
basically to log the whole page on each change, with the exception of
completely unmodified segments up to the first modified one. That was not
too bad when appending to the end of the page, as only the last segment had
to be WAL-logged, but per Fujii Masao's testing, even that produced 2x the
WAL volume that 9.3 did.

The new strategy is to keep track of changes to the posting lists in a more
fine-grained fashion, and also make the repacking" code smarter to avoid
decoding and re-encoding segments unnecessarily.
2014-03-31 15:23:50 +03:00
Heikki Linnakangas
0cfa34c25a Rename GinLogicValue to GinTernaryValue.
It's more descriptive. Also, get rid of the enum, and use #defines instead,
per Greg Stark's suggestion.
2014-03-31 10:26:38 +03:00
Andrew Dunstan
7317d8d961 Use separate output dirs for test_decoding's two runs.
contrib/test_decoding's "make check" runs two sets of tests. Unless we
specify separate output directories for each set the isolation tests
will overwrite the output from the  normal regression set. Doing this
will help the buildfarm collect complete logs.
2014-03-30 18:30:59 -04:00
Bruce Momjian
9d66116444 psql: display "Replica Identity" only for FULL and NOTHING
INDEX is already displayed on the index, and we now exclude pg_catalog.
DEFAULT is not displayed.
2014-03-29 19:00:11 -04:00
Tom Lane
62215de292 Fix dumping of a materialized view that depends on a table's primary key.
It is possible for a view or materialized view to depend on a table's
primary key, if the view query relies on functional dependency to
abbreviate a GROUP BY list.  This is problematic for pg_dump since we
ordinarily want to dump view definitions in the pre-data section but
indexes in post-data.  pg_dump knows how to deal with this situation for
regular views, by breaking the view's ON SELECT rule apart from the view
proper.  But it had not been taught what to do about materialized views,
and in fact mistakenly dumped them as regular views in such cases, as
seen in bug #9616 from Jesse Denardo.

If we had CREATE OR REPLACE MATERIALIZED VIEW, we could fix this in a
manner analogous to what's done for regular views; but we don't yet,
and we'd not back-patch such a thing into 9.3 anyway.  As a hopefully-
temporary workaround, break the circularity by postponing the matview
into post-data altogether when this case occurs.
2014-03-29 17:34:00 -04:00
Noah Misch
8f5578d0f9 Revert "Secure Unix-domain sockets of "make check" temporary clusters."
About half of the buildfarm members use too-long directory names,
strongly suggesting that this approach is a dead end.
2014-03-29 03:12:00 -04:00
Noah Misch
31c6e54ec9 Secure Unix-domain sockets of "make check" temporary clusters.
Any OS user able to access the socket can connect as the bootstrap
superuser and in turn execute arbitrary code as the OS user running the
test.  Protect against that by placing the socket in the temporary data
directory, which has mode 0700 thanks to initdb.  Back-patch to 8.4 (all
supported versions).  The hazard remains wherever the temporary cluster
accepts TCP connections, notably on Windows.

Attempts to run "make check" from a directory with a long name will now
fail.  An alternative not sharing that problem was to place the socket
in a subdirectory of /tmp, but that is only secure if /tmp is sticky.
The PG_REGRESS_SOCK_DIR environment variable is available as a
workaround when testing from long directory paths.

As a convenient side effect, this lets testing proceed smoothly in
builds that override DEFAULT_PGSOCKET_DIR.  Popular non-default values
like /var/run/postgresql are often unwritable to the build user.

Security: CVE-2014-0067
2014-03-29 00:52:56 -04:00
Noah Misch
fbd32b0cab Document platform-specificity of unix_socket_permissions.
Back-patch to 8.4 (all supported versions).
2014-03-29 00:52:31 -04:00