]> granicus.if.org Git - postgresql/log
postgresql
9 years agoUse the typcache to cache constraints for domain types.
Tom Lane [Sun, 1 Mar 2015 19:06:50 +0000 (14:06 -0500)]
Use the typcache to cache constraints for domain types.

Previously, we cached domain constraints for the life of a query, or
really for the life of the FmgrInfo struct that was used to invoke
domain_in() or domain_check().  But plpgsql (and probably other places)
are set up to cache such FmgrInfos for the whole lifespan of a session,
which meant they could be enforcing really stale sets of constraints.
On the other hand, searching pg_constraint once per query gets kind of
expensive too: testing says that as much as half the runtime of a
trivial query such as "SELECT 0::domaintype" went into that.

To fix this, delegate the responsibility for tracking a domain's
constraints to the typcache, which has the infrastructure needed to
detect syscache invalidation events that signal possible changes.
This not only removes unnecessary repeat reads of pg_constraint,
but ensures that we never apply stale constraint data: whatever we
use is the current data according to syscache rules.

Unfortunately, the current configuration of the system catalogs means
we have to flush cached domain-constraint data whenever either pg_type
or pg_constraint changes, which happens rather a lot (eg, creation or
deletion of a temp table will do it).  It might be worth rearranging
things to split pg_constraint into two catalogs, of which the domain
constraint one would probably be very low-traffic.  That's a job for
another patch though, and in any case this patch should improve matters
materially even with that handicap.

This patch makes use of the recently-added memory context reset callback
feature to manage the lifespan of domain constraint caches, so that we
don't risk deleting a cache that might be in the midst of evaluation.

Although this is a bug fix as well as a performance improvement, no
back-patch.  There haven't been many if any field complaints about
stale domain constraint checks, so it doesn't seem worth taking the
risk of modifying data structures as basic as MemoryContexts in back
branches.

9 years agoAdd transform functions for AT TIME ZONE.
Noah Misch [Sun, 1 Mar 2015 18:22:34 +0000 (13:22 -0500)]
Add transform functions for AT TIME ZONE.

This makes "ALTER TABLE tabname ALTER tscol TYPE ... USING tscol AT TIME
ZONE 'UTC'" skip rewriting the table when altering from "timestamp" to
"timestamptz" or vice versa.  While it would be nicer still to optimize
this in the absence of the USING clause given timezone==UTC, transform
functions must consult IMMUTABLE facts only.

9 years agoUnlink static libraries before rebuilding them.
Noah Misch [Sun, 1 Mar 2015 18:05:23 +0000 (13:05 -0500)]
Unlink static libraries before rebuilding them.

When the library already exists in the build directory, "ar" preserves
members not named on its command line.  This mattered when, for example,
a "configure" rerun dropped a file from $(LIBOBJS).  libpgport carried
the obsolete member until "make clean".  Back-patch to 9.0 (all
supported versions).

9 years agoMove memory context callback declarations into palloc.h.
Tom Lane [Sun, 1 Mar 2015 17:31:32 +0000 (12:31 -0500)]
Move memory context callback declarations into palloc.h.

Initial experience with this feature suggests that instances of
MemoryContextCallback are likely to propagate into some widely-used headers
over time.  As things stood, that would result in pulling memutils.h or
at least memnodes.h into common headers, which does not seem desirable.
Instead, let's decide that this feature is part of the "ordinary palloc
user" API rather than the "specialized context management" API, and as
such should be declared in palloc.h not memutils.h.

9 years agoFix intermittent failure in event_trigger test
Alvaro Herrera [Sun, 1 Mar 2015 14:58:07 +0000 (11:58 -0300)]
Fix intermittent failure in event_trigger test

As evidenced by measles in buildfarm.  Pointed out by Tom.

9 years agoTrack typmods in plpgsql expression evaluation and assignment.
Tom Lane [Sat, 28 Feb 2015 19:34:35 +0000 (14:34 -0500)]
Track typmods in plpgsql expression evaluation and assignment.

The main value of this change is to avoid expensive I/O conversions when
assigning to a variable that has a typmod specification, if the value
to be assigned is already known to have the right typmod.  This is
particularly valuable for arrays with typmod specifications; formerly,
in an assignment to an array element the entire array would invariably
get put through double I/O conversion to check the typmod, to absolutely
no purpose since we'd already properly coerced the new element value.

Extracted from my "expanded arrays" patch; this seems worth committing
separately, whatever becomes of that patch, since it's really an
independent issue.

As long as we're changing the function signatures, take the opportunity
to rationalize the argument lists of exec_assign_value, exec_cast_value,
and exec_simple_cast_value; that is, put the arguments into a saner order,
and get rid of the bizarre choice to pass exec_assign_value's isNull flag
by reference.

9 years agoFix planning of star-schema-style queries.
Tom Lane [Sat, 28 Feb 2015 17:43:04 +0000 (12:43 -0500)]
Fix planning of star-schema-style queries.

Part of the intent of the parameterized-path mechanism was to handle
star-schema queries efficiently, but some overly-restrictive search
limiting logic added in commit e2fa76d80ba571d4de8992de6386536867250474
prevented such cases from working as desired.  Fix that and add a
regression test about it.  Per gripe from Marc Cousin.

This is arguably a bug rather than a new feature, so back-patch to 9.2
where parameterized paths were introduced.

9 years agoImprove mmgr README.
Tom Lane [Sat, 28 Feb 2015 01:32:34 +0000 (20:32 -0500)]
Improve mmgr README.

Add documentation about the new reset callback mechanism.

Also, at long last, recast the existing text so that it describes the
current context mechanisms as established fact rather than something
we're going to implement.  Shoulda done that in 2001 or so ...

9 years agoSuppress uninitialized-variable warning from less-bright compilers.
Tom Lane [Fri, 27 Feb 2015 23:19:22 +0000 (18:19 -0500)]
Suppress uninitialized-variable warning from less-bright compilers.

The type variable must get set on first iteration of the while loop,
but there are reasonably modern gcc versions that don't realize that.
Initialize it with a dummy value.  This undoes a removal of initialization
in commit 654809e770ce270c0bb9de726c5df1ab193d60f0.

9 years agoRedefine MemoryContextReset() as deleting, not resetting, child contexts.
Tom Lane [Fri, 27 Feb 2015 23:09:42 +0000 (18:09 -0500)]
Redefine MemoryContextReset() as deleting, not resetting, child contexts.

That is, MemoryContextReset() now means what was formerly meant by
MemoryContextResetAndDeleteChildren(), and the latter is now just a macro
alias for the former.  If you really want the functionality that was
formerly provided by MemoryContextReset(), what you have to do is
MemoryContextResetChildren() plus MemoryContextResetOnly() (which is a
new API to reset *only* the named context and not touch its children).

The reason for this change is that near fifteen years of experience has
proven that there is noplace where old-style MemoryContextReset() is
actually what you want.  Making that the default behavior has led to lots
of context-leakage bugs, while we've not found anyplace where it's actually
necessary to keep the child contexts; at least the standard regression
tests do not reveal anyplace where this change breaks anything.  And there
are upcoming patches that will introduce additional reasons why child
contexts need to be removed.

We could change existing calls of MemoryContextResetAndDeleteChildren to be
just MemoryContextReset, but for the moment I'll leave them alone; they're
not costing anything.

9 years agoMake CREATE OR REPLACE VIEW internally more consistent
Alvaro Herrera [Fri, 27 Feb 2015 22:19:34 +0000 (19:19 -0300)]
Make CREATE OR REPLACE VIEW internally more consistent

The way that columns are added to a view is by calling
AlterTableInternal with special subtype AT_AddColumnToView; but that
subtype is changed to AT_AddColumnRecurse by ATPrepAddColumn.  This has
no visible effect in the current code, since views cannot have
inheritance children (thus the recursion step is a no-op) and adding a
column to a view is executed identically to doing it to a table; but it
does make a difference for future event trigger code keeping track of
commands, because the current situation leads to confusing the case with
a normal ALTER TABLE ADD COLUMN.

