]> granicus.if.org Git - postgresql/log
postgresql
9 years agoUse a safer method for determining whether relcache init file is stale.
Tom Lane [Sun, 7 Jun 2015 19:32:09 +0000 (15:32 -0400)]
Use a safer method for determining whether relcache init file is stale.

When we invalidate the relcache entry for a system catalog or index, we
must also delete the relcache "init file" if the init file contains a copy
of that rel's entry.  The old way of doing this relied on a specially
maintained list of the OIDs of relations present in the init file: we made
the list either when reading the file in, or when writing the file out.
The problem is that when writing the file out, we included only rels
present in our local relcache, which might have already suffered some
deletions due to relcache inval events.  In such cases we correctly decided
not to overwrite the real init file with incomplete data --- but we still
used the incomplete initFileRelationIds list for the rest of the current
session.  This could result in wrong decisions about whether the session's
own actions require deletion of the init file, potentially allowing an init
file created by some other concurrent session to be left around even though
it's been made stale.

Since we don't support changing the schema of a system catalog at runtime,
the only likely scenario in which this would cause a problem in the field
involves a "vacuum full" on a catalog concurrently with other activity, and
even then it's far from easy to provoke.  Remarkably, this has been broken
since 2002 (in commit 786340441706ac1957a031f11ad1c2e5b6e18314), but we had
never seen a reproducible test case until recently.  If it did happen in
the field, the symptoms would probably involve unexpected "cache lookup
failed" errors to begin with, then "could not open file" failures after the
next checkpoint, as all accesses to the affected catalog stopped working.
Recovery would require manually removing the stale "pg_internal.init" file.

To fix, get rid of the initFileRelationIds list, and instead consult
syscache.c's list of relations used in catalog caches to decide whether a
relation is included in the init file.  This should be a tad more efficient
anyway, since we're replacing linear search of a list with ~100 entries
with a binary search.  It's a bit ugly that the init file contents are now
so directly tied to the catalog caches, but in practice that won't make
much difference.

Back-patch to all supported branches.

9 years agoGet rid of a //-style comment.
Tom Lane [Fri, 5 Jun 2015 21:04:07 +0000 (17:04 -0400)]
Get rid of a //-style comment.

Not sure how "//XXX" got into a committed patch in the first place,
as it's both content-free and against project style.  pgindent made a
bit of a hash of it, too.

Going forward, we should have at least one buildfarm member using
"gcc -ansi" to catch such things, at least till such time as we
decide the project target language isn't C90 any more.  I've turned
this option on on dromedary.

9 years agoFix incorrect order of database-locking operations in InitPostgres().
Tom Lane [Fri, 5 Jun 2015 17:22:27 +0000 (13:22 -0400)]
Fix incorrect order of database-locking operations in InitPostgres().

We should set MyProc->databaseId after acquiring the per-database lock,
not beforehand.  The old way risked deadlock against processes trying to
copy or delete the target database, since they would first acquire the lock
and then wait for processes with matching databaseId to exit; that left a
window wherein an incoming process could set its databaseId and then block
on the lock, while the other process had the lock and waited in vain for
the incoming process to exit.

CountOtherDBBackends() would time out and fail after 5 seconds, so this
just resulted in an unexpected failure not a permanent lockup, but it's
still annoying when it happens.  A real-world example of a use-case is that
short-duration connections to a template database should not cause CREATE
DATABASE to fail.

Doing it in the other order should be fine since the contract has always
been that processes searching the ProcArray for a database ID must hold the
relevant per-database lock while searching.  Thus, this actually removes
the former race condition that required an assumption that storing to
MyProc->databaseId is atomic.

It's been like this for a long time, so back-patch to all active branches.

9 years agoCope with possible failure of the oldest MultiXact to exist.
Robert Haas [Fri, 5 Jun 2015 12:34:52 +0000 (08:34 -0400)]
Cope with possible failure of the oldest MultiXact to exist.

Recent commits, mainly b69bf30b9bfacafc733a9ba77c9587cf54d06c0c and
53bb309d2d5a9432d2602c93ed18e58bd2924e15, introduced mechanisms to
protect against wraparound of the MultiXact member space: the number
of multixacts that can exist at one time is limited to 2^32, but the
total number of members in those multixacts is also limited to 2^32,
and older code did not take care to enforce the second limit,
potentially allowing old data to be overwritten while it was still
needed.

Unfortunately, these new mechanisms failed to account for the fact
that the code paths in which they run might be executed during
recovery or while the cluster was in an inconsistent state.  Also,
they failed to account for the fact that users who used pg_upgrade
to upgrade a PostgreSQL version between 9.3.0 and 9.3.4 might have
might oldestMultiXid = 1 in the control file despite the true value
being larger.

To fix these problems, first, avoid unnecessarily examining the
mmembers of MultiXacts when the cluster is not known to be consistent.
TruncateMultiXact has done this for a long time, and this patch does
not fix that.  But the new calls used to prevent member wraparound
are not needed until we reach normal running, so avoid calling them
earlier.  (SetMultiXactIdLimit is actually called before InRecovery
is set, so we can't rely on that; we invent our own multixact-specific
flag instead.)

Second, make failure to look up the members of a MultiXact a non-fatal
error.  Instead, if we're unable to determine the member offset at
which wraparound would occur, postpone arming the member wraparound
defenses until we are able to do so.  If we're unable to determine the
member offset that should force autovacuum, force it continuously
until we are able to do so.  If we're unable to deterine the member
offset at which we should truncate the members SLRU, log a message and
skip truncation.

An important consequence of these changes is that anyone who does have
a bogus oldestMultiXid = 1 value in pg_control will experience
immediate emergency autovacuuming when upgrading to a release that
contains this fix.  The release notes should highlight this fact.  If
a user has no pg_multixact/offsets/0000 file, but has oldestMultiXid = 1
in the control file, they may wish to vacuum any tables with
relminmxid = 1 prior to upgrading in order to avoid an immediate
emergency autovacuum after the upgrade.  This must be done with a
PostgreSQL version 9.3.5 or newer and with vacuum_multixact_freeze_min_age
and vacuum_multixact_freeze_table_age set to 0.

This patch also adds an additional log message at each database server
startup, indicating either that protections against member wraparound
have been engaged, or that they have not.  In the latter case, once
autovacuum has advanced oldestMultiXid to a sane value, the message
indicating that the guards have been engaged will appear at the next
checkpoint.  A few additional messages have also been added at the DEBUG1
level so that the correct operation of this code can be properly audited.

Along the way, this patch fixes another, related bug in TruncateMultiXact
that has existed since PostgreSQL 9.3.0: when no MultiXacts exist at
all, the truncation code looks up NextMultiXactId, which doesn't exist
yet.  This can lead to TruncateMultiXact removing every file in
pg_multixact/offsets instead of keeping one around, as it should.
This in turn will cause the database server to refuse to start
afterwards.

Patch by me.  Review by Álvaro Herrera, Andres Freund, Noah Misch, and
Thomas Munro.

9 years agodoc: Session identifiers truncate, not round, the backend start time.
Robert Haas [Thu, 4 Jun 2015 21:57:39 +0000 (17:57 -0400)]
doc: Session identifiers truncate, not round, the backend start time.

Joel Jacobson

9 years agodocs: Fix list of object types pg_table_is_visible() can handle.
Robert Haas [Thu, 4 Jun 2015 21:48:00 +0000 (17:48 -0400)]
docs: Fix list of object types pg_table_is_visible() can handle.

Materialized views and foreign tables were missing from the list,
probably because they are newer than the other object types that were
mentioned.

