]> granicus.if.org Git - postgresql/log
postgresql
9 years agoFix old bug in get_loop_count().
Tom Lane [Thu, 12 Mar 2015 02:53:32 +0000 (22:53 -0400)]
Fix old bug in get_loop_count().

While poking at David Kubečka's issue I noticed an ancient logic error
in get_loop_count(): it used 1.0 as a "no data yet" indicator, but since
that is actually a valid rowcount estimate, this doesn't work.  If we
have one input relation with 1.0 as rowcount and then another one with
a larger rowcount, we should use 1.0 as the result, but we picked the
larger rowcount instead.  (I think when I coded this, I recognized the
conflict, but mistakenly thought that the logic would pick the desired
count anyway.)

Fixing this changed the plan for one existing regression test case.
Since the point of that test is to exercise creation of a particular
shape of nestloop plan, I tweaked the query a little bit so it still
results in the same plan choice.

This is definitely a bug, but I'm hesitant to back-patch since it might
change plan choices unexpectedly, and anyway failure to implement a
heuristic precisely as intended is a pretty low-grade bug.

9 years agoImprove planner's cost estimation in the presence of semijoins.
Tom Lane [Thu, 12 Mar 2015 01:21:00 +0000 (21:21 -0400)]
Improve planner's cost estimation in the presence of semijoins.

If we have a semijoin, say
SELECT * FROM x WHERE x1 IN (SELECT y1 FROM y)
and we're estimating the cost of a parameterized indexscan on x, the number
of repetitions of the indexscan should not be taken as the size of y; it'll
really only be the number of distinct values of y1, because the only valid
plan with y on the outside of a nestloop would require y to be unique-ified
before joining it to x.  Most of the time this doesn't make that much
difference, but sometimes it can lead to drastically underestimating the
cost of the indexscan and hence choosing a bad plan, as pointed out by
David Kubečka.

Fixing this is a bit difficult because parameterized indexscans are costed
out quite early in the planning process, before we have the information
that would be needed to call estimate_num_groups() and thereby estimate the
number of distinct values of the join column(s).  However we can move the
code that extracts a semijoin RHS's unique-ification columns, so that it's
done in initsplan.c rather than on-the-fly in create_unique_path().  That
shouldn't make any difference speed-wise and it's really a bit cleaner too.

The other bit of information we need is the size of the semijoin RHS,
which is easy if it's a single relation (we make those estimates before
considering indexscan costs) but problematic if it's a join relation.
The solution adopted here is just to use the product of the sizes of the
join component rels.  That will generally be an overestimate, but since
estimate_num_groups() only uses this input as a clamp, an overestimate
shouldn't hurt us too badly.  In any case we don't allow this new logic
to produce a value larger than we would have chosen before, so that at
worst an overestimate leaves us no wiser than we were before.

9 years agoPL/Python: Fix regression tests for Python 3
Peter Eisentraut [Wed, 11 Mar 2015 22:30:34 +0000 (18:30 -0400)]
PL/Python: Fix regression tests for Python 3

9 years agoSupport default ACLs in get_object_address
Alvaro Herrera [Wed, 11 Mar 2015 22:23:47 +0000 (19:23 -0300)]
Support default ACLs in get_object_address

In the spirit of 890192e99af, this time add support for the things
living in the pg_default_acl catalog.  These are not really "objects",
but they show up as such in event triggers.

There is no "DROP DEFAULT PRIVILEGES" or similar command, so it doesn't
look like the new representation given would be useful anywhere else, so
I didn't try to use it outside objectaddress.c.  (That might be a bug in
itself, but that would be material for another commit.)

Reviewed by Stephen Frost.

9 years agoFix libpq test expected output file
Alvaro Herrera [Wed, 11 Mar 2015 20:01:30 +0000 (17:01 -0300)]
Fix libpq test expected output file

Evidently, this test is not run very frequently ...

9 years agoSupport user mappings in get_object_address
Alvaro Herrera [Wed, 11 Mar 2015 20:01:13 +0000 (17:01 -0300)]
Support user mappings in get_object_address

Since commit 72dd233d3ef we were trying to obtain object addressing
information in sql_drop event triggers, but that caused failures when
the drops involved user mappings.  This addition enables that to work
again.  Naturally, pg_get_object_address can work with these objects
now, too.

I toyed with the idea of removing DropUserMappingStmt as a node and
using DropStmt instead in the DropUserMappingStmt grammar production,
but that didn't go very well: for one thing the messages thrown by the
specific code are specialized (you get "server not found" if you specify
the wrong server, instead of a generic "user mapping for ... not found"
which you'd get it we were to merge this with RemoveObjects --- unless
we added even more special cases).  For another thing, it would require
to pass RoleSpec nodes through the objname/objargs representation used
by RemoveObjects, which works in isolation, but gets messy when
pg_get_object_address is involved.  So I dropped this part for now.