Fix the problem by passing a flag to ATPrepAddColumn to prevent it from
changing the command subtype.  The event trigger code can then properly
ignore the subcommand.  (We could remove the call to ATPrepAddColumn,
since views are never typed, and there is never a need for recursion,
which are the two conditions that are checked by ATPrepAddColumn; but it
seems more future-proof to keep the call in place.)

9 years agoInvent a memory context reset/delete callback mechanism.
Tom Lane [Fri, 27 Feb 2015 22:16:40 +0000 (17:16 -0500)]
Invent a memory context reset/delete callback mechanism.

This allows cleanup actions to be registered to be called just before a
particular memory context's contents are flushed (either by deletion or
MemoryContextReset).  The patch in itself has no use-cases for this, but
several likely reasons for wanting this exist.

In passing, per discussion, rearrange some boolean fields in struct
MemoryContextData so as to avoid wasted padding space.  For safety,
this requires making allowInCritSection's existence unconditional;
but I think that's a better approach than what was there anyway.

9 years agoFix a couple of trivial issues in jsonb.c
Alvaro Herrera [Fri, 27 Feb 2015 21:54:49 +0000 (18:54 -0300)]
Fix a couple of trivial issues in jsonb.c

Typo "aggreagate" appeared three times, and the return value of function
JsonbIteratorNext() was being assigned to an int variable in a bunch of
places.

9 years agoFix table_rewrite event trigger for ALTER TYPE/SET DATA TYPE CASCADE
Alvaro Herrera [Fri, 27 Feb 2015 21:38:33 +0000 (18:38 -0300)]
Fix table_rewrite event trigger for ALTER TYPE/SET DATA TYPE CASCADE

When a composite type being used in a typed table is modified by way
of ALTER TYPE, a table rewrite occurs appearing to come from ALTER TYPE.
The existing event_trigger.c code was unable to cope with that
and raised a spurious error.  The fix is just to accept that command
tag for the event, and document this properly.

Noted while fooling with deparsing of DDL commands.  This appears to be
an oversight in commit 618c9430a.

Thanks to Mark Wong for documentation wording help.

9 years agoRender infinite date/timestamps as 'infinity' for json/jsonb
Andrew Dunstan [Thu, 26 Feb 2015 17:25:21 +0000 (12:25 -0500)]
Render infinite date/timestamps as 'infinity' for json/jsonb

Commit ab14a73a6c raised an error in these cases and later the
behaviour was copied to jsonb. This is what the XML code, which we
then adopted, does, as the XSD types don't accept infinite values.
However, json dates and timestamps are just strings as far as json is
concerned, so there is no reason not to render these values as
'infinity'.

The json portion of this is backpatched to 9.4 where the behaviour was
introduced. The jsonb portion only affects the development branch.

Per gripe on pgsql-general.

9 years agoReconsider when to wait for WAL flushes/syncrep during commit.
Andres Freund [Thu, 26 Feb 2015 11:50:07 +0000 (12:50 +0100)]
Reconsider when to wait for WAL flushes/syncrep during commit.

Up to now RecordTransactionCommit() waited for WAL to be flushed (if
synchronous_commit != off) and to be synchronously replicated (if
enabled), even if a transaction did not have a xid assigned. The primary
reason for that is that sequence's nextval() did not assign a xid, but
are worthwhile to wait for on commit.

This can be problematic because sometimes read only transactions do
write WAL, e.g. HOT page prune records. That then could lead to read only
transactions having to wait during commit. Not something people expect
in a read only transaction.

This lead to such strange symptoms as backends being seemingly stuck
during connection establishment when all synchronous replicas are
down. Especially annoying when said stuck connection is the standby
trying to reconnect to allow syncrep again...

This behavior also is involved in a rather complicated <= 9.4 bug where
the transaction started by catchup interrupt processing waited for
syncrep using latches, but didn't get the wakeup because it was already
running inside the same overloaded signal handler. Fix the issue here
doesn't properly solve that issue, merely papers over the problems. In
9.5 catchup interrupts aren't processed out of signal handlers anymore.

To fix all this, make nextval() acquire a top level xid, and only wait for
transaction commit if a transaction both acquired a xid and emitted WAL
records.  If only a xid has been assigned we don't uselessly want to
wait just because of writes to temporary/unlogged tables; if only WAL
has been written we don't want to wait just because of HOT prunes.

The xid assignment in nextval() is unlikely to cause overhead in
real-world workloads. For one it only happens SEQ_LOG_VALS/32 values
anyway, for another only usage of nextval() without using the result in
an insert or similar is affected.

Discussion: 20150223165359.GF30784@awork2.anarazel.de,
    369698E947874884A77849D8FE3680C2@maumau,
    5CF4ABBA67674088B3941894E22A0D25@maumau

Per complaint from maumau and Thom Brown

Backpatch all the way back; 9.0 doesn't have syncrep, but it seems
better to be consistent behavior across all maintained branches.

9 years agoAdd note about how to make the SRF detoasted arguments live accross calls.
Fujii Masao [Thu, 26 Feb 2015 06:48:07 +0000 (15:48 +0900)]
Add note about how to make the SRF detoasted arguments live accross calls.

Andrew Gierth and Ali Akbar

9 years agoFree SQLSTATE and SQLERRM no earlier than other PL/pgSQL variables.
Noah Misch [Thu, 26 Feb 2015 04:48:28 +0000 (23:48 -0500)]
Free SQLSTATE and SQLERRM no earlier than other PL/pgSQL variables.

"RETURN SQLERRM" prompted plpgsql_exec_function() to read from freed
memory.  Back-patch to 9.0 (all supported versions).  Little code ran
between the premature free and the read, so non-assert builds are
unlikely to witness user-visible consequences.

9 years agoAdd hasRowSecurity to copyfuncs/outfuncs
Stephen Frost [Thu, 26 Feb 2015 04:32:18 +0000 (23:32 -0500)]
Add hasRowSecurity to copyfuncs/outfuncs

The RLS patch added a hasRowSecurity field to PlannerGlobal and
PlannedStmt but didn't update nodes/copyfuncs.c and nodes/outfuncs.c to
reflect those additional fields.

Correct that by adding entries to the appropriate functions for those
fields.

Pointed out by Robert.

9 years agoAdd locking clause for SB views for update/delete
Stephen Frost [Thu, 26 Feb 2015 02:36:29 +0000 (21:36 -0500)]
Add locking clause for SB views for update/delete

In expand_security_qual(), we were handling locking correctly when a
PlanRowMark existed, but not when we were working with the target
relation (which doesn't have any PlanRowMarks, but the subquery created
for the security barrier quals still needs to lock the rows under it).

Noted by Etsuro Fujita when working with the Postgres FDW, which wasn't
properly issuing a SELECT ... FOR UPDATE to the remote side under a
DELETE.

Back-patch to 9.4 where updatable security barrier views were
introduced.

Per discussion with Etsuro and Dean Rasheed.

9 years agoFix over-optimistic caching in fetch_array_arg_replace_nulls().
Tom Lane [Wed, 25 Feb 2015 19:19:13 +0000 (14:19 -0500)]
Fix over-optimistic caching in fetch_array_arg_replace_nulls().

When I rewrote this in commit 56a79a869bedc4bf6c35853642694cc0b0594dd2,
I forgot that it's possible for the input array type to change from one
call to the next (this can happen when applying the function to
pg_statistic columns, for instance).  Fix that.

9 years agoFix dumping of views that are just VALUES(...) but have column aliases.
Tom Lane [Wed, 25 Feb 2015 17:01:12 +0000 (12:01 -0500)]
Fix dumping of views that are just VALUES(...) but have column aliases.