Etsuro Fujita

9 years agoSecond try at stabilizing query plans in rowsecurity regression test.
Tom Lane [Thu, 4 Jun 2015 20:42:23 +0000 (16:42 -0400)]
Second try at stabilizing query plans in rowsecurity regression test.

This reverts commit 5cdf25e16843dff33dbc2ddc02941458032e3ad4,
which was almost immediately proven insufficient by the buildfarm.

On second thought, the tables involved are not large enough that
autovacuum or autoanalyze would notice them; what seems far more
likely to be the culprit is the database-wide "vacuum analyze"
in the concurrent gist test.  That thing has given us one headache
too many, so get rid of it in favor of targeted vacuuming of that
test's own tables only.

9 years agoFix brin regression test so it actually tests cidr.
Tom Lane [Thu, 4 Jun 2015 19:24:22 +0000 (15:24 -0400)]
Fix brin regression test so it actually tests cidr.

The problem noted in my previous commit was simpler than I thought:
we weren't getting an index plan because the column wasn't indexed.

9 years agoTighten the per-operator testing done in brin regression test.
Tom Lane [Thu, 4 Jun 2015 18:39:52 +0000 (14:39 -0400)]
Tighten the per-operator testing done in brin regression test.

Verify that the number of matches is exactly what it should be, not just
that it not be zero.  This should help us detect any environment-dependent
issues.

Also, verify that we're getting the expected type of scan plan (either
bitmap or seqscan as appropriate).  Right now, this is failing on the
cidrcol test cases, as shown in the output file.  I'll look into that
in a bit, but it seems good to commit this as-is temporarily to verify
that it behaves as expected on the buildfarm.

9 years agoFix brin "char" test to actually test what it meant to test.
Tom Lane [Thu, 4 Jun 2015 17:50:32 +0000 (13:50 -0400)]
Fix brin "char" test to actually test what it meant to test.

Casting to char, without quotes, does not give the same results as casting
to "char".  That meant we were not testing the brin "char" paths at all,
since we ended up with a text operator not a "char" operator.

9 years agoStabilize results of brin regression test.
Tom Lane [Thu, 4 Jun 2015 17:46:34 +0000 (13:46 -0400)]
Stabilize results of brin regression test.

This test used seqscans on tenk1, with LIMIT, to build test data.
That works most of the time, but if the synchronized-seqscan logic
kicks in, we get varying test data.  This seems likely to explain
the erratic test failures on buildfarm member chipmunk, which uses
smaller-than-default shared_buffers.  To fix, add ORDER BY clauses to
force the ordering to be what it was implicitly being assumed to be.

Peter Geoghegan had noticed this with respect to one of the trouble
spots, though not the ones actually causing the chipmunk issue.

9 years agoStabilize query plans in rowsecurity regression test.
Tom Lane [Thu, 4 Jun 2015 14:37:06 +0000 (10:37 -0400)]
Stabilize query plans in rowsecurity regression test.

Some recent buildfarm failures can be explained by supposing that
autovacuum or autoanalyze fired on the tables created by this test,
resulting in plan changes.  Do a proactive VACUUM ANALYZE on the
test's principal tables to try to forestall such changes.

9 years agoRemove -i/--ignore-version option from pg_dump, pg_dumpall and pg_restore.
Fujii Masao [Thu, 4 Jun 2015 10:54:43 +0000 (19:54 +0900)]
Remove -i/--ignore-version option from pg_dump, pg_dumpall and pg_restore.

The commit c22ed3d523782c43836c163c16fa5a7bb3912826 turned
the -i/--ignore-version options into no-ops and marked as deprecated.
Considering we shipped that in 8.4, it's time to remove all trace of
those switches, per discussion. We'd still have to wait a couple releases
before it'd be safe to use -i for something else, but it'd be a start.

9 years agoFix some issues in pg_class.relminmxid and pg_database.datminmxid documentation.
Fujii Masao [Thu, 4 Jun 2015 04:22:49 +0000 (13:22 +0900)]
Fix some issues in pg_class.relminmxid and pg_database.datminmxid documentation.

- Correct the name of directory which those catalog columns allow to be shrunk.
- Correct the name of symbol which is used as the value of pg_class.relminmxid
  when the relation is not a table.
- Fix "ID ID" typo.

Backpatch to 9.3 where those cataog columns were introduced.

9 years agodoc: Fix PDF build with FOP
Peter Eisentraut [Thu, 4 Jun 2015 00:19:47 +0000 (20:19 -0400)]
doc: Fix PDF build with FOP

Because of a bug in the DocBook XSL FO style sheet, an xref to a
varlistentry whose term includes an indexterm fails to build.  One such
instance was introduced in commit
5086dfceba79ecd5d1eb28b8f4ed5221838ff3a6.  Fix by adding the upstream
bug fix to our customization layer.

9 years agoFix some questionable edge-case behaviors in add_path() and friends.
Tom Lane [Wed, 3 Jun 2015 22:02:39 +0000 (18:02 -0400)]
Fix some questionable edge-case behaviors in add_path() and friends.

add_path_precheck was doing exact comparisons of path costs, but it really
needs to do them fuzzily to be sure it won't reject paths that could
survive add_path's comparisons.  (This can only matter if the initial cost
estimate is very close to the final one, but that turns out to often be
true.)

Also, it should ignore startup cost for this purpose if and only if
compare_path_costs_fuzzily would do so.  The previous coding always ignored
startup cost for parameterized paths, which is wrong as of commit
3f59be836c555fa6; it could result in improper early rejection of paths that
we care about for SEMI/ANTI joins.  It also always considered startup cost
for unparameterized paths, which is just as wrong though the only effect is
to waste planner cycles on paths that can't survive.  Instead, it should
consider startup cost only when directed to by the consider_startup/
consider_param_startup relation flags.

Likewise, compare_path_costs_fuzzily should have symmetrical behavior
for parameterized and unparameterized paths.  In this case, the best
answer seems to be that after establishing that total costs are fuzzily
equal, we should compare startup costs whether or not the consider_xxx
flags are on.  That is what it's always done for unparameterized paths,
so let's make the behavior for parameterized  paths match.

These issues were noted while developing the SEMI/ANTI join costing fix
of commit 3f59be836c555fa6, but we chose not to back-patch these fixes,
because they can cause changes in the planner's choices among
nearly-same-cost plans.  (There is in fact one minor change in plan choice
within the core regression tests.)  Destabilizing plan choices in back
branches without very clear improvements is frowned on, so we'll just fix
this in HEAD.

9 years agoFix planner's cost estimation for SEMI/ANTI joins with inner indexscans.
Tom Lane [Wed, 3 Jun 2015 15:58:47 +0000 (11:58 -0400)]
Fix planner's cost estimation for SEMI/ANTI joins with inner indexscans.

When the inner side of a nestloop SEMI or ANTI join is an indexscan that
uses all the join clauses as indexquals, it can be presumed that both
matched and unmatched outer rows will be processed very quickly: for
matched rows, we'll stop after fetching one row from the indexscan, while
for unmatched rows we'll have an indexscan that finds no matching index
entries, which should also be quick.  The planner already knew about this,
but it was nonetheless charging for at least one full run of the inner
indexscan, as a consequence of concerns about the behavior of materialized
inner scans --- but those concerns don't apply in the fast case.  If the
inner side has low cardinality (many matching rows) this could make an
indexscan plan look far more expensive than it actually is.  To fix,
rearrange the work in initial_cost_nestloop/final_cost_nestloop so that we
don't add the inner scan cost until we've inspected the indexquals, and
then we can add either the full-run cost or just the first tuple's cost as
appropriate.

