aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Make locale-dependent regex character classes work for large char codes.Tom Lane2016-09-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, we failed to recognize Unicode characters above U+7FF as being members of locale-dependent character classes such as [[:alpha:]]. (Actually, the same problem occurs for large pg_wchar values in any multibyte encoding, but UTF8 is the only case people have actually complained about.) It's impractical to get Spencer's original code to handle character classes or ranges containing many thousands of characters, because it insists on considering each member character individually at regex compile time, whether or not the character will ever be of interest at run time. To fix, choose a cutoff point MAX_SIMPLE_CHR below which we process characters individually as before, and deal with entire ranges or classes as single entities above that. We can actually make things cheaper than before for chars below the cutoff, because the color map can now be a simple linear array for those chars, rather than the multilevel tree structure Spencer designed. It's more expensive than before for chars above the cutoff, because we must do a binary search in a list of high chars and char ranges used in the regex pattern, plus call iswalpha() and friends for each locale-dependent character class used in the pattern. However, multibyte encodings are normally designed to give smaller codes to popular characters, so that we can expect that the slow path will be taken relatively infrequently. In any case, the speed penalty appears minor except when we have to apply iswalpha() etc. to high character codes at runtime --- and the previous coding gave wrong answers for those cases, so whether it was faster is moot. Tom Lane, reviewed by Heikki Linnakangas Discussion: <15563.1471913698@sss.pgh.pa.us>
* C comment: align dashes in GroupState node headerBruce Momjian2016-09-05
| | | | Author: Jim Nasby
* Relax transactional restrictions on ALTER TYPE ... ADD VALUE.Tom Lane2016-09-05
| | | | | | | | | | | | | | | | | | | | To prevent possibly breaking indexes on enum columns, we must keep uncommitted enum values from getting stored in tables, unless we can be sure that any such column is new in the current transaction. Formerly, we enforced this by disallowing ALTER TYPE ... ADD VALUE from being executed at all in a transaction block, unless the target enum type had been created in the current transaction. This patch removes that restriction, and instead insists that an uncommitted enum value can't be referenced unless it belongs to an enum type created in the same transaction as the value. Per discussion, this should be a bit less onerous. It does require each function that could possibly return a new enum value to SQL operations to check this restriction, but there aren't so many of those that this seems unmaintainable. Andrew Dunstan and Tom Lane Discussion: <4075.1459088427@sss.pgh.pa.us>
* Add debug check function LWLockHeldByMeInMode()Simon Riggs2016-09-05
| | | | | | | Tests whether my process holds a lock in given mode. Add initial usage in MarkBufferDirty(). Thomas Munro
* Dirty replication slots when using sql interfaceSimon Riggs2016-09-05
| | | | | | | | | | | | | | | | | | When pg_logical_slot_get_changes(...) sets confirmed_flush_lsn to the point at which replay stopped, it doesn't dirty the replication slot. So if the replay didn't cause restart_lsn or catalog_xmin to change as well, this change will not get written out to disk. Even on a clean shutdown. If Pg crashes or restarts, a subsequent pg_logical_slot_get_changes(...) call will see the same changes already replayed since it uses the slot's confirmed_flush_lsn as the start point for fetching changes. The caller can't specify a start LSN when using the SQL interface. Mark the slot as dirty after reading changes using the SQL interface so that users won't see repeated changes after a clean shutdown. Repeated changes still occur when using the walsender interface or after an unclean shutdown. Craig Ringer
* Remove duplicate code from ReorderBufferCleanupTXN().Tom Lane2016-09-04
| | | | | | | | | | | Andres is apparently the only hacker who thinks this code is better as-is. I (tgl) follow some of his logic, but the fact that it's setting off warnings from static code analyzers seems like a sufficient reason to put the complexity into a comment rather than the code. Aleksander Alekseev Discussion: <20160404190345.54d84ee8@fujitsu>
* Add regression test coverage for non-default timezone abbreviation sets.Tom Lane2016-09-04
| | | | | | | | | | After further reflection about the mess cleaned up in commit 39b691f25, I decided the main bit of test coverage that was still missing was to check that the non-default abbreviation-set files we supply are usable. Add that. Back-patch to supported branches, just because it seems like a good idea to keep this all in sync.
* Remove vestigial references to "zic" in favor of "IANA database".Tom Lane2016-09-04
| | | | | | | | | | | | Commit b2cbced9e instituted a policy of referring to the timezone database as the "IANA timezone database" in our user-facing documentation. Propagate that wording into a couple of places that were still using "zic" to refer to the database, which is definitely not right (zic is the compilation tool, not the data). Back-patch, not because this is very important in itself, but because we routinely cherry-pick updates to the tznames files and I don't want to risk future merge failures.
* Remove useless pg_strdup() operations.Tom Lane2016-09-04
| | | | | | | | | split_to_stringlist() doesn't modify its first argument nor expect it to remain valid after exit, so there's no need to duplicate the optarg string at the call sites. Per Coverity. (This has been wrong all along, but commit 052cc223d changed the useless calls from "strdup" to "pg_strdup", which apparently made Coverity think it's a new bug. It's not, but it's also not worth back-patching.)
* Clarify the new Red-Black post-order traversal code a bit.Heikki Linnakangas2016-09-04
| | | | | | | | | | Coverity complained about the for(;;) loop, because it never actually iterated. It was used just to be able to use "break" to exit it early. I agree with Coverity, that's a bit confusing, so refactor the code to use if-else instead. While we're at it, use a local variable to hold the "current" node. That's shorter and clearer than referring to "iter->last_visited" all the time.
* Improve readability of the output of psql's \timing command.Tom Lane2016-09-03
| | | | | | | | | | | | In addition to the existing decimal-milliseconds output value, display the same value in mm:ss.fff format if it exceeds one second. Tack on hours and even days fields if the interval is large enough. This avoids needing mental arithmetic to convert the values into customary time units. Corey Huinker, reviewed by Gerdan Santos; bikeshedding by many Discussion: <CADkLM=dbC4R8sbbuFXQVBFWoJGQkTEW8RWnC0PbW9nZsovZpJQ@mail.gmail.com>
* Fix multiple bugs in numeric_poly_deserialize().Tom Lane2016-09-03
| | | | | | | | | | These were evidently introduced by yesterday's commit 9cca11c91, which perhaps needs more review than it got. Per report from Andreas Seltenreich and additional examination of nearby code. Report: <87oa45qfwq.fsf@credativ.de>
* Fix corrupt GIN_SEGMENT_ADDITEMS WAL records on big-endian hardware.Tom Lane2016-09-03
| | | | | | | | | | | | | | | | | | | | computeLeafRecompressWALData() tried to produce a uint16 WAL log field by memcpy'ing the first two bytes of an int-sized variable. That accidentally works on little-endian hardware, but not at all on big-endian. Replay then thinks it's looking at an ADDITEMS action with zero entries, and reads the first two bytes of the first TID therein as the next segno/action, typically leading to "unexpected GIN leaf action" errors during replay. Even if replay failed to crash, the resulting GIN index page would surely be incorrect. To fix, just declare the variable as uint16 instead. Per bug #14295 from Spencer Thomason (much thanks to Spencer for turning his problem into a self-contained test case). This likely also explains a previous report of the same symptom from Bernd Helmle. Back-patch to 9.4 where the problem was introduced (by commit 14d02f0bb). Discussion: <20160826072658.15676.7628@wrigleys.postgresql.org> Possible-Report: <2DA7350F7296B2A142272901@eje.land.credativ.lan>
* New recovery target recovery_target_lsnSimon Riggs2016-09-03
| | | | Michael Paquier
* Don't require dynamic timezone abbreviations to match underlying time zone.Tom Lane2016-09-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, we threw an error if a dynamic timezone abbreviation did not match any abbreviation recorded in the referenced IANA time zone entry. That seemed like a good consistency check at the time, but it turns out that a number of the abbreviations in the IANA database are things that Olson and crew made up out of whole cloth. Their current policy is to remove such names in favor of using simple numeric offsets. Perhaps unsurprisingly, a lot of these made-up abbreviations have varied in meaning over time, which meant that our commit b2cbced9e and later changes made them into dynamic abbreviations. So with newer IANA database versions that don't mention these abbreviations at all, we fail, as reported in bug #14307 from Neil Anderson. It's worse than just a few unused-in-the-wild abbreviations not working, because the pg_timezone_abbrevs view stops working altogether (since its underlying function tries to compute the whole view result in one call). We considered deleting these abbreviations from our abbreviations list, but the problem with that is that we can't stay ahead of possible future IANA changes. Instead, let's leave the abbreviations list alone, and treat any "orphaned" dynamic abbreviation as just meaning the referenced time zone. It will behave a bit differently than it used to, in that you can't any longer override the zone's standard vs. daylight rule by using the "wrong" abbreviation of a pair, but that's better than failing entirely. (Also, this solution can be interpreted as adding a small new feature, which is that any abbreviation a user wants can be defined as referencing a time zone name.) Back-patch to all supported branches, since this problem affects all of them when using tzdata 2016f or newer. Report: <20160902031551.15674.67337@wrigleys.postgresql.org> Discussion: <6189.1472820913@sss.pgh.pa.us>
* Move code shared between libpq and backend from backend/libpq/ to common/.Heikki Linnakangas2016-09-02
| | | | | | | | | | | | | | | | | | | | | | When building libpq, ip.c and md5.c were symlinked or copied from src/backend/libpq into src/interfaces/libpq, but now that we have a directory specifically for routines that are shared between the server and client binaries, src/common/, move them there. Some routines in ip.c were only used in the backend. Keep those in src/backend/libpq, but rename to ifaddr.c to avoid confusion with the file that's now in common. Fix the comment in src/common/Makefile to reflect how libpq actually links those files. There are two more files that libpq symlinks directly from src/backend: encnames.c and wchar.c. I don't feel compelled to move those right now, though. Patch by Michael Paquier, with some changes by me. Discussion: <69938195-9c76-8523-0af8-eb718ea5b36e@iki.fi>
* Speed up SUM calculation in numeric aggregates.Heikki Linnakangas2016-09-02
| | | | | | | | | | | | This introduces a numeric sum accumulator, which performs better than repeatedly calling add_var(). The performance comes from using wider digits and delaying carry propagation, tallying positive and negative values separately, and avoiding a round of palloc/pfree on every value. This speeds up SUM(), as well as other standard aggregates like AVG() and STDDEV() that also calculate a sum internally. Reviewed-by: Andrey Borodin Discussion: <c0545351-a467-5b76-6d46-4840d1ea8aa4@iki.fi>
* Support multiple iterators in the Red-Black Tree implementation.Heikki Linnakangas2016-09-02
| | | | | | | While we don't need multiple iterators at the moment, the interface is nicer and less dangerous this way. Aleksander Alekseev, with some changes by me.
* Improve tab completion for BEGIN & START|SET TRANSACTION.Kevin Grittner2016-09-01
| | | | | Andreas Karlsson with minor change by me for SET TRANSACTION SNAPSHOT.
* Change API of ShmemAlloc() so it throws error rather than returning NULL.Tom Lane2016-09-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | A majority of callers seem to have believed that this was the API spec already, because they omitted any check for a NULL result, and hence would crash on an out-of-shared-memory failure. The original proposal was to just add such error checks everywhere, but that does nothing to prevent similar omissions in future. Instead, let's make ShmemAlloc() throw the error (so we can remove the caller-side checks that do exist), and introduce a new function ShmemAllocNoError() that has the previous behavior of returning NULL, for the small number of callers that need that and are prepared to do the right thing. This also lets us remove the rather wishy-washy behavior of printing a WARNING for out-of-shmem, which never made much sense: either the caller has a strategy for dealing with that, or it doesn't. It's not ShmemAlloc's business to decide whether a warning is appropriate. The v10 release notes will need to call this out as a significant source-code change. It's likely that it will be a bug fix for extension callers too, but if not, they'll need to change to using ShmemAllocNoError(). This is nominally a bug fix, but the odds that it's fixing any live bug are actually rather small, because in general the requests being made by the unchecked callers were already accounted for in determining the overall shmem size, so really they ought not fail. Between that and the possible impact on extensions, no back-patch. Discussion: <24843.1472563085@sss.pgh.pa.us>
* Improve memory management for PL/Perl functions.Tom Lane2016-08-31
| | | | | | | | | | | | | | | | | | Unlike PL/Tcl, PL/Perl at least made an attempt to clean up after itself when a function gets redefined. But it was still using TopMemoryContext for the fn_mcxt of argument/result I/O functions, resulting in the potential for memory leaks depending on what those functions did, and the retail alloc/free logic was pretty bulky as well. Fix things to use a per-function memory context like the other PLs now do. Tweak a couple of places where things were being done in a not-very-safe order (on the principle that a memory leak is better than leaving global state inconsistent after an error). Also make some minor cosmetic adjustments, mostly in field names, to make the code look similar to the way PL/Tcl does now wherever it's essentially the same logic. Michael Paquier and Tom Lane Discussion: <CAB7nPqSOyAsHC6jL24J1B+oK3p=yyNoFU0Vs_B6fd2kdd5g5WQ@mail.gmail.com>
* Improve memory management for PL/Tcl functions.Tom Lane2016-08-31
| | | | | | | | | | | | | | | | | | | | Formerly, the memory used to represent a PL/Tcl function was allocated with malloc() or in TopMemoryContext, and we'd leak it all if the function got redefined during the session. Instead, create a per-function context and keep everything in or under that context. Add a reference-counting mechanism (like the one plpgsql has long had) so that we can safely clean up an old function definition, either immediately if it's not being executed or at the end of the outermost execution. Currently, we only detect that a cached function is obsolete when we next attempt to call that function. So this covers the updated-definition case but leaves cruft around after DROP FUNCTION. It's not clear whether it's worth installing a syscache invalidation callback to watch for drops; none of the other PLs do, so for now we won't do it here either. Michael Paquier and Tom Lane Discussion: <CAB7nPqSOyAsHC6jL24J1B+oK3p=yyNoFU0Vs_B6fd2kdd5g5WQ@mail.gmail.com>
* Try to fix portability issue in enum renumbering (again).Tom Lane2016-08-31
| | | | | | | | | | | | | | | | The hack embodied in commit 4ba61a487 no longer works after today's change to allow DatumGetFloat4/Float4GetDatum to be inlined (commit 14cca1bf8). Probably what's happening is that the faulty compilers are deciding that the now-inlined assignment is a no-op and so they're not required to round to float4 width. We had a bunch of similar issues earlier this year in the degree-based trig functions, and eventually settled on using volatile intermediate variables as the least ugly method of forcing recalcitrant compilers to do what the C standard says (cf commit 82311bcdd). Let's see if that method works here. Discussion: <4640.1472664476@sss.pgh.pa.us>
* Remove no-longer-useful SSL-specific Port.count field.Tom Lane2016-08-31
| | | | | | | | | Since we removed SSL renegotiation, there's no longer any reason to keep track of the amount of data transferred over the link. Daniel Gustafsson Discussion: <FEA7F89C-ECDF-4799-B789-2F8DDCBA467F@yesql.se>
* Use static inline functions for float <-> Datum conversions.Heikki Linnakangas2016-08-31
| | | | | | | | | | | | | | Now that we are OK with using static inline functions, we can use them to avoid function call overhead of pass-by-val versions of Float4GetDatum, DatumGetFloat8, and Float8GetDatum. Those functions are only a few CPU instructions long, but they could not be written into macros previously, because we need a local union variable for the conversion. I kept the pass-by-ref versions as regular functions. They are very simple too, but they call palloc() anyway, so shaving a few instructions from the function call doesn't seem so important there. Discussion: <dbb82a4a-2c15-ba27-dd0a-009d2aa72b77@iki.fi>
* Prevent starting a standalone backend with standby_mode on.Tom Lane2016-08-31
| | | | | | | | | | | | | | | | | | | This can't really work because standby_mode expects there to be more WAL arriving, which there will not ever be because there's no WAL receiver process to fetch it. Moreover, if standby_mode is on then hot standby might also be turned on, causing even more strangeness because that expects read-only sessions to be executing in parallel. Bernd Helmle reported a case where btree_xlog_delete_get_latestRemovedXid got confused, but rather than band-aiding individual problems it seems best to prevent getting anywhere near this state in the first place. Back-patch to all supported branches. In passing, also fix some omissions of errcodes in other ereport's in readRecoveryCommandFile(). Michael Paquier (errcode hacking by me) Discussion: <00F0B2CEF6D0CEF8A90119D4@eje.credativ.lan>
* Update comments to reflect code rearrangement.Robert Haas2016-08-31
| | | | | | Commit f9143d102ffd0947ca904c62b1d3d6fd587e0c80 falsified these. KaiGai Kohei
* Fix a bunch of places that called malloc and friends with no NULL check.Tom Lane2016-08-30
| | | | | | | | | | | | | | | Where possible, use palloc or pg_malloc instead; otherwise, insert explicit NULL checks. Generally speaking, these are places where an actual OOM is quite unlikely, either because they're in client programs that don't allocate all that much, or they're very early in process startup so that we'd likely have had a fork() failure instead. Hence, no back-patch, even though this is nominally a bug fix. Michael Paquier, with some adjustments by me Discussion: <CAB7nPqRu07Ot6iht9i9KRfYLpDaF2ZuUv5y_+72uP23ZAGysRg@mail.gmail.com>
* Simplify correct use of simple_prompt().Tom Lane2016-08-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The previous API for this function had it returning a malloc'd string. That meant that callers had to check for NULL return, which few of them were doing, and it also meant that callers had to remember to free() the string later, which required extra logic in most cases. Instead, make simple_prompt() write into a buffer supplied by the caller. Anywhere that the maximum required input length is reasonably small, which is almost all of the callers, we can just use a local or static array as the buffer instead of dealing with malloc/free. A fair number of callers used "pointer == NULL" as a proxy for "haven't requested the password yet". Maintaining the same behavior requires adding a separate boolean flag for that, which adds back some of the complexity we save by removing free()s. Nonetheless, this nets out at a small reduction in overall code size, and considerably less code than we would have had if we'd added the missing NULL-return checks everywhere they were needed. In passing, clean up the API comment for simple_prompt() and get rid of a very-unnecessary malloc/free in its Windows code path. This is nominally a bug fix, but it does not seem worth back-patching, because the actual risk of an OOM failure in any of these places seems pretty tiny, and all of them are client-side not server-side anyway. This patch is by me, but it owes a great deal to Michael Paquier who identified the problem and drafted a patch for fixing it the other way. Discussion: <CAB7nPqRu07Ot6iht9i9KRfYLpDaF2ZuUv5y_+72uP23ZAGysRg@mail.gmail.com>
* Fix initdb misbehavior when user mis-enters superuser password.Tom Lane2016-08-30
| | | | | | | | | | | | | | | | | | | While testing simple_prompt() revisions, I happened to notice that current initdb behaves rather badly when --pwprompt is specified and the user miskeys the second password. It complains about the mismatch, does "rm -rf" on the data directory, and exits. The problem is that since commit c4a8812cf, there's a standalone backend sitting waiting for commands at that point. It gets unhappy about its datadir having gone away, and spews a PANIC message at the user, which is not nice. (And the shell then adds to the mess with meaningless bleating about a core dump...) We don't really want that sort of thing to happen unless there's an internal failure in initdb, which this surely is not. The best fix seems to be to move the collection of the password earlier, so that it's done essentially as part of argument collection, rather than at the rather ad-hoc time it was done before. Back-patch to 9.6 where the problem was introduced.
* Split hash.h → hash_xlog.hAlvaro Herrera2016-08-29
| | | | | | | | Since the hash AM is going to be revamped to have WAL, this is a good opportunity to clean up the include file a little bit to avoid including a lot of extra stuff in the future. Author: Amit Kapila
* Remove support for OpenSSL versions older than 0.9.8.Heikki Linnakangas2016-08-29
| | | | | | | | OpenSSL officially only supports 1.0.1 and newer. Some OS distributions still provide patches for 0.9.8, but anything older than that is not interesting anymore. Let's simplify things by removing compatibility code. Andreas Karlsson, with small changes by me.
* Make AllocSetContextCreate throw an error for bad context-size parameters.Tom Lane2016-08-29
| | | | | | | | | | The previous behavior was to silently change them to something valid. That obscured the bugs fixed in commit ea268cdc9, and generally seems less useful than complaining. Unlike the previous commit, though, we'll do this in HEAD only --- it's a bit too late to be possibly breaking third-party code in 9.6. Discussion: <CA+TgmobNcELVd3QmLD3tx=w7+CokRQiC4_U0txjz=WHpfdkU=w@mail.gmail.com>
* Fix pg_receivexlog --synchronousSimon Riggs2016-08-29
| | | | | | | | Make pg_receivexlog work correctly with --synchronous without slots Backpatch to 9.5 Gabriele Bartolini, reviewed by Michael Paquier and Simon Riggs
* Fix typos in comments.Fujii Masao2016-08-29
|
* Fix pg_xlogdump so that it handles cross-page XLP_FIRST_IS_CONTRECORD record.Fujii Masao2016-08-29
| | | | | | | | | | | | | | | | | | Previously pg_xlogdump failed to dump the contents of the WAL file if the file starts with the continuation WAL record which spans more than one pages. Since pg_xlogdump assumed that the continuation record always fits on a page, it could not find the valid WAL record to start reading from in that case. This patch changes pg_xlogdump so that it can handle a continuation WAL record which crosses a page boundary and find the valid record to start reading from. Back-patch to 9.3 where pg_xlogdump was introduced. Author: Pavan Deolasee Reviewed-By: Michael Paquier and Craig Ringer Discussion: CABOikdPsPByMiG6J01DKq6om2+BNkxHTPkOyqHM2a4oYwGKsqQ@mail.gmail.com
* Fix stray reference to the old genbki.sh script.Tom Lane2016-08-28
| | | | Per Tomas Vondra.
* Add macros to make AllocSetContextCreate() calls simpler and safer.Tom Lane2016-08-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I found that half a dozen (nearly 5%) of our AllocSetContextCreate calls had typos in the context-sizing parameters. While none of these led to especially significant problems, they did create minor inefficiencies, and it's now clear that expecting people to copy-and-paste those calls accurately is not a great idea. Let's reduce the risk of future errors by introducing single macros that encapsulate the common use-cases. Three such macros are enough to cover all but two special-purpose contexts; those two calls can be left as-is, I think. While this patch doesn't in itself improve matters for third-party extensions, it doesn't break anything for them either, and they can gradually adopt the simplified notation over time. In passing, change TopMemoryContext to use the default allocation parameters. Formerly it could only be extended 8K at a time. That was probably reasonable when this code was written; but nowadays we create many more contexts than we did then, so that it's not unusual to have a couple hundred K in TopMemoryContext, even without considering various dubious code that sticks other things there. There seems no good reason not to let it use growing blocks like most other contexts. Back-patch to 9.6, mostly because that's still close enough to HEAD that it's easy to do so, and keeping the branches in sync can be expected to avoid some future back-patching pain. The bugs fixed by these changes don't seem to be significant enough to justify fixing them further back. Discussion: <21072.1472321324@sss.pgh.pa.us>
* Add a nonlocalized version of the severity field to client error messages.Tom Lane2016-08-26
| | | | | | | | | | | | | | | | | | | | This has been requested a few times, but the use-case for it was never entirely clear. The reason for adding it now is that transmission of error reports from parallel workers fails when NLS is active, because pq_parse_errornotice() wrongly assumes that the existing severity field is nonlocalized. There are other ways we could have fixed that, but the other options were basically kluges, whereas this way provides something that's at least arguably a useful feature along with the bug fix. Per report from Jakob Egger. Back-patch into 9.6, because otherwise parallel query is essentially unusable in non-English locales. The problem exists in 9.5 as well, but we don't want to risk changing on-the-wire behavior in 9.5 (even though the possibility of new error fields is specifically called out in the protocol document). It may be sufficient to leave the issue unfixed in 9.5, given the very limited usefulness of pq_parse_errornotice in that version. Discussion: <A88E0006-13CB-49C6-95CC-1A77D717213C@eggerapps.at>
* Fix potential memory leakage from HandleParallelMessages().Tom Lane2016-08-26
| | | | | | | | | | | | | | | | HandleParallelMessages leaked memory into the caller's context. Since it's called from ProcessInterrupts, there is basically zero certainty as to what CurrentMemoryContext is, which means we could be leaking into long-lived contexts. Over the processing of many worker messages that would grow to be a problem. Things could be even worse than just a leak, if we happened to service the interrupt while ErrorContext is current: elog.c thinks it can reset that on its own whim, possibly yanking storage out from under HandleParallelMessages. Give HandleParallelMessages its own dedicated context instead, which we can reset during each call to ensure there's no accumulation of wasted memory. Discussion: <16610.1472222135@sss.pgh.pa.us>
* Put static forward declarations in elog.c back into same order as code.Tom Lane2016-08-26
| | | | | | | | The guiding principle for the last few patches in this area apparently involved throwing darts. Cosmetic only, but back-patch to 9.6 because there is no reason for 9.6 and HEAD to diverge yet in this file.
* Fix assorted small bugs in ThrowErrorData().Tom Lane2016-08-26
| | | | | | | | | | | | | | | | | Copy the palloc'd strings into the correct context, ie ErrorContext not wherever the source ErrorData is. This would be a large bug, except that it appears that all catchers of thrown errors do either EmitErrorReport or CopyErrorData before doing anything that would cause transient memory contexts to be cleaned up. Still, it's wrong and it will bite somebody someday. Fix failure to copy cursorpos and internalpos. Utter the appropriate incantations involving recursion_depth, so that we'll behave sanely if we get an error inside pstrdup. (In general, the body of this function ought to act like, eg, errdetail().) Per code reading induced by Jakob Egger's report.
* Fix logic for adding "parallel worker" context line to worker errors.Tom Lane2016-08-26
| | | | | | | | | The previous coding here was capable of adding a "parallel worker" context line to errors that were not, in fact, returned from a parallel worker. Instead of using an errcontext callback to add that annotation, just paste it onto the message by hand; this looks uglier but is more reliable. Discussion: <19757.1472151987@sss.pgh.pa.us>
* Fix instability in parallel regression tests.Tom Lane2016-08-25
| | | | | | | | | | | | | | Commit f0c7b789a added a test case in case.sql that creates and then drops both an '=' operator and the type it's for. Given the right timing, that can cause a "cache lookup failed for type" failure in concurrent sessions, which see the '=' operator as a potential match for '=' in a query, but then the type is gone by the time they inquire into its properties. It might be nice to make that behavior more robust someday, but as a back-patchable solution, adjust the new test case so that the operator is never visible to other sessions. Like the previous commit, back-patch to all supported branches. Discussion: <5983.1471371667@sss.pgh.pa.us>
* Fix small query-lifespan memory leak in bulk updates.Tom Lane2016-08-24
| | | | | | | | | | | | | When there is an identifiable REPLICA IDENTITY index on the target table, heap_update leaks the id_attrs bitmapset. That's not many bytes, but it adds up over enough rows, since the code typically runs in a query-lifespan context. Bug introduced in commit e55704d8b, which did a rather poor job of cloning the existing use-pattern for RelationGetIndexAttrBitmap(). Per bug #14293 from Zhou Digoal. Back-patch to 9.4 where the bug was introduced. Report: <20160824114320.15676.45171@wrigleys.postgresql.org>
* Fix improper repetition of previous results from a hashed aggregate.Tom Lane2016-08-24
| | | | | | | | | | | | | | | | | | | | | | ExecReScanAgg's check for whether it could re-use a previously calculated hashtable neglected the possibility that the Agg node might reference PARAM_EXEC Params that are not referenced by its input plan node. That's okay if the Params are in upper tlist or qual expressions; but if one appears in aggregate input expressions, then the hashtable contents need to be recomputed when the Param's value changes. To avoid unnecessary performance degradation in the case of a Param that isn't within an aggregate input, add logic to the planner to determine which Params are within aggregate inputs. This requires a new field in struct Agg, but fortunately we never write plans to disk, so this isn't an initdb-forcing change. Per report from Jeevan Chalke. This has been broken since forever, so back-patch to all supported branches. Andrew Gierth, with minor adjustments by me Report: <CAM2+6=VY8ykfLT5Q8vb9B6EbeBk-NGuLbT6seaQ+Fq4zXvrDcA@mail.gmail.com>
* Remove unnecessary #include.Kevin Grittner2016-08-24
| | | | | | Accidentally added in 8b65cf4c5edabdcae45ceaef7b9ac236879aae50. Pointed out by Álvaro Herrera
* Build libpgfeutils before src/bin/pg_basebackup programs.Noah Misch2016-08-23
| | | | Oversight in commit 9132c014290d02435999c81892fa8b0b384497d8.
* Build libpgfeutils before pg_isready.Noah Misch2016-08-23
| | | | | | Every program having -lpgfeutils in LDFLAGS must have this dependency, whether or not the program uses a libpgfeutils symbol. Back-patch to 9.6, where libpgfeutils was introduced.
* Suppress compiler warnings in non-cassert builds.Tom Lane2016-08-23
| | | | | | With Asserts off, these variables are set but never used, resulting in warnings from pickier compilers. Fix that with our standard solution. Per report from Jeff Janes.