The "simple" path for printing VALUES clauses doesn't work if we need
to attach nondefault column aliases, because there's noplace to do that
in the minimal VALUES() syntax.  So modify get_simple_values_rte() to
detect nondefault aliases and treat that as a non-simple case.  This
further exposes that the "non-simple" path never actually worked;
it didn't produce valid syntax.  Fix that too.  Per bug #12789 from
Curtis McEnroe, and analysis by Andrew Gierth.

Back-patch to all supported branches.  Before 9.3, this also requires
back-patching the part of commit 092d7ded29f36b0539046b23b81b9f0bf2d637f1
that created get_simple_values_rte() to begin with; inserting the extra
test into the old factorization of that logic would've been too messy.

9 years agoRemove null-pointer checks that are not needed.
Michael Meskes [Wed, 25 Feb 2015 10:46:15 +0000 (11:46 +0100)]
Remove null-pointer checks that are not needed.

If a pointer is guaranteed to carry information there is no need to check
for NULL again. Patch by Michael Paquier.

9 years agoImprove parser's one-extra-token lookahead mechanism.
Tom Lane [Tue, 24 Feb 2015 22:53:42 +0000 (17:53 -0500)]
Improve parser's one-extra-token lookahead mechanism.

There are a couple of places in our grammar that fail to be strict LALR(1),
by requiring more than a single token of lookahead to decide what to do.
Up to now we've dealt with that by using a filter between the lexer and
parser that merges adjacent tokens into one in the places where two tokens
of lookahead are necessary.  But that creates a number of user-visible
anomalies, for instance that you can't name a CTE "ordinality" because
"WITH ordinality AS ..." triggers folding of WITH and ORDINALITY into one
token.  I realized that there's a better way.

In this patch, we still do the lookahead basically as before, but we never
merge the second token into the first; we replace just the first token by
a special lookahead symbol when one of the lookahead pairs is seen.

This requires a couple extra productions in the grammar, but it involves
fewer special tokens, so that the grammar tables come out a bit smaller
than before.  The filter logic is no slower than before, perhaps a bit
faster.

I also fixed the filter logic so that when backing up after a lookahead,
the current token's terminator is correctly restored; this eliminates some
weird behavior in error message issuance, as is shown by the one change in
existing regression test outputs.

I believe that this patch entirely eliminates odd behaviors caused by
lookahead for WITH.  It doesn't really improve the situation for NULLS
followed by FIRST/LAST unfortunately: those sequences still act like a
reserved word, even though there are cases where they should be seen as two
ordinary identifiers, eg "SELECT nulls first FROM ...".  I experimented
with additional grammar hacks but couldn't find any simple solution for
that.  Still, this is better than before, and it seems much more likely
that we *could* somehow solve the NULLS case on the basis of this filter
behavior than the previous one.

9 years agoError when creating names too long for tar format
Peter Eisentraut [Tue, 24 Feb 2015 18:41:07 +0000 (13:41 -0500)]
Error when creating names too long for tar format

The tar format (at least the version we are using), does not support
file names or symlink targets longer than 99 bytes.  Until now, the tar
creation code would silently truncate any names that are too long.  (Its
original application was pg_dump, where this never happens.)  This
creates problems when running base backups over the replication
protocol.

The most important problem is when a tablespace path is longer than 99
bytes, which will result in a truncated tablespace path being backed up.
Less importantly, the basebackup protocol also promises to back up any
other files it happens to find in the data directory, which would also
lead to file name truncation if someone put a file with a long name in
there.

Now both of these cases result in an error during the backup.

Add tests that fail when a too-long file name or symlink is attempted to
be backed up.

Reviewed-by: Robert Hass <robertmhaas@gmail.com>
9 years agoFix recovery_command -> restore_command typo in 8.3 release notes.
Heikki Linnakangas [Tue, 24 Feb 2015 12:41:13 +0000 (14:41 +0200)]
Fix recovery_command -> restore_command typo in 8.3 release notes.

Kyotaro Horiguchi

9 years agoFix typo in README.
Heikki Linnakangas [Tue, 24 Feb 2015 12:32:56 +0000 (14:32 +0200)]
Fix typo in README.

Kyotaro Horiguchi

9 years agoFix invalid DocBook XML
Peter Eisentraut [Mon, 23 Feb 2015 21:57:54 +0000 (16:57 -0500)]
Fix invalid DocBook XML

9 years agoFix stupid merge errors in previous commit
Alvaro Herrera [Mon, 23 Feb 2015 18:05:37 +0000 (15:05 -0300)]
Fix stupid merge errors in previous commit

Brown paper bag installed permanently.

9 years agoFurther tweaking of raw grammar output to distinguish different inputs.
Tom Lane [Mon, 23 Feb 2015 17:46:46 +0000 (12:46 -0500)]
Further tweaking of raw grammar output to distinguish different inputs.

Use a different A_Expr_Kind for LIKE/ILIKE/SIMILAR TO constructs, so that
they can be distinguished from direct invocation of the underlying
operators.  Also, postpone selection of the operator name when transforming
"x IN (select)" to "x = ANY (select)", so that those syntaxes can be told
apart at parse analysis time.

I had originally thought I'd also have to do something special for the
syntaxes IS NOT DISTINCT FROM, IS NOT DOCUMENT, and x NOT IN (SELECT...),
which the grammar translates as though they were NOT (construct).
On reflection though, we can distinguish those cases reliably by noting
whether the parse location shown for the NOT is the same as for its child
node.  This only requires tweaking the parse locations for NOT IN, which
I've done here.

These changes should have no effect outside the parser; they're just in
support of being able to give accurate warnings for planned operator
precedence changes.

9 years agoSupport more commands in event triggers
Alvaro Herrera [Mon, 23 Feb 2015 17:22:42 +0000 (14:22 -0300)]
Support more commands in event triggers

COMMENT, SECURITY LABEL, and GRANT/REVOKE now also fire
ddl_command_start and ddl_command_end event triggers, when they operate
on database-local objects.

Reviewed-By: Michael Paquier, Andres Freund, Stephen Frost
9 years agoReplace checkpoint_segments with min_wal_size and max_wal_size.
Heikki Linnakangas [Mon, 23 Feb 2015 16:53:02 +0000 (18:53 +0200)]
Replace checkpoint_segments with min_wal_size and max_wal_size.

Instead of having a single knob (checkpoint_segments) that both triggers
checkpoints, and determines how many checkpoints to recycle, they are now
separate concerns. There is still an internal variable called
CheckpointSegments, which triggers checkpoints. But it no longer determines
how many segments to recycle at a checkpoint. That is now auto-tuned by
keeping a moving average of the distance between checkpoints (in bytes),
and trying to keep that many segments in reserve. The advantage of this is
that you can set max_wal_size very high, but the system won't actually
consume that much space if there isn't any need for it. The min_wal_size
sets a floor for that; you can effectively disable the auto-tuning behavior
by setting min_wal_size equal to max_wal_size.

The max_wal_size setting is now the actual target size of WAL at which a
new checkpoint is triggered, instead of the distance between checkpoints.
Previously, you could calculate the actual WAL usage with the formula
"(2 + checkpoint_completion_target) * checkpoint_segments + 1". With this
patch, you set the desired WAL usage with max_wal_size, and the system
calculates the appropriate CheckpointSegments with the reverse of that
formula. That's a lot more intuitive for administrators to set.

Reviewed by Amit Kapila and Venkata Balaji N.

9 years agoRenumber GUC_* constants.
Heikki Linnakangas [Mon, 23 Feb 2015 16:33:16 +0000 (18:33 +0200)]
Renumber GUC_* constants.

This moves all the regular flags back together (for aesthetic reasons), and
makes room for more GUC_UNIT_* types.

9 years agoRefactor unit conversions code in guc.c.
Heikki Linnakangas [Mon, 23 Feb 2015 16:06:16 +0000 (18:06 +0200)]
Refactor unit conversions code in guc.c.

