- 05 Apr, 2016 4 commits
-
-
Robert Haas authored
This lets us use parallel aggregate for a variety of useful cases that didn't work before, like sum(int8), sum(numeric), several versions of avg(), and various other functions. Add some regression tests, as well, testing the general sanity of these and future catalog entries. David Rowley, reviewed by Tomas Vondra, with a few further changes by me.
-
Magnus Hagander authored
Previously non-exclusive backups had to be done using the replication protocol and pg_basebackup. With this commit it's now possible to make them using pg_start_backup/pg_stop_backup as well, as long as the backup program can maintain a persistent connection to the database. Doing this, backup_label and tablespace_map are returned as results from pg_stop_backup() instead of being written to the data directory. This makes the server safe from a crash during an ongoing backup, which can be a problem with exclusive backups. The old syntax of the functions remain and work exactly as before, but since the new syntax is safer this should eventually be deprecated and removed. Only reference documentation is included. The main section on backup still needs to be rewritten to cover this, but since that is already scheduled for a separate large rewrite, it's not included in this patch. Reviewed by David Steele and Amit Kapila
-
Magnus Hagander authored
Etsuro Fujita
-
Peter Eisentraut authored
found by Ian Barwick
-
- 04 Apr, 2016 12 commits
-
-
Tom Lane authored
As noted by Julian Schauder in bug #14063, the configuration-file parser doesn't support embedded newlines in string literals. While there might someday be a good reason to remove that restriction, there doesn't seem to be one right now. However, ALTER SYSTEM SET could accept strings containing newlines, since many of the variable-specific value-checking routines would just see a newline as whitespace. This led to writing a postgresql.auto.conf file that was broken and had to be removed manually. Pending a reason to work harder, just throw an error if someone tries this. In passing, fix several places in the ALTER SYSTEM logic that failed to provide an errcode() for an ereport(), and thus would falsely log the failure as an internal XX000 error. Back-patch to 9.4 where ALTER SYSTEM was introduced.
-
Alvaro Herrera authored
This makes it easier to identify the source of a recovery problem in case of a bug or data corruption.
-
Tom Lane authored
Alex Shulgin complained that the underlying strategy wasn't all that apparent, particularly not the fact that we intentionally have two code paths depending on whether we think the column has a limited set of possible values or not. Try to make it clearer.
-
Tom Lane authored
On reflection, the pre-existing logic in ANALYZE is specifically meant to compare the frequency of a candidate MCV against the estimated frequency of a random distinct value across the whole table. The change to compare it against the average frequency of values actually seen in the sample doesn't seem very principled, and if anything it would make us less likely not more likely to consider a value an MCV. So revert that, but keep the aspect of considering only nonnull values, which definitely is correct. In passing, rename the local variables in these stanzas to "ndistinct_table", to avoid confusion with the "ndistinct" that appears at an outer scope in compute_scalar_stats.
-
Alvaro Herrera authored
Reported by Peter Eisentraut to occur on 32bit systems
-
Tom Lane authored
\gexec executes the just-entered query, like \g, but instead of printing the results it takes each field as a SQL command to send to the server. Computing a series of queries to be executed is a fairly common thing, but up to now you always had to resort to kluges like writing the queries to a file and then inputting the file. Now it can be done with no intermediate step. The implementation is fairly straightforward except for its interaction with FETCH_COUNT. ExecQueryUsingCursor isn't capable of being called recursively, and even if it were, its need to create a transaction block interferes unpleasantly with the desired behavior of \gexec after a failure of a generated query (i.e., that it can continue). Therefore, disable use of ExecQueryUsingCursor when doing the master \gexec query. We can still apply it to individual generated queries, however, and there might be some value in doing so. While testing this feature's interaction with single-step mode, I (tgl) was led to conclude that SendQuery needs to recognize SIGINT (cancel_pressed) as a negative response to the single-step prompt. Perhaps that's a back-patchable bug fix, but for now I just included it here. Corey Huinker, reviewed by Jim Nasby, Daniel Vérité, and myself
-
Tom Lane authored
This elevel is useful for logging audit messages and similar information that should not be passed to the client. It's equivalent to LOG in terms of decisions about logging priority in the postmaster log, but messages with this elevel will never be sent to the client. In the current implementation, it's just an alias for the longstanding COMMERROR elevel (or more accurately, we've made COMMERROR an alias for this). At some point it might be interesting to allow a LOG_ONLY flag to be attached to any elevel, but that would be considerably more complicated, and it's not clear there's enough use-cases to justify the extra work. For now, let's just take the easy 90% solution. David Steele, reviewed by Fabien Coelho, Petr Jelínek, and myself
-
Tom Lane authored
The first iteration of the signal-checking loop would compute sigmask(0) which expands to 1<<(-1) which is undefined behavior according to the C standard. The lack of field reports of trouble suggest that it evaluates to 0 on all existing Windows compilers, but that's hardly something to rely on. Since signal 0 isn't a queueable signal anyway, we can just make the loop iterate from 1 instead, and save a few cycles as well as avoiding the undefined behavior. In passing, avoid evaluating the volatile expression UNBLOCKED_SIGNAL_QUEUE twice in a row; there's no reason to waste cycles like that. Noted by Aleksander Alekseev, though this isn't his proposed fix. Back-patch to all supported branches.
-
Teodor Sigaev authored
Michael Paquier
-
Teodor Sigaev authored
Andreas Ulbrich
-
Dean Rasheed authored
When adjusting the estimate for the number of distinct values from a rel in a grouped query to take into account the selectivity of the rel's restrictions, use a formula that is less likely to produce under-estimates. The old formula simply multiplied the number of distinct values in the rel by the restriction selectivity, which would be correct if the restrictions were fully correlated with the grouping expressions, but can produce significant under-estimates in cases where they are not well correlated. The new formula is based on the random selection probability, and so assumes that the restrictions are not correlated with the grouping expressions. This is guaranteed to produce larger estimates, and of course risks over-estimating in cases where the restrictions are correlated, but that has less severe consequences than under-estimating, which might lead to a HashAgg that consumes an excessive amount of memory. This could possibly be improved upon in the future by identifying correlated restrictions and using a hybrid of the old and new formulae. Author: Tomas Vondra, with some hacking be me Reviewed-by: Mark Dilger, Alexander Korotkov, Dean Rasheed and Tom Lane Discussion: http://www.postgresql.org/message-id/flat/56CD0381.5060502@2ndquadrant.com
-
Simon Riggs authored
If archive_timeout > 0 we should avoid logging XLOG_RUNNING_XACTS if idle. Bug 13685 reported by Laurence Rowe, investigated in detail by Michael Paquier, though this is not his proposed fix. 20151016203031.3019.72930@wrigleys.postgresql.org Simple non-invasive patch to allow later backpatch to 9.4 and 9.5
-
- 03 Apr, 2016 6 commits
-
-
Tom Lane authored
The restore() function assumed that the result of sprintf() with %e format would necessarily contain an 'e', which is false: what if the supplied number is an infinity or NaN? If that did happen, we'd get a null-pointer-dereference core dump. The case appears impossible currently, because seg_in() does not accept such values, and there are no seg-creating functions that would create one. But it seems unwise to rely on it never happening in future. Quite aside from that, the code was pretty ugly: it relied on modifying a static format string when it could use a "*" precision argument, and it used strtok() entirely gratuitously, and it stripped off trailing spaces by hand instead of just not asking for them to begin with. Coverity noticed the potential null pointer dereference (though I wonder why it didn't complain years ago, since this code is ancient). Since this is just code cleanup and forestalling a hypothetical future bug, there seems no need for back-patching.
-
Tom Lane authored
The code was supposing that rd_amcache wouldn't disappear from under it during a scan; which is wrong. Copy the data out of the relcache rather than trying to reference it there.
-
Tom Lane authored
Coverity complained about implicit sign-extension in the BloomPageGetFreeSpace macro, probably because sizeOfBloomTuple isn't wide enough for size calculations. No overflow is really possible as long as maxoff and sizeOfBloomTuple are small enough to represent a realistic situation, but it seems like a good idea to declare sizeOfBloomTuple as Size not int32. Add missing check on BloomPageAddItem() result, again from Coverity. Avoid core dump due to not allocating so->sign array when scan->numberOfKeys is zero. Also thanks to Coverity. Use FLEXIBLE_ARRAY_MEMBER rather than declaring an array as size 1 when it isn't necessarily. Very minor beautification of related code. Unfortunately, none of the Coverity-detected mistakes look like they could account for the remaining buildfarm unhappiness with this module. It's barely possible that the FLEXIBLE_ARRAY_MEMBER mistake does account for that, if it's enabling bogus compiler optimizations; but I'm not terribly optimistic. We probably still have bugs to find here.
-
Simon Riggs authored
Replay of XLOG_BTREE_VACUUM during Hot Standby was previously thought to require complex interlocking that matched the requirements on the master. This required an O(N) operation that became a significant problem with large indexes, causing replication delays of seconds or in some cases minutes while the XLOG_BTREE_VACUUM was replayed. This commit skips the pin scan that was previously required, by observing in detail when and how it is safe to do so, with full documentation. The pin scan is skipped only in replay; the VACUUM code path on master is not touched here and WAL is identical. The current commit applies in all cases, effectively replacing commit 687f2cd7.
-
Tom Lane authored
Often, upon getting an unexpected error in psql, one's first wish is that the verbosity setting had been higher; for example, to be able to see the schema-name field or the server code location info. Up to now the only way has been to adjust the VERBOSITY variable and repeat the failing query. That's a pain, and it doesn't work if the error isn't reproducible. This commit adds a psql feature that redisplays the most recent server error at full verbosity, without needing to make any variable changes or re-execute the failed command. We just need to hang onto the latest error PGresult in case the user executes \errverbose, and then apply libpq's new PQresultVerboseErrorMessage() function to it. This will consume some trivial amount of psql memory, but otherwise the cost when the feature isn't used should be negligible. Alex Shulgin, reviewed by Daniel Vérité, some improvements by me
-
Tom Lane authored
Often, upon getting an unexpected error in psql, one's first wish is that the verbosity setting had been higher; for example, to be able to see the schema-name field or the server code location info. Up to now the only way has been to adjust the VERBOSITY variable and repeat the failing query. That's a pain, and it doesn't work if the error isn't reproducible. This commit adds support in libpq for regenerating the error message for an existing error PGresult at any desired verbosity level. This is almost just a matter of refactoring the existing code into a subroutine, but there is one bit of possibly-needed information that was not getting put into PGresults: the text of the last query sent to the server. We must add that string to the contents of an error PGresult. But we only need to save it if it might be used, which with the existing error-formatting code only happens if there is a PG_DIAG_STATEMENT_POSITION error field, which is probably pretty rare for errors in production situations. So really the overhead when the feature isn't used should be negligible. Alex Shulgin, reviewed by Daniel Vérité, some improvements by me
-
- 02 Apr, 2016 9 commits
-
-
Tom Lane authored
Per buildfarm member pademelon.
-
Tom Lane authored
The inconsistency here triggered compiler warnings on some buildfarm members, and it's surely pretty pointless.
-
Tom Lane authored
Some buildfarm members are showing "comparison is always false due to limited range of data type" complaints on this test, so #ifdef it out on machines with 32-bit int.
-
Teodor Sigaev authored
Comment is right, but if - not.
-
Stephen Frost authored
s/afer/after Pointed out by Andreas 'ads' Scherbaum
-
Teodor Sigaev authored
Vacuum might put page into list of pages with some free space and mark as deleted at the same time.
-
Teodor Sigaev authored
Looking at result of buildfarm member jaguarundi it seems to me that BloomOptions isn't inited sometime, but I don't see yet how it's possible. Nevertheless, check of signature length's is missed, so, add a limit of it. Also add missed GenericXLogAbort() in case of already deleted page in vacuum + minor code refactoring.
-
Noah Misch authored
This corrects messages for can't-happen errors. The corresponding "user token" appears in the HANDLE argument of GetTokenInformation().
-
Noah Misch authored
-
- 01 Apr, 2016 9 commits
-
-
Alvaro Herrera authored
-
Tom Lane authored
As with the previous patch, large numbers of null rows could skew this calculation unfavorably, causing us to discard values that have a legitimate claim to be MCVs, since our definition of MCV is that it's most common among the non-null population of the column. Hence, make the numerator of avgcount be the number of non-null sample values not the number of sample rows; likewise for maxmincount in the compute_scalar_stats variant. Also, make the denominator be the number of distinct values actually observed in the sample, rather than reversing it back out of the computed stadistinct. This avoids depending on the accuracy of the Haas-Stokes approximation, and really it's what we want anyway; the threshold should depend only on what we see in the sample, not on what we extrapolate about the contents of the whole column. Alex Shulgin, reviewed by Tomas Vondra and myself
-
Alvaro Herrera authored
For some reason this parameter was introduced as unused in 3da0dfb4, and has never been used for anything. Remove it. Author: Fabien Coelho
-
Tom Lane authored
Previously, we included null rows in the values of n and N that went into the formula, which amounts to considering null as a value in its own right; but the d and f1 values do not include nulls. This is inconsistent, and it contributes to significant underestimation of ndistinct when the column is mostly nulls. In any case stadistinct is defined as the number of distinct non-null values, so we should exclude nulls when doing this computation. This is an aboriginal bug in our application of the Haas-Stokes formula, but we'll refrain from back-patching for fear of destabilizing plan choices in released branches. While at it, make the code a bit more readable by omitting unnecessary casts and intermediate variables. Observation and original patch by Tomas Vondra, adjusted to fix both uses of the formula by Alex Shulgin, cosmetic improvements by me
-
Alvaro Herrera authored
In the test_slot_timelines test module, we were abusing passing NULL values which was received as zeroes in x86, but this breaks in ARM (buildfarm member hamster) by crashing instead. Fix the breakage by marking these functions as STRICT; the InvalidXid value that was previously implicit in NULL values (on x86 at least) can now be passed as 0. Failing to follow the fmgr protocol to check for NULLs beforehand was causing ARM to fail, as evidenced by segmentation faults in buildfarm member hamster. In order to use the new functionality in the test script, use COALESCE in the right spot to avoid forwarding NULL values. This was diagnosed from the hamster crash by Craig Ringer, who also proposed a different patch (checking for NULL values explicitely in the C function code, and keeping the non-strictness in the C functions). I decided to go with this approach instead.
-
Teodor Sigaev authored
- macroses llike (var & FLAG) are changed to ((var & FLAG) != 0) - do not copy uninitialized part of notFullPage array to page
-
Alvaro Herrera authored
Our actual convention, contrary to what I said in 59a2111b, is not to quote type names, as evidenced by unquoted use of format_type_be() result value in error messages. Remove quotes from recently tweaked messages accordingly. Per note from Tom Lane
-
Tom Lane authored
Commit acdf2a8b added a test case involving minus zero as a box endpoint. This is not very portable, as evidenced by the several older buildfarm members that are failing on the test because they print minus zero as just "0". If there were any significant reason to test this behavior, we could consider carrying a separate expected-file; but it doesn't look to me like there's adequate justification to accept such a maintenance burden. Just change the test to use plain zero, instead.
-
Tom Lane authored
When getParamDescriptions was changed to handle out-of-memory better by cribbing error recovery logic from getRowDescriptions/getAnotherTuple, somebody omitted to copy the stanza about checking for excess data in the message. But you need to do that, since continue'ing out of the switch in pqParseInput3 means no such check gets applied there anymore. Noted while looking at Michael Paquier's patch that made yet another copy of this advance_and_error logic. (This whole business desperately needs refactoring, because I sure don't want to see a dozen copies of this code, but that's where we seem to be headed. What's more, the "suspend parsing on EOF return" convention is a holdover from protocol 2 and shouldn't exist at all in protocol 3, because we don't process partial messages anymore. But for now, just fix the obvious bug.) Also, fix some wrong/missing comments about what the API spec is for these three functions. This doesn't seem worthy of back-patching, even though it's a bug; the case shouldn't ever arise in the field.
-