Reviewed by Stephen Frost.

9 years agoPL/Python: Avoid lossiness in float conversion
Peter Eisentraut [Wed, 11 Mar 2015 19:44:17 +0000 (15:44 -0400)]
PL/Python: Avoid lossiness in float conversion

PL/Python uses str() to convert Python values back to PostgreSQL, but
str() is lossy for float values, so use repr() instead in that case.

Author: Marko Kreen <markokr@gmail.com>

9 years agoRequire non-NULL pstate for all addRangeTableEntryFor* functions.
Robert Haas [Wed, 11 Mar 2015 19:26:43 +0000 (15:26 -0400)]
Require non-NULL pstate for all addRangeTableEntryFor* functions.

Per discussion, it's better to have a consistent coding rule here.

Michael Paquier, per a node from Greg Stark referencing an old post
from Tom Lane.

9 years agoMake operator precedence follow the SQL standard more closely.
Tom Lane [Wed, 11 Mar 2015 17:22:52 +0000 (13:22 -0400)]
Make operator precedence follow the SQL standard more closely.

While the SQL standard is pretty vague on the overall topic of operator
precedence (because it never presents a unified BNF for all expressions),
it does seem reasonable to conclude from the spec for <boolean value
expression> that OR has the lowest precedence, then AND, then NOT, then IS
tests, then the six standard comparison operators, then everything else
(since any non-boolean operator in a WHERE clause would need to be an
argument of one of these).

We were only sort of on board with that: most notably, while "<" ">" and
"=" had properly low precedence, "<=" ">=" and "<>" were treated as generic
operators and so had significantly higher precedence.  And "IS" tests were
even higher precedence than those, which is very clearly wrong per spec.

Another problem was that "foo NOT SOMETHING bar" constructs, such as
"x NOT LIKE y", were treated inconsistently because of a bison
implementation artifact: they had the documented precedence with respect
to operators to their right, but behaved like NOT (i.e., very low priority)
with respect to operators to their left.

Fixing the precedence issues is just a small matter of rearranging the
precedence declarations in gram.y, except for the NOT problem, which
requires adding an additional lookahead case in base_yylex() so that we
can attach a different token precedence to NOT LIKE and allied two-word
operators.

The bulk of this patch is not the bug fix per se, but adding logic to
parse_expr.c to allow giving warnings if an expression has changed meaning
because of these precedence changes.  These warnings are off by default
and are enabled by the new GUC operator_precedence_warning.  It's believed
that very few applications will be affected by these changes, but it was
agreed that a warning mechanism is essential to help debug any that are.

9 years agoAllocate ParamListInfo once per plpgsql function, not once per expression.
Tom Lane [Wed, 11 Mar 2015 16:40:43 +0000 (12:40 -0400)]
Allocate ParamListInfo once per plpgsql function, not once per expression.

setup_param_list() was allocating a fresh ParamListInfo for each query or
expression evaluation requested by a plpgsql function.  There was probably
once good reason to do it like that, but for a long time we've had a
convention that there's a one-to-one mapping between the function's
PLpgSQL_datum array and the ParamListInfo slots, which means that a single
ParamListInfo can serve all the function's evaluation requests: the data
that would need to be passed is the same anyway.

In this patch, we retain the pattern of zeroing out the ParamListInfo
contents during each setup_param_list() call, because some of the slots may
be stale and we don't know exactly which ones.  So this patch only saves a
palloc/pfree per evaluation cycle and nothing more; still, that seems to be
good for a couple percent overall speedup on simple-arithmetic type
statements.  In future, though, we might be able to improve matters still
more by managing the param array contents more carefully.

Also, unify the former use of estate->cur_expr with that of
paramLI->parserSetupArg; they both were used to point to the active
expression, so we can combine the variables into just one.

9 years agosepgsql: Improve error message when unsupported object type is labeled.
Robert Haas [Wed, 11 Mar 2015 16:12:10 +0000 (12:12 -0400)]
sepgsql: Improve error message when unsupported object type is labeled.

KaiGai Kohei, reviewed by Álvaro Herrera and myself

9 years agoSuggest to the user the column they may have meant to reference.
Robert Haas [Wed, 11 Mar 2015 14:44:04 +0000 (10:44 -0400)]
Suggest to the user the column they may have meant to reference.

Error messages informing the user that no such column exists can
sometimes provoke a perplexed response.  This often happens due to
a subtle typo in the column name or, perhaps less likely, in the
alias name.  To speed discovery of what the real issue is in such
cases, we'll now search the range table for approximate matches.
If there are one or two such matches that are good enough to think
that they might be what the user intended to type, and better than
all other approximate matches, we'll issue a hint suggesting that
the user might have intended to reference those columns.

Peter Geoghegan and Robert Haas

9 years agoAdd macros wrapping all usage of gcc's __attribute__.
Andres Freund [Wed, 11 Mar 2015 13:19:54 +0000 (14:19 +0100)]
Add macros wrapping all usage of gcc's __attribute__.