Replace the if-switch-case constructs with two conversion tables,
containing all the supported conversions between human-readable unit
strings and the base units used in GUC variables. This makes the code
easier to read, and makes adding new units simpler.

9 years agoGuard against spurious signals in LockBufferForCleanup.
Andres Freund [Mon, 23 Feb 2015 15:11:11 +0000 (16:11 +0100)]
Guard against spurious signals in LockBufferForCleanup.

When LockBufferForCleanup() has to wait for getting a cleanup lock on a
buffer it does so by setting a flag in the buffer header and then wait
for other backends to signal it using ProcWaitForSignal().
Unfortunately LockBufferForCleanup() missed that ProcWaitForSignal() can
return for other reasons than the signal it is hoping for. If such a
spurious signal arrives the wait flags on the buffer header will still
be set. That then triggers "ERROR: multiple backends attempting to wait
for pincount 1".

The fix is simple, unset the flag if still set when retrying. That
implies an additional spinlock acquisition/release, but that's unlikely
to matter given the cost of waiting for a cleanup lock.  Alternatively
it'd have been possible to move responsibility for maintaining the
relevant flag to the waiter all together, but that might have had
negative consequences due to possible floods of signals. Besides being
more invasive.

This looks to be a very longstanding bug. The relevant code in
LockBufferForCleanup() hasn't changed materially since its introduction
and ProcWaitForSignal() was documented to return for unrelated reasons
since 8.2.  The master only patch series removing ImmediateInterruptOK
made it much easier to hit though, as ProcSendSignal/ProcWaitForSignal
now uses a latch shared with other tasks.

Per discussion with Kevin Grittner, Tom Lane and me.

Backpatch to all supported branches.

Discussion: 11553.1423805224@sss.pgh.pa.us

9 years agoAdd GUC to control the time to wait before retrieving WAL after failed attempt.
Fujii Masao [Mon, 23 Feb 2015 11:55:17 +0000 (20:55 +0900)]
Add GUC to control the time to wait before retrieving WAL after failed attempt.

Previously when the standby server failed to retrieve WAL files from any sources
(i.e., streaming replication, local pg_xlog directory or WAL archive), it always
waited for five seconds (hard-coded) before the next attempt. For example,
this is problematic in warm-standby because restore_command can fail
every five seconds even while new WAL file is expected to be unavailable for
a long time and flood the log files with its error messages.

This commit adds new parameter, wal_retrieve_retry_interval, to control that
wait time.

Alexey Vasiliev and Michael Paquier, reviewed by Andres Freund and me.

9 years agoFix potential deadlock with libpq non-blocking mode.
Heikki Linnakangas [Mon, 23 Feb 2015 11:32:34 +0000 (13:32 +0200)]
Fix potential deadlock with libpq non-blocking mode.

If libpq output buffer is full, pqSendSome() function tries to drain any
incoming data. This avoids deadlock, if the server e.g. sends a lot of
NOTICE messages, and blocks until we read them. However, pqSendSome() only
did that in blocking mode. In non-blocking mode, the deadlock could still
happen.

To fix, take a two-pronged approach:

1. Change the documentation to instruct that when PQflush() returns 1, you
should wait for both read- and write-ready, and call PQconsumeInput() if it
becomes read-ready. That fixes the deadlock, but applications are not going
to change overnight.

2. In pqSendSome(), drain the input buffer before returning 1. This
alleviates the problem for applications that only wait for write-ready. In
particular, a slow but steady stream of NOTICE messages during COPY FROM
STDIN will no longer cause a deadlock. The risk remains that the server
attempts to send a large burst of data and fills its output buffer, and at
the same time the client also sends enough data to fill its output buffer.
The application will deadlock if it goes to sleep, waiting for the socket
to become write-ready, before the server's data arrives. In practice,
NOTICE messages and such that the server might be sending are usually
short, so it's highly unlikely that the server would fill its output buffer
so quickly.

Backpatch to all supported versions.

9 years agoAdd parse location fields to NullTest and BooleanTest structs.
Tom Lane [Sun, 22 Feb 2015 19:40:27 +0000 (14:40 -0500)]
Add parse location fields to NullTest and BooleanTest structs.

We did not need a location tag on NullTest or BooleanTest before, because
no error messages referred directly to their locations.  That's planned
to change though, so add these fields in a separate housekeeping commit.

Catversion bump because stored rules may change.

9 years agoGet rid of multiple applications of transformExpr() to the same tree.
Tom Lane [Sun, 22 Feb 2015 18:59:09 +0000 (13:59 -0500)]
Get rid of multiple applications of transformExpr() to the same tree.

transformExpr() has for many years had provisions to do nothing when
applied to an already-transformed expression tree.  However, this was
always ugly and of dubious reliability, so we'd be much better off without
it.  The primary historical reason for it was that gram.y sometimes
returned multiple links to the same subexpression, which is no longer true
as of my BETWEEN fixes.  We'd also grown some lazy hacks in CREATE TABLE
LIKE (failing to distinguish between raw and already-transformed index
specifications) and one or two other places.

This patch removes the need for and support for re-transforming already
transformed expressions.  The index case is dealt with by adding a flag
to struct IndexStmt to indicate that it's already been transformed;
which has some benefit anyway in that tablecmds.c can now Assert that
transformation has happened rather than just assuming.  The other main
reason was some rather sloppy code for array type coercion, which can
be fixed (and its performance improved too) by refactoring.

I did leave transformJoinUsingClause() still constructing expressions
containing untransformed operator nodes being applied to Vars, so that
transformExpr() still has to allow Var inputs.  But that's a much narrower,
and safer, special case than before, since Vars will never appear in a raw
parse tree, and they don't have any substructure to worry about.

In passing fix some oversights in the patch that added CREATE INDEX
IF NOT EXISTS (missing processing of IndexStmt.if_not_exists).  These
appear relatively harmless, but still sloppy coding practice.

9 years agoRepresent BETWEEN as a special node type in raw parse trees.
Tom Lane [Sun, 22 Feb 2015 18:57:56 +0000 (13:57 -0500)]
Represent BETWEEN as a special node type in raw parse trees.

Previously, gram.y itself converted BETWEEN into AND (or AND/OR) nests of
expression comparisons.  This was always as bogus as could be, but fixing
it hasn't risen to the top of the to-do list.  The present patch invents an
A_Expr representation for BETWEEN expressions, and does the expansion to
comparison trees in parse_expr.c which is at least a slightly saner place
to be doing semantic conversions.  There should be no change in the post-
parse-analysis results.

This does nothing for the semantic issues with BETWEEN (dubious connection
to btree-opclass semantics, and multiple evaluation of possibly volatile
subexpressions) ... but it's a necessary preliminary step before we could
fix any of that.  The main immediate benefit is that preserving BETWEEN as
an identifiable raw-parse-tree construct will enable better error messages.

While at it, fix the code so that multiply-referenced subexpressions are
physically duplicated before being passed through transformExpr().  This
gets rid of one of the principal reasons why transformExpr() has
historically had to allow already-processed input.

9 years agoRename variable in AllocSetContextCreate to be consistent.
Jeff Davis [Sun, 22 Feb 2015 07:17:52 +0000 (23:17 -0800)]
Rename variable in AllocSetContextCreate to be consistent.

Everywhere else in the file, "context" is of type MemoryContext and
"set" is of type AllocSet. AllocSetContextCreate uses a variable of
type AllocSet, so rename it from "context" to "set".

9 years agoIn array_agg(), don't create a new context for every group.
Jeff Davis [Sun, 22 Feb 2015 01:24:48 +0000 (17:24 -0800)]
In array_agg(), don't create a new context for every group.

Previously, each new array created a new memory context that started
out at 8kB. This is incredibly wasteful when there are lots of small
groups of just a few elements each.