Experimentation with this fix uncovered another problem: add_path and
friends were coded to disregard cheap startup cost when considering
parameterized paths.  That's usually okay (and desirable, because it thins
the path herd faster); but in this fast case for SEMI/ANTI joins, it could
result in throwing away the desired plain indexscan path in favor of a
bitmap scan path before we ever get to the join costing logic.  In the
many-matching-rows cases of interest here, a bitmap scan will do a lot more
work than required, so this is a problem.  To fix, add a per-relation flag
consider_param_startup that works like the existing consider_startup flag,
but applies to parameterized paths, and set it for relations that are the
inside of a SEMI or ANTI join.

To make this patch reasonably safe to back-patch, care has been taken to
avoid changing the planner's behavior except in the very narrow case of
SEMI/ANTI joins with inner indexscans.  There are places in
compare_path_costs_fuzzily and add_path_precheck that are not terribly
consistent with the new approach, but changing them will affect planner
decisions at the margins in other cases, so we'll leave that for a
HEAD-only fix.

Back-patch to 9.3; before that, the consider_startup flag didn't exist,
meaning that the second aspect of the patch would be too invasive.

Per a complaint from Peter Holzer and analysis by Tomas Vondra.

9 years agoMinor improvement to txid_current() documentation.
Fujii Masao [Wed, 3 Jun 2015 03:12:48 +0000 (12:12 +0900)]
Minor improvement to txid_current() documentation.

Michael Paquier, reviewed by Christoph Berg and Naoya Anzai

9 years agoRelease notes for 9.4.3, 9.3.8, 9.2.12, 9.1.17, 9.0.21.
Tom Lane [Mon, 1 Jun 2015 17:27:43 +0000 (13:27 -0400)]
Release notes for 9.4.3, 9.3.8, 9.2.12, 9.1.17, 9.0.21.

Also sneak entries for commits 97ff2a564 et al into the sections for
the previous releases in the relevant branches.  Those fixes did go out
in the previous releases, but missed getting documented.

9 years agopgindent: add typedef blog URL
Bruce Momjian [Mon, 1 Jun 2015 15:27:30 +0000 (11:27 -0400)]
pgindent:  add typedef blog URL

9 years agoAvoid naming a variable "new", and remove bogus initializer.
Andrew Dunstan [Mon, 1 Jun 2015 02:56:53 +0000 (22:56 -0400)]
Avoid naming a variable "new", and remove bogus initializer.

Per gripe from Tom Lane.

9 years agoAdd a couple of missing JsonbValue type initialisers.
Andrew Dunstan [Mon, 1 Jun 2015 02:51:58 +0000 (22:51 -0400)]
Add a couple of missing JsonbValue type initialisers.

9 years agoRename jsonb_replace to jsonb_set and allow it to add new values
Andrew Dunstan [Mon, 1 Jun 2015 00:34:10 +0000 (20:34 -0400)]
Rename jsonb_replace to jsonb_set and allow it to add new values

The function is given a fourth parameter, which defaults to true. When
this parameter is true, if the last element of the path is missing
in the original json, jsonb_set creates it in the result and assigns it
the new value. If it is false then the function does nothing unless all
elements of the path are present, including the last.

Based on some original code from Dmitry Dolgov, heavily modified by me.

Catalog version bumped.

9 years agoMake Python tests more portable
Peter Eisentraut [Sun, 31 May 2015 11:10:45 +0000 (07:10 -0400)]
Make Python tests more portable

Newer Python versions randomize the hash seed for dictionaries,
resulting in a random output order, which messes up the regression test
diffs.

Instead, use Python assert to compare the dictionaries with their
expected value.

9 years agopg_upgrade: add missing period in C comment
Bruce Momjian [Fri, 29 May 2015 21:44:14 +0000 (17:44 -0400)]
pg_upgrade:  add missing period in C comment

9 years agoinitdb -S should now have an explicit check that $PGDATA is valid.
Tom Lane [Fri, 29 May 2015 21:02:58 +0000 (17:02 -0400)]
initdb -S should now have an explicit check that $PGDATA is valid.

The fsync code from the backend essentially assumes that somebody's already
validated PGDATA, at least to the extent of it being a readable directory.
That's safe enough for initdb's normal code path too, but "initdb -S"
doesn't have any other processing at all that touches the target directory.
To have reasonable error-case behavior, add a pg_check_dir call.
Per gripe from Peter E.

9 years agoRemove special cases for ETXTBSY from new fsync'ing logic.
Tom Lane [Fri, 29 May 2015 19:11:36 +0000 (15:11 -0400)]
Remove special cases for ETXTBSY from new fsync'ing logic.

The argument that this is a sufficiently-expected case to be silently
ignored seems pretty thin.  Andres had brought it up back when we were
still considering that most fsync failures should be hard errors, and it
probably would be legit not to fail hard for ETXTBSY --- but the same is
true for EROFS and other cases, which is why we gave up on hard failures.
ETXTBSY is surely not a normal case, so logging the failure seems fine
from here.

9 years agoCheck that all aliases of a built-in function have same leakproof property.
Tom Lane [Fri, 29 May 2015 17:26:21 +0000 (13:26 -0400)]
Check that all aliases of a built-in function have same leakproof property.

opr_sanity.sql has a test checking that relevant properties of built-in
functions match when the same C function is referenced by multiple pg_proc
entries.  The test neglected to check proleakproof, though, and when
I added that condition it exposed that xideqint4 hadn't been updated to
match xideq.  So fix that as well, and in consequence bump catversion.

This isn't very critical, so no need to worry about fixing back branches.

9 years agoAdjust initdb to also not consider fsync'ing failures fatal.
Tom Lane [Fri, 29 May 2015 17:05:16 +0000 (13:05 -0400)]
Adjust initdb to also not consider fsync'ing failures fatal.

Make initdb's version of this logic look as much like the backend's
as possible.  This is much less critical than in the backend since not
so many people use "initdb -S", but we want the same corner-case error
handling in both cases.

Back-patch to 9.3 where initdb -S option was introduced.  Before that,
initdb only had to deal with freshly-created data directories, wherein
no failures should be expected.

Abhijit Menon-Sen

9 years agoRevert exporting of internal GUC variable "data_directory".
Tom Lane [Fri, 29 May 2015 15:57:33 +0000 (11:57 -0400)]
Revert exporting of internal GUC variable "data_directory".

This undoes a poorly-thought-out choice in commit 970a18687f9b3058, namely
to export guc.c's internal variable data_directory.  The authoritative
variable so far as C code is concerned is DataDir; there is no reason for
anything except specific bits of GUC code to look at the GUC variable.

After yesterday's commits fixing the fsync-on-restart patch, the only
remaining misuse of data_directory was in AlterSystemSetConfigFile(),
which would be much better off just using a relative path anyhow: it's
less code and it doesn't break if the DBA moves the data directory of a
running system, which is a case we've taken some pains over in the past.

This is mostly cosmetic, so no need for a back-patch (and I'd be hesitant
to remove a global variable in stable branches anyway).

9 years agoFix fsync-at-startup code to not treat errors as fatal.
Tom Lane [Thu, 28 May 2015 21:33:03 +0000 (17:33 -0400)]
Fix fsync-at-startup code to not treat errors as fatal.

Commit 2ce439f3379aed857517c8ce207485655000fc8e introduced a rather serious
regression, namely that if its scan of the data directory came across any
un-fsync-able files, it would fail and thereby prevent database startup.
Worse yet, symlinks to such files also caused the problem, which meant that
crash restart was guaranteed to fail on certain common installations such
as older Debian.

