- 17 Dec, 2017 2 commits
-
-
Tom Lane authored
Commit c04d35f4 didn't quite do the job here, because it still allowed the compiler to deduce that the function call could be optimized away. Prevent that by putting the arguments and results in global variables. Discussion: https://postgr.es/m/20171213213754.pydkyjs6bt2hvsdb@alap3.anarazel.de
-
- 16 Dec, 2017 3 commits
-
-
Andres Freund authored
On some systems the results of 64 bit __builtin_mul_overflow() operations can be computed at compile time, but not at runtime. The known cases are arm buildfar animals using clang where the runtime operation is implemented in a unavailable function. Try to avoid compile-time computation by using volatile arguments to __builtin_mul_overflow(). In that case we hopefully will get a link error when unavailable, similar to what buildfarm animals dangomushi and gull are reporting. Author: Andres Freund Discussion: https://postgr.es/m/20171213213754.pydkyjs6bt2hvsdb@alap3.anarazel.de
-
Noah Misch authored
Don't write to stdin of a psql process that could have already exited with an authentication failure. Buildfarm members crake and mandrill have failed once by doing so. Ignore SIGPIPE in all TAP tests. Back-patch to v10, where these tests were introduced. Reviewed by Michael Paquier. Discussion: https://postgr.es/m/20171209210203.GC3362632@rfd.leadboat.com
-
- 15 Dec, 2017 2 commits
-
-
Andres Freund authored
The previous commit has shown that the sanity checks around freezing aren't strong enough. Strengthening them seems especially important because the existance of the bug has caused corruption that we don't want to make even worse during future vacuum cycles. The errors are emitted with ereport rather than elog, despite being "should never happen" messages, so a proper error code is emitted. To avoid superflous translations, mark messages as internal. Author: Andres Freund and Alvaro Herrera Reviewed-By: Alvaro Herrera, Michael Paquier Discussion: https://postgr.es/m/20171102112019.33wb7g5wp4zpjelu@alap3.anarazel.de Backpatch: 9.3-
-
Andres Freund authored
Previously it was possible that a tuple was not pruned during vacuum, even though its update xmax (i.e. the updating xid in a multixact with both key share lockers and an updater) was below the cutoff horizon. As the freezing code assumed, rightly so, that that's not supposed to happen, xmax would be preserved (as a member of a new multixact or xmax directly). That causes two problems: For one the tuple is below the xmin horizon, which can cause problems if the clog is truncated or once there's an xid wraparound. The bigger problem is that that will break HOT chains, which in turn can lead two to breakages: First, failing index lookups, which in turn can e.g lead to constraints being violated. Second, future hot prunes / vacuums can end up making invisible tuples visible again. There's other harmful scenarios. Fix the problem by recognizing that tuples can be DEAD instead of RECENTLY_DEAD, even if the multixactid has alive members, if the update_xid is below the xmin horizon. That's safe because newer versions of the tuple will contain the locking xids. A followup commit will harden the code somewhat against future similar bugs and already corrupted data. Author: Andres Freund, with changes by Alvaro Herrera Reported-By: Daniel Wood Analyzed-By: Andres Freund, Alvaro Herrera, Robert Haas, Peter Geoghegan, Daniel Wood, Yi Wen Wong, Michael Paquier Reviewed-By: Alvaro Herrera, Robert Haas, Michael Paquier Discussion: https://postgr.es/m/E5711E62-8FDF-4DCA-A888-C200BF6B5742@amazon.com https://postgr.es/m/20171102112019.33wb7g5wp4zpjelu@alap3.anarazel.de Backpatch: 9.3-
-
- 14 Dec, 2017 4 commits
-
-
Tom Lane authored
Commit 9fa6f00b assumed that __builtin_constant_p("string literal") is TRUE, if the compiler has that function at all. Buildfarm results show that Sun Studio 12, at least, breaks that assumption. Removing that usage would leave us with no mechanical check for a very fragile coding requirement, so instead teach configure to ignore __builtin_constant_p() if it doesn't behave that way. We could complicate matters by distinguishing three cases (no such function, vs does, vs doesn't work for string literals); but for now, that seems unnecessary because our other existing uses of this function are just fairly minor optimizations of non-returning elog/ereport. We can live without that on the small population of compilers that act this way. Discussion: https://postgr.es/m/22997.1513264066@sss.pgh.pa.us
-
Andres Freund authored
Author: Christoph Berg Discussion: https://postgr.es/m/20171214082808.GA5775@msg.df7cb.de
-
Andrew Dunstan authored
The logical slots have a fast code path for sending data so as not to impose too high a per message overhead. The fast path skips checks for interrupts and timeouts. However, the existing coding failed to consider the fact that a transaction with a large number of changes may take a very long time to be processed and sent to the client. This causes the walsender to ignore interrupts for potentially a long time and more importantly it will result in the walsender being killed due to timeout at the end of such a transaction. This commit changes the fast path to also check for interrupts and only allows calling the fast path when the last keepalive check happened less than half the walsender timeout ago. Otherwise the slower code path will be taken. Backpatched to 9.4 Petr Jelinek, reviewed by Kyotaro HORIGUCHI, Yura Sokolov, Craig Ringer and Robert Haas. Discussion: https://postgr.es/m/e082a56a-fd95-a250-3bae-0fff93832510@2ndquadrant.com
-
Teodor Sigaev authored
Generator helps to make close to real-world tests. Author: Alik Khilazhev Reviewed-By: Fabien COELHO Discussion: https://www.postgresql.org/message-id/flat/BF3B6F54-68C3-417A-BFAB-FB4D66F2B410@postgrespro.ru
-
- 13 Dec, 2017 15 commits
-
-
Andres Freund authored
In order for executor nodes to be able to change their ExecProcNode function after ExecInitNode() has finished, provide ExecSetExecProcNode(). This allows any wrappers functions that only execProcnode.c knows about to be reinstalled. The motivation for wanting to change ExecProcNode after ExecInitNode() has finished is that it is not known until later whether parallel query is available, so if a parallel variant is to be installed then ExecInitNode() is too soon to decide. Author: Thomas Munro Reviewed-By: Andres Freund Discussion: https://postgr.es/m/CAEepm=09rr65VN+cAV5FgyM_z=D77Xy8Fuc9CDDDYbq3pQUezg@mail.gmail.com
-
Andres Freund authored
Sometimes it is useful to be able to insist that the compiler inline a function that its normal cost analysis would not normally choose to inline. This can be useful for instantiating different variants of a function that remove branches of code by constant folding. Author: Thomas Munro Reviewed-By: Andres Freund Discussion: https://postgr.es/m/CAEepm=09rr65VN+cAV5FgyM_z=D77Xy8Fuc9CDDDYbq3pQUezg@mail.gmail.com
-
Andres Freund authored
Crash restarts currently don't clean up temporary files, as a debugging aid. If a left-over file happens to have the same name as a segment file we're trying to create, we'll just truncate and reuse it, but there is a problem: BufFileOpenShared() determines how many segment files exist by trying to open .0, .1, .2, ... until it finds no more files. It might be confused by a junk file that has the next segment number. To defend against that, make sure we always create a gap after the end file by unlinking the following name if it exists. Also make it an error to try to open a BufFile that doesn't exist (has no segment 0), so as not to encourage the development of client code that depends on an interface that we can't reliably provide. Author: Thomas Munro Reviewed-By: Andres Freund Discussion: https://postgr.es/m/CAEepm%3D2jhCbC_GFQJaaDhWxLB4EXtT3vVd5czuRNaqF5CWSTog%40mail.gmail.com
-
Robert Haas authored
The previous coding forgot to release the scan before seizing it again, leading to a lockup. Report by Patrick Hemmer. Diagnosis by Thomas Munro. Patch by Amit Kapila. Discussion: http://postgr.es/m/CAEepm=2xZUcOGP9V0O_G0=2P2wwXwPrkF=upWTCJSisUxMnuSg@mail.gmail.com
-
Robert Haas authored
This reverts commit 2c09a5c1. Per further discussion, that doesn't seem to be the best possible fix. Discussion: http://postgr.es/m/CAA4eK1LW2aFKzY3=vwvc=t-juzPPVWP2uT1bpx_MeyEqnM+p8g@mail.gmail.com
-
Tom Lane authored
This patch makes a number of interrelated changes to reduce the overhead involved in creating/deleting memory contexts. The key ideas are: * Include the AllocSetContext header of an aset.c context in its first malloc request, rather than allocating it separately in TopMemoryContext. This means that we now always create an initial or "keeper" block in an aset, even if it never receives any allocation requests. * Create freelists in which we can save and recycle recently-destroyed asets (this idea is due to Robert Haas). * In the common case where the name of a context is a constant string, just store a pointer to it in the context header, rather than copying the string. The first change eliminates a palloc/pfree cycle per context, and also avoids bloat in TopMemoryContext, at the price that creating a context now involves a malloc/free cycle even if the context never receives any allocations. That would be a loser for some common usage patterns, but recycling short-lived contexts via the freelist eliminates that pain. Avoiding copying constant strings not only saves strlen() and strcpy() overhead, but is an essential part of the freelist optimization because it makes the context header size constant. Currently we make no attempt to use the freelist for contexts with non-constant names. (Perhaps someday we'll need to think harder about that, but in current usage, most contexts with custom names are long-lived anyway.) The freelist management in this initial commit is pretty simplistic, and we might want to refine it later --- but in common workloads that will never matter because the freelists will never get full anyway. To create a context with a non-constant name, one is now required to call AllocSetContextCreateExtended and specify the MEMCONTEXT_COPY_NAME option. AllocSetContextCreate becomes a wrapper macro, and it includes a test that will complain about non-string-literal context name parameters on gcc and similar compilers. An unfortunate side effect of making AllocSetContextCreate a macro is that one is now *required* to use the size parameter abstraction macros (ALLOCSET_DEFAULT_SIZES and friends) with it; the pre-9.6 habit of writing out individual size parameters no longer works unless you switch to AllocSetContextCreateExtended. Internally to the memory-context-related modules, the context creation APIs are simplified, removing the rather baroque original design whereby a context-type module called mcxt.c which then called back into the context-type module. That saved a bit of code duplication, but not much, and it prevented context-type modules from exercising control over the allocation of context headers. In passing, I converted the test-and-elog validation of aset size parameters into Asserts to save a few more cycles. The original thought was that callers might compute size parameters on the fly, but in practice nobody does that, so it's useless to expend cycles on checking those numbers in production builds. Also, mark the memory context method-pointer structs "const", just for cleanliness. Discussion: https://postgr.es/m/2264.1512870796@sss.pgh.pa.us
-
Peter Eisentraut authored
The plpgsql.sql test file in the main regression tests is now by far the largest after numeric_big, making editing and managing the test cases very cumbersome. The other PLs have their own test suites split up into smaller files by topic. It would be nice to have that for plpgsql as well. So, to get that started, set up test infrastructure in src/pl/plpgsql/src/ and split out the recently added procedure test cases into a new file there. That file now mirrors the test cases added to the other PLs, making managing those matching tests a bit easier too. msvc build system changes with help from Michael Paquier
-
Peter Eisentraut authored
Author: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> Reported-by: Rushabh Lathia <rushabh.lathia@gmail.com>
-
Andres Freund authored
port.h redirects isnan() to _isnan() on windows, which in turn is provided by float.h rather than math.h. Therefore include the latter as well. Per buildfarm.
-
Andres Freund authored
Per buildfarm animal woodlouse.
-
Peter Eisentraut authored
After d0aa965c, one error path in PLy_spi_execute_fetch_result() could result in the variable "result" being dereferenced after being set to NULL. Rearrange the code a bit to fix that. Also add another SPI_freetuptable() call so that that is cleared in all error paths. discovered by John Naylor <jcnaylor@gmail.com> via scan-build ideas and review by Tom Lane
-
Andres Freund authored
Otherwise the detection can spuriously detect symbol as available, because the compiler may just emits reference to non-existant symbol.
-
Andres Freund authored
A previous commit added inline functions that provide fast(er) and correct overflow checks for signed integer math. Use them in a significant portion of backend code. There's more to touch in both backend and frontend code, but these were the easily identifiable cases. The old overflow checks are noticeable in integer heavy workloads. A secondary benefit is that getting rid of overflow checks that rely on signed integer overflow wrapping around, will allow us to get rid of -fwrapv in the future. Which in turn slows down other code. Author: Andres Freund Discussion: https://postgr.es/m/20171024103954.ztmatprlglz3rwke@alap3.anarazel.de
-
Andres Freund authored
It's not easy to get signed integer overflow checks correct and fast. Therefore abstract the necessary infrastructure into a common header providing addition, subtraction and multiplication for 16, 32, 64 bit signed integers. The new macros aren't yet used, but a followup commit will convert several open coded overflow checks. Author: Andres Freund, with some code stolen from Greg Stark Reviewed-By: Robert Haas Discussion: https://postgr.es/m/20171024103954.ztmatprlglz3rwke@alap3.anarazel.de
-
Robert Haas authored
Commit 8b304b8b removed replacement selection, but left behind this comment text. The optimization to which the comment refers is not relevant without replacement selection, because if we had so few tuples as to require only one tape, we would have just completed the sort in memory. Peter Geoghegan Discussion: http://postgr.es/m/CAH2-WznqupLA8CMjp+vqzoe0yXu0DYYbQSNZxmgN76tLnAOZ_w@mail.gmail.com
-
- 12 Dec, 2017 2 commits
-
-
Robert Haas authored
Etsuro Fujita, reviewed by Ashutosh Bapat Discussion: http://postgr.es/m/5A2A60E6.6000008@lab.ntt.co.jp
-
Teodor Sigaev authored
regular expression which contains backslash.
-
- 11 Dec, 2017 3 commits
-
-
Peter Eisentraut authored
Reported-by: Noah Misch <noah@leadboat.com>
-
Tom Lane authored
I noticed that _SPI_execute_plan initially sets spierrcontext.arg = NULL, and only fills it in some time later. If an error were to happen in between, _SPI_error_callback would try to dereference the null pointer. This is unlikely --- there's not much between those points except push-snapshot calls --- but it's clearly not impossible. Tweak the callback to do nothing if the pointer isn't set yet. It's been like this for awhile, so back-patch to all supported branches.
-
Robert Haas authored
Ashutosh Bapat, per discussion with Julien Rouhaund, who also reviewed this patch. Discussion: http://postgr.es/m/CAFjFpReBR3ftK9C23LLCZY_TDXhhjB_dgE-L9+mfTnA=gkvdvQ@mail.gmail.com
-
- 10 Dec, 2017 1 commit
-
-
Tom Lane authored
The test added by commit 390d5813 turns out to have different output in CLOBBER_CACHE_ALWAYS builds: there's an extra CONTEXT line in the error message as a result of detecting the error at a different place. Possibly we should do something to make that more consistent. But as a stopgap measure to make the buildfarm green again, adjust the test to suppress CONTEXT entirely. We can revert this if we do something in the backend to eliminate the inconsistency. Discussion: https://postgr.es/m/31545.1512924904@sss.pgh.pa.us
-
- 09 Dec, 2017 5 commits
-
-
Tom Lane authored
If one exits and re-enters a DECLARE ... BEGIN ... END block within a single execution of a plpgsql function, perhaps due to a surrounding loop, the declared variables are supposed to get re-initialized to null (or whatever their initializer is). But this failed to happen for variables of type "record", because while exec_stmt_block() expected such variables to be included in the block's initvarnos list, plpgsql_add_initdatums() only adds DTYPE_VAR variables to that list. This bug appears to have been there since the aboriginal addition of plpgsql to our tree. Fix by teaching plpgsql_add_initdatums() to include DTYPE_REC variables as well. (We don't need to consider other DTYPEs because they don't represent separately-stored values.) I failed to resist the temptation to make some nearby cosmetic adjustments, too. No back-patch, because there have not been field complaints, and it seems possible that somewhere out there someone has code depending on the incorrect behavior. In any case this change would have no impact on correctly-written code. Discussion: https://postgr.es/m/22994.1512800671@sss.pgh.pa.us
-
Magnus Hagander authored
Missed this in the last commit.
-
Magnus Hagander authored
Reported by Robins Tharakan
-
Noah Misch authored
Notably, this permits linking to the 32-bit Perl binaries advertised on perl.org, namely Strawberry Perl and ActivePerl. This has a side effect of permitting linking to binaries built with obsolete MSVC versions. By default, MSVC 2012 and later require a "safe exception handler table" in each binary. MinGW-built, 32-bit DLLs lack the relevant exception handler metadata, so linking to them failed with error LNK2026. Restore the semantics of MSVC 2010, which omits the table from a given binary if some linker input lacks metadata. This has no effect on 64-bit builds or on MSVC 2010 and earlier. Back-patch to 9.3 (all supported versions). Reported by Victor Wagner. Discussion: https://postgr.es/m/20160326154321.7754ab8f@wagner.wagner.home
-
Noah Misch authored
Commits 5a5c2fec and b5178c5d introduced support for modern MSVC-built, 32-bit Perl, but they broke use of MinGW-built, 32-bit Perl distributions like Strawberry Perl and modern ActivePerl. Perl has no robust means to report whether it expects a -D_USE_32BIT_TIME_T ABI, so test this. Back-patch to 9.3 (all supported versions). The chief alternative was a heuristic of adding -D_USE_32BIT_TIME_T when $Config{gccversion} is nonempty. That banks on every gcc-built Perl using the same ABI. gcc could change its default ABI the way MSVC once did, and one could build Perl with gcc and the non-default ABI. The GNU make build system could benefit from a similar test, without which it does not support MSVC-built Perl. For now, just add a comment. Most users taking the special step of building Perl with MSVC probably build PostgreSQL with MSVC. Discussion: https://postgr.es/m/20171130041441.GA3161526@rfd.leadboat.com
-
- 08 Dec, 2017 3 commits
-
-
Peter Eisentraut authored
Those cases currently crash and supporting them is more work then originally thought, so we'll just prohibit these scenarios for now. Author: Michael Paquier <michael.paquier@gmail.com> Reviewed-by: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> Reported-by: Мансур Галиев <gomer94@yandex.ru> Bug: #14866
-
Peter Eisentraut authored
Reported-by: Masahiko Sawada <sawada.mshk@gmail.com>
-
Tom Lane authored
plpgsql's function exec_move_row() handles assignment of a composite source value to either a PLpgSQL_rec or PLpgSQL_row target variable. Oddly, rather than taking a single target argument which it could do run-time type detection on, it was coded to take two separate arguments (only one of which is allowed to be non-NULL). This choice had then back-propagated into storing two separate target variables in various plpgsql statement nodes, with lots of duplicative coding and awkward interface logic to support that. Simplify matters by folding those pairs down to single variables, distinguishing the two cases only where we must ... which turns out to be only in exec_move_row itself. This is purely refactoring and should not change any behavior. In passing, remove unused field PLpgSQL_stmt_open.returntype. Discussion: https://postgr.es/m/11787.1512713374@sss.pgh.pa.us
-