Change initArrayResult() and friends to accept a "subcontext" argument
to indicate whether the caller wants the ArrayBuildState allocated in
a new subcontext or not. If not, it can no longer be released
separately from the rest of the memory context.

Fixes bug report by Frank van Vugt on 2013-10-19.

Tomas Vondra. Reviewed by Ali Akbar, Tom Lane, and me.

9 years agoTry to fix busted gettimeofday() code.
Tom Lane [Sat, 21 Feb 2015 22:15:13 +0000 (17:15 -0500)]
Try to fix busted gettimeofday() code.

Per buildfarm, we have to match the _stdcall property of the system
functions.

9 years agoUse FLEXIBLE_ARRAY_MEMBER in Windows-specific code.
Tom Lane [Sat, 21 Feb 2015 21:49:28 +0000 (16:49 -0500)]
Use FLEXIBLE_ARRAY_MEMBER in Windows-specific code.

Be a tad more paranoid about overlength input, too.

9 years agoForce some system catalog table columns to be marked NOT NULL.
Andres Freund [Sat, 21 Feb 2015 21:37:05 +0000 (22:37 +0100)]
Force some system catalog table columns to be marked NOT NULL.

In a manual pass over the catalog declaration I found a number of
columns which the boostrap automatism didn't mark NOT NULL even though
they actually were. Add BKI_FORCE_NOT_NULL markings to them.

It's usually not critical if a system table column is falsely determined
to be nullable as the code should always catch relevant cases. But it's
good to have a extra layer in place.

Discussion: 20150215170014.GE15326@awork2.anarazel.de

9 years agoAllow forcing nullness of columns during bootstrap.
Andres Freund [Sat, 21 Feb 2015 21:25:49 +0000 (22:25 +0100)]
Allow forcing nullness of columns during bootstrap.

Bootstrap determines whether a column is null based on simple builtin
rules. Those work surprisingly well, but nonetheless a few existing
columns aren't set correctly. Additionally there is at least one patch
sent to hackers where forcing the nullness of a column would be helpful.

The boostrap format has gained FORCE [NOT] NULL for this, which will be
emitted by genbki.pl when BKI_FORCE_(NOT_)?NULL is specified for a
column in a catalog header.

This patch doesn't change the marking of any existing columns.

Discussion: 20150215170014.GE15326@awork2.anarazel.de

9 years agoDon't need to explain [1] kluge anymore in xfunc.sgml.
Tom Lane [Sat, 21 Feb 2015 21:29:45 +0000 (16:29 -0500)]
Don't need to explain [1] kluge anymore in xfunc.sgml.

9 years agoUse FLEXIBLE_ARRAY_MEMBER in a number of other places.
Tom Lane [Sat, 21 Feb 2015 21:12:14 +0000 (16:12 -0500)]
Use FLEXIBLE_ARRAY_MEMBER in a number of other places.

I think we're about done with this...

9 years agoUse FLEXIBLE_ARRAY_MEMBER for HeapTupleHeaderData.t_bits[].
Tom Lane [Sat, 21 Feb 2015 20:13:06 +0000 (15:13 -0500)]
Use FLEXIBLE_ARRAY_MEMBER for HeapTupleHeaderData.t_bits[].

This requires changing quite a few places that were depending on
sizeof(HeapTupleHeaderData), but it seems for the best.

Michael Paquier, some adjustments by me

9 years agoMinor code beautification in conninfo_uri_parse_params().
Tom Lane [Sat, 21 Feb 2015 18:27:12 +0000 (13:27 -0500)]
Minor code beautification in conninfo_uri_parse_params().

Reading this made me itch, so clean the logic a bit.

9 years agoFix misparsing of empty value in conninfo_uri_parse_params().
Tom Lane [Sat, 21 Feb 2015 17:59:25 +0000 (12:59 -0500)]
Fix misparsing of empty value in conninfo_uri_parse_params().

After finding an "=" character, the pointer was advanced twice when it
should only advance once.  This is harmless as long as the value after "="
has at least one character; but if it doesn't, we'd miss the terminator
character and include too much in the value.

In principle this could lead to reading off the end of memory.  It does not
seem worth treating as a security issue though, because it would happen on
client side, and besides client logic that's taking conninfo strings from
untrusted sources has much worse security problems than this.

Report and patch received off-list from Thomas Fanghaenel.
Back-patch to 9.2 where the faulty code was introduced.

9 years agoDon't require users of src/port/gettimeofday.c to initialize it.
Robert Haas [Sat, 21 Feb 2015 17:13:47 +0000 (12:13 -0500)]
Don't require users of src/port/gettimeofday.c to initialize it.

Commit 8001fe67a3d66c95861ce1f7075ef03953670d13 introduced this
requirement, but per discussion, we want to avoid requirements of
this type to make things easier on the calling code.  An especially
important consideration is that this may be used in frontend code,
not just the backend.

Asif Naeem, reviewed by Michael Paquier

9 years agoSome more FLEXIBLE_ARRAY_MEMBER fixes.
Tom Lane [Sat, 21 Feb 2015 06:46:43 +0000 (01:46 -0500)]
Some more FLEXIBLE_ARRAY_MEMBER fixes.

9 years agoFix statically allocated struct with FLEXIBLE_ARRAY_MEMBER member.
Tom Lane [Fri, 20 Feb 2015 22:50:18 +0000 (17:50 -0500)]
Fix statically allocated struct with FLEXIBLE_ARRAY_MEMBER member.

clang complains about this, not unreasonably, so define another struct
that's explicitly for a WordEntryPos with exactly one element.

While at it, get rid of pretty dubious use of a static variable for
more than one purpose --- if it were being treated as const maybe
I'd be okay with this, but it isn't.

9 years agoUse FLEXIBLE_ARRAY_MEMBER in some more places.
Tom Lane [Fri, 20 Feb 2015 22:32:01 +0000 (17:32 -0500)]
Use FLEXIBLE_ARRAY_MEMBER in some more places.

Fix a batch of structs that are only visible within individual .c files.

Michael Paquier

9 years agoUse FLEXIBLE_ARRAY_MEMBER in struct RecordIOData.
Tom Lane [Fri, 20 Feb 2015 22:03:12 +0000 (17:03 -0500)]
Use FLEXIBLE_ARRAY_MEMBER in struct RecordIOData.

I (tgl) fixed this last night in rowtypes.c, but I missed that the
code had been copied into a couple of other places.

Michael Paquier

9 years agoUse FLEXIBLE_ARRAY_MEMBER in struct varlena.
Tom Lane [Fri, 20 Feb 2015 21:51:53 +0000 (16:51 -0500)]
Use FLEXIBLE_ARRAY_MEMBER in struct varlena.

This forces some minor coding adjustments in tuptoaster.c and inv_api.c,
but the new coding there is cleaner anyway.

Michael Paquier

9 years agoRemove unnecessary and unreliable test
Alvaro Herrera [Fri, 20 Feb 2015 17:03:49 +0000 (14:03 -0300)]
Remove unnecessary and unreliable test

9 years agoUpdate PGSTAT_FILE_FORMAT_ID
Alvaro Herrera [Fri, 20 Feb 2015 15:59:27 +0000 (12:59 -0300)]
Update PGSTAT_FILE_FORMAT_ID

Previous commit should have bumped it but didn't.  Oops.

Per note from Tom.

9 years agoHave TRUNCATE update pgstat tuple counters
Alvaro Herrera [Fri, 20 Feb 2015 15:10:01 +0000 (12:10 -0300)]
Have TRUNCATE update pgstat tuple counters

This works by keeping a per-subtransaction record of the ins/upd/del
counters before the truncate, and then resetting them; this record is
useful to return to the previous state in case the truncate is rolled
back, either in a subtransaction or whole transaction.  The state is
propagated upwards as subtransactions commit.

When the per-table data is sent to the stats collector, a flag indicates
to reset the live/dead counters to zero as well.

Catalog version bumped due to the change in pgstat format.