Until now __attribute__() was defined to be empty for all compilers but
gcc. That's problematic because it prevents using it in other compilers;
which is necessary e.g. for atomics portability.  It's also just
generally dubious to do so in a header as widely included as c.h.

Instead add pg_attribute_format_arg, pg_attribute_printf,
pg_attribute_noreturn macros which are implemented in the compilers that
understand them. Also add pg_attribute_noreturn and pg_attribute_packed,
but don't provide fallbacks, since they can affect functionality.

This means that external code that, possibly unwittingly, relied on
__attribute__ defined to be empty on !gcc compilers may now run into
warnings or errors on those compilers. But there shouldn't be many
occurances of that and it's hard to work around...

Discussion: 54B58BA3.8040302@ohmu.fi
Author: Oskari Saarenmaa, with some minor changes by me.

9 years agoRefactor Mkvcbuild.pm to facilitate modules migrations
Alvaro Herrera [Wed, 11 Mar 2015 13:21:01 +0000 (10:21 -0300)]
Refactor Mkvcbuild.pm to facilitate modules migrations

This is in preparation to "upgrade" some modules from contrib/ to
src/bin/, per discussion.

Author: Michael Paquier

9 years agoAdd GUC to enable compression of full page images stored in WAL.
Fujii Masao [Wed, 11 Mar 2015 06:52:24 +0000 (15:52 +0900)]
Add GUC to enable compression of full page images stored in WAL.

When newly-added GUC parameter, wal_compression, is on, the PostgreSQL server
compresses a full page image written to WAL when full_page_writes is on or
during a base backup. A compressed page image will be decompressed during WAL
replay. Turning this parameter on can reduce the WAL volume without increasing
the risk of unrecoverable data corruption, but at the cost of some extra CPU
spent on the compression during WAL logging and on the decompression during
WAL replay.

This commit changes the WAL format (so bumping WAL version number) so that
the one-byte flag indicating whether a full page image is compressed or not is
included in its header information. This means that the commit increases the
WAL volume one-byte per a full page image even if WAL compression is not used
at all. We can save that one-byte by borrowing one-bit from the existing field
like hole_offset in the header and using it as the flag, for example. But which
would reduce the code readability and the extensibility of the feature.
Per discussion, it's not worth paying those prices to save only one-byte, so we
decided to add the one-byte flag to the header.

This commit doesn't introduce any new compression algorithm like lz4.
Currently a full page image is compressed using the existing PGLZ algorithm.
Per discussion, we decided to use it at least in the first version of the
feature because there were no performance reports showing that its compression
ratio is unacceptably lower than that of other algorithm. Of course,
in the future, it's worth considering the support of other compression
algorithm for the better compression.

Rahila Syed and Michael Paquier, reviewed in various versions by myself,
Andres Freund, Robert Haas, Abhijit Menon-Sen and many others.

9 years agoClean up the mess from => patch.
Tom Lane [Tue, 10 Mar 2015 15:48:34 +0000 (11:48 -0400)]
Clean up the mess from => patch.

Commit 865f14a2d31af23a05bbf2df04c274629c5d5c4d was quite a few bricks
shy of a load: psql, ecpg, and plpgsql were all left out-of-step with
the core lexer.  Of these only the last was likely to be a fatal
problem; but still, a minimal amount of grepping, or even just reading
the comments adjacent to the places that were changed, would have found
the other places that needed to be changed.

9 years agoFix stray sentence fragment in shared_preload_libraries documentation
Alvaro Herrera [Tue, 10 Mar 2015 15:36:17 +0000 (12:36 -0300)]
Fix stray sentence fragment in shared_preload_libraries documentation

The introduction in the Shared Library Preloading section already
instructs the user to separate multiple library names with commas, so
just remove the fragment from here.

Author: Dagfinn Ilmari Mannsåker

9 years agoMove BRIN page type to page's last two bytes
Alvaro Herrera [Tue, 10 Mar 2015 15:26:34 +0000 (12:26 -0300)]
Move BRIN page type to page's last two bytes

... which is the usual convention among AMs, so that pg_filedump and
similar utilities can tell apart pages of different AMs.  It was also
the intent of the original code, but I failed to realize that alignment
considerations would move the whole thing to the previous-to-last word
in the page.

The new definition of the associated macro makes surrounding code a bit
leaner, too.

Per note from Heikki at
http://www.postgresql.org/message-id/546A16EF.9070005@vmware.com

9 years agoAllow named parameters to be specified using => in addition to :=
Robert Haas [Tue, 10 Mar 2015 14:59:11 +0000 (10:59 -0400)]
Allow named parameters to be specified using => in addition to :=

