Tom Lane [Thu, 11 Dec 2014 00:06:27 +0000 (19:06 -0500)]
Fix minor thinko in convertToJsonb().
The amount of space to reserve for the value's varlena header is
VARHDRSZ, not sizeof(VARHDRSZ). The latter coding accidentally
failed to fail because of the way the VARHDRSZ macro is currently
defined; but if we ever change it to return size_t (as one might
reasonably expect it to do), convertToJsonb() would have failed.
It's not run by the global "check" or "installcheck" targets, because the
temporary installation it creates accepts TCP connections from any user
the same host, which is insecure.
Simon Riggs [Tue, 9 Dec 2014 09:05:36 +0000 (18:05 +0900)]
Silence REINDEX
Previously REINDEX DATABASE and REINDEX SCHEMA
produced a stream of NOTICE messages. Removing that
since it is inconsistent for such a command to
produce output without a VERBOSE option.
Simon Riggs [Mon, 8 Dec 2014 14:36:06 +0000 (23:36 +0900)]
Windows: use GetSystemTimePreciseAsFileTime if available
PostgreSQL on Windows 8 or Windows Server 2012 will now
get high-resolution timestamps by dynamically loading the
GetSystemTimePreciseAsFileTime function. It'll fall back to
to GetSystemTimeAsFileTime if the higher precision variant
isn't found, so the same binaries without problems on older
Windows releases.
No attempt is made to detect the Windows version. Only the
presence or absence of the desired function is considered.
Simon Riggs [Mon, 8 Dec 2014 14:32:03 +0000 (23:32 +0900)]
Use GetSystemTimeAsFileTime directly in win32
PostgreSQL was calling GetSystemTime followed by SystemTimeToFileTime in the
win32 port gettimeofday function. This is not necessary and limits the reported
precision to the 1ms granularity that the SYSTEMTIME struct can represent. By
using GetSystemTimeAsFileTime we avoid unnecessary conversions and capture
timestamps at 100ns granularity, which is then rounded to 1µs granularity for
storage in a PostgreSQL timestamp.
On most Windows systems this change will actually have no significant effect on
timestamp resolution as the system timer tick is typically between 1ms and 15ms
depending on what timer resolution currently running applications have
requested. You can check this with clockres.exe from sysinternals. Despite the
platform limiation this change still permits capture of finer timestamps where
the system is capable of producing them and it gets rid of an unnecessary
syscall.
The higher resolution GetSystemTimePreciseAsFileTime call available on Windows
8 and Windows Server 2012 has the same interface as GetSystemTimeAsFileTime, so
switching to GetSystemTimeAsFileTime makes it easier to use the Precise variant
later.
Don't include file type bits in tar archive's mode field.
The "file mode" bits in the tar file header is not supposed to include the
file type bits, e.g. S_IFREG or S_IFDIR. The file type is stored in a
separate field. This isn't a problem in practice, all tar programs ignore
the extra bits, but let's be tidy.
This came up in a discussion around bug #11949, reported by Hendrik Grewe,
although this doesn't fix the issue with tar --append. That turned out to be
a bug in GNU tar. Schilly's tartest program revealed this defect in the tar
created by pg_basebackup.
This problem goes as far as we we've had pg_basebackup, but since this
hasn't caused any problems in practice, let's be conservative and fix in
master only.
Fix PGXS vpath build when PostgreSQL is built with vpath
PGXS computes srcdir from VPATH, PostgreSQL proper computes VPATH from
srcdir, and doing both results in an error from make. Conditionalize so
only one of these takes effect.
Peter Eisentraut [Thu, 20 Nov 2014 03:26:32 +0000 (22:26 -0500)]
Revert haphazard pgxs makefile changes
These changes were originally submitted as "adds support for VPATH with
USE_PGXS", but they are not necessary for VPATH support, so they just
add more lines of code for no reason.
Fix SHLIB_PREREQS use in contrib, allowing PGXS builds
dblink and postgres_fdw use SHLIB_PREREQS = submake-libpq to build libpq
first. This doesn't work in a PGXS build, because there is no libpq to
build. So just omit setting SHLIB_PREREQS in this case.
Note that PGXS users can still use SHLIB_PREREQS (although it is not
documented). The problem here is only that contrib modules can be built
in-tree or using PGXS, and the prerequisite is only applicable in the
former case.
Commit 6697aa2bc25c83b88d6165340348a31328c35de6 previously attempted to
address this by creating a somewhat fake submake-libpq target in
Makefile.global. That was not the right fix, and it was also done in a
nonportable way, so revert that.
Since this is not something that a user should change,
pg_config_manual.h was an inappropriate place for it.
In initdb.c, remove the use of the macro, because utils/guc.h can't be
included by non-backend code. But we hardcode all the other
configuration file names there, so this isn't a disaster.
Alvaro Herrera [Wed, 3 Dec 2014 14:53:02 +0000 (11:53 -0300)]
Keep track of transaction commit timestamps
Transactions can now set their commit timestamp directly as they commit,
or an external transaction commit timestamp can be fed from an outside
system using the new function TransactionTreeSetCommitTsData(). This
data is crash-safe, and truncated at Xid freeze point, same as pg_clog.
This module is disabled by default because it causes a performance hit,
but can be enabled in postgresql.conf requiring only a server restart.
A new test in src/test/modules is included.
Catalog version bumped due to the new subdirectory within PGDATA and a
couple of new SQL functions.
Authors: Álvaro Herrera and Petr Jelínek
Reviewed to varying degrees by Michael Paquier, Andres Freund, Robert
Haas, Amit Kapila, Fujii Masao, Jaime Casanova, Simon Riggs, Steven
Singer, Peter Eisentraut
Alvaro Herrera [Wed, 3 Dec 2014 02:43:53 +0000 (23:43 -0300)]
Install kludges to fix check-world for src/test/modules
check-world failed in a completely clean tree, because src/test/modules
fail to build unless errcodes.h is generated first. To fix this,
install a dependency in src/test/modules' Makefile so that the necessary
file is generated. Even with this, running "make check" within
individual module subdirs will still fail because the dependency is not
considered there, but this case is less interesting and would be messier
to fix.
check-world still failed with the above fix in place, this time because
dummy_seclabel used LOAD to load the dynamic library, which doesn't work
because the @libdir@ (expanded by the makefile) is expanded to the final
install path, not the temporary installation directory used by make
check. To fix, tweak things so that CREATE EXTENSION can be used
instead, which solves the problem because the library path is expanded
by the backend, which is aware of the true libdir.
Tom Lane [Tue, 2 Dec 2014 23:23:16 +0000 (18:23 -0500)]
Improve error messages for malformed array input strings.
Make the error messages issued by array_in() uniformly follow the style
ERROR: malformed array literal: "actual input string"
DETAIL: specific complaint here
and rewrite many of the specific complaints to be clearer.
The immediate motivation for doing this is a complaint from Josh Berkus
that json_to_record() produced an unintelligible error message when
dealing with an array item, because it tries to feed the JSON-format
array value to array_in(). Really it ought to be smart enough to
perform JSON-to-Postgres array conversion, but that's a future feature
not a bug fix. In the meantime, this change is something we agreed
we could back-patch into 9.4, and it should help de-confuse things a bit.
Andres Freund [Tue, 2 Dec 2014 22:42:26 +0000 (23:42 +0100)]
Don't skip SQL backends in logical decoding for visibility computation.
The logical decoding patchset introduced PROC_IN_LOGICAL_DECODING flag
PGXACT flag, that allows such backends to be skipped when computing
the xmin horizon/snapshots. That's fine and sensible for walsenders
streaming out logical changes, but not at all fine for SQL backends
doing logical decoding. If the latter set that flag any change they
have performed outside of logical decoding will not be regarded as
visible - which e.g. can lead to that change being vacuumed away.
Note that not setting the flag for SQL backends isn't particularly
bothersome - the SQL backend doesn't do streaming, so it only runs for
a limited amount of time.
Per buildfarm member 'tick' and Alvaro.
Backpatch to 9.4, where logical decoding was introduced.
Tom Lane [Tue, 2 Dec 2014 20:02:37 +0000 (15:02 -0500)]
Fix JSON aggregates to work properly when final function is re-executed.
Davide S. reported that json_agg() sometimes produced multiple trailing
right brackets. This turns out to be because json_agg_finalfn() attaches
the final right bracket, and was doing so by modifying the aggregate state
in-place. That's verboten, though unfortunately it seems there's no way
for nodeAgg.c to check for such mistakes.
Fix that back to 9.3 where the broken code was introduced. In 9.4 and
HEAD, likewise fix json_object_agg(), which had copied the erroneous logic.
Make some cosmetic cleanups as well.
Tom Lane [Tue, 2 Dec 2014 19:07:54 +0000 (14:07 -0500)]
Minor cleanup of function declarations for BRIN.
Get rid of PG_FUNCTION_INFO_V1() macros, which are quite inappropriate
for built-in functions (possibly leftovers from testing as a loadable
module?). Also, fix gratuitous inconsistency between SQL-level and
C-level names of the minmax support functions.
Tom Lane [Mon, 1 Dec 2014 20:25:02 +0000 (15:25 -0500)]
Guard against bad "dscale" values in numeric_recv().
We were not checking to see if the supplied dscale was valid for the given
digit array when receiving binary-format numeric values. While dscale can
validly be more than the number of nonzero fractional digits, it shouldn't
be less; that case causes fractional digits to be hidden on display even
though they're there and participate in arithmetic.
Bug #12053 from Tommaso Sala indicates that there's at least one broken
client library out there that sometimes supplies an incorrect dscale value,
leading to strange behavior. This suggests that simply throwing an error
might not be the best response; it would lead to failures in applications
that might seem to be working fine today. What seems the least risky fix
is to truncate away any digits that would be hidden by dscale. This
preserves the existing behavior in terms of what will be printed for the
transmitted value, while preventing subsequent arithmetic from producing
results inconsistent with that.
In passing, throw a specific error for the case of dscale being outside
the range that will fit into a numeric's header. Before you got "value
overflows numeric format", which is a bit misleading.
Alvaro Herrera [Mon, 1 Dec 2014 19:12:43 +0000 (16:12 -0300)]
Move security_label test
Rather than have the core security_label regression test depend on the
dummy_seclabel module, have that part of the test be executed by
dummy_seclabel itself directly. This simplifies the testing rig a bit;
in particular it should silence the problems from the MSVC buildfarm
phylum, which haven't yet gotten taught how to install src/test/modules.
Andrew Dunstan [Mon, 1 Dec 2014 16:28:45 +0000 (11:28 -0500)]
Fix hstore_to_json_loose's detection of valid JSON number values.
We expose a function IsValidJsonNumber that internally calls the lexer
for json numbers. That allows us to use the same test everywhere,
instead of inventing a broken test for hstore conversions. The new
function is also used in datum_to_json, replacing the code that is now
moved to the new function.
Backpatch to 9.3 where hstore_to_json_loose was introduced.
Tom Lane [Mon, 1 Dec 2014 00:43:43 +0000 (19:43 -0500)]
Add a #define for the inet overlaps operator.
Extracted from pending inet selectivity patch. The rest of it isn't
quite ready to commit, but we might as well push this part so the patch
doesn't have to track the moving target of pg_operator.h.
Coverity complained that the "else" added to fillPGconn() was unreachable,
which it was. Remove the dead code. In passing, rearrange the tests so as
not to bother trying to fetch values for options that can't be assigned.
Pre-9.3 did not have that issue, but it did have a "return" that should be
"goto oom_error" to ensure that a suitable error message gets filled in.
Alvaro Herrera [Sun, 30 Nov 2014 02:55:00 +0000 (23:55 -0300)]
Move test modules from contrib to src/test/modules
This is advance preparation for introducing even more test modules; the
easy solution is to add them to contrib, but that's bloated enough that
it seems a good time to think of something different.
Moved modules are dummy_seclabel, test_shm_mq, test_parser and
worker_spi.
(test_decoding was also a candidate, but there was too much opposition
to moving that one. We can always reconsider later.)
Apart from ignoring "hostaddr" set to the empty string, this behaves
identically to its predecessor. Back-patch to 9.4, where the original
commit first appeared.
Noah Misch [Sat, 29 Nov 2014 17:31:21 +0000 (12:31 -0500)]
Revert "Add libpq function PQhostaddr()."
This reverts commit 9f80f4835a55a1cbffcda5d23a617917f3286c14. The
function returned the raw value of a connection parameter, a task served
by PQconninfo(). The next commit will reimplement the psql \conninfo
change that way. Back-patch to 9.4, where that commit first appeared.
Alvaro Herrera [Fri, 28 Nov 2014 21:09:19 +0000 (18:09 -0300)]
Fix BRIN operator family definitions
The original definitions were leaving no room for cross-type operators,
so queries that compared a column of one type against something of a
different type were not taking advantage of the index. Fix by making
the opfamilies more like the ones for Btree, and include a few
cross-type operator classes.
Catalog version bumped.
Per complaints from Hubert Lubaczewski, Mark Wong, Heikki Linnakangas.
Tom Lane [Fri, 28 Nov 2014 19:16:24 +0000 (14:16 -0500)]
Add bms_get_singleton_member(), and use it where appropriate.
This patch adds a function that replaces a bms_membership() test followed
by a bms_singleton_member() call, performing both the test and the
extraction of a singleton set's member in one scan of the bitmapset.
The performance advantage over the old way is probably minimal in current
usage, but it seems worthwhile on notational grounds anyway.
Tom Lane [Fri, 28 Nov 2014 18:37:25 +0000 (13:37 -0500)]
Add bms_next_member(), and use it where appropriate.
This patch adds a way of iterating through the members of a bitmapset
nondestructively, unlike the old way with bms_first_member(). While
bms_next_member() is very slightly slower than bms_first_member()
(at least for typical-size bitmapsets), eliminating the need to palloc
and pfree a temporary copy of the target bitmapset is a significant win.
So this method should be preferred in all cases where a temporary copy
would be necessary.
Tom Lane, with suggestions from Dean Rasheed and David Rowley
Tom Lane [Fri, 28 Nov 2014 17:37:27 +0000 (12:37 -0500)]
Improve performance of OverrideSearchPathMatchesCurrent().
This function was initially coded on the assumption that it would not be
performance-critical, but that turns out to be wrong in workloads that
are heavily dependent on the speed of plpgsql functions. Speed it up by
hard-coding the comparison rules, thereby avoiding palloc/pfree traffic
from creating and immediately freeing an OverrideSearchPath object.
Per report from Scott Marlowe.
Previously, if the typcache had for example tried and failed to find a hash
opclass for a given data type, it would nonetheless repeat the unsuccessful
catalog lookup each time it was asked again. This can lead to a
significant amount of useless bufmgr traffic, as in a recent report from
Scott Marlowe. Like the catalog caches, typcache should be able to cache
negative results. This patch arranges that by making use of separate flag
bits to remember whether a particular item has been looked up, rather than
treating a zero OID as an indicator that no lookup has been done.
Also, install a credible invalidation mechanism, namely watching for inval
events in pg_opclass. The sole advantage of the lack of negative caching
was that the code would cope if operators or opclasses got added for a type
mid-session; to preserve that behavior we have to be able to invalidate
stale lookup results. Updates in pg_opclass should be pretty rare in
production systems, so it seems sufficient to just invalidate all the
dependent data whenever one happens.
Adding proper invalidation also means that this code will now react sanely
if an opclass is dropped mid-session. Arguably, that's a back-patchable
bug fix, but in view of the lack of complaints from the field I'll refrain
from back-patching. (Probably, in most cases where an opclass is dropped,
the data type itself is dropped soon after, so that this misfeasance has
no bad consequences.)
InitXLogInsert() cannot be called in a critical section, because it
allocates memory. But CreateCheckPoint() did that, when called for the
end-of-recovery checkpoint by the startup process.
In the passing, fix the scratch space allocation in InitXLogInsert to go to
the right memory context. Also update the comment at InitXLOGAccess, which
hasn't been totally accurate since hot standby was introduced (in a hot
standby backend, InitXLOGAccess isn't called at backend startup).
Fujii Masao [Thu, 27 Nov 2014 17:42:43 +0000 (02:42 +0900)]
Make \watch respect the user's \pset null setting.
Previously \watch always ignored the user's \pset null setting.
\pset null setting should be ignored for \d and similar queries.
For those, the code can reasonably have an opinion about what
the presentation should be like, since it knows what SQL query
it's issuing. This argument surely doesn't apply to \watch,
so this commit makes \watch use the user's \pset null setting.
Tom Lane [Thu, 27 Nov 2014 16:12:44 +0000 (11:12 -0500)]
Free libxml2/libxslt resources in a safer order.
Mark Simonetti reported that libxslt sometimes crashes for him, and that
swapping xslt_process's object-freeing calls around to do them in reverse
order of creation seemed to fix it. I've not reproduced the crash, but
valgrind clearly shows a reference to already-freed memory, which is
consistent with the idea that shutdown of the xsltTransformContext is
trying to reference the already-freed stylesheet or input document.
With this patch, valgrind is no longer unhappy.
I have an inquiry in to see if this is a libxslt bug or if we're just
abusing the library; but even if it's a library bug, we'd want to adjust
our code so it doesn't fail with unpatched libraries.
Back-patch to all supported branches, because we've been doing this in
the wrong(?) order for a long time.
Stephen Frost [Thu, 27 Nov 2014 06:06:36 +0000 (01:06 -0500)]
Rename pg_rowsecurity -> pg_policy and other fixes
As pointed out by Robert, we should really have named pg_rowsecurity
pg_policy, as the objects stored in that catalog are policies. This
patch fixes that and updates the column names to start with 'pol' to
match the new catalog name.
The security consideration for COPY with row level security, also
pointed out by Robert, has also been addressed by remembering and
re-checking the OID of the relation initially referenced during COPY
processing, to make sure it hasn't changed under us by the time we
finish planning out the query which has been built.
Robert and Alvaro also commented on missing OCLASS and OBJECT entries
for POLICY (formerly ROWSECURITY or POLICY, depending) in various
places. This patch fixes that too, which also happens to add the
ability to COMMENT on policies.
In passing, attempt to improve the consistency of messages, comments,
and documentation as well. This removes various incarnations of
'row-security', 'row-level security', 'Row-security', etc, in favor
of 'policy', 'row level security' or 'row_security' as appropriate.
Tom Lane [Tue, 25 Nov 2014 20:16:49 +0000 (15:16 -0500)]
Fix uninitialized-variable warning.
In passing, add an Assert defending the presumption that bytes_left
is positive to start with. (I'm not exactly convinced that using an
unsigned type was such a bright thing here, but let's at least do
this much.)
Tom Lane [Tue, 25 Nov 2014 20:02:09 +0000 (15:02 -0500)]
De-reserve most statement-introducing keywords in plpgsql.
Add a bit of context sensitivity to plpgsql_yylex() so that it can
recognize when the word it is looking at is the first word of a new
statement, and if so whether it is the target of an assignment statement.
When we are at start of statement and it's not an assignment, we can
prefer recognizing unreserved keywords over recognizing variable names,
thereby allowing most statements' initial keywords to be demoted from
reserved to unreserved status. This is rather useful already (there are
15 such words that get demoted here), and what's more to the point is
that future patches proposing to add new plpgsql statements can avoid
objections about having to add new reserved words.
The keywords BEGIN, DECLARE, FOR, FOREACH, LOOP, WHILE need to remain
reserved because they can be preceded by block labels, and the logic
added here doesn't understand about block labels. In principle we
could probably fix that, but it would take more than one token of
lookback and the benefit doesn't seem worth extra complexity.
Also note I didn't de-reserve EXECUTE, because it is used in more places
than just statement start. It's possible it could be de-reserved with
more work, but that would be an independent fix.
In passing, also de-reserve COLLATE and DEFAULT, which shouldn't have
been reserved in the first place since they only need to be recognized
within DECLARE sections.
Tom Lane [Tue, 25 Nov 2014 17:21:22 +0000 (12:21 -0500)]
Support arrays as input to array_agg() and ARRAY(SELECT ...).
These cases formerly failed with errors about "could not find array type
for data type". Now they yield arrays of the same element type and one
higher dimension.
The implementation involves creating functions with API similar to the
existing accumArrayResult() family. I (tgl) also extended the base family
by adding an initArrayResult() function, which allows callers to avoid
special-casing the zero-inputs case if they just want an empty array as
result. (Not all do, so the previous calling convention remains valid.)
This allowed simplifying some existing code in xml.c and plperl.c.
Ali Akbar, reviewed by Pavel Stehule, significantly modified by me
Stephen Frost [Tue, 25 Nov 2014 16:48:16 +0000 (11:48 -0500)]
Add int64 -> int8 mapping to genbki
Per discussion with Tom and Andrew, 64bit integers are no longer a
problem for the catalogs, so go ahead and add the mapping from the C
int64 type to the int8 SQL identification to allow using them.
The old method of appending options to the connection string didn't work if
the primary_conninfo was a postgres:// style URI, instead of a traditional
connection string. Use PQconnectdbParams instead.
Allow "dbname" from connection string to be overridden in PQconnectDBParams
If the "dbname" attribute in PQconnectDBParams contained a connection string
or URI (and expand_dbname = TRUE), the database name from the connection
string could not be overridden by a subsequent "dbname" keyword in the
array. That was not intentional; all other options can be overridden.
Furthermore, any subsequent "dbname" caused the connection string from the
first dbname value to be processed again, overriding any values for the same
options that were given between the connection string and the second dbname
option.
In the passing, clarify in the docs that only the first dbname option in the
array is parsed as a connection string.
Alex Shulgin. Backpatch to all supported versions.
Stephen Frost [Tue, 25 Nov 2014 14:49:48 +0000 (09:49 -0500)]
Suppress DROP CASCADE notices in regression tests
In the regression tests, when doing cascaded drops, we need to suppress
the notices from DROP CASCADE or there can be transient regression
failures as the order of drops can depend on the physical row order in
pg_depend.
Check return value of strdup() in libpq connection option parsing.
An out-of-memory in most of these would lead to strange behavior, like
connecting to a different database than intended, but some would lead to
an outright segfault.
Alex Shulgin and me. Backpatch to all supported versions.
Make Port->ssl_in_use available, even when built with !USE_SSL
Code that check the flag no longer need #ifdef's, which is more convenient.
In particular, makes it easier to write extensions that depend on it.
In the passing, modify sslinfo's ssl_is_used function to check ssl_in_use
instead of the OpenSSL specific 'ssl' pointer. It doesn't make any
difference currently, as sslinfo is only compiled when built with OpenSSL,
but seems cleaner anyway.
Distinguish XLOG_FPI records generated for hint-bit updates.
Add a new XLOG_FPI_FOR_HINT record type, and use that for full-page images
generated for hint bit updates, when checksums are enabled. The new record
type is replayed exactly the same as XLOG_FPI, but allows them to be tallied
separately e.g. in pg_xlogdump.
Tom Lane [Sun, 23 Nov 2014 20:31:32 +0000 (15:31 -0500)]
Get rid of redundant production in plpgsql grammar.
There may once have been a reason for the intermediate proc_stmts
production in the plpgsql grammar, but it isn't doing anything useful
anymore, so let's collapse it into proc_sect. Saves some code and
probably a small number of nanoseconds per statement list.
In passing, correctly alphabetize keyword lists to match pl_scanner.c;
note that for "rowtype" vs "row_count", pl_scanner.c must sort on the
basis of the lower-case spelling.
Noted while fooling with a patch to de-reserve more plpgsql keywords.
Tom Lane [Sun, 23 Nov 2014 00:12:38 +0000 (19:12 -0500)]
Allow simplification of EXISTS() subqueries containing LIMIT.
The locution "EXISTS(SELECT ... LIMIT 1)" seems to be rather common among
people who don't realize that the database already performs optimizations
equivalent to putting LIMIT 1 in the sub-select. Unfortunately, this was
actually making things worse, because it prevented us from optimizing such
EXISTS clauses into semi or anti joins. Teach simplify_EXISTS_query() to
suppress constant-positive LIMIT clauses. That fixes the semi/anti-join
case, and may help marginally even for cases that have to be left as
sub-SELECTs.
Tom Lane [Sat, 22 Nov 2014 21:01:05 +0000 (16:01 -0500)]
Fix mishandling of system columns in FDW queries.
postgres_fdw would send query conditions involving system columns to the
remote server, even though it makes no effort to ensure that system
columns other than CTID match what the remote side thinks. tableoid,
in particular, probably won't match and might have some use in queries.
Hence, prevent sending conditions that include non-CTID system columns.
Also, create_foreignscan_plan neglected to check local restriction
conditions while determining whether to set fsSystemCol for a foreign
scan plan node. This again would bollix the results for queries that
test a foreign table's tableoid.
Back-patch the first fix to 9.3 where postgres_fdw was introduced.
Back-patch the second to 9.2. The code is probably broken in 9.1 as
well, but the patch doesn't apply cleanly there; given the weak state
of support for FDWs in 9.1, it doesn't seem worth fixing.
Etsuro Fujita, reviewed by Ashutosh Bapat, and somewhat modified by me
Andrew Dunstan [Sat, 22 Nov 2014 14:39:01 +0000 (09:39 -0500)]
Rework echo_hidden for \sf and \ef from commit e4d2817.
PSQLexec's error reporting turns out to be too verbose for this case, so
revert to using PQexec instead with minimal error reporting. Prior to
calling PQexec, we call a function that mimics just the echo_hidden
piece of PSQLexec.
Tom Lane [Fri, 21 Nov 2014 23:21:46 +0000 (18:21 -0500)]
Rearrange CustomScan API.
Make it work more like FDW plans do: instead of assuming that there are
expressions in a CustomScan plan node that the core code doesn't know
about, insist that all subexpressions that need planner attention be in
a "custom_exprs" list in the Plan representation. (Of course, the
custom plugin can break the list apart again at executor initialization.)
This lets us revert the parts of the patch that exposed setrefs.c and
subselect.c processing to the outside world.
Also revert the GetSpecialCustomVar stuff in ruleutils.c; that concept
may work in future, but it's far from fully baked right now.
Tom Lane [Fri, 21 Nov 2014 19:05:46 +0000 (14:05 -0500)]
Simplify API for initially hooking custom-path providers into the planner.
Instead of register_custom_path_provider and a CreateCustomScanPath
callback, let's just provide a standard function hook in set_rel_pathlist.
This is more flexible than what was previously committed, is more like the
usual conventions for planner hooks, and requires less support code in the
core. We had discussed this design (including centralizing the
set_cheapest() calls) back in March or so, so I'm not sure why it wasn't
done like this already.
Andrew Dunstan [Fri, 21 Nov 2014 17:37:09 +0000 (12:37 -0500)]
Fix an error in psql that overcounted output lines.
This error counted the first line of a cell as "extra". The effect was
to cause far too frequent invocation of the pager. In most cases this
can be worked around (for example, by using the "less" pager with the -F
flag), so don't backpatch.
Andrew Dunstan [Fri, 21 Nov 2014 17:14:05 +0000 (12:14 -0500)]
Make psql's \sf and \ef honor ECHO_HIDDEN.
These commands were calling the database direct rather than calling
PSQLexec like other slash commands that needed database data.
The code is also changed not to pass the connection as a parameter to
the helper functions. It's available in a global variable, and that's
what PSQLexec uses.
Tom Lane [Fri, 21 Nov 2014 01:20:54 +0000 (20:20 -0500)]
Remove dead code supporting mark/restore in SeqScan, TidScan, ValuesScan.
There seems no prospect that any of this will ever be useful, and indeed
it's questionable whether some of it would work if it ever got called;
it's certainly not been exercised in a very long time, if ever. So let's
get rid of it, and make the comments about mark/restore in execAmi.c less
wishy-washy.
The mark/restore support for Result nodes is also currently dead code,
but that's due to planner limitations not because it's impossible that
it could be useful. So I left it in.
Also, rearrange the CustomFoo struct/typedef definitions so that all the
typedef names are seen as used by the compiler. Without this pgindent
will mess things up a bit, which is not so important perhaps, but it also
removes a bizarre discrepancy between the declaration arrangement used for
CustomExecMethods and that used for CustomScanMethods and
CustomPathMethods.
Clean up the commentary around ExecSupportsMarkRestore to reflect the
rather large change in its API.
Const-ify register_custom_path_provider's argument. This necessitates
casting away const in the function, but that seems better than forcing
callers of the function to do so (or else not const-ify their method
pointer structs, which was sort of the whole point).
De-export fix_expr_common. I don't like the exporting of fix_scan_expr
or replace_nestloop_params either, but this one surely has got little
excuse.
Tom Lane [Thu, 20 Nov 2014 20:56:39 +0000 (15:56 -0500)]
Fix another oversight in CustomScan patch.
execCurrent.c's search_plan_tree() must recognize a CustomScan on the
target relation. This would only be helpful for custom providers that
support CurrentOfExpr quals, which is probably a bit far-fetched, but
it's not impossible I think. But even without assuming that, we need
to recognize a scanned-relation match so that we will properly throw
error if the desired relation is being scanned with both a CustomScan
and a regular scan (ie, self-join).
Also recognize ForeignScanState for similar reasons. Supporting WHERE
CURRENT OF on a foreign table is probably even more far-fetched than
it is for custom scans, but I think in principle you could do it with
postgres_fdw (or another FDW that supports the ctid column). This
would be a back-patchable bug fix if existing FDWs handled CurrentOfExpr,
but I doubt any do so I won't bother back-patching.
Tom Lane [Thu, 20 Nov 2014 18:16:14 +0000 (13:16 -0500)]
Remove no-longer-needed phony typedefs in genbki.h.
Now that we have a policy of hiding varlena catalog fields behind
"#ifdef CATALOG_VARLEN", there is no need for their type names to be
acceptable to the C compiler. And experimentation shows that it does
not matter to pgindent either. (If it did, we'd have problems anyway,
since these typedefs are unreferenced so far as the C compiler is
concerned, and find_typedef fails to identify such typedefs.)
Hence, remove the phony typedefs that genbki.h provided to make
some varlena field definitions compilable.
In passing, rearrange #define's into what seemed a more logical order.
Silence compiler warning about variable being used uninitialized.
It's a false positive - the variable is only used when 'onleft' is true,
and it is initialized in that case. But the compiler doesn't necessarily
see that.
Each WAL record now carries information about the modified relation and
block(s) in a standardized format. That makes it easier to write tools that
need that information, like pg_rewind, prefetching the blocks to speed up
recovery, etc.
There's a whole new API for building WAL records, replacing the XLogRecData
chains used previously. The new API consists of XLogRegister* functions,
which are called for each buffer and chunk of data that is added to the
record. The new API also gives more control over when a full-page image is
written, by passing flags to the XLogRegisterBuffer function.
This also simplifies the XLogReadBufferForRedo() calls. The function can dig
the relation and block number from the WAL record, so they no longer need to
be passed as arguments.
For the convenience of redo routines, XLogReader now disects each WAL record
after reading it, copying the main data part and the per-block data into
MAXALIGNed buffers. The data chunks are not aligned within the WAL record,
but the redo routines can assume that the pointers returned by XLogRecGet*
functions are. Redo routines are now passed the XLogReaderState, which
contains the record in the already-disected format, instead of the plain
XLogRecord.
The new record format also makes the fixed size XLogRecord header smaller,
by removing the xl_len field. The length of the "main data" portion is now
stored at the end of the WAL record, and there's a separate header after
XLogRecord for it. The alignment padding at the end of XLogRecord is also
removed. This compansates for the fact that the new format would otherwise
be more bulky than the old format.
Reviewed by Andres Freund, Amit Kapila, Michael Paquier, Alvaro Herrera,
Fujii Masao.