After discussion, we agreed that (1) failure to start is worse than any
consequence of not fsync'ing is likely to be, therefore treat all errors
in this code as nonfatal; (2) we should not chase symlinks other than
those that are expected to exist, namely pg_xlog/ and tablespace links
under pg_tblspc/.  The latter restriction avoids possibly fsync'ing a
much larger part of the filesystem than intended, if the user has left
random symlinks hanging about in the data directory.

This commit takes care of that and also does some code beautification,
mainly moving the relevant code into fd.c, which seems a much better place
for it than xlog.c, and making sure that the conditional compilation for
the pre_sync_fname pass has something to do with whether pg_flush_data
works.

I also relocated the call site in xlog.c down a few lines; it seems a
bit silly to be doing this before ValidateXLOGDirectoryStructure().

The similar logic in initdb.c ought to be made to match this, but that
change is noncritical and will be dealt with separately.

Back-patch to all active branches, like the prior commit.

Abhijit Menon-Sen and Tom Lane

9 years agoRemove *pgaudit* references also.
Stephen Frost [Thu, 28 May 2015 17:02:09 +0000 (13:02 -0400)]
Remove *pgaudit* references also.

Fixes the docs build.

9 years agoFinish removing pg_audit
Stephen Frost [Thu, 28 May 2015 16:48:25 +0000 (12:48 -0400)]
Finish removing pg_audit

9 years agoFix pg_rewind's handling of top-level symlinks.
Tom Lane [Thu, 28 May 2015 16:44:31 +0000 (12:44 -0400)]
Fix pg_rewind's handling of top-level symlinks.

The previous coding suffered a null-pointer dereference if it found any
symlink at the top level of $PGDATA.  Fix that, and teach it to recurse
into a symlink for pg_xlog, but not anything else.

Per note from Abhijit Menon-Sen.

9 years agoRemove pg_audit
Stephen Frost [Thu, 28 May 2015 16:41:26 +0000 (12:41 -0400)]
Remove pg_audit

This removes pg_audit, per discussion:

20150528082038.GU26667@tamriel.snowman.net

9 years agoFix assorted inconsistencies in our calls of readlink().
Tom Lane [Thu, 28 May 2015 16:17:22 +0000 (12:17 -0400)]
Fix assorted inconsistencies in our calls of readlink().

Ensure that we null-terminate the result string (one place in pg_rewind).
Be paranoid about out-of-range results from readlink() (should not happen,
but there is no good reason for some call sites to be careful about it and
others not).  Consistently use the whole buffer, not sometimes one byte
less.  Ensure we emit an appropriate errcode() in all cases.  Spell the
error messages the same way.

The only serious bug here is the missing null-termination in pg_rewind,
which is new code, so no need for a back-patch.

Abhijit Menon-Sen and Tom Lane

9 years agoFix pg_get_functiondef() to print a function's LEAKPROOF property.
Tom Lane [Thu, 28 May 2015 15:24:37 +0000 (11:24 -0400)]
Fix pg_get_functiondef() to print a function's LEAKPROOF property.

Seems to have been an oversight in the original leakproofness patch.
Per report and patch from Jeevan Chalke.

In passing, prettify some awkward leakproof-related code in AlterFunction.

9 years agoFix portability issue in isolationtester grammar.
Tom Lane [Wed, 27 May 2015 23:14:39 +0000 (19:14 -0400)]
Fix portability issue in isolationtester grammar.

specparse.y and specscanner.l used "string" as a token name.  Now, bison
likes to define each token name as a macro for the token code it assigns,
which means those names are basically off-limits for any other use within
the grammar file or included headers.  So names as generic as "string" are
dangerous.  This is what was causing the recent failures on protosciurus:
some versions of Solaris' sys/kstat.h use "string" as a field name.
With late-model bison we don't see this problem because the token macros
aren't defined till later (that is why castoroides didn't show the problem
even though it's on the same machine).  But protosciurus uses bison 1.875
which defines the token macros up front.

This land mine has been there from day one; we'd have found it sooner
except that protosciurus wasn't trying to run the isolation tests till
recently.

To fix, rename the token to "string_literal" which is hopefully less
likely to collide with names used by system headers.  Back-patch to
all branches containing the isolation tests.

9 years agoRevert "Add all structured objects passed to pushJsonbValue piecewise."
Andrew Dunstan [Wed, 27 May 2015 02:54:55 +0000 (22:54 -0400)]
Revert "Add all structured objects passed to pushJsonbValue piecewise."

This reverts commit 54547bd87f49326d67051254c363e6597d16ffda.

This appears to have been a thinko on my part. I will try to come up
wioth a better solution.

9 years agoRevert "Simplify addJsonbToParseState()"
Andrew Dunstan [Wed, 27 May 2015 02:54:11 +0000 (22:54 -0400)]
Revert "Simplify addJsonbToParseState()"

This reverts commit fba12c8c6c4159e1923958a4006b26f3cf873254.

This relied on a commit that is also being reverted.

9 years agoRemove configure check prohibiting threaded libpython on OpenBSD.
Tom Lane [Wed, 27 May 2015 02:14:59 +0000 (22:14 -0400)]
Remove configure check prohibiting threaded libpython on OpenBSD.

According to recent tests, this case now works fine, so there's no reason
to reject it anymore.  (Even if there are still some OpenBSD platforms
in the wild where it doesn't work, removing the check won't break any case
that worked before.)

We can actually remove the entire test that discovers whether libpython
is threaded, since without the OpenBSD case there's no need to know that
at all.

Per report from Davin Potts.  Back-patch to all active branches.

9 years agoSuppress occasional failures in brin regression test.
Tom Lane [Tue, 26 May 2015 18:10:46 +0000 (14:10 -0400)]
Suppress occasional failures in brin regression test.

brin.sql included a call of brin_summarize_new_values(), and expected
it to always report exactly 5 summarization events.  This failed sometimes
during parallel regression tests, as a consequence of the database-wide
VACUUM in gist.sql getting there first.  The most future-proof way
to avoid variation in the test results is to forget about using
brin_summarize_new_values() and just do a plain "VACUUM brintest",
which will exercise the same code anyway.

Having done that, there's no need for preventing autovacuum on brintest;
doing so just reduces the scope of test coverage, so let's not.

9 years agoSimplify addJsonbToParseState()
Andrew Dunstan [Tue, 26 May 2015 15:46:02 +0000 (11:46 -0400)]
Simplify addJsonbToParseState()

This function no longer needs to walk non-scalar structures passed to
it, following commit 54547bd87f49326d67051254c363e6597d16ffda.

9 years agoAdd all structured objects passed to pushJsonbValue piecewise.
Andrew Dunstan [Tue, 26 May 2015 15:16:52 +0000 (11:16 -0400)]
Add all structured objects passed to pushJsonbValue piecewise.

Commit 9b74f32cdbff8b9be47fc69164eae552050509ff did this for objects of
type jbvBinary, but in trying further to simplify some of the new jsonb
code I discovered that objects of type jbvObject or jbvArray passed as
WJB_ELEM or WJB_VALUE also caused problems. These too are now added
component by component.

Backpatch to 9.4.

9 years agoFix valgrind's "unaddressable bytes" whining about BRIN code.
Tom Lane [Tue, 26 May 2015 01:56:19 +0000 (21:56 -0400)]
Fix valgrind's "unaddressable bytes" whining about BRIN code.

