summaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2018-09-26Implement %m in src/port/snprintf.c, and teach elog.c to rely on that.Tom Lane
I started out with the idea that we needed to detect use of %m format specs in contexts other than elog/ereport calls, because we couldn't rely on that working in *printf calls. But a better answer is to fix things so that it does work. Now that we're using snprintf.c all the time, we can implement %m in that and we've fixed the problem. This requires also adjusting our various printf-wrapping functions so that they ensure "errno" is preserved when they call snprintf.c. Remove elog.c's handmade implementation of %m, and let it rely on snprintf to support the feature. That should provide some performance gain, though I've not attempted to measure it. There are a lot of places where we could now simplify 'printf("%s", strerror(errno))' into 'printf("%m")', but I'm not in any big hurry to make that happen. Patch by me, reviewed by Michael Paquier Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us
2018-09-26Always use our own versions of *printf().Tom Lane
We've spent an awful lot of effort over the years in coping with platform-specific vagaries of the *printf family of functions. Let's just forget all that mess and standardize on always using src/port/snprintf.c. This gets rid of a lot of configure logic, and it will allow a saner approach to dealing with %m (though actually changing that is left for a follow-on patch). Preliminary performance testing suggests that as it stands, snprintf.c is faster than the native printf functions for some tasks on some platforms, and slower for other cases. A pending patch will improve that, though cases with floating-point conversions will doubtless remain slower unless we want to put a *lot* of effort into that. Still, we've not observed that *printf is really a performance bottleneck for most workloads, so I doubt this matters much. Patch by me, reviewed by Michael Paquier Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us
2018-09-26Incorporate strerror_r() into src/port/snprintf.c, too.Tom Lane
This provides the features that used to exist in useful_strerror() for users of strerror_r(), too. Also, standardize on the GNU convention that strerror_r returns a char pointer that may not be NULL. I notice that libpq's win32.c contains a variant version of strerror_r that probably ought to be folded into strerror.c. But lacking a Windows environment, I should leave that to somebody else. Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us
2018-09-26Convert elog.c's useful_strerror() into a globally-used strerror wrapper.Tom Lane
elog.c has long had a private strerror wrapper that handles assorted possible failures or deficiencies of the platform's strerror. On Windows, it also knows how to translate Winsock error codes, which the native strerror does not. Move all this code into src/port/strerror.c and define strerror() as a macro that invokes it, so that both our frontend and backend code will have all of this behavior. I believe this constitutes an actual bug fix on Windows, since AFAICS our frontend code did not report Winsock error codes properly before this. However, the main point is to lay the groundwork for implementing %m in src/port/snprintf.c: the behavior we want %m to have is this one, not the native strerror's. Note that this throws away the prior use of src/port/strerror.c, which was to implement strerror() on platforms lacking it. That's been dead code for nigh twenty years now, since strerror() was already required by C89. We should likewise cause strerror_r to use this behavior, but I'll tackle that separately. Patch by me, reviewed by Michael Paquier Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us
2018-09-26Update dummy CREATE ASSERTION grammarPeter Eisentraut
While we are probably still far away from fully implementing assertions, all patch proposals appear to take issue with the existing dummy grammar CREATE/DROP ASSERTION productions, so update those a little bit. Rename the rule, use any_name instead of name, and remove some unused code. Also remove the production for DROP ASSERTION, since that would most likely be handled via the generic DROP support. extracted from a patch by Joe Wildish
2018-09-26Improve test coverage of geometric typesTomas Vondra
This commit significantly increases test coverage of geo_ops.c, adding tests for various issues addressed by 2e2a392de3 (which went undetected for a long time, at least partially due to not being covered). This also removes alternative results expecting -0 on some platforms. Instead the functions are should return the same results everywhere, transforming -0 to 0 if needed. The tests are added to geometric.sql file, sorted by the left hand side of the operators. There are many cross datatype operators, so this seems like the best solution. Author: Emre Hasegeli Reviewed-by: Tomas Vondra Discussion: https://www.postgresql.org/message-id/CAE2gYzxF7-5djV6-cEvqQu-fNsnt%3DEqbOURx7ZDg%2BVv6ZMTWbg%40mail.gmail.com
2018-09-26Fix problems in handling the line data typeTomas Vondra
According to the source history, the internal format of line data type has changed, but various functions working with it did were not updated and thus were producing wrong results. This patch addresses various such issues, in particular: * Reject invalid specification A=B=0 on receive * Reject same points on line_construct_pp() * Fix perpendicular operator when negative values are involved * Avoid division by zero on perpendicular operator * Fix intersection and distance operators when neither A nor B are 1 * Return NULL for closest point when objects are parallel * Check whether closest point of line segments is the intersection point * Fix closest point of line segments being on the wrong segment Aside from handling those issues, the patch also aims to make operators more symmetric and less sen to precision loss. The EPSILON interferes with even minor changes, but the least we can do is applying it to both sides of the operators equally. Author: Emre Hasegeli Reviewed-by: Tomas Vondra Discussion: https://www.postgresql.org/message-id/CAE2gYzxF7-5djV6-cEvqQu-fNsnt%3DEqbOURx7ZDg%2BVv6ZMTWbg%40mail.gmail.com
2018-09-26Add basic regression tests for default monitoring rolesMichael Paquier
The following default roles gain some coverage: - pg_read_all_stats - pg_read_all_settings Author: Alexandra Ryzhevich Discussion: https://postgr.es/m/CAOt4E5S5WJmDc9YpS1BfyAMQ5C1NEmiYynD6nUz42qVxphqkpA@mail.gmail.com
2018-09-26Rework activation of commit timestamps during recoveryMichael Paquier
The activation and deactivation of commit timestamp tracking has not been handled consistently for a primary or standbys at recovery. The facility can be activated at three different moments of recovery: - The beginning, where a primary would use the GUC value for the decision-making, and where a standby relies on the contents of the control file. - When replaying a XLOG_PARAMETER_CHANGE record at redo. - The end, where both primary and standby rely on the GUC value. Using the GUC value for a primary at the beginning of recovery causes problems with commit timestamp access when doing crash recovery. Particularly, when replaying transaction commits, it could be possible that an attempt to read commit timestamps is done for a transaction which committed at a moment when track_commit_timestamp was disabled. A test case is added to reproduce the failure. The test works down to v11 as it takes advantage of transaction commits within procedures. Reported-by: Hailong Li Author: Masahiko Sawasa, Michael Paquier Reviewed-by: Kyotaro Horiguchi Discussion: https://postgr.es/m/11224478-a782-203b-1f17-e4797b39bdf0@qunar.com Backpatch-through: 9.5, where commit timestamps have been introduced.
2018-09-25Remove absolete function TupleDescGetSlot().Andres Freund
TupleDescGetSlot() was kept around for backward compatibility for user-written SRFs. With the TupleTableSlot abstraction work, that code will need to be version specific anyway, so there's no point in keeping the function around any longer. Author: Ashutosh Bapat Reviewed-By: Andres Freund Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
2018-09-25Split ExecStoreTuple into ExecStoreHeapTuple and ExecStoreBufferHeapTuple.Andres Freund
Upcoming changes introduce further types of tuple table slots, in preparation of making table storage pluggable. New storage methods will have different representation of tuples, therefore the slot accessor should refer explicitly to heap tuples. Instead of just renaming the functions, split it into one function that accepts heap tuples not residing in buffers, and one accepting ones in buffers. Previously one function was used for both, but that was a bit awkward already, and splitting will allow us to represent slot types for tuples in buffers and normal memory separately. This is split out from the patch introducing abstract slots, as this largely consists out of mechanical changes. Author: Ashutosh Bapat Reviewed-By: Andres Freund Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
2018-09-25Remove function list from prologue of execTuples.c.Andres Freund
That section is never in sync with the actual routines available and their functionality. Author: Ashutosh Bapat Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
2018-09-25Change TupleTableSlot->tts_nvalid to type AttrNumber.Andres Freund
Previously it was an int / 4 bytes. The maximum number of attributes in a tuple is restricted by the maximum value Var->varattno, which is an AttrNumber/int16. Hence use the same data type for TupleTableSlot->tts_nvalid. Author: Ashutosh Bapat Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
2018-09-25Remove obsolete commentAlvaro Herrera
The documented shortcoming was actually fixed in 4c728f3829 so the comment is not true anymore.
2018-09-25Remove fmgr.h inclusion from partition.hAlvaro Herrera
It's not needed anymore.
2018-09-25Collect JIT instrumentation from workers.Andres Freund
Previously, when using parallel query, EXPLAIN (ANALYZE)'s JIT compilation timings did not include the overhead from doing so on the workers. Fix that. We do so by simply aggregating the cost of doing JIT compilation on workers and the leader together. Arguably that's not quite accurate, because the total time spend doing so is spent in parallel - but it's hard to do much better. For additional detail, when VERBOSE is specified, the stats for workers are displayed separately. Author: Amit Khandekar and Andres Freund Discussion: https://postgr.es/m/CAJ3gD9eLrz51RK_gTkod+71iDcjpB_N8eC6vU2AW-VicsAERpQ@mail.gmail.com Backpatch: 11-
2018-09-25Make some fixes to allow building Postgres on macOS 10.14 ("Mojave").Tom Lane
Apple's latest rearrangements of the system-supplied headers have broken building of PL/Perl and PL/Tcl. The only practical way to fix PL/Tcl is to start using the "-isysroot" compiler flag to point to SDK-supplied headers, as Apple expects. We must also start distinguishing where to find Perl's headers from where to find its shared library; but that seems like good cleanup anyway. Extensions that formerly did something like -I$(perl_archlibexp)/CORE should now do -I$(perl_includedir)/CORE instead. perl_archlibexp is still the place to look for libperl.so, though. If for some reason you don't like the default -isysroot setting, you can override that by setting PG_SYSROOT in configure's arguments. I don't currently think people would need to do so, unless maybe for cross-version build purposes. In addition, teach configure where to find tclConfig.sh. Our traditional method of searching $auto_path hasn't worked for the last couple of macOS releases, and it now seems clear that Apple's not going to change that. The workaround of manually specifying --with-tclconfig was annoying already, but Mojave's made it a lot more so because the sysroot path now has to be included as well. Let's just wire the knowledge into configure instead. To avoid breaking builds against non-default Tcl installations (e.g. MacPorts) wherein the $auto_path method probably still works, arrange to try the additional case only after all else has failed. Back-patch to all supported versions, since at least the buildfarm cares about that. The changes are set up to not do anything on macOS releases that are old enough to not have functional sysroot trees.
2018-09-25Avoid unnecessary precision loss for pgbench's --rate target.Tom Lane
It's fairly silly to truncate the throttle_delay to integer when the only math we ever do with it requires converting back to double. Furthermore, given that people are starting to complain about restrictions like only supporting 1K client connections, I don't think we're very far away from situations where the precision loss matters. As the code stood, for example, there's no difference between --rate 100001 and --rate 111111; both get converted to throttle_delay = 9. Somebody trying to run 100 threads and have each one dispatch around 1K TPS would find this lack of precision rather surprising, especially since the required per-thread delays are around 1ms, well within the timing precision of modern systems.
2018-09-25Constify dsa_size_class_map and use a better type.Thomas Munro
Author: Mark G Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAEeOP_Zy_FvVwcAU0UX9nkOhnoR5KN%3D0B6LWX_kv0ZuSc4wbGw%40mail.gmail.com
2018-09-25Ignore publication tables when --no-publications is usedMichael Paquier
96e1cb4 has added support for --no-publications in pg_dump, pg_dumpall and pg_restore, but forgot the fact that publication tables also need to be ignored when this option is used. Author: Gilles Darold Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/3f48e812-b0fa-388e-2043-9a176bdee27e@dalibo.com Backpatch-through: 10, where publications have been added.
2018-09-24Sync our Snowball stemmer dictionaries with current upstream.Tom Lane
We haven't touched these since text search functionality landed in core in 2007 :-(. While the upstream project isn't a beehive of activity, they do make additions and bug fixes from time to time. Update our copies of these files. Also update our documentation about how to keep things in sync, since they're not making distribution tarballs these days. Fortunately, their source code turns out to be a breeze to build. Notable changes: * The non-UTF8 version of the hungarian stemmer now works in LATIN2 not LATIN1. * New stemmers have appeared for arabic, indonesian, irish, lithuanian, nepali, and tamil. These all work in UTF8, and the indonesian and irish ones also work in LATIN1. (There are some new stemmers that I did not incorporate, mainly because their names don't match the underlying languages, suggesting that they're not to be considered mainstream.) Worth noting: the upstream Nepali dictionary was contributed by Arthur Zakirov. initdb forced because the contents of snowball_create.sql have changed. Still TODO: see about updating the stopword lists. Arthur Zakirov, minor mods and doc work by me Discussion: https://postgr.es/m/20180626122025.GA12647@zakirov.localdomain Discussion: https://postgr.es/m/20180219140849.GA9050@zakirov.localdomain
2018-09-24Make EXPLAIN output for JIT compilation more dense.Andres Freund
A discussion about also reporting JIT compilation overhead on workers brought unhappiness with the verbosity of the current explain format to light. Make the text format more dense, and restructure the structured output to mirror that more closely. As we're re-jiggering the output format anyway: The denser format allows us to report all flags for JIT compilation (now also reporting PGJIT_EXPR and PGJIT_DEFORM), and report the total time in addition to the individual times. Per complaint from Tom Lane. Author: Andres Freund Discussion: https://postgr.es/m/27812.1537221015@sss.pgh.pa.us Backpatch: 11-, where JIT compilation was introduced
2018-09-24Fast default trigger and expand_tuple fixesAndrew Dunstan
Ensure that triggers get properly filled in tuples for the OLD value. Also fix the logic of detecting missing null values. The previous logic failed to detect a missing null column before the first missing column with a default. Fixing this has simplified the logic a bit. Regression tests are added to test changes. This should ensure better coverage of expand_tuple(). Original bug reports, and some code and test scripts from Tomas Vondra Backpatch to release 11.
2018-09-24Use ppoll(2), if available, to wait for input in pgbench.Tom Lane
Previously, pgbench always used select(2) for this purpose, but that's problematic for very high client counts, because select() can't deal with file descriptor numbers larger than FD_SETSIZE. It's pretty common for that to be only 1024 or so, whereas modern OSes can allow many more open files than that. Using poll(2) would surmount that problem, but it creates another one: poll()'s timeout resolution is only 1ms, which is poor enough to cause problems with --rate specifications approaching or exceeding 1K TPS. On platforms that have ppoll(2), which includes Linux and recent FreeBSD, we can use that to avoid the FD_SETSIZE problem without any loss of timeout resolution. Hence, add configure logic to test for ppoll(), and use it if available. This patch introduces an abstraction layer into pgbench that could be extended to support other kernel event-wait APIs such as kevents. But actually adding such support is a matter for some future patch. Doug Rady, reviewed by Robert Haas and Fabien Coelho, and whacked around a good bit more by me Discussion: https://postgr.es/m/23D017C9-81B7-484D-8490-FD94DEC4DF59@amazon.com
2018-09-24Fix over-allocation of space for array_out()'s result string.Tom Lane
array_out overestimated the space needed for its output, possibly by a very substantial amount if the array is multi-dimensional, because of wrong order of operations in the loop that counts the number of curly-brace pairs needed. While the output string is normally short-lived, this could still cause problems in extreme cases. An additional minor error was that it counted one more delimiter than is actually needed. Repair those errors, add an Assert that the space is now correctly calculated, and make some minor improvements in the comments. I also failed to resist the temptation to get rid of an integer modulus operation per array element; a simple comparison is sufficient. This bug dates clear back to Berkeley days, so back-patch to all supported versions. Keiichi Hirobe, minor additional work by me Discussion: https://postgr.es/m/CAH=EFxE9W0tRvQkixR2XJRRCToUYUEDkJZk6tnADXugPBRdcdg@mail.gmail.com
2018-09-24Document aclitem functions and operatorsJoe Conway
aclitem functions and operators have been heretofore undocumented. Fix that. While at it, ensure the non-operator aclitem functions have pg_description strings. Does not seem worthwhile to back-patch. Author: Fabien Coelho, with pg_description from John Naylor, and significant refactoring and editorialization by me. Reviewed by: Tom Lane Discussion: https://postgr.es/m/flat/alpine.DEB.2.21.1808010825490.18204%40lancre
2018-09-23Initialize random() in bootstrap/stand-alone postgres and in initdb.Noah Misch
This removes a difference between the standard IsUnderPostmaster execution environment and that of --boot and --single. In a stand-alone backend, "SELECT random()" always started at the same seed. On a system capable of using posix shared memory, initdb could still conclude "selecting dynamic shared memory implementation ... sysv". Crashed --boot or --single postgres processes orphaned shared memory objects having names that collided with the not-actually-random names that initdb probed. The sysv fallback appeared after ten crashes of --boot or --single postgres. Since --boot and --single are rare in production use, systems used for PostgreSQL development are the principal candidate to notice this symptom. Back-patch to 9.3 (all supported versions). PostgreSQL 9.4 introduced dynamic shared memory, but 9.3 does share the "SELECT random()" problem. Reviewed by Tom Lane and Kyotaro HORIGUCHI. Discussion: https://postgr.es/m/20180915221546.GA3159382@rfd.leadboat.com
2018-09-23Fix failure in WHERE CURRENT OF after rewinding the referenced cursor.Tom Lane
In a case where we have multiple relation-scan nodes in a cursor plan, such as a scan of an inheritance tree, it's possible to fetch from a given scan node, then rewind the cursor and fetch some row from an earlier scan node. In such a case, execCurrent.c mistakenly thought that the later scan node was still active, because ExecReScan hadn't done anything to make it look not-active. We'd get some sort of failure in the case of a SeqScan node, because the node's scan tuple slot would be pointing at a HeapTuple whose t_self gets reset to invalid by heapam.c. But it seems possible that for other relation scan node types we'd actually return a valid tuple TID to the caller, resulting in updating or deleting a tuple that shouldn't have been considered current. To fix, forcibly clear the ScanTupleSlot in ExecScanReScan. Another issue here, which seems only latent at the moment but could easily become a live bug in future, is that rewinding a cursor does not necessarily lead to *immediately* applying ExecReScan to every scan-level node in the plan tree. Upper-level nodes will think that they can postpone that call if their child node is already marked with chgParam flags. I don't see a way for that to happen today in a plan tree that's simple enough for execCurrent.c's search_plan_tree to understand, but that's one heck of a fragile assumption. So, add some logic in search_plan_tree to detect chgParam flags being set on nodes that it descended to/through, and assume that that means we should consider lower scan nodes to be logically reset even if their ReScan call hasn't actually happened yet. Per bug #15395 from Matvey Arye. This has been broken for a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/153764171023.14986.280404050547008575@wrigleys.postgresql.org
2018-09-22Replace CAS loop with single TAS in ProcArrayGroupClearXid()Alexander Korotkov
Single pg_atomic_exchange_u32() is expected to be faster than loop of pg_atomic_compare_exchange_u32(). Also, it would be consistent with clog group update code. Discussion: https://postgr.es/m/CAPpHfdtxLsC-bqfxFcHswZ91OxXcZVNDBBVfg9tAWU0jvn1tQA%40mail.gmail.com Reviewed-by: Amit Kapila
2018-09-22Make GUC wal_sender_timeout user-settableMichael Paquier
Being able to use a value that can be changed on a connection basis is useful with clusters distributed geographically, and makes failure detection more flexible. A note is added in the documentation about the use of "options" in primary_conninfo, which can be hard to grasp for newcomers with the need of two single quotes when listing a set of parameters. Author: Tsunakawa Takayuki Reviewed-by: Masahiko Sawada, Michael Paquier Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1FAAD3AE@G01JPEXMBYT05
2018-09-21Get rid of explicit argument-count markings in tab-complete.c.Tom Lane
This replaces the "TailMatchesN" macros with just "TailMatches", and likewise "HeadMatchesN" becomes "HeadMatches" and "MatchesN" becomes "Matches". The various COMPLETE_WITH_LISTn macros are reduced to COMPLETE_WITH, and the single-item COMPLETE_WITH_CONST also gets folded into that. This eliminates a lot of minor annoyance in writing tab-completion rules. Usefully, the compiled code also gets a bit smaller (10% or so, on my machine). The implementation depends on variadic macros, so we couldn't have done this before we required C99. Andres Freund and Thomas Munro; some cosmetic cleanup by me. Discussion: https://postgr.es/m/d8jo9djvm7h.fsf@dalvik.ping.uio.no
2018-09-21Fix bogus tab-completion rule for CREATE PUBLICATION.Tom Lane
You can't use "FOR TABLE" as a single Matches argument, because readline will consider that input to be two words not one. It's necessary to make the pattern contain two arguments. The case accidentally worked anyway because the words_after_create test fired ... but only for the first such table name. Noted by Edmund Horner, though this isn't exactly his proposed fix. Backpatch to v10 where the faulty code came in. Discussion: https://postgr.es/m/CAMyN-kDe=gBmHgxWwUUaXuwK+p+7g1vChR7foPHRDLE592nJPQ@mail.gmail.com
2018-09-21Improve tab completion for ANALYZE, EXPLAIN, and VACUUM.Tom Lane
Previously, we made no attempt to provide tab completion in these statements' optional parenthesized options lists. This patch teaches psql to do so. To prevent the option completions from being offered after we've already seen a complete parenthesized option list, it's necessary to improve word_matches() so that it allows a wildcard '*' in the middle of an alternative, not only at the end as formerly. That requires only a little more code than before, and it allows us to test for "incomplete parenthesized options" with a test like else if (HeadMatches2("EXPLAIN", "(*") && !HeadMatches2("EXPLAIN", "(*)")) In addition, add some logic to offer column names in the context of "ANALYZE tablename ( ...", and likewise for VACUUM. This isn't real complete; it won't offer column names again after a comma. But it's better than before, and it doesn't take much code. Justin Pryzby, reviewed at various times by Álvaro Herrera, Arthur Zakirov, and Edmund Horner; some additional fixups by me Discussion: https://postgr.es/m/20180529000623.GA21896@telsasoft.com
2018-09-21Rationalize Query_for_list_of_[relations] query names in tab-complete.c.Tom Lane
The previous convention was to use names based on the set of relkinds being selected for, which was not at all helpful for maintenance, especially since people had been quite inconsistent about whether to change the names when they changed the relkinds being selected for. Instead, use names based on the functionality we need the relation to have, following the model established by Query_for_list_of_updatables. While at it, sort the list of Query constants a bit better; it had the distinct air of code-assembled-by-dartboard before. Discussion: https://postgr.es/m/14830.1537481254@sss.pgh.pa.us
2018-09-22Use size_t consistently in dsa.{ch}.Thomas Munro
Takeshi Ideriha complained that there is a mixture of Size and size_t in dsa.c and corresponding header. Let's use size_t. Back-patch to 10 where dsa.c landed, to make future back-patching easy. Discussion: https://postgr.es/m/4E72940DA2BF16479384A86D54D0988A6F19ABD9%40G01JPEXMBKW04
2018-09-21Remove special handling for open() in initdb for WindowsMichael Paquier
40cfe86 enforces the translation mode to text for all frontends, so this special handling in initdb is not needed anymore.
2018-09-20Fix psql's tab completion for TABLE.Tom Lane
This should offer the same relation types that SELECT ... FROM would. You can't select from an index for instance, so offering it here is unhelpful. Noted while testing ilmari's recent patch.
2018-09-20Fix psql's tab completion for ALTER DATABASE ... SET TABLESPACE.Tom Lane
We have the infrastructure to offer a list of tablespace names, but it wasn't being used here; instead you got "FROM", "CURRENT", and "TO" which aren't actually legal in this syntax. Dagfinn Ilmari Mannsåker, reviewed by Arthur Zakirov Discussion: https://postgr.es/m/d8jo9djvm7h.fsf@dalvik.ping.uio.no
2018-09-20Add a "return" statement to pacify perlcritic.Tom Lane
Per buildfarm member crake.
2018-09-20Add missing pg_description strings for pg_type entries.Tom Lane
I noticed that all non-composite, non-array entries in pg_type.dat had descr strings, except for "json" and the pseudo-types. The lack for json seems certainly an oversight, and there's surely little reason to not have entries for the pseudo-types either. So add some. "make reformat-dat-files" turned up some formatting issues in pg_amop.dat, too, so fix those in passing. No catversion bump since the backend doesn't care too much what is in pg_description.
2018-09-20Teach genbki.pl to auto-generate pg_type entries for array types.Tom Lane
This eliminates some more tedium in adding new catalog entries, specifically the need to set up an array type when adding a new built-in data type. Now it's sufficient to assign an OID for the array type and write it in an "array_type_oid" metadata field. You don't have to fill the base type's typarray link explicitly, either. No catversion bump since the contents of pg_type aren't changed. (Well, their order might be different, but that doesn't matter.) John Naylor, reviewed and whacked around a bit by Dagfinn Ilmari Mannsåker, and some more by me. Discussion: https://postgr.es/m/CAJVSVGVTb6m9pJF49b3SuA8J+T-THO9c0hxOmoyv-yGKh-FbNg@mail.gmail.com
2018-09-20Fix handling of format string text characters in to_timestamp()/to_date()Alexander Korotkov
cf984672 introduced improvement of handling of spaces and separators in to_timestamp()/to_date() functions. In particular, now we're skipping spaces both before and after fields. That may cause format string text character to consume part of field in the situations, when it didn't happen before cf984672. This commit cause format string text character consume input string characters only when since previous field (or string beginning) number of skipped input string characters is not greater than number of corresponding format string characters (that is we didn't skip any extra characters in input string).
2018-09-20Fix segment_bins corruption in dsa.c.Thomas Munro
If a segment has been freed by dsa.c because it is entirely empty, other backends must make sure to unmap it before following links to new segments that might happen to have the same index number, or they could finish up looking at a defunct segment and then corrupt the segment_bins lists. The correct protocol requires checking freed_segment_counter after acquiring the area lock and before resolving any index number to a segment. Add the missing checks and an assertion. Back-patch to 10, where dsa.c first arrived. Author: Thomas Munro Reported-by: Tomas Vondra Discussion: https://postgr.es/m/CAEepm%3D0thg%2Bja5zGVa7jBy-uqyHrTqTm8HGhEOtMmigGrAqTbw%40mail.gmail.com
2018-09-20Defer restoration of libraries in parallel workers.Thomas Munro
Several users of extensions complained of crashes in parallel workers that turned out to be due to syscache access from their _PG_init() functions. Reorder the initialization of parallel workers so that libraries are restored after the caches are initialized, and inside a transaction. This was reported in bug #15350 and elsewhere. We don't consider it to be a bug: extensions shouldn't do that, because then they can't be used in shared_preload_libraries. However, it's a fairly obscure hazard and these extensions worked in practice before parallel query came along. So let's make it work. Later commits might add a warning message and eventually an error. Back-patch to 9.6, where parallel query landed. Author: Thomas Munro Reviewed-by: Amit Kapila Reported-by: Kieran McCusker, Jimmy Discussion: https://postgr.es/m/153512195228.1489.8545997741965926448%40wrigleys.postgresql.org
2018-09-20Enforce translation mode for Windows frontends to text with open/fopenMichael Paquier
Allowing frontends to use concurrent-safe open() and fopen() via 0ba06e0 has the side-effect of switching the default translation mode from text to binary, so the switch can cause breakages for frontend tools when the caller of those new versions specifies neither binary and text. This commit makes sure to maintain strict compatibility with past versions, so as no frontends should see a difference when upgrading. Author: Laurenz Albe Reviewed-by: Michael Paquier, Tom Lane Discussion: https://postgr.es/m/20180917140202.GF31460@paquier.xyz
2018-09-19Fix minor error message style guide violation.Tom Lane
No periods at the ends of primary error messages, please. Daniel Gustafsson Discussion: https://postgr.es/m/43E004C0-18C6-42B4-A313-003B43EB0571@yesql.se
2018-09-19Don't ignore locktable-full failures in StandbyAcquireAccessExclusiveLock.Tom Lane
Commit 37c54863c removed the code in StandbyAcquireAccessExclusiveLock that checked the return value of LockAcquireExtended. That created a bug, because it's still passing reportMemoryError = false to LockAcquireExtended, meaning that LOCKACQUIRE_NOT_AVAIL will be returned if we're out of shared memory for the lock table. In such a situation, the startup process would believe it had acquired an exclusive lock even though it hadn't, with potentially dire consequences. To fix, just drop the use of reportMemoryError = false, which allows us to simplify the call into a plain LockAcquire(). It's unclear that the locktable-full situation arises often enough that it's worth having a better recovery method than crash-and-restart. (I strongly suspect that the only reason the code path existed at all was that it was relatively simple to do in the pre-37c54863c implementation. But now it's not.) LockAcquireExtended's reportMemoryError parameter is now dead code and could be removed. I refrained from doing so, however, because there was some interest in resurrecting the behavior if we do get reports of locktable-full failures in the field. Also, it seems unwise to remove the parameter concurrently with shipping commit f868a8143, which added a parameter; if there are any third-party callers of LockAcquireExtended, we want them to get a wrong-number-of-parameters compile error rather than a possibly-silent misinterpretation of its last parameter. Back-patch to 9.6 where the bug was introduced. Discussion: https://postgr.es/m/6202.1536359835@sss.pgh.pa.us
2018-09-19Add support for nearest-neighbor (KNN) searches to SP-GiSTAlexander Korotkov
Currently, KNN searches were supported only by GiST. SP-GiST also capable to support them. This commit implements that support. SP-GiST scan stack is replaced with queue, which serves as stack if no ordering is specified. KNN support is provided for three SP-GIST opclasses: quad_point_ops, kd_point_ops and poly_ops (catversion is bumped). Some common parts between GiST and SP-GiST KNNs are extracted into separate functions. Discussion: https://postgr.es/m/570825e8-47d0-4732-2bf6-88d67d2d51c8%40postgrespro.ru Author: Nikita Glukhov, Alexander Korotkov based on GSoC work by Vlad Sterzhanov Review: Andrey Borodin, Alexander Korotkov
2018-09-18Add a debugging option to stress-test outfuncs.c and readfuncs.c.Tom Lane
In the normal course of operation, query trees will be serialized only if they are stored as views or rules; and plan trees will be serialized only if they get passed to parallel-query workers. This leaves an awful lot of opportunity for bugs/oversights to not get detected, as indeed we've just been reminded of the hard way. To improve matters, this patch adds a new compile option WRITE_READ_PARSE_PLAN_TREES, which is modeled on the longstanding option COPY_PARSE_PLAN_TREES; but instead of passing all parse and plan trees through copyObject, it passes them through nodeToString + stringToNode. Enabling this option in a buildfarm animal or two will catch problems at least for cases that are exercised by the regression tests. A small problem with this idea is that readfuncs.c historically has discarded location fields, on the reasonable grounds that parse locations in a retrieved view are not relevant to the current query. But doing that in WRITE_READ_PARSE_PLAN_TREES breaks pg_stat_statements, and it could cause problems for future improvements that might try to report error locations at runtime. To fix that, provide a variant behavior in readfuncs.c that makes it restore location fields when told to. In passing, const-ify the string arguments of stringToNode and its subsidiary functions, just because it annoyed me that they weren't const already. Discussion: https://postgr.es/m/17114.1537138992@sss.pgh.pa.us
2018-09-18Fix some minor issues exposed by outfuncs/readfuncs testing.Tom Lane
A test patch to pass parse and plan trees through outfuncs + readfuncs exposed several issues that need to be fixed to get clean matches: Query.withCheckOptions failed to get copied; it's intentionally ignored by outfuncs/readfuncs on the grounds that it'd always be NIL anyway in stored rules. This seems less than future-proof, and it's not even saving very much, so just undo the decision and treat the field like all others. Several places that convert a view RTE into a subquery RTE, or similar manipulations, failed to clear out fields that were specific to the original RTE type and should be zero in a subquery RTE. Since readfuncs.c will leave such fields as zero, equalfuncs.c thinks the nodes are different leading to a reported mismatch. It seems like a good idea to clear out the no-longer-needed fields, even though in principle nothing should look at them; the node ought to be indistinguishable from how it would look if we'd built a new node instead of scribbling on the old one. BuildOnConflictExcludedTargetlist randomly set the resname of some TargetEntries to "" not NULL. outfuncs/readfuncs don't distinguish those cases, and so the string will read back in as NULL ... but equalfuncs.c does distinguish. Perhaps we ought to try to make things more consistent in this area --- but it's just useless extra code space for BuildOnConflictExcludedTargetlist to not use NULL here, so I fixed it for now by making it do that. catversion bumped because the change in handling of Query.withCheckOptions affects stored rules. Discussion: https://postgr.es/m/17114.1537138992@sss.pgh.pa.us