Michael Meskes [Sun, 19 Feb 2012 13:50:14 +0000 (14:50 +0100)]
In ecpglib rewrote code that used strtok_r to not use library functions
anymore. This way we don't have to worry which compiler on which OS offers
which version of strtok.
There are no functional changes here; the main point is just to lay down
a commit-log marker that somebody has looked at this recently, and to do
what we can to keep the two codebases comparable.
Tom Lane [Fri, 17 Feb 2012 01:00:11 +0000 (20:00 -0500)]
Fix longstanding error in contrib/intarray's int[] & int[] operator.
The array intersection code would give wrong results if the first entry of
the correct output array would be "1". (I think only this value could be
at risk, since the previous word would always be a lower-bound entry with
that fixed value.)
Problem spotted by Julien Rouhaud, initial patch by Guillaume Lelarge,
cosmetic improvements by me.
Tom Lane [Thu, 16 Feb 2012 22:33:28 +0000 (17:33 -0500)]
Improve statistics estimation to make some use of DISTINCT in sub-queries.
Formerly, we just punted when trying to estimate stats for variables coming
out of sub-queries using DISTINCT, on the grounds that whatever stats we
might have for underlying table columns would be inapplicable. But if the
sub-query has only one DISTINCT column, we can consider its output variable
as being unique, which is useful information all by itself. The scope of
this improvement is pretty narrow, but it costs nearly nothing, so we might
as well do it. Per discussion with Andres Freund.
This patch differs from the draft I submitted yesterday in updating various
comments about vardata.isunique (to reflect its extended meaning) and in
tweaking the interaction with security_barrier views. There does not seem
to be a reason why we can't use this sort of knowledge even when the
sub-query is such a view.
Robert Haas [Thu, 16 Feb 2012 18:24:19 +0000 (13:24 -0500)]
pg_dump: Miscellaneous tightening based on recent refactorings.
Use exit_horribly() and ExecuteSqlQueryForSingleRow() in various
places where it's equivalent, or nearly equivalent, to the prior
coding. Apart from being more compact, this also makes the error
messages for the wrong-number-of-tuples case more consistent.
Robert Haas [Thu, 16 Feb 2012 18:00:24 +0000 (13:00 -0500)]
pg_dump: Remove global connection pointer.
Parallel pg_dump wants to have multiple ArchiveHandle objects, and
therefore multiple PGconns, in play at the same time. This should
be just about the end of the refactoring that we need in order to
make that workable.
Robert Haas [Thu, 16 Feb 2012 17:07:06 +0000 (12:07 -0500)]
Refactor pg_dump.c to avoid duplicating returns-one-row check.
Any patches apt to get broken have probably already been broken by the
error-handling cleanups I just did, so we might as well clean this up
at the same time.
Tom Lane [Wed, 15 Feb 2012 21:18:34 +0000 (16:18 -0500)]
Run a portal's cleanup hook immediately when pushing it to FAILED state.
This extends the changes of commit 6252c4f9e201f619e5eebda12fa867acd4e4200e
so that we run the cleanup hook earlier for failure cases as well as
success cases. As before, the point is to avoid an assertion failure from
an Assert I added in commit a874fe7b4c890d1fe3455215a83ca777867beadd, which
was meant to check that no user-written code can be called during portal
cleanup. This fixes a case reported by Pavan Deolasee in which the Assert
could be triggered during backend exit (see the new regression test case),
and also prevents the possibility that the cleanup hook is run after
portions of the portal's state have already been recycled. That doesn't
really matter in current usage, but it foreseeably could matter in the
future.
Back-patch to 9.1 where the Assert in question was added.
Robert Haas [Wed, 15 Feb 2012 18:54:26 +0000 (13:54 -0500)]
sepgsql: Move some code from hooks.c to label.c
This is some preliminary refactoring related to a pending patch
to allow sepgsql-enable sessions to make dynamic label transitions.
But this commit doesn't involve any functional change: it just puts
some bits of code in more logical places.
Robert Haas [Wed, 15 Feb 2012 17:13:32 +0000 (12:13 -0500)]
Speed up in-memory tuplesorting.
Per recent work by Peter Geoghegan, it's significantly faster to
tuplesort on a single sortkey if ApplySortComparator is inlined into
quicksort rather reached via a function pointer. It's also faster
in general to have a version of quicksort which is specialized for
sorting SortTuple objects rather than objects of arbitrary size and
type. This requires a couple of additional copies of the quicksort
logic, which in this patch are generate using a Perl script. There
might be some benefit in adding further specializations here too,
but thus far it's not clear that those gains are worth their weight
in code footprint.
Tom Lane [Tue, 14 Feb 2012 22:34:19 +0000 (17:34 -0500)]
Preserve column names in the execution-time tupledesc for a RowExpr.
The hstore and json datatypes both have record-conversion functions that
pay attention to column names in the composite values they're handed.
We used to not worry about inserting correct field names into tuple
descriptors generated at runtime, but given these examples it seems
useful to do so. Observe the nicer-looking results in the regression
tests whose results changed.
catversion bump because there is a subtle change in requirements for stored
rule parsetrees: RowExprs from ROW() constructs now have to include field
names.
Bruce Momjian [Tue, 14 Feb 2012 16:09:49 +0000 (11:09 -0500)]
Change contrib/pg_test_fsync to control tests in terms of seconds per
test, rather than a number of test cycles. Changes -o/cycles option to
-s/seconds.
Robert Haas [Tue, 14 Feb 2012 03:20:27 +0000 (22:20 -0500)]
Allow LEAKPROOF functions for better performance of security views.
We don't normally allow quals to be pushed down into a view created
with the security_barrier option, but functions without side effects
are an exception: they're OK. This allows much better performance in
common cases, such as when using an equality operator (that might
even be indexable).
There is an outstanding issue here with the CREATE FUNCTION / ALTER
FUNCTION syntax: there's no way to use ALTER FUNCTION to unset the
leakproof flag. But I'm committing this as-is so that it doesn't
have to be rebased again; we can fix up the grammar in a future
commit.
Fix heap_multi_insert to set t_self field in the caller's tuples.
If tuples were toasted, heap_multi_insert didn't update the ctid on the
original tuples. This caused a failure if there was an after trigger
(including a foreign key), on the table, and a tuple got toasted.
Tom Lane [Sat, 11 Feb 2012 23:06:24 +0000 (18:06 -0500)]
Fix I/O-conversion-related memory leaks in plpgsql.
Datatype I/O functions are allowed to leak memory in CurrentMemoryContext,
since they are generally called in short-lived contexts. However, plpgsql
calls such functions for purposes of type conversion, and was calling them
in its procedure context. Therefore, any leaked memory would not be
recovered until the end of the plpgsql function. If such a conversion
was done within a loop, quite a bit of memory could get consumed. Fix by
calling such functions in the transient "eval_econtext", and adjust other
logic to match. Back-patch to all supported versions.
Tom Lane [Fri, 10 Feb 2012 20:22:14 +0000 (15:22 -0500)]
Fix oversight in pg_dump's handling of extension configuration tables.
If an extension has not been selected to be dumped (perhaps because of
a --schema or --table switch), the contents of its configuration tables
surely should not get dumped either. Per gripe from
Hubert Depesz Lubaczewski.
Tom Lane [Fri, 10 Feb 2012 19:09:21 +0000 (14:09 -0500)]
Fix brain fade in previous pg_dump patch.
In pre-7.3 databases, pg_attribute.attislocal doesn't exist. The easiest
way to make sure the new inheritance logic behaves sanely is to assume it's
TRUE, not FALSE. This will result in printing child columns even when
they're not really needed. We could work harder at trying to reconstruct a
value for attislocal, but there is little evidence that anyone still cares
about dumping from such old versions, so just do the minimum necessary to
have a valid dump.
I had this correct in the original draft of the patch, but for some
unaccountable reason decided it wasn't necessary to change the value.
Testing against an old server shows otherwise...
Tom Lane [Fri, 10 Feb 2012 18:28:05 +0000 (13:28 -0500)]
Fix pg_dump for better handling of inherited columns.
Revise pg_dump's handling of inherited columns, which was last looked at
seriously in 2001, to eliminate several misbehaviors associated with
inherited default expressions and NOT NULL flags. In particular make sure
that a column is printed in a child table's CREATE TABLE command if and
only if it has attislocal = true; the former behavior would sometimes cause
a column to become marked attislocal when it was not so marked in the
source database. Also, stop relying on textual comparison of default
expressions to decide if they're inherited; instead, don't use
default-expression inheritance at all, but just install the default
explicitly at each level of the hierarchy. This fixes the
search-path-related misbehavior recently exhibited by Chester Young, and
also removes some dubious assumptions about the order in which ALTER TABLE
SET DEFAULT commands would be executed.
Tom Lane [Fri, 10 Feb 2012 07:33:00 +0000 (02:33 -0500)]
Add ORDER BY to a query to prevent occasional regression test failures.
Per buildfarm, we sometimes get row-ordering variations in the output.
This also makes this query look more like numerous other ones in the same
test file.
psql: Support zero byte field and record separators
Add new psql settings and command-line options to support setting the
field and record separators for unaligned output to a zero byte, for
easier interfacing with other shell tools.
Robert Haas [Thu, 9 Feb 2012 17:22:21 +0000 (12:22 -0500)]
Add a comment to AdjustIntervalForTypmod to reduce chance of future bugs.
It's not entirely evident how the logic here relates to the
interval_transform function, so let's clue people in that they need to
check that if the rules change.
Add new keywords SNAPSHOT and TYPES to the keyword list in gram.y
These were added to kwlist.h as unreserved keywords in separate patches,
but authors forgot to add them to the corresponding list in gram.y.
Because of that, even though they were supposed to be unreserved keywords,
they could not be used as identifiers. src/tools/check_keywords.pl is your
friend.
Tom Lane [Wed, 8 Feb 2012 20:23:00 +0000 (15:23 -0500)]
Fix up dumping conditions for extension configuration tables.
Various filters that were meant to prevent dumping of table data were not
being applied to extension config tables, notably --exclude-table-data and
--no-unlogged-table-data. We also would bogusly try to dump data from
views, sequences, or foreign tables, should an extension try to claim they
were config tables. Fix all that, and refactor/redocument to try to make
this a bit less fragile. This reverts the implementation, though not the
feature, of commit 7b070e896ca835318c90b02c830a5c4844413b64, which had
broken config-table dumping altogether :-(.
It is still the case that the code will dump config-table data even if
--schema is specified. That behavior was intentional, as per the comments
in getExtensionMembership, so I think it requires some more discussion
before we change it.
Tom Lane [Wed, 8 Feb 2012 18:15:02 +0000 (13:15 -0500)]
Check misplaced window functions before checking aggregate/group by sanity.
If somebody puts a window function in WHERE, we should complain about that
in so many words. The previous coding tended to complain about the window
function's arguments instead, which is likely to be misleading to users who
are unclear on the semantics of window functions; as seen for example in
bug #6440 from Matyas Novak.
Just another example of how "add new code at the end" is frequently a bad
heuristic.
Tom Lane [Wed, 8 Feb 2012 17:41:48 +0000 (12:41 -0500)]
Support min/max index optimizations on boolean columns.
Since bool_and() is equivalent to min(), and bool_or() to max(), we might
as well let them be index-optimized in the same way. The practical value
of this is debatable at best, but it seems nearly cost-free to enable it.
Code-wise, we need only adjust the entries in pg_aggregate. There is a
measurable planning speed penalty for a query involving one of these
aggregates, but it is only a few percent in simple cases, so that seems
acceptable.
Tom Lane [Wed, 8 Feb 2012 16:29:29 +0000 (11:29 -0500)]
Mark some more I/O-conversion-invoking functions as stable not volatile.
When written, textanycat, anytextcat, quote_literal, and quote_nullable
were marked volatile, because they could invoke arbitrary type-specific
output functions as part of casting their anyelement arguments to text.
Since then, we have defined a project policy that I/O functions must not
be volatile, as per commit aab353a60b95aadc00f81da0c6d99bde696c4b75.
So these functions can safely be downgraded to stable. Most of the time
this makes no difference since they'll get inlined anyway, but as noted
by Andrew Dunstan, there are cases where the volatile marking prevents
optimizations that the planner does before function inlining. (I think
I might have overlooked these functions in the earlier commit on the
grounds that inlining would make it moot, but not so --- tgl)
This change results in a change in the expected output of the json
regression tests, because the planner can now flatten a sub-select
that it failed to before. The old output is preferable, but getting
that back will require some as-yet-unfinished work on RowExpr handling.
pg_regress: Use target-specific variable instead of overriding make rule
Use a target-specific variable to add to CPPFLAGS instead of writing a
custom .c -> .o rule. This will ensure that dependency tracking is
used when enabled.
Robert Haas [Tue, 7 Feb 2012 18:45:46 +0000 (13:45 -0500)]
Support fls().
The immediate impetus for this is that Noah Misch's patch to elide
unnecessary table and index rebuilds when changing typmod for temporal
types uses it; and this is extracted from that patch, with some
further commentary by me. But it seems logically separate from the
remainder of the patch, so I'm committing it separately; this is not
the first time someone has wanted fls() in the backend and probably
won't be the last.
If we end up using this in more performance-critical spots it may be
worthwhile to add some architecture-specific optimizations to our
src/port version of fls() - e.g. any x86 platform can implement this
using the assembly instruction BSRL. But performance won't matter
a bit for assessing typmod changes, so I'm not worried about that
right now.
Robert Haas [Tue, 7 Feb 2012 17:41:42 +0000 (12:41 -0500)]
Add a transform function for varbit typmod coercisions.
This enables ALTER TABLE to skip table and index rebuilds when the
new type is unconstraint varbit, or when the allowable number of bits
is not decreasing.
Noah Misch, with review and a fix for an OID collision by me.
Robert Haas [Tue, 7 Feb 2012 17:08:26 +0000 (12:08 -0500)]
Add a transform function for numeric typmod coercisions.
This enables ALTER TABLE to skip table and index rebuilds when a column
is changed to an unconstrained numeric, or when the scale is unchanged
and the precision does not decrease.
Noah Misch, with a few stylistic changes and a fix for an OID
collision by me.
Robert Haas [Tue, 7 Feb 2012 16:23:04 +0000 (11:23 -0500)]
Add TIMING option to EXPLAIN, to allow eliminating of timing overhead.
Sometimes it may be useful to get actual row counts out of EXPLAIN
(ANALYZE) without paying the cost of timing every node entry/exit.
With this patch, you can say EXPLAIN (ANALYZE, TIMING OFF) to get that.
Tomas Vondra, reviewed by Eric Theise, with minor doc changes by me.
Robert Haas [Tue, 7 Feb 2012 15:07:02 +0000 (10:07 -0500)]
pg_dump: Further reduce reliance on global variables.
This is another round of refactoring to make things simpler for parallel
pg_dump. pg_dump.c now issues SQL queries through the relevant Archive
object, rather than relying on the global variable g_conn. This commit
isn't quite enough to get rid of g_conn entirely, but it makes a big
dent in its utilization and, along the way, manages to be slightly less
code than before.
When building with LWLOCK_STATS, initialize the stats in LWLockWaitUntilFree.
If LWLockWaitUntilFree was called before the first LWLockAcquire call, you
would either crash because of access to uninitialized array or account the
acquisition incorrectly. LWLockConditionalAcquire doesn't have this problem
because it doesn't update the lwlock stats.
In practice, this never happens because there is no codepath where you would
call LWLockWaitUntilfree before LWLockAcquire after a new process is
launched. But that's just accidental, there's no guarantee that that's
always going to be true in the future.
Tom Lane [Mon, 6 Feb 2012 20:29:26 +0000 (15:29 -0500)]
Fix postmaster to attempt restart after a hot-standby crash.
The postmaster was coded to treat any unexpected exit of the startup
process (i.e., the WAL replay process) as a catastrophic crash, and not try
to restart it. This was OK so long as the startup process could not have
any sibling postmaster children. However, if a hot-standby backend
crashes, we SIGQUIT the startup process along with everything else, and the
resulting exit is hardly "unexpected". Treating it as such meant we failed
to restart a standby server after any child crash at all, not only a crash
of the WAL replay process as intended. Adjust that. Back-patch to 9.0
where hot standby was introduced.
Tom Lane [Mon, 6 Feb 2012 19:43:58 +0000 (14:43 -0500)]
Avoid throwing ERROR during WAL replay of DROP TABLESPACE.
Although we will not even issue an XLOG_TBLSPC_DROP WAL record unless
removal of the tablespace's directories succeeds, that does not guarantee
that the same operation will succeed during WAL replay. Foreseeable
reasons for it to fail include temp files created in the tablespace by Hot
Standby backends, wrong directory permissions on a standby server, etc etc.
The original coding threw ERROR if replay failed to remove the directories,
but that is a serious overreaction. Throwing an error aborts recovery,
and worse means that manual intervention will be needed to get the database
to start again, since otherwise the same error will recur on subsequent
attempts to replay the same WAL record. And the consequence of failing to
remove the directories is only that some probably-small amount of disk
space is wasted, so it hardly seems justified to throw an error.
Accordingly, arrange to report such failures as LOG messages and keep going
when a failure occurs during replay.
Back-patch to 9.0 where Hot Standby was introduced. In principle such
problems can occur in earlier releases, but Hot Standby increases the odds
of trouble significantly. Given the lack of field reports of such issues,
I'm satisfied with patching back as far as the patch applies easily.
Robert Haas [Mon, 6 Feb 2012 19:07:55 +0000 (14:07 -0500)]
pg_dump: Remove global Archive pointer.
Instead, everything that needs the Archive object now gets it as a
parameter. This is necessary infrastructure for parallel pg_dump,
but is also amply justified by the ugliness of the current code
(though a lot more than this is needed to fix that problem).
Robert Haas [Mon, 6 Feb 2012 17:52:36 +0000 (12:52 -0500)]
pg_dump: Reduce dependencies on global variables.
Change various places in the code that are referencing the global
Archive object g_fout to instead reference the Archive object fout
which is already being passed as a parameter. For parallel pg_dump to
work, we're going to need multiple Archive(Handle) objects, so the
real solution here is to pass down the Archive object to everywhere
that it needs to go, but we might as well pick the low-hanging fruit
first.
Tom Lane [Mon, 6 Feb 2012 17:34:10 +0000 (12:34 -0500)]
Add locking around WAL-replay modification of shared-memory variables.
Originally, most of this code assumed that no Postgres backends could be
running concurrently with it, and so no locking could be needed. That
assumption fails in Hot Standby. While it's still true that Hot Standby
backends should never change values like nextXid, they can examine them,
and consistency is important in some cases such as when computing a
snapshot. Therefore, prudence requires that WAL replay code obtain the
relevant locks when modifying such variables, even though it can examine
them without taking a lock. We were following that coding rule in some
places but not all. This commit applies the coding rule uniformly to all
updates of ShmemVariableCache and MultiXactState fields; a search of the
replay routines did not find any other cases that seemed to be at risk.
In addition, this commit fixes a longstanding thinko in replay of NEXTOID
and checkpoint records: we tried to advance nextOid only if it was behind
the value in the WAL record, but the comparison would draw the wrong
conclusion if OID wraparound had occurred since the previous value.
Better to just unconditionally assign the new value, since OID assignment
shouldn't be happening during replay anyway.
The additional locking seems to be more in the nature of future-proofing
than fixing any live bug, so I am not going to back-patch it. The NEXTOID
fix will be back-patched separately.
Tom Lane [Sun, 5 Feb 2012 20:49:17 +0000 (15:49 -0500)]
Fix transient clobbering of shared buffers during WAL replay.
RestoreBkpBlocks was in the habit of zeroing and refilling the target
buffer; which was perfectly safe when the code was written, but is unsafe
during Hot Standby operation. The reason is that we have coding rules
that allow backends to continue accessing a tuple in a heap relation while
holding only a pin on its buffer. Such a backend could see transiently
zeroed data, if WAL replay had occasion to change other data on the page.
This has been shown to be the cause of bug #6425 from Duncan Rance (who
deserves kudos for developing a sufficiently-reproducible test case) as
well as Bridget Frey's re-report of bug #6200. It most likely explains the
original report as well, though we don't yet have confirmation of that.
To fix, change the code so that only bytes that are supposed to change will
change, even transiently. This actually saves cycles in RestoreBkpBlocks,
since it's not writing the same bytes twice.
Also fix seq_redo, which has the same disease, though it has to work a bit
harder to meet the requirement.
So far as I can tell, no other WAL replay routines have this type of bug.
In particular, the index-related replay routines, which would certainly be
broken if they had to meet the same standard, are not at risk because we
do not have coding rules that allow access to an index page when not
holding a buffer lock on it.
Tom Lane [Sun, 5 Feb 2012 03:32:35 +0000 (22:32 -0500)]
Add missing Assert and fix inaccurate elog message in standby_redo().
All other WAL redo routines either call RestoreBkpBlocks() or Assert that
they haven't been passed any backup blocks. Make this one do likewise.
Also, fix incorrect routine name in its failure message.
Tom Lane [Sat, 4 Feb 2012 20:52:09 +0000 (15:52 -0500)]
Revert "Add some regression test cases for denormalized float8 input."
This reverts commit 500cf66d5522b39ddfdc26b309f8b5b0e385f42e. As was
more or less expected, a small minority of platforms won't accept
denormalized input even with the recent changes. It doesn't seem
especially helpful to test this if we're going to have to provide an
alternate expected-file to allow failure.
Further improve on commit c75e1436467f32a06b5ab9d594d2a390e7f4594d.
Instead of building both .o files and binaries in the same make rule,
just rely on the normal .c -> .o rule. This will ensure that
dependency tracking is used when enabled. To do this, disable the
implicit direct .c -> binary rule globally, which will also prevent
the original problem (*.dSYM junk) from reappearing elsewhere.
Robert Haas [Thu, 2 Feb 2012 01:35:42 +0000 (20:35 -0500)]
Avoid re-checking for visibility map extension too frequently.
When testing bits (but not when setting or clearing them), we now
won't check whether the map has been extended. This significantly
improves performance in the case where the visibility map doesn't
exist yet, by avoiding an extra system call per tuple. To make
sure backends notice eventually, send an smgr inval on VM extension.
Tom Lane [Wed, 1 Feb 2012 18:11:16 +0000 (13:11 -0500)]
Try to be more consistent about accepting denormalized float8 numbers.
On some platforms, strtod() reports ERANGE for a denormalized value (ie,
one that can be represented as distinct from zero, but is too small to have
full precision). On others, it doesn't. It seems better to try to accept
these values consistently, so add a test to see if the result value
indicates a true out-of-range condition. This should be okay per Single
Unix Spec. On machines where the underlying math isn't IEEE standard, the
behavior for such small numbers may not be very consistent, but then it
wouldn't be anyway.
Marti Raudsepp, after a proposal by Jeroen Vermeulen
Alvaro Herrera [Wed, 1 Feb 2012 16:56:59 +0000 (13:56 -0300)]
Implement dry-run mode for pg_archivecleanup
In dry-run mode, just the name of the file to be removed is printed to
stdout; this is so the user can easily plug it into another program
through a pipe. If debug mode is also specified, a more verbose message
is printed to stderr.
Tom Lane [Wed, 1 Feb 2012 07:14:37 +0000 (02:14 -0500)]
Code review for plpgsql fn_signature patch.
Don't quote the output of format_procedure(); it's already quoted quite
enough. Remove the fn_name field, which was now just dead weight. Fix
remaining expected-output files.
Robert Haas [Tue, 31 Jan 2012 16:48:23 +0000 (11:48 -0500)]
Built-in JSON data type.
Like the XML data type, we simply store JSON data as text, after checking
that it is valid. More complex operations such as canonicalization and
comparison may come later, but this is enough for not.
There are a few open issues here, such as whether we should attempt to
detect UTF-8 surrogate pairs represented as \uXXXX\uYYYY, but this gets
the basic framework in place.
Fix bug in the new wait-until-lwlock-is-free mechanism.
If there was a wait-until-free process in the head of the wait queue,
followed by an exclusive locker, the exclusive locker was not be woken up
as it should.
Peter Eisentraut [Mon, 30 Jan 2012 19:45:42 +0000 (21:45 +0200)]
Add sequence USAGE privileges to information schema
The sequence USAGE privilege is sufficiently similar to the SQL
standard that it seems reasonable to show in the information schema.
Also add some compatibility notes about it on the GRANT reference
page.
Peter Eisentraut [Mon, 30 Jan 2012 19:38:52 +0000 (21:38 +0200)]
PL/Python: Add result metadata functions
Add result object functions .colnames, .coltypes, .coltypmods to
obtain information about the result column names and types, which was
previously not possible in the PL/Python SPI interface.