SQL has standardized on => as the use of to specify named parameters,
and we've wanted for many years to support the same syntax ourselves,
but this has been complicated by the possible use of => as an operator
name.  In PostgreSQL 9.0, we began emitting a warning when an operator
named => was defined, and in PostgreSQL 9.2, we stopped shipping a
=>(text, text) operator as part of hstore.  By the time the next major
version of PostgreSQL is released, => will have been deprecated for a
full five years, so hopefully there won't be too many people still
relying on it.  We continue to support := for compatibility with
previous PostgreSQL releases.

Pavel Stehule, reviewed by Petr Jelinek, with a few documentation
tweaks by me.

9 years agoKeep CommitTs module in sync in standby and master
Alvaro Herrera [Mon, 9 Mar 2015 20:44:00 +0000 (17:44 -0300)]
Keep CommitTs module in sync in standby and master

We allow this module to be turned off on restarts, so a restart time
check is enough to activate or deactivate the module; however, if there
is a standby replaying WAL emitted from a master which is restarted, but
the standby isn't, the state in the standby becomes inconsistent and can
easily be crashed.

Fix by activating and deactivating the module during WAL replay on
parameter change as well as on system start.

Problem reported by Fujii Masao in
http://www.postgresql.org/message-id/CAHGQGwFhJ3CnHo1CELEfay18yg_RA-XZT-7D8NuWUoYSZ90r4Q@mail.gmail.com

Author: Petr Jelínek

9 years agoFix crasher bugs in previous commit
Alvaro Herrera [Mon, 9 Mar 2015 20:00:43 +0000 (17:00 -0300)]
Fix crasher bugs in previous commit

ALTER DEFAULT PRIVILEGES was trying to decode the list of roles in the
FOR clause as a list of names rather than of RoleSpecs; and the IN
clause in CREATE ROLE was doing the same thing.  This was evidenced by
crashes on some buildfarm machines, though on my platform this doesn't
cause a failure by mere chance; I can reproduce the failures only by
adding some padding in struct RoleSpecs.

Fix by dereferencing those lists as being of RoleSpecs, not string
Values.

9 years agoAllow CURRENT/SESSION_USER to be used in certain commands
Alvaro Herrera [Mon, 9 Mar 2015 18:41:54 +0000 (15:41 -0300)]
Allow CURRENT/SESSION_USER to be used in certain commands

Commands such as ALTER USER, ALTER GROUP, ALTER ROLE, GRANT, and the
various ALTER OBJECT / OWNER TO, as well as ad-hoc clauses related to
roles such as the AUTHORIZATION clause of CREATE SCHEMA, the FOR clause
of CREATE USER MAPPING, and the FOR ROLE clause of ALTER DEFAULT
PRIVILEGES can now take the keywords CURRENT_USER and SESSION_USER as
user specifiers in place of an explicit user name.

This commit also fixes some quite ugly handling of special standards-
mandated syntax in CREATE USER MAPPING, which in particular would fail
to work in presence of a role named "current_user".

The special role specifiers PUBLIC and NONE also have more consistent
handling now.

Also take the opportunity to add location tracking to user specifiers.

Authors: Kyotaro Horiguchi.  Heavily reworked by Álvaro Herrera.
Reviewed by: Rushabh Lathia, Adam Brightwell, Marti Raudsepp.

9 years agoIgnore leftover artifacts from ecpg testsuite runs on Windows.
Michael Meskes [Mon, 9 Mar 2015 17:48:44 +0000 (18:48 +0100)]
Ignore leftover artifacts from ecpg testsuite runs on Windows.

9 years agoRevert "Ignore object files generated by ecpg test suite on Windows"
Michael Meskes [Mon, 9 Mar 2015 17:48:13 +0000 (18:48 +0100)]
Revert "Ignore object files generated by ecpg test suite on Windows"

This reverts commit b9e538b190d9cf4387361214eadc430393ebf852.

9 years agoFix handling of sortKeys field in Tuplesortstate.
Robert Haas [Mon, 9 Mar 2015 14:35:41 +0000 (10:35 -0400)]
Fix handling of sortKeys field in Tuplesortstate.

Commit 5cefbf5a6c4466ac6b1cc2a4316b4eba9108c802 introduced an
assumption that this field would always be non-NULL when doing a merge
pass, but that's not true.  Without this fix, you can crash the server
by building a hash index that is sufficiently large relative to
maintenance_work_mem, or by triggering a large datum sort.

Commit 5ea86e6e65dd2da3e9a3464484985d48328e7fe3 changed the comments
for that field to say that it would be set in all cases except for the
hash index case, but that wasn't (and still isn't) true.

The datum-sort failure was spotted by Tomas Vondra; initial analysis
of that failure was by Peter Geoghegan.  The remaining issues were
spotted by me during review of the surrounding code, and the patch is
all my fault.

9 years agoMove WAL-related definitions from dbcommands.h to separate header file.
Heikki Linnakangas [Mon, 9 Mar 2015 13:49:10 +0000 (15:49 +0200)]
Move WAL-related definitions from dbcommands.h to separate header file.