Author: Alexander Shulgin
Discussion: 1007.1207238291@sss.pgh.pa.us
Discussion: 548F7D38.2000401@BlueTreble.com
Reviewed-by: Álvaro Herrera, Jim Nasby
9 years agoSome more FLEXIBLE_ARRAY_MEMBER hacking.
Tom Lane [Fri, 20 Feb 2015 07:28:03 +0000 (02:28 -0500)]
Some more FLEXIBLE_ARRAY_MEMBER hacking.

9 years agoRemove unused variable.
Tom Lane [Fri, 20 Feb 2015 05:47:28 +0000 (00:47 -0500)]
Remove unused variable.

Per buildfarm.

9 years agoUse "#ifdef CATALOG_VARLEN" to protect nullable fields of pg_authid.
Tom Lane [Fri, 20 Feb 2015 05:23:48 +0000 (00:23 -0500)]
Use "#ifdef CATALOG_VARLEN" to protect nullable fields of pg_authid.

This gives a stronger guarantee than a mere comment against accessing these
fields as simple struct members.  Since rolpassword is in fact varlena,
it's not clear why these didn't get marked from the beginning, but let's
do it now.

Michael Paquier

9 years agoUse FLEXIBLE_ARRAY_MEMBER in a bunch more places.
Tom Lane [Fri, 20 Feb 2015 05:11:42 +0000 (00:11 -0500)]
Use FLEXIBLE_ARRAY_MEMBER in a bunch more places.

Replace some bogus "x[1]" declarations with "x[FLEXIBLE_ARRAY_MEMBER]".
Aside from being more self-documenting, this should help prevent bogus
warnings from static code analyzers and perhaps compiler misoptimizations.

This patch is just a down payment on eliminating the whole problem, but
it gets rid of a lot of easy-to-fix cases.

Note that the main problem with doing this is that one must no longer rely
on computing sizeof(the containing struct), since the result would be
compiler-dependent.  Instead use offsetof(struct, lastfield).  Autoconf
also warns against spelling that offsetof(struct, lastfield[0]).

Michael Paquier, review and additional fixes by me.

9 years agoAdd pg_stat_get_snapshot_timestamp() to show statistics snapshot timestamp.
Tom Lane [Fri, 20 Feb 2015 02:36:50 +0000 (21:36 -0500)]
Add pg_stat_get_snapshot_timestamp() to show statistics snapshot timestamp.

Per discussion, this could be useful for purposes such as programmatically
detecting a nonresponding stats collector.  We already have the timestamp
anyway, it's just a matter of providing a SQL-accessible function to fetch
it.

Matt Kelly, reviewed by Jim Nasby

9 years agoRemove dead structs.
Heikki Linnakangas [Thu, 19 Feb 2015 19:13:15 +0000 (21:13 +0200)]
Remove dead structs.

These are not used with the new WAL format anymore. GIN split records are
simply always recorded as full-page images.

Michael Paquier

9 years agoUpdate assorted TOAST-related documentation.
Tom Lane [Thu, 19 Feb 2015 03:33:39 +0000 (22:33 -0500)]
Update assorted TOAST-related documentation.

While working on documentation for expanded arrays, I noticed a number of
details in the TOAST-related documentation that were already inaccurate or
obsolete.  This should be fixed independently of whether expanded arrays
get in or not.  One issue is that the already existing indirect-pointer
facility was not documented at all.  Also, the documentation says that you
only need to use VARSIZE/SET_VARSIZE if you've made your variable-length
type TOAST-aware, but actually we've forced that business on all varlena
types even if they've opted out of TOAST by setting storage = plain.
Wordsmith a few other things too, like an amusingly archaic claim that
there are few 64-bit machines.

I thought about back-patching this, but since all this doco is oriented
to hackers and C-coded extension authors, fixing it in HEAD is probably
good enough.

9 years agoSplit array_push into separate array_append and array_prepend functions.
Tom Lane [Thu, 19 Feb 2015 01:53:14 +0000 (20:53 -0500)]
Split array_push into separate array_append and array_prepend functions.

There wasn't any good reason for a single C function to implement both
these SQL functions: it saved very little code overall, and it required
significant pushups to re-determine at runtime which case applied.  Redoing
it as two functions ends up with just slightly more lines of code, but it's
simpler to understand, and faster too because we need not repeat syscache
lookups on every call.

An important side benefit is that this eliminates the only case in which
different aliases of the same C function had both anyarray and anyelement
arguments at the same position, which would almost always be a mistake.
The opr_sanity regression test will now notice such mistakes since there's
no longer a valid case where it happens.

9 years agoFix Perl coding error in msvc build system
Peter Eisentraut [Thu, 19 Feb 2015 01:24:30 +0000 (20:24 -0500)]
Fix Perl coding error in msvc build system

Code like

    open(P, "cl /? 2>&1 |") || die "cl command not found";

does not actually catch any errors, because the exit status of the
command before the pipe is ignored.  The fix is to look at $?.

This also gave the opportunity to clean up the logic of this code a bit.

9 years agoFix opclass/opfamily identity strings
Alvaro Herrera [Wed, 18 Feb 2015 17:44:27 +0000 (14:44 -0300)]
Fix opclass/opfamily identity strings

The original representation uses "opcname for amname", which is good
enough; but if we replace "for" with "using", we can apply the returned
identity directly in a DROP command, as in

DROP OPERATOR CLASS opcname USING amname

This slightly simplifies code using object identities to programatically
execute commands on these kinds of objects.

Note backwards-incompatible change:
The previous representation dates back to 9.3 when object identities
were introduced by commit f8348ea3, but we don't want to change the
behavior on released branches unnecessarily and so this is not
backpatched.

9 years agoFix object identities for pg_conversion objects
Alvaro Herrera [Wed, 18 Feb 2015 17:28:11 +0000 (14:28 -0300)]
Fix object identities for pg_conversion objects

We were neglecting to schema-qualify them.

Backpatch to 9.3, where object identities were introduced as a concept
by commit f8348ea32ec8.

9 years agoFix placement of "SET row_security" command issuance in pg_dump.
Tom Lane [Wed, 18 Feb 2015 17:23:40 +0000 (12:23 -0500)]
Fix placement of "SET row_security" command issuance in pg_dump.

Somebody apparently threw darts at the code to decide where to insert
these.  They certainly didn't proceed by adding them where other similar
SETs were handled.  This at least broke pg_restore, and perhaps other
use-cases too.

9 years agoFix failure to honor -Z compression level option in pg_dump -Fd.
Tom Lane [Wed, 18 Feb 2015 16:43:00 +0000 (11:43 -0500)]
Fix failure to honor -Z compression level option in pg_dump -Fd.

cfopen() and cfopen_write() failed to pass the compression level through
to zlib, so that you always got the default compression level if you got
any at all.

In passing, also fix these and related functions so that the correct errno
is reliably returned on failure; the original coding supposes that free()
cannot change errno, which is untrue on at least some platforms.

Per bug #12779 from Christoph Berg.  Back-patch to 9.1 where the faulty
code was introduced.

Michael Paquier

9 years agoFix EXPLAIN output for cases where parent table is excluded by constraints.
Tom Lane [Tue, 17 Feb 2015 23:04:11 +0000 (18:04 -0500)]
Fix EXPLAIN output for cases where parent table is excluded by constraints.

The previous coding in EXPLAIN always labeled a ModifyTable node with the
name of the target table affected by its first child plan.  When originally
written, this was necessarily the parent table of the inheritance tree,
so everything was unconfusing.  But when we added NO INHERIT constraints,
it became possible for the parent table to be deleted from the plan by
constraint exclusion while still leaving child tables present.  This led to
the ModifyTable plan node being labeled with the first surviving child,
which was deemed confusing.  Fix it by retaining the parent table's RT
index in a new field in ModifyTable.

Etsuro Fujita, reviewed by Ashutosh Bapat and myself