brin_form_tuple calculated an exact tuple size, then palloc'd and
filled just that much.  Later, brin_doinsert or brin_doupdate would
MAXALIGN the tuple size and tell PageAddItem that that was the size
of the tuple to insert.  If the original tuple size wasn't a multiple
of MAXALIGN, the net result would be that PageAddItem would memcpy
a few more bytes than the palloc request had been for.

AFAICS, this is totally harmless in the real world: the error is a
read overrun not a write overrun, and palloc would certainly have
rounded the request up to a MAXALIGN multiple internally, so there's
no chance of the memcpy fetching off the end of memory.  Valgrind,
however, is picky to the byte level not the MAXALIGN level.

Fix it by pushing the MAXALIGN step back to brin_form_tuple.  (The other
possible source of tuples in this code, brin_form_placeholder_tuple,
was already producing a MAXALIGN'd result.)

In passing, be a bit more paranoid about internal allocations in
brin_form_tuple.

9 years agopgindent: document location of "all" typedef lists
Bruce Momjian [Mon, 25 May 2015 20:53:48 +0000 (16:53 -0400)]
pgindent:  document location of "all" typedef lists

9 years agoExplain CHECK constraint handling in postgres_fdw's IMPORT FOREIGN SCHEMA.
Tom Lane [Mon, 25 May 2015 18:12:51 +0000 (14:12 -0400)]
Explain CHECK constraint handling in postgres_fdw's IMPORT FOREIGN SCHEMA.

The existing documentation could easily be misinterpreted, and it failed to
explain the inconsistent-evaluation hazard that deterred us from supporting
automatic importing of check constraints.  Revise it.

Etsuro Fujita, further expanded by me

9 years agoUpdate README.tuplock
Alvaro Herrera [Mon, 25 May 2015 18:09:05 +0000 (15:09 -0300)]
Update README.tuplock

Multixact truncation is now handled differently, and this file hadn't
gotten the memo.

Per note from Amit Langote.  I didn't use his patch, though.

Also update the description of infomask bits, which weren't completely up
to date either.  This commit also propagates b01a4f6838 back to 9.3 and
9.4, which apparently I failed to do back then.

9 years agoClean up and simplify jsonb_concat code.
Andrew Dunstan [Mon, 25 May 2015 15:43:06 +0000 (11:43 -0400)]
Clean up and simplify jsonb_concat code.

Some of this is made possible by commit
9b74f32cdbff8b9be47fc69164eae552050509ff which lets pushJsonbValue
handle binary Jsonb values, meaning that clients no longer have to, and
some is just doing things in simpler and more straightforward ways.

9 years agopgindent: fix typo
Bruce Momjian [Mon, 25 May 2015 12:08:05 +0000 (08:08 -0400)]
pgindent:  fix typo

Report by Michael Paquier

9 years agoFix rescan of IndexScan node with the new lossy GiST distance functions.
Heikki Linnakangas [Mon, 25 May 2015 11:42:21 +0000 (14:42 +0300)]
Fix rescan of IndexScan node with the new lossy GiST distance functions.

Must reset the "reached end" flag and reorder queue at rescan.

Per report from Regina Obe, bug #13349

9 years agopgindent: more doc updates for skipping __asm__ files
Bruce Momjian [Mon, 25 May 2015 01:51:42 +0000 (21:51 -0400)]
pgindent:  more doc updates for skipping __asm__ files

9 years agoRevert 9.5 pgindent changes to atomics directory files
Bruce Momjian [Mon, 25 May 2015 01:44:57 +0000 (21:44 -0400)]
Revert 9.5 pgindent changes to atomics directory files

This is because there are many __asm__ blocks there that pgindent messes
up.  Also configure pgindent to skip that directory in the future.

9 years agoManual cleanup of pgindent results.
Tom Lane [Sun, 24 May 2015 19:04:10 +0000 (15:04 -0400)]
Manual cleanup of pgindent results.

Fix some places where pgindent did silly stuff, often because project
style wasn't followed to begin with.  (I've not touched the atomics
headers, though.)

9 years agoRename pg_shdepend.c's typedef "objectType" to SharedDependencyObjectType.
Tom Lane [Sun, 24 May 2015 17:03:45 +0000 (13:03 -0400)]
Rename pg_shdepend.c's typedef "objectType" to SharedDependencyObjectType.

The name objectType is widely used as a field name, and it's pure luck that
this conflict has not caused pgindent to go crazy before.  It messed up
pg_audit.c pretty good though.  Since pg_shdepend.c doesn't export this
typedef and only uses it in three places, changing that seems saner than
changing the field usages.

Back-patch because we're contemplating using the union of all branch
typedefs for future pgindent runs, so this won't fix anything if it
stays the same in back branches.

9 years agoAdd a bit more commentary about regex's colormap tree data structure.
Tom Lane [Sun, 24 May 2015 16:40:38 +0000 (12:40 -0400)]
Add a bit more commentary about regex's colormap tree data structure.

Per an off-list question from Piotr Stefaniak.

9 years agoRemove no-longer-required function declarations.
Tom Lane [Sun, 24 May 2015 16:20:23 +0000 (12:20 -0400)]
Remove no-longer-required function declarations.

Remove a bunch of "extern Datum foo(PG_FUNCTION_ARGS);" declarations that
are no longer needed now that PG_FUNCTION_INFO_V1(foo) provides that.

Some of these were evidently missed in commit e7128e8dbb305059, but others
were cargo-culted in in code added since then.  Possibly that can be blamed
in part on the fact that we'd not fixed relevant documentation examples,
which I've now done.

9 years agopgindent run for 9.5
Bruce Momjian [Sun, 24 May 2015 01:35:49 +0000 (21:35 -0400)]
pgindent run for 9.5

9 years agoUpdate typedef file in preparation for pgindent run
Bruce Momjian [Sun, 24 May 2015 01:20:37 +0000 (21:20 -0400)]
Update typedef file in preparation for pgindent run

9 years agoImprove pgindent instructions regarding Perl backup files
Bruce Momjian [Sun, 24 May 2015 01:09:00 +0000 (21:09 -0400)]
Improve pgindent instructions regarding Perl backup files

9 years agoAdd error check for lossy distance functions in index-only scans.
Tom Lane [Sat, 23 May 2015 20:24:31 +0000 (16:24 -0400)]
Add error check for lossy distance functions in index-only scans.

Maybe we should actually support this, but for the moment let's just
throw an error if the opclass tries it.

9 years agoFix incorrect snprintf() limit.
Tom Lane [Sat, 23 May 2015 20:05:52 +0000 (16:05 -0400)]
Fix incorrect snprintf() limit.

Typo in commit 7cbee7c0a.  No practical effect since the buffer should
never actually be overrun, but various compilers and static analyzers will
whine about it.

Petr Jelinek

9 years agoStill more fixes for lossy-GiST-distance-functions patch.
Tom Lane [Sat, 23 May 2015 19:22:25 +0000 (15:22 -0400)]
Still more fixes for lossy-GiST-distance-functions patch.

Fix confusion in documentation, substantial memory leakage if float8 or
float4 are pass-by-reference, and assorted comments that were obsoleted
by commit 98edd617f3b62a02cb2df9b418fcc4ece45c7ec0.

9 years agoFix yet another bug in ON CONFLICT rule deparsing.
Andres Freund [Sat, 23 May 2015 00:16:24 +0000 (02:16 +0200)]
Fix yet another bug in ON CONFLICT rule deparsing.

Expand testing of rule deparsing a good bit, it's evidently needed.