This makes it easier to write frontend programs that needs to understand
the WAL record format of CREATE/DROP DATABASE. dbcommands.h cannot easily
be #included in a frontend program, because it pulls in other header files
that need backend stuff, but the new dbcommands_xlog.h header file has
fewer dependencies.

9 years agoIgnore object files generated by ecpg test suite on Windows
Michael Meskes [Mon, 9 Mar 2015 13:38:22 +0000 (14:38 +0100)]
Ignore object files generated by ecpg test suite on Windows

Patch by Michael Paquier

9 years agoFix typo in comment.
Fujii Masao [Mon, 9 Mar 2015 05:39:46 +0000 (14:39 +0900)]
Fix typo in comment.

9 years agoAdd missing "goto err" statements in xlogreader.c.
Fujii Masao [Mon, 9 Mar 2015 05:31:10 +0000 (14:31 +0900)]
Add missing "goto err" statements in xlogreader.c.

Spotted by Andres Freund.

9 years agoSort SUBDIRS variable in src/bin/Makefile
Peter Eisentraut [Mon, 15 Dec 2014 01:41:58 +0000 (20:41 -0500)]
Sort SUBDIRS variable in src/bin/Makefile

The previous order appears to have been historically grown randomness.

9 years agoCast to (void *) rather than (int *) when passing int64's to PQfn().
Tom Lane [Sun, 8 Mar 2015 17:58:28 +0000 (13:58 -0400)]
Cast to (void *) rather than (int *) when passing int64's to PQfn().

This is a possibly-vain effort to silence a Coverity warning about
bogus endianness dependency.  The code's fine, because it takes care
of endianness issues for itself, but Coverity sees an int64 being
passed to an int* argument and not unreasonably suspects something's
wrong.  I'm not sure if putting the void* cast in the way will shut it
up; but it can't hurt and seems better from a documentation standpoint
anyway, since the pointer is not used as an int* in this code path.

Just for a bit of additional safety, verify that the result length
is 8 bytes as expected.

Back-patch to 9.3 where the code in question was added.

9 years agoRemove struct PQArgBlock from server-side header libpq/libpq.h.
Tom Lane [Sun, 8 Mar 2015 17:42:59 +0000 (13:42 -0400)]
Remove struct PQArgBlock from server-side header libpq/libpq.h.

This struct is purely a client-side artifact.  Perhaps there was once
reason for the server to know it, but any such reason is lost in the
mists of time.  We certainly don't need two independent declarations
of it.

9 years agoFix documentation for libpq's PQfn().
Tom Lane [Sun, 8 Mar 2015 17:35:28 +0000 (13:35 -0400)]
Fix documentation for libpq's PQfn().

The SGML docs claimed that 1-byte integers could be sent or received with
the "isint" options, but no such behavior has ever been implemented in
pqGetInt() or pqPutInt().  The in-code documentation header for PQfn() was
even less in tune with reality, and the code itself used parameter names
matching neither the SGML docs nor its libpq-fe.h declaration.  Do a bit
of additional wordsmithing on the SGML docs while at it.

Since the business about 1-byte integers is a clear documentation bug,
back-patch to all supported branches.

9 years agoCode cleanup for REINDEX DATABASE/SCHEMA/SYSTEM.
Tom Lane [Sun, 8 Mar 2015 16:18:43 +0000 (12:18 -0400)]
Code cleanup for REINDEX DATABASE/SCHEMA/SYSTEM.

Fix some minor infelicities.  Some of these things were introduced in
commit fe263d115a7dd16095b8b8f1e943aff2bb4574d2, and some are older.

9 years agoFix erroneous error message for REINDEX SYSTEM.
Tom Lane [Sun, 8 Mar 2015 15:51:04 +0000 (11:51 -0400)]
Fix erroneous error message for REINDEX SYSTEM.

Missed case in commit fe263d115a7dd16095b8b8f1e943aff2bb4574d2.

Sawada Masahiko

9 years agoBuild fls.o only when AC_REPLACE_FUNCS so dictates via $(LIBOBJS).
Noah Misch [Sat, 7 Mar 2015 05:48:04 +0000 (00:48 -0500)]
Build fls.o only when AC_REPLACE_FUNCS so dictates via $(LIBOBJS).

By building it unconditionally, libpgport inadvertently replaced any
libc version of the function.  This is essentially a code cleanup; any
effect on performance is almost surely too small to notice.

9 years agoAdd CHECK_FOR_INTERRUPTS() to the wait_pid() loop.
Noah Misch [Sat, 7 Mar 2015 05:47:38 +0000 (00:47 -0500)]
Add CHECK_FOR_INTERRUPTS() to the wait_pid() loop.

Though the one contemporary caller uses it in a limited way, this
function could loop indefinitely if pointed to an arbitrary PID.