9 years agoFix a bug in pairing heap removal code.
Heikki Linnakangas [Tue, 17 Feb 2015 20:55:53 +0000 (22:55 +0200)]
Fix a bug in pairing heap removal code.

After removal, the next_sibling pointer of a node was sometimes incorrectly
left to point to another node in the heap, which meant that a node was
sometimes linked twice into the heap. Surprisingly that didn't cause any
crashes in my testing, but it was clearly wrong and could easily segfault
in other scenarios.

Also always keep the prev_or_parent pointer as NULL on the root node. That
was not a correctness issue AFAICS, but let's be tidy.

Add a debugging function, to dump the contents of a pairing heap as a
string. It's #ifdef'd out, as it's not used for anything in any normal
code, but it was highly useful in debugging this. Let's keep it handy for
further reference.

9 years agoFix knn-GiST queue comparison function to return heap tuples first.
Heikki Linnakangas [Tue, 17 Feb 2015 20:33:38 +0000 (22:33 +0200)]
Fix knn-GiST queue comparison function to return heap tuples first.

The part of the comparison function that was supposed to keep heap tuples
ahead of index items was backwards. It would not lead to incorrect results,
but it is more efficient to return heap tuples first, before scanning more
index pages, when both have the same distance.

Alexander Korotkov

9 years agoRemove code to match IPv4 pg_hba.conf entries to IPv4-in-IPv6 addresses.
Tom Lane [Tue, 17 Feb 2015 17:49:18 +0000 (12:49 -0500)]
Remove code to match IPv4 pg_hba.conf entries to IPv4-in-IPv6 addresses.

In investigating yesterday's crash report from Hugo Osvaldo Barrera, I only
looked back as far as commit f3aec2c7f51904e7 where the breakage occurred
(which is why I thought the IPv4-in-IPv6 business was undocumented).  But
actually the logic dates back to commit 3c9bb8886df7d56a and was simply
broken by erroneous refactoring in the later commit.  A bit of archives
excavation shows that we added the whole business in response to a report
that some 2003-era Linux kernels would report IPv4 connections as having
IPv4-in-IPv6 addresses.  The fact that we've had no complaints since 9.0
seems to be sufficient confirmation that no modern kernels do that, so
let's just rip it all out rather than trying to fix it.

Do this in the back branches too, thus essentially deciding that our
effective behavior since 9.0 is correct.  If there are any platforms on
which the kernel reports IPv4-in-IPv6 addresses as such, yesterday's fix
would have made for a subtle and potentially security-sensitive change in
the effective meaning of IPv4 pg_hba.conf entries, which does not seem like
a good thing to do in minor releases.  So let's let the post-9.0 behavior
stand, and change the documentation to match it.

In passing, I failed to resist the temptation to wordsmith the description
of pg_hba.conf IPv4 and IPv6 address entries a bit.  A lot of this text
hasn't been touched since we were IPv4-only.

9 years agoImprove pg_check_dir code and comments.
Robert Haas [Tue, 17 Feb 2015 15:19:30 +0000 (10:19 -0500)]
Improve pg_check_dir code and comments.

Avoid losing errno if readdir() fails and closedir() works.  Consistently
return 4 rather than 3 if both a lost+found directory and other files are
found, rather than returning one value or the other depending on the
order of the directory listing.  Update comments to match the actual
behavior.

These oversights date to commits 6f03927fce038096f53ca67eeab9adb24938f8a6
and 17f15239325a88581bb4f9cf91d38005f1f52d69.

Marco Nenciarini

9 years agoEliminate unnecessary NULL checks in picksplit method of intarray.
Kevin Grittner [Mon, 16 Feb 2015 21:26:23 +0000 (15:26 -0600)]
Eliminate unnecessary NULL checks in picksplit method of intarray.

Where these checks were being done there was no code path which
could leave them NULL.

Michael Paquier per Coverity

9 years agoFix misuse of memcpy() in check_ip().
Tom Lane [Mon, 16 Feb 2015 21:17:48 +0000 (16:17 -0500)]
Fix misuse of memcpy() in check_ip().

The previous coding copied garbage into a local variable, pretty much
ensuring that the intended test of an IPv6 connection address against a
promoted IPv4 address from pg_hba.conf would never match.  The lack of
field complaints likely indicates that nobody realized this was supposed
to work, which is unsurprising considering that no user-facing docs suggest
it should work.

In principle this could have led to a SIGSEGV due to reading off the end of
memory, but since the source address would have pointed to somewhere in the
function's stack frame, that's quite unlikely.  What led to discovery of
the bug is Hugo Osvaldo Barrera's report of a crash after an OS upgrade,
which is probably because he is now running a system in which memcpy raises
abort() upon detecting overlapping source and destination areas.  (You'd
have to additionally suppose some things about the stack frame layout to
arrive at this conclusion, but it seems plausible.)

This has been broken since the code was added, in commit f3aec2c7f51904e7,
so back-patch to all supported branches.

9 years agoFix comment in libpq OpenSSL code about why a substitue BIO is used.
Heikki Linnakangas [Mon, 16 Feb 2015 21:05:20 +0000 (23:05 +0200)]
Fix comment in libpq OpenSSL code about why a substitue BIO is used.

The comment was copy-pasted from the backend code along with the
implementation, but libpq has different reasons for using the BIO.

9 years agoRestore the SSL_set_session_id_context() call to OpenSSL renegotiation.
Heikki Linnakangas [Mon, 16 Feb 2015 20:34:32 +0000 (22:34 +0200)]
Restore the SSL_set_session_id_context() call to OpenSSL renegotiation.

This reverts the removal of the call in commit (272923a0). It turns out it
wasn't superfluous after all: without it, renegotiation fails if a client
certificate was used. The rest of the changes in that commit are still OK
and not reverted.

Per investigation of bug #12769 by Arne Scheffer, although this doesn't fix
the reported bug yet.

9 years agoUse fast path in plpgsql's RETURN/RETURN NEXT in more cases.
Tom Lane [Mon, 16 Feb 2015 20:28:40 +0000 (15:28 -0500)]
Use fast path in plpgsql's RETURN/RETURN NEXT in more cases.

exec_stmt_return() and exec_stmt_return_next() have fast-path code for
handling a simple variable reference (i.e. "return var") without going
through the full expression evaluation machinery.  For some reason,
pl_gram.y was under the impression that this fast path only applied for
record/row variables; but in reality code for handling regular scalar
variables has been there all along.  Adjusting the logic to allow that
code to be used actually results in a net savings of code in pl_gram.y
(by eliminating some redundancy), and it buys a measurable though not
very impressive amount of speedup.

Noted while fooling with my expanded-array patch, wherein this makes a much
bigger difference because it enables returning an expanded array variable
without an extra flattening step.  But AFAICS this is a win regardless,
so commit it separately.

9 years agoIn the SSL test suite, use a root CA cert that won't expire (so quickly)
Heikki Linnakangas [Mon, 16 Feb 2015 20:08:41 +0000 (22:08 +0200)]
In the SSL test suite, use a root CA cert that won't expire (so quickly)

All the other certificates were created to be valid for 10000 days, because
we don't want to have to recreate them. But I missed the root CA cert, and
the pre-created certificates included in the repository expired in January.
Fix, and re-create all the certificates.

9 years agoRationalize the APIs of array element/slice access functions.
Tom Lane [Mon, 16 Feb 2015 17:23:58 +0000 (12:23 -0500)]
Rationalize the APIs of array element/slice access functions.

The four functions array_ref, array_set, array_get_slice, array_set_slice
have traditionally declared their array inputs and results as being of type
"ArrayType *".  This is a lie, and has been since Berkeley days, because
they actually also support "fixed-length array" types such as "name" and
"point"; not to mention that the inputs could be toasted.  These values
should be declared Datum instead to avoid confusion.  The current coding
already risks possible misoptimization by compilers, and it'll get worse
when "expanded" array representations become a valid alternative.

