- 04 Dec, 2015 1 commit
-
-
Tom Lane authored
In commit 1ea0c73c I added a section to user-manag.sgml about how to drop roles that own objects; but as pointed out by Stephen Frost, I neglected that shared objects (databases or tablespaces) may need special treatment. Fix that. Back-patch to supported versions, like the previous patch.
-
- 03 Dec, 2015 6 commits
-
-
Alvaro Herrera authored
As pointed out by Fujii Masao, we weren't quite there on a standby behaving sanely: first because we were failing to acquire the correct state in the case where no XLOG_PARAMETER_CHANGE message was sent (because a checkpoint had already happened after the setting was changed in the master, and then the standby was restarted); and second because promoting the standby with the feature enabled failed to activate it if the master had the feature disabled. This patch fixes both those misbehaviors hopefully without re-introducing any old problems. Also change the hint emitted in a standby together with the error message about the feature being disabled, to make it point out that the place to chance the setting is the master. Otherwise, if the setting is already enabled in the standby, it is very confusing to have it say that the setting must be enabled ... Authors: Álvaro Herrera, Petr Jelínek. Backpatch to 9.5.
-
Tom Lane authored
Formerly, if "psql -o foo" failed to open the output file "foo", it would print an error message but then carry on as though -o had not been specified at all. This seems contrary to expectation: a program that cannot open its output file normally fails altogether. Make psql do exit(1) after reporting the error. If "\o foo" failed to open "foo", it would print an error message but then reset the output file to stdout, as if the argument had been omitted. This is likewise pretty surprising behavior. Make it keep the previous output state, instead. psql keeps SIGPIPE interrupts disabled when it is writing to a pipe, either a pipe specified by -o/\o or a transient pipe opened for purposes such as using a pager on query output. The logic for this was too simple and could sometimes re-enable SIGPIPE when a -o pipe was still active, thus possibly leading to an unexpected psql crash later. Fixing the last point required getting rid of the kluge in PrintQueryTuples and ExecQueryUsingCursor whereby they'd transiently change the global queryFout state, but that seems like good cleanup anyway. Back-patch to 9.5 but not further; these are minor-enough issues that changing the behavior in stable branches doesn't seem appropriate.
-
Peter Eisentraut authored
-
Peter Eisentraut authored
-
Peter Eisentraut authored
-
Alvaro Herrera authored
Michael Paquier
-
- 02 Dec, 2015 3 commits
-
-
Tom Lane authored
The formatting modes that depend on knowledge of the terminal window width did not work right when printing a query result that's been fetched in sections (as a result of FETCH_SIZE). ExecQueryUsingCursor() would force use of the pager as soon as there's more than one result section, and then print.c would see an output file pointer that's not stdout and incorrectly conclude that the terminal window width isn't relevant. This has been broken all along for non-expanded "wrapped" output format, and as of 9.5 the issue affects expanded mode as well. The problem also caused "\pset expanded auto" mode to invariably *not* switch to expanded output in a segmented result, which seems to me to be exactly backwards. To fix, we need to pass down an "is_pager" flag to inform the print.c subroutines that some calling level has already replaced stdout with a pager pipe, so they should (a) not do that again and (b) nonetheless honor the window size. (Notably, this makes the first is_pager test in print_aligned_text() not be dead code anymore.) This patch is a bit invasive because there are so many existing calls of printQuery()/printTable(), but fortunately all but a couple can just pass "false" for the added parameter. Back-patch to 9.5 but no further. Given the lack of field complaints, it's not clear that we should change the behavior in stable branches. Also, the API change for printQuery()/printTable() might possibly break third-party code, again something we don't like to do in stable branches. However, it's not quite too late to do this in 9.5, and with the larger scope of the problem there, it seems worth doing.
-
Alvaro Herrera authored
The original code was a bit clunky; make it more amenable for further reuse by creating a new Perl package PostgresNode, which is an object-oriented representation of a single server, with some support routines such as init, start, stop, psql. This serves as a better basis on which to build further test code, and enables writing tests that use more than one server without too much complication. This commit modifies a lot of the existing test files, mostly to remove explicit calls to system commands (pg_ctl) replacing them with method calls of a PostgresNode object. The result is quite a bit more straightforward. Also move some initialization code to BEGIN and INIT blocks instead of having it straight in as top-level code. This commit also introduces package RecursiveCopy so that we can copy whole directories without having to depend on packages that may not be present on vanilla Perl 5.8 installations. I also ran perltidy on the modified files, which changes some code sites that are not otherwise touched by this patch. I tried to avoid this, but it ended up being more trouble than it's worth. Authors: Michael Paquier, Álvaro Herrera Review: Noah Misch
-
Robert Haas authored
Peter Geoghegan
-
- 01 Dec, 2015 5 commits
-
-
Tom Lane authored
We tried to fetch statistics data from the index metapage, which does not work if the index isn't actually present. If the index is hypothetical, instead extrapolate some plausible internal statistics based on the index page count provided by the index-advisor plugin. There was already some code in gincostestimate() to invent internal stats in this way, but since it was only meant as a stopgap for pre-9.1 GIN indexes that hadn't been vacuumed since upgrading, it was pretty crude. If we want it to support index advisors, we should try a little harder. A small amount of testing says that it's better to estimate the entry pages as 90% of the index, not 100%. Also, estimating the number of entries (keys) as equal to the heap tuple count could be wildly wrong in either direction. Instead, let's estimate 100 entries per entry page. Perhaps someday somebody will want the index advisor to be able to provide these numbers more directly, but for the moment this should serve. Problem report and initial patch by Julien Rouhaud; modified by me to invent less-bogus internal statistics. Back-patch to all supported branches, since we've supported index advisors since 9.0.
-
Tom Lane authored
Don't force the data width to extend all the way to the right margin if it doesn't need to. This reverts the behavior in non-wrapping cases to be what it was in 9.4. Also, make the logic that ensures the data line width is at least equal to the record-header line width a little less obscure. In passing, avoid possible calculation of log10(0). Probably that's harmless, given the lack of field complaints, but it seems risky: conversion of NaN to an integer isn't well defined.
-
Tom Lane authored
The previous coding could overrun the provided buffer size for a very large input, or lose precision for a very small input. Adopt the methodology that's been in use in the equivalent backend code for a long time. Per private report from Bas van Schaik. Back-patch to all supported branches.
-
Tom Lane authored
We should ignore output_columns unless it's greater than zero. A zero means we couldn't get any information from ioctl(TIOCGWINSZ); in that case the expected behavior is to print the data at native width, not to wrap it at the smallest possible value. print_aligned_text() gets this consideration right, but print_aligned_vertical() lost track of this detail somewhere along the line.
-
Teodor Sigaev authored
Allow pg_rewind to work when target timeline was switched. Now user can return promoted standby to old master. Target timeline history becomes a global variable. Index in target timeline history is used in function interfaces instead of specifying TLI directly. Thus, SimpleXLogPageRead() can easily start reading XLOGs from next timeline when current timeline ends. Author: Alexander Korotkov Review: Michael Paquier
-
- 30 Nov, 2015 2 commits
-
-
Tom Lane authored
This area was rather heavily whacked around in 6513633b and follow-on commits, and it was showing it, because the logic to calculate the allowable data width in wrapped expanded mode had only the vaguest relationship to the logic that was actually printing the data. It was not very close to being right about the conditions requiring overhead columns to be added. Aside from being wrong, it was pretty unreadable and under-commented. Rewrite it so it corresponds to what the printing code actually does. In passing, remove a couple of dead tests in the printing logic, too. Per a complaint from Jeff Janes, though this doesn't look much like his patch because it fixes a number of other corner-case bogosities too. One such fix that's visible in the regression test results is that although the code was attempting to enforce a minimum data width of 3 columns, it sometimes left less space than that available.
-
Robert Haas authored
It's amazing how fast things become obsolete these days. Amit Langote
-
- 29 Nov, 2015 1 commit
-
-
Tom Lane authored
In commit 8abb3cda I attempted to cache the expression state trees constructed for domain CHECK constraints for the life of the backend (assuming the domain's constraints don't get redefined). However, this turns out not to work very well, because execQual.c will run those state trees with ecxt_per_query_memory pointing to a query-lifespan context, and in some situations we'll end up with pointers into that context getting stored into the state trees. This happens in particular with SQL-language functions, as reported by Emre Hasegeli, but there are many other cases. To fix, keep only the expression plan trees for domain CHECK constraints in the typcache's data structure, and revert to performing ExecInitExpr (at least) once per query to set up expression state trees in the query's context. Eventually it'd be nice to undo this, but that will require some careful thought about memory management for expression state trees, and it seems far too late for any such redesign in 9.5. This way is still much more efficient than what happened before 8abb3cda.
-
- 28 Nov, 2015 1 commit
-
-
Tom Lane authored
Previously, we did many conversions for Cyrillic and Central European single-byte encodings by converting to a related MULE_INTERNAL coding scheme before converting to the destination. This seems unnecessarily inefficient. Moreover, if the conversion encounters an untranslatable character, the error message will confusingly complain about failure to convert to or from MULE_INTERNAL, rather than the user-visible encodings. Worse still, this approach results in some completely unnecessary conversion failures; there are cases where the chosen MULE subset lacks characters that exist in both of the user-visible encodings, causing a conversion failure that need not occur. This patch fixes the first two of those deficiencies by introducing a new local2local() conversion support subroutine for direct conversion between any two single-byte character sets, and adding new conversion tables where needed. However, I generated the new conversion tables by testing PG 9.5's behavior, so that the actual conversion behavior is bug-compatible with previous releases; the only user-visible behavior change is that the error messages for conversion failures are saner. Changes in the conversion behavior will probably ensue after discussion. Interestingly, although this approach requires more tables, the .so files actually end up smaller (at least on my x86_64 machine); the tables are smaller than the management code needed for double conversion. Per a complaint from Albe Laurenz.
-
- 27 Nov, 2015 4 commits
-
-
Tom Lane authored
-
Tom Lane authored
Some of the Unicode/*.map files had identification comments added to them, evidently by hand. Others did not. Modify the generating scripts to produce these comments automatically, and update the generated files that lacked them. This is just minor cleanup as a by-product of trying to verify that the *.map files can indeed be reproduced from authoritative data. There are a depressingly large number that fail to reproduce from the claimed sources. I have not touched those in this commit, except for the JIS 2004-related files which required only a single comment update to match. Since this only affects comments, no need to consider a back-patch.
-
Tom Lane authored
Previously, if no host information had been specified at connection time, PQhost() would return NULL (unless you are on Windows, in which case you got "localhost"). This is an unhelpful definition for a couple of reasons: it can cause corner-case crashes in applications (cf commit c5ef8ce5), and there's no well-defined way for applications to find out the socket directory path that's actually in use. As an example of the latter problem, psql substituted DEFAULT_PGSOCKET_DIR for NULL in a couple of places, but this is subtly wrong because it's conceivable that psql is using a libpq shared library that was built with a different setting. Hence, change PQhost() to return DEFAULT_PGSOCKET_DIR when appropriate, and strip out the now-dead substitutions in psql. (There is still one remaining reference to DEFAULT_PGSOCKET_DIR in psql, in prompt.c, which I don't see a nice way to get rid of. But it only controls a prompt abbreviation decision, so it seems noncritical.) Also update the docs for PQhost, which had never previously mentioned the possibility of a socket directory path being returned. In passing fix the outright-incorrect code comment about PGconn.pgunixsocket.
-
Teodor Sigaev authored
Attached is a patch for being able to do COPY (query) without a CTE. Author: Marko Tiikkaja Review: Michael Paquier
-
- 26 Nov, 2015 1 commit
-
-
Tom Lane authored
Failure to initially palloc the comboCids array, or to realloc it bigger when needed, left combocid's data structures in an inconsistent state that would cause trouble if the top transaction continues to execute. Noted while examining a user complaint about the amount of memory used for this. (There's not much we can do about that, but it does point up that repalloc failure has a non-negligible chance of occurring here.) In HEAD/9.5, also avoid possible invocation of memcpy() with a null pointer in SerializeComboCIDState; cf commit 13bba022.
-
- 25 Nov, 2015 4 commits
-
-
Tom Lane authored
PQhost() can return NULL in non-error situations, namely when a Unix-socket connection has been selected by default. That behavior is a tad debatable perhaps, but for the moment we should make sure that psql copes with it. Unfortunately, do_connect() failed to: it could pass a NULL pointer to strcmp(), resulting in crashes on most platforms. This was reported as a security issue by ChenQin of Topsec Security Team, but the consensus of the security list is that it's just a garden-variety bug with no security implications. For paranoia's sake, I made the keep_password test not trust PQuser or PQport either, even though I believe those will never return NULL given a valid PGconn. Back-patch to all supported branches.
-
Tom Lane authored
The integer overflow situation in div_var_fast() is a great deal more complicated than the pre-existing comments would suggest. Moreover, the comments were also flat out incorrect as to the precise statement of the maxdiv loop invariant. Upon clarifying that, it becomes apparent that the way in which we updated maxdiv after a carry propagation pass was overly slow, complex, and conservative: we can just reset it to one, which is much easier and also reduces the number of times carry propagation occurs. Fix that and improve the relevant comments. Since this is mostly a comment fix, with only a rather marginal performance boost, no need for back-patch. Tom Lane and Dean Rasheed
-
Teodor Sigaev authored
-
Teodor Sigaev authored
Now pageinspect can show data stored in the heap tuple. Nikolay Shaplov
-
- 24 Nov, 2015 2 commits
-
-
Bruce Momjian authored
Also fix getErrorText() to return the right error string on failure. This behavior now matches that of other operating systems. Report by Noah Misch Backpatch through 9.1
-
Peter Eisentraut authored
-
- 23 Nov, 2015 2 commits
-
-
Teodor Sigaev authored
Per http://www.postgresql.org/message-id/flat/564C4CE6.9000509@postgrespro.ru Pavel Luzanov <p.luzanov@postgrespro.ru>
-
Peter Eisentraut authored
from Michael Paquier
-
- 22 Nov, 2015 1 commit
-
-
Tom Lane authored
The POSIX standard for tar headers requires archive member sizes to be printed in octal with at most 11 digits, limiting the representable file size to 8GB. However, GNU tar and apparently most other modern tars support a convention in which oversized values can be stored in base-256, allowing any practical file to be a tar member. Adopt this convention to remove two limitations: * pg_dump with -Ft output format failed if the contents of any one table exceeded 8GB. * pg_basebackup failed if the data directory contained any file exceeding 8GB. (This would be a fatal problem for installations configured with a table segment size of 8GB or more, and it has also been seen to fail when large core dump files exist in the data directory.) File sizes under 8GB are still printed in octal, so that no compatibility issues are created except in cases that would have failed entirely before. In addition, this patch fixes several bugs in the same area: * In 9.3 and later, we'd defined tarCreateHeader's file-size argument as size_t, which meant that on 32-bit machines it would write a corrupt tar header for file sizes between 4GB and 8GB, even though no error was raised. This broke both "pg_dump -Ft" and pg_basebackup for such cases. * pg_restore from a tar archive would fail on tables of size between 4GB and 8GB, on machines where either "size_t" or "unsigned long" is 32 bits. This happened even with an archive file not affected by the previous bug. * pg_basebackup would fail if there were files of size between 4GB and 8GB, even on 64-bit machines. * In 9.3 and later, "pg_basebackup -Ft" failed entirely, for any file size, on 64-bit big-endian machines. In view of these potential data-loss bugs, back-patch to all supported branches, even though removal of the documented 8GB limit might otherwise be considered a new feature rather than a bug fix.
-
- 20 Nov, 2015 2 commits
-
-
Tom Lane authored
The previous way of reconstructing check constraints was to do a separate "ALTER TABLE ONLY tab ADD CONSTRAINT" for each table in an inheritance hierarchy. However, that way has no hope of reconstructing the check constraints' own inheritance properties correctly, as pointed out in bug #13779 from Jan Dirk Zijlstra. What we should do instead is to do a regular "ALTER TABLE", allowing recursion, at the topmost table that has a particular constraint, and then suppress the work queue entries for inherited instances of the constraint. Annoyingly, we'd tried to fix this behavior before, in commit 5ed6546c, but we failed to notice that it wasn't reconstructing the pg_constraint field values correctly. As long as I'm touching pg_get_constraintdef_worker anyway, tweak it to always schema-qualify the target table name; this seems like useful backup to the protections installed by commit 5f173040. In HEAD/9.5, get rid of get_constraint_relation_oids, which is now unused. (I could alternatively have modified it to also return conislocal, but that seemed like a pretty single-purpose API, so let's not pretend it has some other use.) It's unused in the back branches as well, but I left it in place just in case some third-party code has decided to use it. In HEAD/9.5, also rename pg_get_constraintdef_string to pg_get_constraintdef_command, as the previous name did nothing to explain what that entry point did differently from others (and its comment was equally useless). Again, that change doesn't seem like material for back-patching. I did a bit of re-pgindenting in tablecmds.c in HEAD/9.5, as well. Otherwise, back-patch to all supported branches.
-
Robert Haas authored
The previous coding attempts to destroy the DSM in this case, but child nodes might have stored data there and still be holding onto pointers in this case. So don't do that. Also, free the reader array instead of leaking it. Extracted from two different patch versions both by Amit Kapila.
-
- 19 Nov, 2015 5 commits
-
-
Robert Haas authored
Amit Langote
-
Robert Haas authored
Reported by Andres Freund.
-
Tom Lane authored
Some versions of Perl export a macro named HS_KEY. This creates a conflict in contrib/hstore_plperl against hstore's macro of the same name. The most future-proof solution seems to be to rename our macro; I chose HSTORE_KEY. For consistency, rename HS_VAL and related macros similarly. Back-patch to 9.5. contrib/hstore_plperl doesn't exist before that so there is no need to worry about the conflict in older releases. Per reports from Marco Atzeri and Mike Blackwell.
-
Peter Eisentraut authored
-
Tom Lane authored
Silly mistake in my commit 09cecdf2, reported by Erik Rijkers. The fact that the buildfarm didn't find this implies that we are not testing Perl builds that lack MULTIPLICITY, which is a bit disturbing from a coverage standpoint. Until today I'd have said nobody cared about such configurations anymore; but maybe not.
-