9 years agoRemove rolcatupdate
Peter Eisentraut [Sat, 7 Mar 2015 04:42:38 +0000 (23:42 -0500)]
Remove rolcatupdate

This role attribute is an ancient PostgreSQL feature, but could only be
set by directly updating the system catalogs, and it doesn't have any
clearly defined use.

Author: Adam Brightwell <adam.brightwell@crunchydatasolutions.com>

9 years agoAdd some more tests on event triggers
Alvaro Herrera [Fri, 6 Mar 2015 22:14:28 +0000 (19:14 -0300)]
Add some more tests on event triggers

Fabien Coelho
Reviewed by Robert Haas

9 years agoRethink function argument sorting in pg_dump.
Tom Lane [Fri, 6 Mar 2015 18:27:46 +0000 (13:27 -0500)]
Rethink function argument sorting in pg_dump.

Commit 7b583b20b1c95acb621c71251150beef958bb603 created an unnecessary
dump failure hazard by applying pg_get_function_identity_arguments()
to every function in the database, even those that won't get dumped.
This could result in snapshot-related problems if concurrent sessions are,
for example, creating and dropping temporary functions, as noted by Marko
Tiikkaja in bug #12832.  While this is by no means pg_dump's only such
issue with concurrent DDL, it's unfortunate that we added a new failure
mode for cases that used to work, and even more so that the failure was
created for basically cosmetic reasons (ie, to sort overloaded functions
more deterministically).

To fix, revert that patch and instead sort function arguments using
information that pg_dump has available anyway, namely the names of the
argument types.  This will produce a slightly different sort ordering for
overloaded functions than the previous coding; but applying strcmp
directly to the output of pg_get_function_identity_arguments really was
a bit odd anyway.  The sorting will still be name-based and hence
independent of possibly-installation-specific OID assignments.  A small
additional benefit is that sorting now works regardless of server version.

Back-patch to 9.3, where the previous commit appeared.

9 years agoFix contrib/file_fdw's expected file
Alvaro Herrera [Fri, 6 Mar 2015 14:47:09 +0000 (11:47 -0300)]
Fix contrib/file_fdw's expected file

I forgot to update it on yesterday's cf34e373fcf.

9 years agoFix user mapping object description
Alvaro Herrera [Thu, 5 Mar 2015 21:03:16 +0000 (18:03 -0300)]
Fix user mapping object description

We were using "user mapping for user XYZ" as description for user mappings, but
that's ambiguous because users can have mappings on multiple foreign
servers; therefore change it to "for user XYZ on server UVW" instead.
Object identities for user mappings are also updated in the same way, in
branches 9.3 and above.

The incomplete description string was introduced together with the whole
SQL/MED infrastructure by commit cae565e503 of 8.4 era, so backpatch all
the way back.

9 years agoSilence warning in non-assert-enabled build
Alvaro Herrera [Thu, 5 Mar 2015 15:11:26 +0000 (12:11 -0300)]
Silence warning in non-assert-enabled build

An OID return value was being used only for a (rather pointless) assert.
Silence by removing the variable and the assert.

Per note from Peter Geoghegan

9 years agoRemove comment claiming that PARAM_EXTERN Params always have typmod -1.
Tom Lane [Thu, 5 Mar 2015 18:16:27 +0000 (13:16 -0500)]
Remove comment claiming that PARAM_EXTERN Params always have typmod -1.

This hasn't been true in quite some time, cf plpgsql's make_datum_param().

9 years agoFix typo in comment.
Fujii Masao [Thu, 5 Mar 2015 11:15:16 +0000 (20:15 +0900)]
Fix typo in comment.

9 years agoAvoid unused-variable warning in non-assert builds.
Tom Lane [Thu, 5 Mar 2015 03:00:36 +0000 (22:00 -0500)]
Avoid unused-variable warning in non-assert builds.

Oversight in my commit b9896198cfbc1b0cd0c631d2af72ffe34bd4c7e5.

9 years agoChange plpgsql's cast cache to consider source typmod as significant.
Tom Lane [Thu, 5 Mar 2015 01:23:13 +0000 (20:23 -0500)]
Change plpgsql's cast cache to consider source typmod as significant.

I had thought that there was no need to maintain separate cache entries
for different source typmods, but further experimentation shows that there
is an advantage to doing so in some cases.  In particular, if a domain has
a typmod (say, "CREATE DOMAIN d AS numeric(20,0)"), failing to notice the
source typmod leads to applying a length-coercion step even when the
source has the correct typmod.

9 years agoNeed to special-case RECORD as well as UNKNOWN in plpgsql's casting logic.
Tom Lane [Thu, 5 Mar 2015 00:10:57 +0000 (19:10 -0500)]
Need to special-case RECORD as well as UNKNOWN in plpgsql's casting logic.

This is because can_coerce_type thinks that RECORD can be cast to any
composite type, but coerce_record_to_complex only works for inputs that are
RowExprs or whole-row Vars, so we get a hard failure on a CaseTestExpr.