However, there's a fair amount of code using array_ref and array_set with
arrays that *are* known to be ArrayType structures, and there might be more
such places in third-party code.  Rather than cluttering those call sites
with PointerGetDatum/DatumGetArrayTypeP cruft, what I did was to rename the
existing functions to array_get_element/array_set_element, fix their
signatures, then reincarnate array_ref/array_set as backwards compatibility
wrappers.

array_get_slice/array_set_slice have no such constituency in the core code,
and probably not in third-party code either, so I just changed their APIs.

9 years agoCorrect the path of pg_lzcompress.c in doc.
Fujii Masao [Mon, 16 Feb 2015 05:50:13 +0000 (14:50 +0900)]
Correct the path of pg_lzcompress.c in doc.

Commit 40bede5 moved pg_lzcompress.c to src/common, but forgot to
update its path in doc. This commit fixes that oversight.

9 years agoFix null-pointer-deref crash while doing COPY IN with check constraints.
Tom Lane [Mon, 16 Feb 2015 04:26:45 +0000 (23:26 -0500)]
Fix null-pointer-deref crash while doing COPY IN with check constraints.

In commit bf7ca15875988a88e97302e012d7c4808bef3ea9 I introduced an
assumption that an RTE referenced by a whole-row Var must have a valid eref
field.  This is false for RTEs constructed by DoCopy, and there are other
places taking similar shortcuts.  Perhaps we should make all those places
go through addRangeTableEntryForRelation or its siblings instead of having
ad-hoc logic, but the most reliable fix seems to be to make the new code in
ExecEvalWholeRowVar cope if there's no eref.  We can reasonably assume that
there's no need to insert column aliases if no aliases were provided.

Add a regression test case covering this, and also verifying that a sane
column name is in fact available in this situation.

Although the known case only crashes in 9.4 and HEAD, it seems prudent to
back-patch the code change to 9.2, since all the ingredients for a similar
failure exist in the variant patch applied to 9.3 and 9.2.

Per report from Jean-Pierre Pelletier.

9 years agoMinor housekeeping on JSON docs.
Andrew Dunstan [Sun, 15 Feb 2015 22:08:37 +0000 (17:08 -0500)]
Minor housekeeping on JSON docs.

9 years agopg_regress: Write processed input/*.source into output dir
Peter Eisentraut [Sun, 15 Feb 2015 02:33:41 +0000 (21:33 -0500)]
pg_regress: Write processed input/*.source into output dir

Before, it was writing the processed files into the input directory,
which is incorrect in a vpath build.

9 years agoAvoid returning undefined bytes in chkpass_in().
Tom Lane [Sat, 14 Feb 2015 17:20:56 +0000 (12:20 -0500)]
Avoid returning undefined bytes in chkpass_in().

We can't really fix the problem that the result is defined to depend on
random(), so it is still going to fail the "unstable input conversion"
test in parse_type.c.  However, we can at least satify valgrind.  (It
looks like this code used to be valgrind-clean, actually, until somebody
did a careless s/strncpy/strlcpy/g on it.)

In passing, let's just make real sure that chkpass_out doesn't overrun
its output buffer.

No need for backpatch, I think, since this is just to satisfy debugging
tools.

Asif Naeem

9 years agoFix broken #ifdef for __sparcv8
Heikki Linnakangas [Fri, 13 Feb 2015 21:51:23 +0000 (23:51 +0200)]
Fix broken #ifdef for __sparcv8

Rob Rowan. Backpatch to all supported versions, like the patch that added
the broken #ifdef.

9 years agoSimplify waiting logic in reading from / writing to client.
Heikki Linnakangas [Fri, 13 Feb 2015 19:46:14 +0000 (21:46 +0200)]
Simplify waiting logic in reading from / writing to client.

The client socket is always in non-blocking mode, and if we actually want
blocking behaviour, we emulate it by sleeping and retrying. But we have
retry loops at different layers for reads and writes, which was confusing.
To simplify, remove all the sleeping and retrying code from the lower
levels, from be_tls_read and secure_raw_read and secure_raw_write, and put
all the logic in secure_read() and secure_write().

9 years agoSimplify the way OpenSSL renegotiation is initiated in server.
Heikki Linnakangas [Fri, 13 Feb 2015 19:46:08 +0000 (21:46 +0200)]
Simplify the way OpenSSL renegotiation is initiated in server.

At least in all modern versions of OpenSSL, it is enough to call
SSL_renegotiate() once, and then forget about it. Subsequent SSL_write()
and SSL_read() calls will finish the handshake.

The SSL_set_session_id_context() call is unnecessary too. We only have
one SSL context, and the SSL session was created with that to begin with.

9 years agopg_upgrade: improve checksum mismatch error message
Bruce Momjian [Thu, 12 Feb 2015 03:22:26 +0000 (22:22 -0500)]
pg_upgrade:  improve checksum mismatch error message

Patch by Greg Sabino Mullane, slight adjustments by me

9 years agopg_upgrade: quote directory names in delete_old_cluster script
Bruce Momjian [Thu, 12 Feb 2015 03:06:04 +0000 (22:06 -0500)]
pg_upgrade:  quote directory names in delete_old_cluster script

This allows the delete script to properly function when special
characters appear in directory paths, e.g. spaces.

Backpatch through 9.0

9 years agopg_upgrade: preserve freeze info for postgres/template1 dbs
Bruce Momjian [Thu, 12 Feb 2015 02:02:07 +0000 (21:02 -0500)]
pg_upgrade:  preserve freeze info for postgres/template1 dbs

pg_database.datfrozenxid and pg_database.datminmxid were not preserved
for the 'postgres' and 'template1' databases.  This could cause missing
clog file errors on access to user tables and indexes after upgrades in
these databases.

Backpatch through 9.0

9 years agoFix typo in logicaldecoding.sgml.
Andres Freund [Thu, 12 Feb 2015 00:16:32 +0000 (01:16 +0100)]
Fix typo in logicaldecoding.sgml.

Author: Tatsuo Ishii

Backpatch to 9.4, where logicaldecoding was introduced.

9 years agoFix missing PQclear() in libpqrcv_endstreaming().
Tom Lane [Thu, 12 Feb 2015 00:20:49 +0000 (19:20 -0500)]
Fix missing PQclear() in libpqrcv_endstreaming().

This omission leaked one PGresult per WAL streaming cycle, which possibly
would never be enough to notice in the real world, but it's still a leak.

Per Coverity.  Back-patch to 9.3 where the error was introduced.

9 years agoFix minor memory leak in ident_inet().
Tom Lane [Thu, 12 Feb 2015 00:09:54 +0000 (19:09 -0500)]
Fix minor memory leak in ident_inet().

We'd leak the ident_serv data structure if the second pg_getaddrinfo_all
(the one for the local address) failed.  This is not of great consequence
because a failure return here just leads directly to backend exit(), but
if this function is going to try to clean up after itself at all, it should
not have such holes in the logic.  Try to fix it in a future-proof way by
having all the failure exits go through the same cleanup path, rather than
"optimizing" some of them.

Per Coverity.  Back-patch to 9.2, which is as far back as this patch
applies cleanly.

9 years agoFix more memory leaks in failure path in buildACLCommands.
Tom Lane [Wed, 11 Feb 2015 23:35:23 +0000 (18:35 -0500)]
Fix more memory leaks in failure path in buildACLCommands.

We already had one go at this issue in commit d73b7f973db5ec7e, but we
failed to notice that buildACLCommands also leaked several PQExpBuffers
along with a simply malloc'd string.  This time let's try to make the
fix a bit more future-proof by eliminating the separate exit path.

It's still not exactly critical because pg_dump will curl up and die on
failure; but since the amount of the potential leak is now several KB,
it seems worth back-patching as far as 9.2 where the previous fix landed.

Per Coverity, which evidently is smarter than clang's static analyzer.