Author: Peter Geoghegan, Andres Freund
Discussion: CAM3SWZQmXxZhQC32QVEOTYfNXJBJ_Q2SDENL7BV14Cq-zL0FLg@mail.gmail.com

9 years agoRemove the new UPSERT command tag and use INSERT instead.
Andres Freund [Fri, 22 May 2015 22:49:27 +0000 (00:49 +0200)]
Remove the new UPSERT command tag and use INSERT instead.

Previously, INSERT with ON CONFLICT DO UPDATE specified used a new
command tag -- UPSERT.  It was introduced out of concern that INSERT as
a command tag would be a misrepresentation for ON CONFLICT DO UPDATE, as
some affected rows may actually have been updated.

Alvaro Herrera noticed that the implementation of that new command tag
was incomplete; in subsequent discussion we concluded that having it
doesn't provide benefits that are in line with the compatibility breaks
it requires.

Catversion bump due to the removal of PlannedStmt->isUpsert.

Author: Peter Geoghegan
Discussion: 20150520215816.GI5885@postgresql.org

9 years agoFix recently-introduced crash in array_contain_compare().
Tom Lane [Fri, 22 May 2015 22:36:48 +0000 (18:36 -0400)]
Fix recently-introduced crash in array_contain_compare().

Silly oversight in commit 1dc5ebc9077ab742079ce5dac9a6664248d42916:
when array2 is an expanded array, it might have array2->xpn.dnulls equal
to NULL, indicating the array is known null-free.  The code wasn't
expecting that, because it formerly always used deconstruct_array() which
always delivers a nulls array.

Per bug #13334 from Regina Obe.

9 years agoUnpack jbvBinary objects passed to pushJsonbValue
Andrew Dunstan [Fri, 22 May 2015 14:21:41 +0000 (10:21 -0400)]
Unpack jbvBinary objects passed to pushJsonbValue

pushJsonbValue was accepting jbvBinary objects passed as WJB_ELEM or
WJB_VALUE data. While this succeeded, when those objects were later
encountered in attempting to convert the result to Jsonb, errors
occurred. With this change we ghuarantee that a JSonbValue constructed
from calls to pushJsonbValue does not contain any jbvBinary objects.
This cures a problem observed with jsonb_delete.

This means callers of pushJsonbValue no longer need to perform this
unpacking themselves. A subsequent patch will perform some cleanup in
that area.

The error was not triggered by any 9.4 code, but this is a publicly
visible routine, and so the error could be exercised by third party
code, therefore backpatch to 9.4.

Bug report from Peter Geoghegan, fix by me.

9 years agoMinor enhancement of readability of ALTER TABLE syntax in the doc.
Fujii Masao [Fri, 22 May 2015 12:42:15 +0000 (21:42 +0900)]
Minor enhancement of readability of ALTER TABLE syntax in the doc.

Fabrízio Mello

9 years agoAt promotion, don't leave behind a partial segment on the old timeline.
Heikki Linnakangas [Thu, 21 May 2015 12:28:22 +0000 (15:28 +0300)]
At promotion, don't leave behind a partial segment on the old timeline.

With commit de768844, a copy of the partial segment was archived with the
.partial suffix, but the original file was still left in pg_xlog, so it
didn't actually solve the problems with archiving the partial segment that
it was supposed to solve. With this patch, the partial segment is renamed
rather than copied, so we only archive it with the .partial suffix.

Also be more robust in detecting if the last segment is already being
archived. Previously I used XLogArchiveIsBusy() for that, but that's not
quite right. With archive_mode='always', there might be a .ready file for
it, and we don't want to rename it to .partial in that case.

The old segment is needed until we're fully committed to the new timeline,
i.e. until we've written the end-of-recovery WAL record and updated the
min recovery point and timeline in the control file. So move the renaming
later in the startup sequence, after all that's been done.

9 years agoMore fixes for lossy-GiST-distance-functions patch.
Tom Lane [Thu, 21 May 2015 23:47:48 +0000 (19:47 -0400)]
More fixes for lossy-GiST-distance-functions patch.

Paul Ramsey reported that commit 35fcb1b3d038a501f3f4c87c05630095abaaadab
induced a core dump on commuted ORDER BY expressions, because it was
assuming that the indexorderby expression could be found verbatim in the
relevant equivalence class, but it wasn't there.  We really don't need
anything that complicated anyway; for the data types likely to be used for
index ORDER BY operators in the foreseeable future, the exprType() of the
ORDER BY expression will serve fine.  (The case where we'd have to work
harder is where the ORDER BY expression's result is only binary-compatible
with the declared input type of the ordering operator; long before worrying
about that, one would need to get rid of GiST's hard-wired assumption that
said datatype is float8.)

Aside from fixing that crash and adding a regression test for the case,
I did some desultory code review:

nodeIndexscan.c was likewise overthinking how hard it ought to work to
identify the datatype of the ORDER BY expressions.

Add comments explaining how come nodeIndexscan.c can get away with
simplifying assumptions about NULLS LAST ordering and no backward scan.

Revert no-longer-needed changes of find_ec_member_for_tle(); while the
new definition was no worse than the old, it wasn't better either, and
it might cause back-patching pain.

Revert entirely bogus additions to genam.h.

9 years agoImprove packing/alignment annotation for ItemPointerData.
Tom Lane [Thu, 21 May 2015 21:21:46 +0000 (17:21 -0400)]
Improve packing/alignment annotation for ItemPointerData.

We want this struct to be exactly a series of 3 int16 words, no more
and no less.  Historically, at least, some ARM compilers preferred to
pad it to 8 bytes unless coerced.  Our old way of doing that was just
to use __attribute__((packed)), but as pointed out by Piotr Stefaniak,
that does too much: it also licenses the compiler to give the struct
only byte-alignment.  We don't want that because it adds access overhead,
possibly quite significant overhead.  According to the GCC manual, what
we want requires also specifying __attribute__((align(2))).  It's not
entirely clear if all the relevant compilers accept this pragma as well,
but we can hope the buildfarm will tell us if not.  We can also add a
static assertion that should fire if the compiler padded the struct.

Since the combination of these pragmas should define exactly what we
want on any compiler that accepts them, let's try using them wherever
we think they exist, not only for __arm__.  (This is likely to expose
that the conditional definitions in c.h are inadequate, but finding
that out would be a good thing.)

The immediate motivation for this is that the current definition of
ExecRowMark allows its curCtid field to be misaligned.  It is not clear
whether there are any other uses of ItemPointerData with a similar hazard.
We could change the definition of ExecRowMark if this doesn't work, but
it would be far better to have a future-proof fix.

Piotr Stefaniak, some further hacking by me

9 years agoCorrect two mistakes in the ALTER FOREIGN TABLE reference page.
Robert Haas [Thu, 21 May 2015 15:16:33 +0000 (11:16 -0400)]
Correct two mistakes in the ALTER FOREIGN TABLE reference page.

Etsuro Fujita

9 years agoCorrect the names of pgstattuple_approx output columns in the doc.
Fujii Masao [Thu, 21 May 2015 11:51:52 +0000 (20:51 +0900)]
Correct the names of pgstattuple_approx output columns in the doc.

9 years agoMake recovery_target_action = pause work.
Fujii Masao [Thu, 21 May 2015 04:56:17 +0000 (13:56 +0900)]
Make recovery_target_action = pause work.

Previously even if recovery_target_action was set to pause and
the recovery target was reached, the recovery could never be paused.
Because the setting of pause was *always* overridden with that of
shutdown unexpectedly. This override is valid and intentional
if hot_standby is not enabled because there is no way to resume
the paused recovery in this case and the setting of pause is
completely useless. But not if hot_standby is enabled.