Perhaps these corner cases ought to be fixed so that coerce_to_target_type
actually returns NULL as per its specification, rather than failing ...
but for the moment an extra check here is the path of least resistance.

9 years agoUse standard casting mechanism to convert types in plpgsql, when possible.
Tom Lane [Wed, 4 Mar 2015 16:04:30 +0000 (11:04 -0500)]
Use standard casting mechanism to convert types in plpgsql, when possible.

plpgsql's historical method for converting datatypes during assignments was
to apply the source type's output function and then the destination type's
input function.  Aside from being miserably inefficient in most cases, this
method failed outright in many cases where a user might expect it to work;
an example is that "declare x int; ... x := 3.9;" would fail, not round the
value to 4.

Instead, let's convert by applying the appropriate assignment cast whenever
there is one.  To avoid breaking compatibility unnecessarily, fall back to
the I/O conversion method if there is no assignment cast.

So far as I can tell, there is just one case where this method produces a
different result than the old code in a case where the old code would not
have thrown an error.  That is assignment of a boolean value to a string
variable (type text, varchar, or bpchar); the old way gave boolean's output
representation, ie 't'/'f', while the new way follows the behavior of the
bool-to-text cast and so gives 'true' or 'false'.  This will need to be
called out as an incompatibility in the 9.5 release notes.

Aside from handling many conversion cases more sanely, this method is
often significantly faster than the old way.  In part that's because
of more effective caching of the conversion info.

9 years agoFix cost estimation for indexscans on expensive indexed expressions.
Tom Lane [Wed, 4 Mar 2015 04:23:17 +0000 (23:23 -0500)]
Fix cost estimation for indexscans on expensive indexed expressions.

genericcostestimate() and friends used the cost of the entire indexqual
expressions as the charge for initial evaluation of indexscan arguments.
But of course the index column is not evaluated, only the other side
of the qual expression, so this was a bad overestimate if the index
column was an expensive expression.

To fix, refactor the logic in this area so that there's a single routine
charged with deconstructing index quals and figuring out what is the index
column and what is the comparison expression.  This is more or less free in
the case of btree indexes, since btcostestimate() was doing equivalent
deconstruction already.  It probably adds a bit of new overhead in the cases
of other index types, but not a lot.  (In the case of GIN I think I saved
something by getting rid of code that wasn't aware that the index column
associations were already available "for free".)

Per recent gripe from Jeff Janes.

Arguably this is a bug fix, but I'm hesitant to back-patch because of the
possibility of destabilizing plan choices that people may be happy with.

9 years agoFix an obsolete reference to SnapshotNow in comment.
Fujii Masao [Wed, 4 Mar 2015 03:25:48 +0000 (12:25 +0900)]
Fix an obsolete reference to SnapshotNow in comment.

Peter Geoghegan

9 years agoFix long-obsolete code for separating filter conditions in cost_index().
Tom Lane [Wed, 4 Mar 2015 02:19:42 +0000 (21:19 -0500)]
Fix long-obsolete code for separating filter conditions in cost_index().

This code relied on pointer equality to identify which restriction clauses
also appear in the indexquals (and, therefore, don't need to be applied as
simple filter conditions).  That was okay once upon a time, years ago,
before we introduced the equivalence-class machinery.  Now there's about a
50-50 chance that an equality clause appearing in the indexquals will be
the mirror image (commutator) of its mate in the restriction list.  When
that happens, we'd erroneously think that the clause would be re-evaluated
at each visited row, and therefore inflate the cost estimate for the
indexscan by the clause's cost.

Add some logic to catch this case.  It seems to me that it continues not to
be worthwhile to expend the extra predicate-proof work that createplan.c
will do on the finally-selected plan, but this case is common enough and
cheap enough to handle that we should do so.

This will make a small difference (about one cpu_operator_cost per row)
in simple cases; but in situations where there's an expensive function in
the indexquals, it can make a very large difference, as seen in recent
example from Jeff Janes.

This is a long-standing bug, but I'm hesitant to back-patch because of the
possibility of destabilizing plan choices that people may be happy with.

9 years agoRemove residual NULL-pstate handling in addRangeTableEntry.
Robert Haas [Tue, 3 Mar 2015 21:31:26 +0000 (16:31 -0500)]
Remove residual NULL-pstate handling in addRangeTableEntry.

Passing a NULL pstate wouldn't actually work, because isLockedRefname()
isn't prepared to cope with it; and there hasn't been any in-core code
that tries in over a decade.  So just remove the residual NULL handling.

Spotted by Coverity; analysis and patch by Michael Paquier.

9 years agoChange many routines to return ObjectAddress rather than OID
Alvaro Herrera [Tue, 3 Mar 2015 17:10:50 +0000 (14:10 -0300)]
Change many routines to return ObjectAddress rather than OID

