aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Fix issues around EXPLAIN with JIT.Andres Freund2018-10-03
| | | | | | | | | | | | | | | | | I (Andres) was more than a bit hasty in committing 33001fd7a7072d48327 after last minute changes, leading to a number of problems (jit output was only shown for JIT in parallel workers, and just EXPLAIN without ANALYZE didn't work). Lukas luckily found these issues quickly. Instead of combining instrumentation in in standard_ExecutorEnd(), do so on demand in the new ExplainPrintJITSummary(). Also update a documentation example of the JIT output, changed in 52050ad8ebec8d831. Author: Lukas Fittl, with minor changes by me Discussion: https://postgr.es/m/CAP53PkxmgJht69pabxBXJBM+0oc6kf3KHMborLP7H2ouJ0CCtQ@mail.gmail.com Backpatch: 11, where JIT compilation was introduced
* Rationalize snprintf.c's handling of "ll" formats.Tom Lane2018-10-03
| | | | | | | | | | | | | | | | | | | Although all known platforms define "long long" as 64 bits, it still feels a bit shaky to be using "va_arg(args, int64)" to pull out an argument that the caller thought was declared "long long". The reason it was coded like this, way back in commit 3311c7669, was to work around the possibility that the compiler had no type named "long long" --- and, at the time, that it maybe didn't have 64-bit ints at all. Now that we're requiring compilers to support C99, those concerns are moot. Let's make the code clearer and more bulletproof by writing "long long" where we mean "long long". This does introduce a hazard that we'd inefficiently use 128-bit arithmetic to convert plain old integers. The way to tackle that would be to provide two versions of fmtint(), one for "long long" and one for narrower types. Since, as of today, no platforms require that, we won't bother with the extra code for now. Discussion: https://postgr.es/m/1680.1538587115@sss.pgh.pa.us
* Provide fast path in snprintf.c for conversion specs that are just "%s".Tom Lane2018-10-03
| | | | | | | | | This case occurs often enough (around 45% of conversion specs executed in our regression tests are just "%s") that it's worth an extra test per conversion spec to allow skipping all the logic associated with field widths and padding when it happens. Discussion: https://postgr.es/m/26193.1538582367@sss.pgh.pa.us
* Make assorted performance improvements in snprintf.c.Tom Lane2018-10-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In combination, these changes make our version of snprintf as fast or faster than most platforms' native snprintf, except for cases involving floating-point conversion (which we still delegate to the native sprintf). The speed penalty for a float conversion is down to around 10% though, much better than before. Notable changes: * Rather than always parsing the format twice to see if it contains instances of %n$, do the extra scan only if we actually find a $. This obviously wins for non-localized formats, and even when there is use of %n$, we can avoid scanning text before the first % twice. * Use strchrnul() if available to find the next %, and emit the literal text between % escapes as strings rather than char-by-char. * Create a bespoke function (dopr_outchmulti) for the common case of emitting N copies of the same character, in place of writing loops around dopr_outch. * Simplify construction of the format string for invocations of sprintf for floats. * Const-ify some internal functions, and avoid unnecessary use of pass-by-reference arguments. Patch by me, reviewed by Andres Freund Discussion: https://postgr.es/m/11787.1534530779@sss.pgh.pa.us
* MAXALIGN the target address where we store flattened value.Amit Kapila2018-10-03
| | | | | | | | | | | | The API (EOH_flatten_into) that flattens the expanded value representation expects the target address to be maxaligned. All it's usage adhere to that principle except when serializing datums for parallel query. Fix that usage. Diagnosed-by: Tom Lane Author: Tom Lane and Amit Kapila Backpatch-through: 9.6 Discussion: https://postgr.es/m/11629.1536550032@sss.pgh.pa.us
* Don't build static libraries on CygwinAndrew Dunstan2018-10-02
| | | | | | | | | | | | | | Cygwin has been building and linking against static libraries. Although a bug this has been relatively harmless until now, when this has caused errors due to changes in the way we build certain libraries. So this patch makes things work the way we always intended, namely that we would link against the dynamic libraries (cygpq.dll etc.) and just not build the static libraries. The downstream packagers have been doing this for some time, so this just aligns with their practice. Extracted from a patch by Marco Atzeri, with a suggestion from Tom Lane. Discussion: https://postgr.es/m/1056.1538235347@sss.pgh.pa.us
* Change rewriter/planner/executor/plancache to depend on RTE rellockmode.Tom Lane2018-10-02
| | | | | | | | | | | | | | | | | | | | | | | | | Instead of recomputing the required lock levels in all these places, just use what commit fdba460a2 made the parser store in the RTE fields. This already simplifies the code measurably in these places, and follow-on changes will remove a bunch of no-longer-needed infrastructure. In a few cases, this change causes us to acquire a higher lock level than we did before. This is OK primarily because said higher lock level should've been acquired already at query parse time; thus, we're saving a useless extra trip through the shared lock manager to acquire a lesser lock alongside the original lock. The only known exception to this is that re-execution of a previously planned SELECT FOR UPDATE/SHARE query, for a table that uses ROW_MARK_REFERENCE or ROW_MARK_COPY methods, might have gotten only AccessShareLock before. Now it will get RowShareLock like the first execution did, which seems fine. While there's more to do, push it in this state anyway, to let the buildfarm help verify that nothing bad happened. Amit Langote, reviewed by David Rowley and Jesper Pedersen, and whacked around a bit more by me Discussion: https://postgr.es/m/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp
* Use slots more widely in tuple mapping code and make naming more consistent.Andres Freund2018-10-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | It's inefficient to use a single slot for mapping between tuple descriptors for multiple tuples, as previously done when using ConvertPartitionTupleSlot(), as that means the slot's tuple descriptors change for every tuple. Previously we also, via ConvertPartitionTupleSlot(), built new tuples after the mapping even in cases where we, immediately afterwards, access individual columns again. Refactor the code so one slot, on demand, is used for each partition. That avoids having to change the descriptor (and allows to use the more efficient "fixed" tuple slots). Then use slot->slot mapping, to avoid unnecessarily forming a tuple. As the naming between the tuple and slot mapping functions wasn't consistent, rename them to execute_attr_map_{tuple,slot}. It's likely that we'll also rename convert_tuples_by_* to denote that these functions "only" build a map, but that's left for later. Author: Amit Khandekar and Amit Langote, editorialized by me Reviewed-By: Amit Langote, Amit Khandekar, Andres Freund Discussion: https://postgr.es/m/CAJ3gD9fR0wRNeAE8VqffNTyONS_UfFPRpqxhnD9Q42vZB+Jvpg@mail.gmail.com https://postgr.es/m/e4f9d743-cd4b-efb0-7574-da21d86a7f36%40lab.ntt.co.jp Backpatch: -
* Set snprintf.c's maximum number of NL arguments to be 31.Tom Lane2018-10-02
| | | | | | | | | | | | | | | | | | | | | | | | Previously, we used the platform's NL_ARGMAX if any, otherwise 16. The trouble with this is that the platform value is hugely variable, ranging from the POSIX-minimum 9 to as much as 64K on recent FreeBSD. Values of more than a dozen or two have no practical use and slow down the initialization of the argtypes array. Worse, they cause snprintf.c to consume far more stack space than was the design intention, possibly resulting in stack-overflow crashes. Standardize on 31, which is comfortably more than we need (it looks like no existing translatable message has more than about 10 parameters). I chose that, not 32, to make the array sizes powers of 2, for some possible small gain in speed of the memset. The lack of reported crashes suggests that the set of platforms we use snprintf.c on (in released branches) may have no overlap with the set where NL_ARGMAX has unreasonably large values. But that's not entirely clear, so back-patch to all supported branches. Per report from Mateusz Guzik (via Thomas Munro). Discussion: https://postgr.es/m/CAEepm=3VF=PUp2f8gU8fgZB22yPE_KBS0+e1AHAtQ=09schTHg@mail.gmail.com
* Fix corner-case failures in has_foo_privilege() family of functions.Tom Lane2018-10-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The variants of these functions that take numeric inputs (OIDs or column numbers) are supposed to return NULL rather than failing on bad input; this rule reduces problems with snapshot skew when queries apply the functions to all rows of a catalog. has_column_privilege() had careless handling of the case where the table OID didn't exist. You might get something like this: select has_column_privilege(9999,'nosuchcol','select'); ERROR: column "nosuchcol" of relation "(null)" does not exist or you might get a crash, depending on the platform's printf's response to a null string pointer. In addition, while applying the column-number variant to a dropped column returned NULL as desired, applying the column-name variant did not: select has_column_privilege('mytable','........pg.dropped.2........','select'); ERROR: column "........pg.dropped.2........" of relation "mytable" does not exist It seems better to make this case return NULL as well. Also, the OID-accepting variants of has_foreign_data_wrapper_privilege, has_server_privilege, and has_tablespace_privilege didn't follow the principle of returning NULL for nonexistent OIDs. Superusers got TRUE, everybody else got an error. Per investigation of Jaime Casanova's report of a new crash in HEAD. These behaviors have been like this for a long time, so back-patch to all supported branches. Patch by me; thanks to Stephen Frost for discussion and review Discussion: https://postgr.es/m/CAJGNTeP=-6Gyqq5TN9OvYEydi7Fv1oGyYj650LGTnW44oAzYCg@mail.gmail.com
* Test passing expanded-value representations to workers.Amit Kapila2018-10-02
| | | | | | | | | | | | Currently, we don't have an explicit test to pass expanded-value representations to workers, so we don't know whether it works on all kind of platforms. We suspect that the current code won't work on alignment-sensitive hardware. This commit will test that aspect and can lead to failure on some of the buildfarm machines which we will fix in the later commit. Author: Tom Lane and Amit Kapila Discussion: https://postgr.es/m/11629.1536550032@sss.pgh.pa.us
* Refactor relation opening for VACUUM and ANALYZEMichael Paquier2018-10-02
| | | | | | | | | | | | | | | VACUUM and ANALYZE share similar logic when it comes to opening a relation to work on in terms of how the relation is opened, in which order locks are tried and how logs should be generated when something does not work as expected. This commit refactors things so as both use the same code path to handle the way a relation is opened, so as the integration of new options becomes easier. Author: Michael Paquier Reviewed-by: Nathan Bossart Discussion: https://postgr.es/m/20180927075152.GT1659@paquier.xyz
* Change PROCEDURE to FUNCTION in CREATE EVENT TRIGGER syntaxPeter Eisentraut2018-10-01
| | | | | | | This was claimed to have been done in 0a63f996e018ac508c858e87fa39cc254a5db49f, but that actually only changed the documentation and not the grammar. (That commit did fully change it for CREATE TRIGGER.)
* Add assertions that we hold some relevant lock during relation open.Tom Lane2018-10-01
| | | | | | | | | | | | | | | | | | | | | | | | | Opening a relation with no lock at all is unsafe; there's no guarantee that we'll see a consistent state of the relevant catalog entries. While use of MVCC scans to read the catalogs partially addresses that complaint, it's still possible to switch to a new catalog snapshot partway through loading the relcache entry. Moreover, whether or not you trust the reasoning behind sometimes using less than AccessExclusiveLock for ALTER TABLE, that reasoning is certainly not valid if concurrent users of the table don't hold a lock corresponding to the operation they want to perform. Hence, add some assertion-build-only checks that require any caller of relation_open(x, NoLock) to hold at least AccessShareLock. This isn't a full solution, since we can't verify that the lock level is semantically appropriate for the action --- but it's definitely of some use, because it's already caught two bugs. We can also assert that callers of addRangeTableEntryForRelation() hold at least the lock level specified for the new RTE. Amit Langote and Tom Lane Discussion: https://postgr.es/m/16565.1538327894@sss.pgh.pa.us
* Fix ALTER COLUMN TYPE to not open a relation without any lock.Tom Lane2018-10-01
| | | | | | | | | | | | | | | | | | | If the column being modified is referenced by a foreign key constraint of another table, ALTER TABLE would open the other table (to re-parse the constraint's definition) without having first obtained a lock on it. This was evidently intentional, but that doesn't mean it's really safe. It's especially not safe in 9.3, which pre-dates use of MVCC scans for catalog reads, but even in current releases it doesn't seem like a good idea. We know we'll need AccessExclusiveLock shortly to drop the obsoleted constraint, so just get that a little sooner to close the hole. Per testing with a patch that complains if we open a relation without holding any lock on it. I don't plan to back-patch that patch, but we should close the holes it identifies in all supported branches. Discussion: https://postgr.es/m/2038.1538335244@sss.pgh.pa.us
* Create an RTE field to record the query's lock mode for each relation.Tom Lane2018-09-30
| | | | | | | | | | | | | | | | | | | | | | | | Add RangeTblEntry.rellockmode, which records the appropriate lock mode for each RTE_RELATION rangetable entry (either AccessShareLock, RowShareLock, or RowExclusiveLock depending on the RTE's role in the query). This patch creates the field and makes all creators of RTE nodes fill it in reasonably, but for the moment nothing much is done with it. The plan is to replace assorted post-parser logic that re-determines the right lockmode to use with simple uses of rte->rellockmode. For now, just add Asserts in each of those places that the rellockmode matches what they are computing today. (In some cases the match isn't perfect, so the Asserts are weaker than you might expect; but this seems OK, as per discussion.) This passes check-world for me, but it seems worth pushing in this state to see if the buildfarm finds any problems in cases I failed to test. catversion bump due to change of stored rules. Amit Langote, reviewed by David Rowley and Jesper Pedersen, and whacked around a bit more by me Discussion: https://postgr.es/m/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp
* Add application_name to connection authorized msgStephen Frost2018-09-28
| | | | | | | | | | | | | | | | | | | | | The connection authorized message has quite a bit of useful information in it, but didn't include the application_name (when provided), so let's add that as it can be very useful. Note that at the point where we're emitting the connection authorized message, we haven't processed GUCs, so it's not possible to get this by using log_line_prefix (which pulls from the GUC). There's also something to be said for having this included in the connection authorized message and then not needing to repeat it for every line, as having it in log_line_prefix would do. The GUC cleans the application name to pure-ascii, so do that here too, but pull out the logic for cleaning up a string into its own function in common and re-use it from those places, and check_cluster_name which was doing the same thing. Author: Don Seiler <don@seiler.us> Discussion: https://postgr.es/m/CAHJZqBB_Pxv8HRfoh%2BAB4KxSQQuPVvtYCzMg7woNR3r7dfmopw%40mail.gmail.com
* Improve error reporting for unsupported effective_io_concurrency setting.Tom Lane2018-09-28
| | | | | | | | | | | | | | Give a specific error complaining about lack of posix_fadvise() when someone tries to set effective_io_concurrency > 0 on platforms without that. This probably isn't worth extensive back-patching, but I (tgl) felt cramming it into v11 was reasonable. James Robinson Discussion: https://postgr.es/m/153771876450.14994.560017943128223619@wrigleys.postgresql.org Discussion: https://postgr.es/m/A3942987-5BC7-4F05-B54D-2A0EC2914B33@jlr-photo.com
* Tweak MSVC build system to match changes in 7143b3e82.Tom Lane2018-09-28
| | | | | | | Looks like we need to pull in $libpgcommon in a couple more places than before. Per buildfarm.
* Tweak MSVC build system to match changes in 7143b3e82.Tom Lane2018-09-28
| | | | | | | Also try to make the comment suggesting that this might be needed more intelligible. Per buildfarm.
* Build src/common files as a library with -fPIC.Tom Lane2018-09-28
| | | | | | | | | | | | | | Build a third version of libpgcommon.a, with -fPIC and -DFRONTEND, as commit ea53100d5 did for src/port. Use that in libpq to avoid symlinking+rebuilding source files retail. Also adjust ecpg to use the new src/port and src/common libraries. Arrange to install these libraries, too, to simplify out-of-tree builds of shared libraries that need any of these modules. Discussion: https://postgr.es/m/13022.1538003440@sss.pgh.pa.us Discussion: https://postgr.es/m/E1g5Y8r-0006vs-QA@gemulon.postgresql.org
* Remove pqsignal() from libpq's official exports list.Tom Lane2018-09-28
| | | | | | | | | | | | | | | | | | | | | Client applications should get this function, if they need it, from libpgport. The fact that it's exported from libpq is a hack left over from before we set up libpgport. It's never been documented, and there's no good reason for non-PG code to be calling it anyway, so hopefully this won't cause any problems. Moreover, with the previous setup it was not real clear whether our clients that use the function were getting it from libpgport or libpq, so this might actually prevent problems. The reason for changing it now is that in the wake of commit ea53100d5, some linkers won't export the symbol, apparently because it's coming from a .a library instead of a .o file. We could get around that by continuing to symlink pqsignal.c into libpq as before; but unless somebody complains very hard, I don't want to adopt such a kluge. Discussion: https://postgr.es/m/13022.1538003440@sss.pgh.pa.us Discussion: https://postgr.es/m/E1g5Y8r-0006vs-QA@gemulon.postgresql.org
* Fix assertion failure when updating full_page_writes for checkpointer.Amit Kapila2018-09-28
| | | | | | | | | | | | | | | | | | When the checkpointer receives a SIGHUP signal to update its configuration, it may need to update the shared memory for full_page_writes and need to write a WAL record for it. Now, it is quite possible that the XLOG machinery has not been initialized by that time and it will lead to assertion failure while doing that. Fix is to allow the initialization of the XLOG machinery outside critical section. This bug has been introduced by the commit 2c03216d83 which added the XLOG machinery initialization in RecoveryInProgress code path. Reported-by: Dilip Kumar Author: Dilip Kumar Reviewed-by: Michael Paquier and Amit Kapila Backpatch-through: 9.5 Discussion: https://postgr.es/m/CAFiTN-u4BA8KXcQUWDPNgaKAjDXC=C2whnzBM8TAcv=stckYUw@mail.gmail.com
* Correct overflow handling in pgbench.Andres Freund2018-09-27
| | | | | | | | | | This patch attempts, although it's quite possible there are a few holes, to properly detect and reported signed integer overflows in pgbench. Author: Fabien Coelho Reviewed-By: Andres Freund Discussion: https://postgr.es/m/20171212052943.k2hlckfkeft3eiio@alap3.anarazel.de
* Fix WAL recycling on standbys depending on archive_modeMichael Paquier2018-09-28
| | | | | | | | | | | | | | | | | | | | | | | | A restart point or a checkpoint recycling WAL segments treats segments marked with neither ".done" (archiving is done) or ".ready" (segment is ready to be archived) in archive_status the same way for archive_mode being "on" or "always". While for a primary this is fine, a standby running a restart point with archive_mode = on would try to mark such a segment as ready for archiving, which is something that will never happen except after the standby is promoted. Note that this problem applies only to WAL segments coming from the local pg_wal the first time archive recovery is run. Segments part of a self-contained base backup are the most common case where this could happen, however even in this case normally the .done markers would be most likely part of the backup. Segments recovered from an archive are marked as .ready or .done by the startup process, and segments finished streaming are marked as such by the WAL receiver, so they are handled already. Reported-by: Haruka Takatsuka Author: Michael Paquier Discussion: https://postgr.es/m/15402-a453c90ed4cf88b2@postgresql.org Backpatch-through: 9.5, where archive_mode = always has been added.
* Fix assorted bugs in pg_get_partition_constraintdef().Tom Lane2018-09-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It failed if passed a nonexistent relation OID, or one that was a non-heap relation, because of blindly applying heap_open to a user-supplied OID. This is not OK behavior for a SQL-exposed function; we have a project policy that we should return NULL in such cases. Moreover, since pg_get_partition_constraintdef ought now to work on indexes, restricting it to heaps is flat wrong anyway. The underlying function generate_partition_qual() wasn't on board with indexes having partition quals either, nor for that matter with rels having relispartition set but yet null relpartbound. (One wonders whether the person who wrote the function comment blocks claiming that these functions allow a missing relpartbound had ever tested it.) Fix by testing relispartition before opening the rel, and by using relation_open not heap_open. (If any other relkinds ever grow the ability to have relispartition set, the code will work with them automatically.) Also, don't reject null relpartbound in generate_partition_qual. Back-patch to v11, and all but the null-relpartbound change to v10. (It's not really necessary to change generate_partition_qual at all in v10, but I thought s/heap_open/relation_open/ would be a good idea anyway just to keep the code in sync with later branches.) Per report from Justin Pryzby. Discussion: https://postgr.es/m/20180927200020.GJ776@telsasoft.com
* Minor formatting cleanup for 2a6368343fAlexander Korotkov2018-09-27
|
* Remove extra usage of BoxPGetDatum() macroAlexander Korotkov2018-09-27
| | | | | Author: Mark Dilger Discussion: https://postgr.es/m/B2AEFCD0-836D-4654-9D59-3DF616E0A6F3%40gmail.com
* Clean up in the wake of TupleDescGetSlot() removal / 10763358c3f.Andres Freund2018-09-27
| | | | | | | | | | | | | | | The previous commit wasn't careful enough to remove all traces of TupleDescGetSlot(). Besides fixing the oversight of not removing TupleDescGetSlot()'s declaration, this also removes FuncCallContext->slot. That was documented to be for use in combination with TupleDescGetSlot(), a cursory search over extensions finds no users, and there doesn't seem to be convincing reasons to keep it around. If we later in the v12 release cycle find users, we can re-consider this part of the commit. Reported-By: Michael Paquier Discussion: https://postgr.es/m/20180926000413.GC1659@paquier.xyz
* Build src/port files as a library with -fPIC, and use that in libpq.Tom Lane2018-09-27
| | | | | | | | | | | | | | | | | | | | libpq and ecpg need shared-library-friendly versions of assorted src/port/ and src/common/ modules. Up to now, they got those by symlinking the individual source files and compiling them locally. That's baroque, and a pain to maintain, and it results in some amount of duplicated compile work. It might've made sense when only a couple of files were needed, but the list has grown and grown and grown :-( It makes more sense to have the originating directory build a third variant of libpgport.a/libpgcommon.a containing modules built with $(CFLAGS_SL), and just link that into the shared library. Unused files won't get linked, so the end result should be the same. This patch makes a down payment on that idea by having src/port/ build such a library and making libpq use it. If the buildfarm doesn't expose fatal problems with the approach, I'll extend it to the other cases. Discussion: https://postgr.es/m/13022.1538003440@sss.pgh.pa.us
* Fix another portability issue from commit 758ce9b77.Tom Lane2018-09-26
| | | | | | strerror.c now requires strlcpy() in some cases, and a couple of the ecpg libraries did not have that at hand. Pull it in from src/port/ following the usual recipe. Per buildfarm.
* Switch flags tracking pending interrupts to sig_atomic_tMichael Paquier2018-09-27
| | | | | | | | | | | Those previously used bool, which should be safe on any modern platforms, however the C standard is clear that it is better to use sig_atomic_t for variables manipulated in signal handlers. This commit adds at the same time PGDLLIMPORT to ClientConnectionLost. Author: Michael Paquier Reviewed-by: Tom Lane, Chris Travers, Andres Freund Discussion: https://postgr.es/m/20180925011311.GD1354@paquier.xyz
* Try another way to detect the result type of strerror_r().Tom Lane2018-09-26
| | | | | | | | | | | | | | | | | | | The method we've traditionally used, of redeclaring strerror_r() to see if the compiler complains of inconsistent declarations, turns out not to work reliably because some compilers only report a warning, not an error. Amazingly, this has gone undetected for years, even though it certainly breaks our detection of whether strerror_r succeeded. Let's instead test whether the compiler will take the result of strerror_r() as a switch() argument. It's possible this won't work universally either, but it's the best idea I could come up with on the spur of the moment. We should probably back-patch this once the dust settles, but first let's see what the buildfarm thinks of it. Discussion: https://postgr.es/m/10877.1537993279@sss.pgh.pa.us
* Clean up *printf macros to avoid conflict with format archetypes.Tom Lane2018-09-26
| | | | | | | | | | | | | | | | | We must define the macro "printf" with arguments, else it can mess up format archetype attributes in builds where PG_PRINTF_ATTRIBUTE is just "printf". Fortunately, that's easy to do now that we're requiring C99; we can use __VA_ARGS__. On the other hand, it's better not to use __VA_ARGS__ for the rest of the *printf crew, so that one can take the addresses of those functions without surprises. I'd proposed doing this some time ago, but forgot to make it happen; buildfarm failures subsequent to 96bf88d52 reminded me. Discussion: https://postgr.es/m/22709.1535135640@sss.pgh.pa.us Discussion: https://postgr.es/m/20180926190934.ea4xvzhkayuw7gkx@alap3.anarazel.de
* Fix link failures due to snprintf/strerror changes.Tom Lane2018-09-26
| | | | | | | | | | | | | | snprintf.c requires isnan(), which requires -lm on some platforms. libpq never bothered with -lm before, but now it needs it. strerror.c tries to translate a string or two, which requires -lintl. We'd managed never to need that anywhere in ecpg/pgtypeslib/ before, but now we do. Per buildfarm and a report from Peter Eisentraut. Discussion: https://postgr.es/m/20180926190934.ea4xvzhkayuw7gkx@alap3.anarazel.de Discussion: https://postgr.es/m/f67b5008-9f01-057f-2bff-558cb53af851@2ndquadrant.com
* Recurse to sequences on ownership change for all relkindsPeter Eisentraut2018-09-26
| | | | | | | | | | | | | | When a table ownership is changed, we must apply that also to any owned sequences. (Otherwise, it would result in a situation that cannot be restored, because linked sequences must have the same owner as the table.) But this was previously only applied to regular tables and materialized views. But it should also apply to at least foreign tables. This patch removes the relkind check altogether, because it doesn't save very much and just introduces the possibility of similar omissions. Bug: #15238 Reported-by: Christoph Berg <christoph.berg@credativ.de>
* Implement %m in src/port/snprintf.c, and teach elog.c to rely on that.Tom Lane2018-09-26
| | | | | | | | | | | | | | | | | | | | | | | 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
* Always use our own versions of *printf().Tom Lane2018-09-26
| | | | | | | | | | | | | | | | | | | | | 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
* Incorporate strerror_r() into src/port/snprintf.c, too.Tom Lane2018-09-26
| | | | | | | | | | | | 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
* Convert elog.c's useful_strerror() into a globally-used strerror wrapper.Tom Lane2018-09-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Update dummy CREATE ASSERTION grammarPeter Eisentraut2018-09-26
| | | | | | | | | | | 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
* Improve test coverage of geometric typesTomas Vondra2018-09-26
| | | | | | | | | | | | | | | | | | | 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
* Fix problems in handling the line data typeTomas Vondra2018-09-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Add basic regression tests for default monitoring rolesMichael Paquier2018-09-26
| | | | | | | | | 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
* Rework activation of commit timestamps during recoveryMichael Paquier2018-09-26
| | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* Remove absolete function TupleDescGetSlot().Andres Freund2018-09-25
| | | | | | | | | | | 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
* Split ExecStoreTuple into ExecStoreHeapTuple and ExecStoreBufferHeapTuple.Andres Freund2018-09-25
| | | | | | | | | | | | | | | | | | | | 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
* Remove function list from prologue of execTuples.c.Andres Freund2018-09-25
| | | | | | | | 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
* Change TupleTableSlot->tts_nvalid to type AttrNumber.Andres Freund2018-09-25
| | | | | | | | | | 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
* Remove obsolete commentAlvaro Herrera2018-09-25
| | | | | The documented shortcoming was actually fixed in 4c728f3829 so the comment is not true anymore.