This patch changes the code so that the setting of pause is overridden
with that of shutdown only when hot_standby is not enabled.

Bug reported by Andres Freund

9 years agoAnother typo fix.
Tom Lane [Wed, 20 May 2015 18:50:22 +0000 (14:50 -0400)]
Another typo fix.

In the spirit of the season.

9 years agoFix more typos in comments.
Heikki Linnakangas [Wed, 20 May 2015 16:44:46 +0000 (19:44 +0300)]
Fix more typos in comments.

Patch by CharSyam, plus a few more I spotted with grep.

9 years agoCollection of typo fixes.
Heikki Linnakangas [Wed, 20 May 2015 13:18:11 +0000 (16:18 +0300)]
Collection of typo fixes.

Use "a" and "an" correctly, mostly in comments. Two error messages were
also fixed (they were just elogs, so no translation work required). Two
function comments in pg_proc.h were also fixed. Etsuro Fujita reported one
of these, but I found a lot more with grep.

Also fix a few other typos spotted while grepping for the a/an typos.
For example, "consists out of ..." -> "consists of ...". Plus a "though"/
"through" mixup reported by Euler Taveira.

Many of these typos were in old code, which would be nice to backpatch to
make future backpatching easier. But much of the code was new, and I didn't
feel like crafting separate patches for each branch. So no backpatching.

9 years agoFix spelling in comment
Simon Riggs [Tue, 19 May 2015 22:37:46 +0000 (18:37 -0400)]
Fix spelling in comment

9 years agoLast-minute updates for release notes.
Tom Lane [Tue, 19 May 2015 22:33:58 +0000 (18:33 -0400)]
Last-minute updates for release notes.

Revise description of CVE-2015-3166, in line with scaled-back patch.
Change release date.

Security: CVE-2015-3166

9 years agoRevert error-throwing wrappers for the printf family of functions.
Tom Lane [Tue, 19 May 2015 22:14:52 +0000 (18:14 -0400)]
Revert error-throwing wrappers for the printf family of functions.

This reverts commit 16304a013432931e61e623c8d85e9fe24709d9ba, except
for its changes in src/port/snprintf.c; as well as commit
cac18a76bb6b08f1ecc2a85e46c9d2ab82dd9d23 which is no longer needed.

Fujii Masao reported that the previous commit caused failures in psql on
OS X, since if one exits the pager program early while viewing a query
result, psql sees an EPIPE error from fprintf --- and the wrapper function
thought that was reason to panic.  (It's a bit surprising that the same
does not happen on Linux.)  Further discussion among the security list
concluded that the risk of other such failures was far too great, and
that the one-size-fits-all approach to error handling embodied in the
previous patch is unlikely to be workable.

This leaves us again exposed to the possibility of the type of failure
envisioned in CVE-2015-3166.  However, that failure mode is strictly
hypothetical at this point: there is no concrete reason to believe that
an attacker could trigger information disclosure through the supposed
mechanism.  In the first place, the attack surface is fairly limited,
since so much of what the backend does with format strings goes through
stringinfo.c or psprintf(), and those already had adequate defenses.
In the second place, even granting that an unprivileged attacker could
control the occurrence of ENOMEM with some precision, it's a stretch to
believe that he could induce it just where the target buffer contains some
valuable information.  So we concluded that the risk of non-hypothetical
problems induced by the patch greatly outweighs the security risks.
We will therefore revert, and instead undertake closer analysis to
identify specific calls that may need hardening, rather than attempt a
universal solution.

We have kept the portion of the previous patch that improved snprintf.c's
handling of errors when it calls the platform's sprintf().  That seems to
be an unalloyed improvement.

Security: CVE-2015-3166

9 years agoVarious fixes around ON CONFLICT for rule deparsing.
Andres Freund [Tue, 19 May 2015 19:07:28 +0000 (21:07 +0200)]
Various fixes around ON CONFLICT for rule deparsing.

Neither the deparsing of the new alias for INSERT's target table, nor of
the inference clause was supported. Also fixup a typo in an error
message.

Add regression tests to test those code paths.

Author: Peter Geoghegan

9 years agoRefactor ON CONFLICT index inference parse tree representation.
Andres Freund [Tue, 19 May 2015 19:17:52 +0000 (21:17 +0200)]
Refactor ON CONFLICT index inference parse tree representation.

Defer lookup of opfamily and input type of a of a user specified opclass
until the optimizer selects among available unique indexes; and store
the opclass in the parse analyzed tree instead.  The primary reason for
doing this is that for rule deparsing it's easier to use the opclass
than the previous representation.

While at it also rename a variable in the inference code to better fit
it's purpose.

This is separate from the actual fixes for deparsing to make review
easier.

9 years agoFix off-by-one error in Assertion.
Heikki Linnakangas [Tue, 19 May 2015 16:21:46 +0000 (19:21 +0300)]
Fix off-by-one error in Assertion.

The point of the assertion is to ensure that the arrays allocated in stack
are large enough, but the check was one item short.

This won't matter in practice because MaxIndexTuplesPerPage is an
overestimate, so you can't have that many items on a page in reality.
But let's be tidy.

Spotted by Anastasia Lubennikova. Backpatch to all supported versions, like
the patch that added the assertion.

9 years agoAvoid collation dependence in indexes of system catalogs.
Tom Lane [Tue, 19 May 2015 15:47:42 +0000 (11:47 -0400)]
Avoid collation dependence in indexes of system catalogs.

No index in template0 should have collation-dependent ordering, especially
not indexes on shared catalogs.  For most textual columns we avoid this
issue by using type "name" (which sorts per strcmp()).  However there are a
few indexed columns that we'd prefer to use "text" for, and for that, the
default opclass text_ops is unsafe.  Fortunately, text_pattern_ops is safe
(it sorts per memcmp()), and it has no real functional disadvantage for our
purposes.  So change the indexes on pg_seclabel.provider and
pg_shseclabel.provider to use text_pattern_ops.

In passing, also mark pg_replication_origin.roname as using
text_pattern_ops --- for some reason it was labeled varchar_pattern_ops
which is just wrong, even though it accidentally worked.

Add regression test queries to catch future errors of these kinds.