The changed routines are mostly those that can be directly called by
ProcessUtilitySlow; the intention is to make the affected object
information more precise, in support for future event trigger changes.
Originally it was envisioned that the OID of the affected object would
be enough, and in most cases that is correct, but upon actually
implementing the event trigger changes it turned out that ObjectAddress
is more widely useful.

Additionally, some command execution routines grew an output argument
that's an object address which provides further info about the executed
command.  To wit:

* for ALTER DOMAIN / ADD CONSTRAINT, it corresponds to the address of
  the new constraint

* for ALTER OBJECT / SET SCHEMA, it corresponds to the address of the
  schema that originally contained the object.

* for ALTER EXTENSION {ADD, DROP} OBJECT, it corresponds to the address
  of the object added to or dropped from the extension.

There's no user-visible change in this commit, and no functional change
either.

Discussion: 20150218213255.GC6717@tamriel.snowman.net
Reviewed-By: Stephen Frost, Andres Freund
9 years agoAdd comment for "is_internal" parameter
Alvaro Herrera [Tue, 3 Mar 2015 17:03:33 +0000 (14:03 -0300)]
Add comment for "is_internal" parameter

This was missed in my commit f4c4335 of 9.3 vintage, so backpatch to
that.

9 years agoReduce json <=> jsonb casts from explicit-only to assignment level.
Tom Lane [Tue, 3 Mar 2015 16:26:04 +0000 (11:26 -0500)]
Reduce json <=> jsonb casts from explicit-only to assignment level.

There's no reason to make users write an explicit cast to store a
json value in a jsonb column or vice versa.

We could probably even make these implicit, but that might open us up
to problems with ambiguous function calls, so for now just do this.

9 years agopgbench: Fix mistakes in Makefile.
Robert Haas [Tue, 3 Mar 2015 15:32:08 +0000 (10:32 -0500)]
pgbench: Fix mistakes in Makefile.

My commit 878fdcb843e087cc1cdeadc987d6ef55202ddd04 was not quite
right.  Tom Lane pointed out one of the mistakes fixed here, and I
noticed the other myself while reviewing what I'd committed.

9 years agoFix busted markup.
Tom Lane [Tue, 3 Mar 2015 04:28:31 +0000 (23:28 -0500)]
Fix busted markup.

Evidently from commit 878fdcb843e087cc1cdeadc987d6ef55202ddd04.
Per buildfarm.

9 years agopgbench: Add a real expression syntax to \set
Robert Haas [Mon, 2 Mar 2015 19:21:41 +0000 (14:21 -0500)]
pgbench: Add a real expression syntax to \set

Previously, you could do \set variable operand1 operator operand2, but
nothing more complicated.  Now, you can \set variable expression, which
makes it much simpler to do multi-step calculations here.  This also
adds support for the modulo operator (%), with the same semantics as in
C.

Robert Haas and Fabien Coelho, reviewed by Álvaro Herrera and
Stephen Frost

9 years agoFix pg_dump handling of extension config tables
Stephen Frost [Mon, 2 Mar 2015 19:12:21 +0000 (14:12 -0500)]
Fix pg_dump handling of extension config tables

Since 9.1, we've provided extensions with a way to denote
"configuration" tables- tables created by an extension which the user
may modify.  By marking these as "configuration" tables, the extension
is asking for the data in these tables to be pg_dump'd (tables which
are not marked in this way are assumed to be entirely handled during
CREATE EXTENSION and are not included at all in a pg_dump).

Unfortunately, pg_dump neglected to consider foreign key relationships
between extension configuration tables and therefore could end up
trying to reload the data in an order which would cause FK violations.

This patch teaches pg_dump about these dependencies, so that the data
dumped out is done so in the best order possible.  Note that there's no
way to handle circular dependencies, but those have yet to be seen in
the wild.

The release notes for this should include a caution to users that
existing pg_dump-based backups may be invalid due to this issue.  The
data is all there, but restoring from it will require extracting the
data for the configuration tables and then loading them in the correct
order by hand.

Discussed initially back in bug #6738, more recently brought up by
Gilles Darold, who provided an initial patch which was further reworked
by Michael Paquier.  Further modifications and documentation updates
by me.

Back-patch to 9.1 where we added the concept of extension configuration
tables.

9 years agoFix targetRelation initializiation in prepsecurity
Stephen Frost [Sun, 1 Mar 2015 20:26:55 +0000 (15:26 -0500)]
Fix targetRelation initializiation in prepsecurity

In 6f9bd50eabb0a4960e94c83dac8855771c9f340d, we modified
expand_security_quals() to tell expand_security_qual() about when the
current RTE was the targetRelation.  Unfortunately, that commit
initialized the targetRelation variable used outside of the loop over
the RTEs instead of at the start of it.

This patch moves the variable and the initialization of it into the
loop, where it should have been to begin with.

Pointed out by Dean Rasheed.

Back-patch to 9.4 as the original commit was.

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.