We still can't do anything about the misdeclared pg_seclabel and
pg_shseclabel indexes in back branches :-(

9 years agoRevert "Change pg_seclabel.provider and pg_shseclabel.provider to type "name"."
Tom Lane [Tue, 19 May 2015 14:40:04 +0000 (10:40 -0400)]
Revert "Change pg_seclabel.provider and pg_shseclabel.provider to type "name"."

This reverts commit b82a7be603f1811a0a707b53c62de6d5d9431740.  There
is a better (less invasive) way to fix it, which I will commit next.

9 years agoMessage string improvements
Peter Eisentraut [Tue, 19 May 2015 03:01:48 +0000 (23:01 -0400)]
Message string improvements

9 years agoFix parse tree of DROP TRANSFORM and COMMENT ON TRANSFORM
Peter Eisentraut [Tue, 19 May 2015 02:55:14 +0000 (22:55 -0400)]
Fix parse tree of DROP TRANSFORM and COMMENT ON TRANSFORM

The plain C string language name needs to be wrapped in makeString() so
that the parse tree is copyable.  This is detectable by
-DCOPY_PARSE_PLAN_TREES.  Add a test case for the COMMENT case.

Also make the quoting in the error messages more consistent.

discovered by Tom Lane

9 years agoChange pg_seclabel.provider and pg_shseclabel.provider to type "name".
Tom Lane [Tue, 19 May 2015 00:07:44 +0000 (20:07 -0400)]
Change pg_seclabel.provider and pg_shseclabel.provider to type "name".

These were "text", but that's a bad idea because it has collation-dependent
ordering.  No index in template0 should have collation-dependent ordering,
especially not indexes on shared catalogs.  There was general agreement
that provider names don't need to be longer than other identifiers, so we
can fix this at a small waste of table space by changing from text to name.

There's no way to fix the problem in the back branches, but we can hope
that security labels don't yet have widespread-enough usage to make it
urgent to fix.

There needs to be a regression sanity test to prevent us from making this
same mistake again; but before putting that in, we'll need to get rid of
similar brain fade in the recently-added pg_replication_origin catalog.

Note: for lack of a suitable testing environment, I've not really exercised
this change.  I trust the buildfarm will show up any mistakes.

9 years agoAttach ON CONFLICT SET ... WHERE to the correct planstate.
Andres Freund [Mon, 18 May 2015 23:55:10 +0000 (01:55 +0200)]
Attach ON CONFLICT SET ... WHERE to the correct planstate.

The previous coding was a leftover from attempting to hang all the on
conflict logic onto modify table's child nodes. It appears to not have
actually caused problems except for explain.

Add test exercising the broken and some other code paths.

Author: Peter Geoghegan and Andres Freund

9 years agoPut back a backwards-compatible version of sampling support functions.
Tom Lane [Mon, 18 May 2015 22:34:37 +0000 (18:34 -0400)]
Put back a backwards-compatible version of sampling support functions.

Commit 83e176ec18d2a91dbea1d0d1bd94c38dc47cd77c removed the longstanding
support functions for block sampling without any consideration of the
impact this would have on third-party FDWs.  The new API is not notably
more functional for FDWs than the old, so forcing them to change doesn't
seem like a good thing.  We can provide the old API as a wrapper (more
or less) around the new one for a minimal amount of extra code.

9 years agoRecognize "REGRESS_OPTS += ..." syntax in MSVC build scripts.
Tom Lane [Mon, 18 May 2015 17:40:06 +0000 (13:40 -0400)]
Recognize "REGRESS_OPTS += ..." syntax in MSVC build scripts.

Necessitated by commit b14cf229f4bd7238be2e31d873dc5dd241d3871e.
Per buildfarm.

9 years agoFix error message in pre_sync_fname.
Robert Haas [Mon, 18 May 2015 16:53:09 +0000 (12:53 -0400)]
Fix error message in pre_sync_fname.

The old one didn't include %m anywhere, and required extra
translation.

Report by Peter Eisentraut. Fix by me. Review by Tom Lane.

9 years agoLast-minute updates for release notes.
Tom Lane [Mon, 18 May 2015 16:09:02 +0000 (12:09 -0400)]
Last-minute updates for release notes.

Add entries for security issues.

Security: CVE-2015-3165 through CVE-2015-3167

9 years agopgcrypto: Report errant decryption as "Wrong key or corrupt data".
Noah Misch [Mon, 18 May 2015 14:02:31 +0000 (10:02 -0400)]
pgcrypto: Report errant decryption as "Wrong key or corrupt data".

This has been the predominant outcome.  When the output of decrypting
with a wrong key coincidentally resembled an OpenPGP packet header,
pgcrypto could instead report "Corrupt data", "Not text data" or
"Unsupported compression algorithm".  The distinct "Corrupt data"
message added no value.  The latter two error messages misled when the
decrypted payload also exhibited fundamental integrity problems.  Worse,
error message variance in other systems has enabled cryptologic attacks;
see RFC 4880 section "14. Security Considerations".  Whether these
pgcrypto behaviors are likewise exploitable is unknown.

In passing, document that pgcrypto does not resist side-channel attacks.
Back-patch to 9.0 (all supported versions).

Security: CVE-2015-3167

9 years agoCheck return values of sensitive system library calls.
Noah Misch [Mon, 18 May 2015 14:02:31 +0000 (10:02 -0400)]
Check return values of sensitive system library calls.

PostgreSQL already checked the vast majority of these, missing this
handful that nearly cannot fail.  If putenv() failed with ENOMEM in
pg_GSS_recvauth(), authentication would proceed with the wrong keytab
file.  If strftime() returned zero in cache_locale_time(), using the
unspecified buffer contents could lead to information exposure or a
crash.  Back-patch to 9.0 (all supported versions).

Other unchecked calls to these functions, especially those in frontend
code, pose negligible security concern.  This patch does not address
them.  Nonetheless, it is always better to check return values whose
specification provides for indicating an error.

In passing, fix an off-by-one error in strftime_win32()'s invocation of
WideCharToMultiByte().  Upon retrieving a value of exactly MAX_L10N_DATA
bytes, strftime_win32() would overrun the caller's buffer by one byte.
MAX_L10N_DATA is chosen to exceed the length of every possible value, so
the vulnerable scenario probably does not arise.

Security: CVE-2015-3166

9 years agoAdd error-throwing wrappers for the printf family of functions.
Noah Misch [Mon, 18 May 2015 14:02:31 +0000 (10:02 -0400)]
Add error-throwing wrappers for the printf family of functions.

All known standard library implementations of these functions can fail
with ENOMEM.  A caller neglecting to check for failure would experience
missing output, information exposure, or a crash.  Check return values
within wrappers and code, currently just snprintf.c, that bypasses the
wrappers.  The wrappers do not return after an error, so their callers
need not check.  Back-patch to 9.0 (all supported versions).

Popular free software standard library implementations do take pains to
bypass malloc() in simple cases, but they risk ENOMEM for floating point
numbers, positional arguments, large field widths, and large precisions.
No specification demands such caution, so this commit regards every call
to a printf family function as a potential threat.

Injecting the wrappers implicitly is a compromise between patch scope
and design goals.  I would prefer to edit each call site to name a
wrapper explicitly.  libpq and the ECPG libraries would, ideally, convey
errors to the caller rather than abort().  All that would be painfully
invasive for a back-patched security fix, hence this compromise.

Security: CVE-2015-3166

9 years agoPermit use of vsprintf() in PostgreSQL code.
Noah Misch [Mon, 18 May 2015 14:02:31 +0000 (10:02 -0400)]
Permit use of vsprintf() in PostgreSQL code.

The next commit needs it.  Back-patch to 9.0 (all supported versions).

9 years agoPrevent a double free by not reentering be_tls_close().
Noah Misch [Mon, 18 May 2015 14:02:31 +0000 (10:02 -0400)]
Prevent a double free by not reentering be_tls_close().

Reentering this function with the right timing caused a double free,
typically crashing the backend.  By synchronizing a disconnection with
the authentication timeout, an unauthenticated attacker could achieve
this somewhat consistently.  Call be_tls_close() solely from within
proc_exit_prepare().  Back-patch to 9.0 (all supported versions).

Benkocs Norbert Attila

Security: CVE-2015-3165

9 years agoFix typo in comment.
Heikki Linnakangas [Mon, 18 May 2015 07:38:52 +0000 (10:38 +0300)]
Fix typo in comment.

Jim Nasby

9 years agoPut back stats-collector restarting code, removed accidentally.
Heikki Linnakangas [Mon, 18 May 2015 07:18:46 +0000 (10:18 +0300)]
Put back stats-collector restarting code, removed accidentally.

Removed that code snippet accidentally in the archive_mode='always' patch.

Also, use varname-tags for archive_command in the docs.

Fujii Masao