aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* pg_verify_checksums: Message style improvements and NLS supportPeter Eisentraut2018-08-28
| | | | | | The source code was already set up for NLS support, so just a nls.mk file needed to be added. Also, fix the old problem of putting the int64 format specifier right into the string, which breaks NLS.
* Code review for simplehash.h.Thomas Munro2018-08-28
| | | | | | | | | | | | | | | | | | Fix reference to non-existent file in comment. Add SH_ prefix to the EMPTY and IN_USE tokens, to reduce likelihood of collisions with unrelated macros. Add include guards around the function definitions that are not "parameterized", so the header can be used again in the same translation unit. Undefine SH_EQUAL macro where other "parameter" macros are undefined, for the same reason. Author: Thomas Munro Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAEepm%3D1LdXZ3mMTM8tHt_b%3DK1kREit%3Dp8sikesak%3DkzHHM07Nw%40mail.gmail.com
* Fix snapshot leak warning for some proceduresPeter Eisentraut2018-08-27
| | | | | | | | | | | | | | | | | | | | The problem arises with the combination of CALL with output parameters and doing a COMMIT inside the procedure. When a CALL has output parameters, the portal uses the strategy PORTAL_UTIL_SELECT instead of PORTAL_MULTI_QUERY. Using PORTAL_UTIL_SELECT causes the portal's snapshot to be registered with the current resource owner (portal->holdSnapshot); see 9ee1cf04ab6bcefe03a11837b53f29ca9dc24c7a for the reason. Normally, PortalDrop() unregisters the snapshot. If not, then ResourceOwnerRelease() will print a warning about a snapshot leak on transaction commit. A transaction commit normally drops all portals (PreCommit_Portals()), except the active portal. So in case of the active portal, we need to manually release the snapshot to avoid the warning. Reported-by: Prabhat Sahu <prabhat.sahu@enterprisedb.com> Reviewed-by: Jonathan S. Katz <jkatz@postgresql.org>
* Fix missing dependency for pg_dump's ENABLE ROW LEVEL SECURITY items.Tom Lane2018-08-27
| | | | | | | | | | | | | | The archive should show a dependency on the item's table, but it failed to include one. This could cause failures in parallel restore due to emitting ALTER TABLE ... ENABLE ROW LEVEL SECURITY before restoring the table's data. In practice the odds of a problem seem low, since you would typically need to have set FORCE ROW LEVEL SECURITY as well, and you'd also need a very high --jobs count to have any chance of this happening. That probably explains the lack of field reports. Still, it's a bug, so back-patch to 9.5 where RLS was introduced. Discussion: https://postgr.es/m/19784.1535390902@sss.pgh.pa.us
* Add some not null constraints to catalogsPeter Eisentraut2018-08-27
| | | | | Use BKI_FORCE_NOT_NULL on some catalog field declarations that are never null (according to the source code that accesses them).
* Improve VACUUM and ANALYZE by avoiding early lock queueMichael Paquier2018-08-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A caller of VACUUM can perform early lookup obtention which can cause other sessions to block on the request done, causing potentially DOS attacks as even a non-privileged user can attempt a vacuum fill of a critical catalog table to block even all incoming connection attempts. Contrary to TRUNCATE, a client could attempt a system-wide VACUUM after building the list of relations to VACUUM, which can cause vacuum_rel() or analyze_rel() to try to lock the relation but the operation would just block. When the client specifies a list of relations and the relation needs to be skipped, ownership checks are done when building the list of relations to work on, preventing a later lock attempt. vacuum_rel() already had the sanity checks needed, except that those were applied too late. This commit refactors the code so as relation skips are checked beforehand, making it safer to avoid too early locks, for both manual VACUUM with and without a list of relations specified. An isolation test is added emulating the fact that early locks do not happen anymore, issuing a WARNING message earlier if the user calling VACUUM is not a relation owner. When a partitioned table is listed in a manual VACUUM or ANALYZE command, its full list of partitions is fetched, all partitions get added to the list to work on, and then each one of them is processed one by one, with ownership checks happening at the later phase of vacuum_rel() or analyze_rel(). Trying to do early ownership checks for each partition is proving to be tedious as this would result in deadlock risks with lock upgrades, and skipping all partitions if the listed partitioned table is not owned would result in a behavior change compared to how Postgres 10 has implemented vacuum for partitioned tables. The original problem reported related to early lock queue for critical relations is fixed anyway, so priority is given to avoiding a backward-incompatible behavior. Reported-by: Lloyd Albin, Jeremy Schneider Author: Michael Paquier Reviewed by: Nathan Bossart, Kyotaro Horiguchi Discussion: https://postgr.es/m/152512087100.19803.12733865831237526317@wrigleys.postgresql.org Discussion: https://postgr.es/m/20180812222142.GA6097@paquier.xyz
* Fix typos.Thomas Munro2018-08-27
| | | | | Author: David Rowley Discussion: https://postgr.es/m/CAKJS1f8du35u5DprpykWvgNEScxapbWYJdHq%2Bz06Wj3Y2KFPbw%40mail.gmail.com
* Make syslogger more robust against failures in opening CSV log files.Tom Lane2018-08-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The previous coding figured it'd be good enough to postpone opening the first CSV log file until we got a message we needed to write there. This is unsafe, though, because if the open fails we end up in infinite recursion trying to report the failure. Instead make the CSV log file management code look as nearly as possible like the longstanding logic for the stderr log file. In particular, open it immediately at postmaster startup (if enabled), or when we get a SIGHUP in which we find that log_destination has been changed to enable CSV logging. It seems OK to fail if a postmaster-start-time open attempt fails, as we've long done for the stderr log file. But we can't die if we fail to open a CSV log file during SIGHUP, so we're still left with a problem. In that case, write any output meant for the CSV log file to the stderr log file. (This will also cover race-condition cases in which backends send CSV log data before or after we have the CSV log file open.) This patch also fixes an ancient oversight that, if CSV logging was turned off during a SIGHUP, we never actually closed the last CSV log file. In passing, remember to reset whereToSendOutput = DestNone during syslogger start, since (unlike all other postmaster children) it's forked before the postmaster has done that. This made for a platform-dependent difference in error reporting behavior between the syslogger and other children: except on Windows, it'd report problems to the original postmaster stderr as well as the normal error log file(s). It's barely possible that that was intentional at some point; but it doesn't seem likely to be desirable in production, and the platform dependency definitely isn't desirable. Per report from Alexander Kukushkin. It's been like this for a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/CAFh8B==iLUD_gqC-dAENS0V+kVrCeGiKujtKqSQ7++S-caaChw@mail.gmail.com
* Reconsider new file extension in commit 91f26d5f.Jeff Davis2018-08-25
| | | | | | | Andres and Tom objected to the choice of the ".tmp" extension. Changing to Andres's suggestion of ".spill". Discussion: https://postgr.es/m/88092095-3348-49D8-8746-EB574B1D30EA%40anarazel.de
* Change extension of spilled ReorderBufferChange data to ".tmp".Jeff Davis2018-08-25
| | | | | | | The previous extension, ".snap", was chosen for historical reasons and became confusing. Discussion: https://postgr.es/m/CAMp0ubd_P8vBGx8=MfDXQJZxHA5D_Zarw5cCkDxJ_63+pWRJ9w@mail.gmail.com
* Comment fix for rewriteheap.h.Jeff Davis2018-08-25
| | | | | The description of the filename for mapping files did not match the code.
* Remove test for VA_ARGS, implied by C99.Andres Freund2018-08-24
| | | | | | | This simplifies logic / reduces duplication in a few headers. Author: Andres Freund Discussion: https://postgr.es/m/97d4b165-192d-3605-749c-f614a0c4e783@2ndquadrant.com
* LLVMJIT: LLVMGetHostCPUFeatures now is upstream, use LLMV version if available.Andres Freund2018-08-24
| | | | | | | Noticed thanks to buildfarm animal seawasp. Author: Andres Freund Backpatch: v11-, where LLVM based JIT compliation was introduced.
* Suppress uninitialized-variable warning in new SCRAM code.Tom Lane2018-08-24
| | | | | | | While we generally don't sweat too much about "may be used uninitialized" warnings from older compilers, I noticed that there's a fair number of buildfarm animals that are producing such a warning *only* for this variable. So it seems worth silencing.
* Introduce minimal C99 usage to verify compiler support.Andres Freund2018-08-23
| | | | | | | | | This just converts a few for loops in postgres.c to declare variables in the loop initializer, and uses designated initializers in smgr.c's definition of smgr callbacks. Author: Andres Freund Discussion: https://postgr.es/m/97d4b165-192d-3605-749c-f614a0c4e783@2ndquadrant.com
* Require C99 (and thus MSCV 2013 upwards).Andres Freund2018-08-23
| | | | | | | | | | | | | | | | | | | | | | | In 86d78ef50e01 I enabled configure to check for C99 support, with the goal of checking which platforms support C99. While there are a few machines without C99 support among our buildfarm animals, de-supporting them for v12 was deemed acceptable. While not tested in aforementioned commit, the biggest increase in minimum compiler version comes from MSVC, which gained C99 support fairly late. The subset in MSVC 2013 is sufficient for our needs, at this point. While that is a significant increase in minimum version, the existing windows binaries are already built with a new enough version. Make configure error out if C99 support could not be detected. For MSVC builds, increase the minimum version to 2013. The increase to MSVC 2013 allows us to get rid of VCBuildProject.pm, as that was only required for MSVC 2005/2008. Author: Andres Freund Discussion: https://postgr.es/m/97d4b165-192d-3605-749c-f614a0c4e783@2ndquadrant.com
* Add more tests for VACUUM skips with partitioned tablesMichael Paquier2018-08-24
| | | | | | | | | | | | | | | | A VACUUM or ANALYZE command listing directly a partitioned table expands it to its partitions, causing all elements of a tree to be processed with individual ownership checks done. This results in different relation skips depending on the ownership policy of a tree, which may not be consistent for a partition tree. This commit adds more tests to ensure that any future refactoring allows to keep a consistent behavior, or at least that any changes done are easily identified and checked. The current behavior of VACUUM with partitioned tables is present since 10. Author: Nathan Bossart Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/DC186201-B01F-4A66-9EC4-F855A957C1F9@amazon.com
* Deduplicate code between slot_getallattrs() and slot_getsomeattrs().Andres Freund2018-08-23
| | | | | | | | | | | | | | | Code in slot_getallattrs() is the same as if slot_getsomeattrs() is called with number of attributes specified in the tuple descriptor. Implement it that way instead of duplicating the code between those two functions. This is part of a patchseries abstracting TupleTableSlots so they can store arbitrary forms of tuples, but is a nice enough cleanup on its own. Author: Ashutosh Bapat Reviewed-By: Andres Freund Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
* Fix lexing of standard multi-character operators in edge cases.Andrew Gierth2018-08-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits c6b3c939b (which fixed the precedence of >=, <=, <> operators) and 865f14a2d (which added support for the standard => notation for named arguments) created a class of lexer tokens which look like multi-character operators but which have their own token IDs distinct from Op. However, longest-match rules meant that following any of these tokens with another operator character, as in (1<>-1), would cause them to be incorrectly returned as Op. The error here isn't immediately obvious, because the parser would usually still find the correct operator via the Op token, but there were more subtle problems: 1. If immediately followed by a comment or +-, >= <= <> would be given the old precedence of Op rather than the correct new precedence; 2. If followed by a comment, != would be returned as Op rather than as NOT_EQUAL, causing it not to be found at all; 3. If followed by a comment or +-, the => token for named arguments would be lexed as Op, causing the argument to be mis-parsed as a simple expression, usually causing an error. Fix by explicitly checking for the operators in the {operator} code block in addition to all the existing special cases there. Backpatch to 9.5 where the problem was introduced. Analysis and patch by me; review by Tom Lane. Discussion: https://postgr.es/m/87va851ppl.fsf@news-spur.riddles.org.uk
* Reduce an unnecessary O(N^3) loop in lexer.Andrew Gierth2018-08-23
| | | | | | | | | | | | The lexer's handling of operators contained an O(N^3) hazard when dealing with long strings of + or - characters; it seems hard to prevent this case from being O(N^2), but the additional N multiplier was not needed. Backpatch all the way since this has been there since 7.x, and it presents at least a mild hazard in that trying to do Bind, PREPARE or EXPLAIN on a hostile query could take excessive time (without honouring cancels or timeouts) even if the query was never executed.
* In libpq, don't look up all the hostnames at once.Tom Lane2018-08-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Historically, we looked up the target hostname in connectDBStart, so that PQconnectPoll did not need to do DNS name resolution. The patches that added multiple-target-host support to libpq preserved this division of labor; but it's really nonsensical now, because it means that if any one of the target hosts fails to resolve in DNS, the connection fails. That negates the no-single-point-of-failure goal of the feature. Additionally, DNS lookups aren't exactly cheap, but the code did them all even if the first connection attempt succeeds. Hence, rearrange so that PQconnectPoll does the lookups, and only looks up a hostname when it's time to try that host. This does mean that PQconnectPoll could block on a DNS lookup --- but if you wanted to avoid that, you should be using hostaddr, as the documentation has always specified. It seems fairly unlikely that any applications would really care whether the lookup occurs inside PQconnectStart or PQconnectPoll. In addition to calling out that fact explicitly, do some other minor wordsmithing in the docs around the multiple-target-host feature. Since this seems like a bug in the multiple-target-host feature, backpatch to v10 where that was introduced. In the back branches, avoid moving any existing fields of struct pg_conn, just in case any third-party code is looking into that struct. Tom Lane, reviewed by Fabien Coelho Discussion: https://postgr.es/m/4913.1533827102@sss.pgh.pa.us
* Copy-editing of pg_verify_checksums help and ref pagePeter Eisentraut2018-08-23
| | | | | Reformat synopsis, put options into better order, make the desciption line a bit shorter, and put more details into the description.
* PL/pgSQL: Extend test casePeter Eisentraut2018-08-23
| | | | | | | | This test was supposed to check the interaction of INOUT and default parameters in a procedure call, but it only checked the case where the parameter was not supplied. Now it also checks the case where the parameter was supplied. It was already working correctly, so no code changes required.
* Change PROCEDURE to FUNCTION in CREATE TRIGGER syntaxPeter Eisentraut2018-08-22
| | | | | | | | | | | | | Since procedures are now a different thing from functions, change the CREATE TRIGGER and CREATE EVENT TRIGGER syntax to use FUNCTION in the clause that specifies the function. PROCEDURE is still accepted for compatibility. pg_dump and ruleutils.c output is not changed yet, because that would require a change in information_schema.sql and thus a catversion change. Reported-by: Peter Geoghegan <pg@bowt.ie> Reviewed-by: Jonathan S. Katz <jonathan.katz@excoventures.com>
* Change PROCEDURE to FUNCTION in CREATE OPERATOR syntaxPeter Eisentraut2018-08-22
| | | | | | | | | Since procedures are now a different thing from functions, change the CREATE OPERATOR syntax to use FUNCTION in the clause that specifies the function. PROCEDURE is still accepted for compatibility. Reported-by: Peter Geoghegan <pg@bowt.ie> Reviewed-by: Jonathan S. Katz <jonathan.katz@excoventures.com>
* doc: Update uses of the word "procedure"Peter Eisentraut2018-08-22
| | | | | | | | | | | | | | | | | | | Historically, the term procedure was used as a synonym for function in Postgres/PostgreSQL. Now we have procedures as separate objects from functions, so we need to clean up the documentation to not mix those terms. In particular, mentions of "trigger procedures" are changed to "trigger functions", and access method "support procedures" are changed to "support functions". (The latter already used FUNCTION in the SQL syntax anyway.) Also, the terminology in the SPI chapter has been cleaned up. A few tests, examples, and code comments are also adjusted to be consistent with documentation changes, but not everything. Reported-by: Peter Geoghegan <pg@bowt.ie> Reviewed-by: Jonathan S. Katz <jonathan.katz@excoventures.com>
* Wrap long line in postgresql.conf.sample.Thomas Munro2018-08-22
| | | | Per complaint from Michael Paquier.
* Provide plan_cache_mode options in postgresql.conf.sample.Thomas Munro2018-08-22
| | | | | Author: David Rowley Discussion: https://postgr.es/m/CAKJS1f8YkwojSTSg8YjNYCLCXzx0fR7wBR3Gf%2BrA9_52eoPZKg%40mail.gmail.com
* Do not dump identity sequences with excluded parent tableMichael Paquier2018-08-22
| | | | | | | | | | | | | | This commit prevents a crash of pg_dump caused by the exclusion of a table which has identity columns, as the table would be correctly excluded but not its identity sequence. In order to fix that, identity sequences are excluded if the parent table is defined as such. Knowing about such sequences has no meaning without their parent table anyway. Reported-by: Andy Abelisto Author: David Rowley Reviewed-by: Peter Eisentraut, Michael Paquier Discussion: https://postgr.es/m/153479393218.1316.8472285660264976457@wrigleys.postgresql.org Backpatch-through: 10
* Add regression tests for VACUUM and ANALYZE with relation skipsMichael Paquier2018-08-22
| | | | | | | | | | | | When a user does not have ownership on a relation, then specific log messages are generated. This new test suite adds coverage for all the possible log messages generated, which will be useful to check the consistency of any refactoring related to ownership checks for relations vacuumed or analyzed. Author: Michael Paquier Reviewed-by: Nathan Bossart Discussion: https://postgr.es/m/20180812222142.GA6097@paquier.xyz
* Fix typoAlvaro Herrera2018-08-21
|
* fix typoAlvaro Herrera2018-08-21
|
* Fix set of NLS translation issuesMichael Paquier2018-08-21
| | | | | | | | | | | | | | | | | | | While monitoring the code, a couple of issues related to string translation has showed up: - Some routines for auto-updatable views return an error string, which sometimes missed the shot. A comment regarding string translation is added for each routine to help with future features. - GSSAPI authentication missed two translations. - vacuumdb handles non-translated strings. - GetConfigOptionByNum should translate strings. This part is not back-patched as after a minor upgrade this could be surprising for users. Reported-by: Kyotaro Horiguchi Author: Kyotaro Horiguchi Reviewed-by: Michael Paquier, Tom Lane Discussion: https://postgr.es/m/20180810.152131.31921918.horiguchi.kyotaro@lab.ntt.co.jp Backpatch-through: 9.3
* Fix typo in description of enable_parallel_hashMichael Paquier2018-08-21
| | | | | Author: Kyotaro Horiguchi Discussion: https://postgr.es/m/20180821.115841.93250330.horiguchi.kyotaro@lab.ntt.co.jp
* Clarify comment about assignment and reset of temp namespace ID in MyProcMichael Paquier2018-08-21
| | | | | | | | | The new wording comes from Álvaro, which I modified a bit. Reported-by: Andres Freund, Álvaro Herrera Author: Álvaro Herrera, Michael Paquier Discussion: https://postgr.es/m/20180809165047.GK13638@paquier.xyz Backpatch-through: 11
* MSVC: Finish clean.bat tmp_check coverage.Noah Misch2018-08-19
| | | | | Use wildcards, so one can add a TAP test suite without updating this file. Back-patch to v11, which omitted multiple new suites.
* MSVC: Remove any tmp_check directory before running a TAP test suite.Noah Misch2018-08-19
| | | | | | | Back-patch to v11, where commit 90627cf98a8e7d0531789391fd798c9bfcc3bc1a made the GNU make build system do likewise. Without this, when a typical PostgresNode-using test failed, subsequent runs bailed out with a "File exists" error.
* Improve error messages for CREATE OR REPLACE PROCEDUREPeter Eisentraut2018-08-18
| | | | | | | | | Change the hint to recommend DROP PROCEDURE instead of FUNCTION. Also make the error message when changing the return type more specific to the case of procedures. Reported-by: Jeremy Evans <code@jeremyevans.net> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
* Ensure schema qualification in pg_restore DISABLE/ENABLE TRIGGER commands.Tom Lane2018-08-17
| | | | | | | | | | | | | | | | | | | | | | | | | | Previously, this code blindly followed the common coding pattern of passing PQserverVersion(AH->connection) as the server-version parameter of fmtQualifiedId. That works as long as we have a connection; but in pg_restore with text output, we don't. Instead we got a zero from PQserverVersion, which fmtQualifiedId interpreted as "server is too old to have schemas", and so the name went unqualified. That still accidentally managed to work in many cases, which is probably why this ancient bug went undetected for so long. It only became obvious in the wake of the changes to force dump/restore to execute with restricted search_path. In HEAD/v11, let's deal with this by ripping out fmtQualifiedId's server- version behavioral dependency, and just making it schema-qualify all the time. We no longer support pg_dump from servers old enough to need the ability to omit schema name, let alone restoring to them. (Also, the few callers outside pg_dump already didn't work with pre-schema servers.) In older branches, that's not an acceptable solution, so instead just tweak the DISABLE/ENABLE TRIGGER logic to ensure it will schema-qualify its output regardless of server version. Per bug #15338 from Oleg somebody. Back-patch to all supported branches. Discussion: https://postgr.es/m/153452458706.1316.5328079417086507743@wrigleys.postgresql.org
* InsertPgAttributeTuple() to set attcacheoffPeter Eisentraut2018-08-17
| | | | | | | | | InsertPgAttributeTuple() is the interface between in-memory tuple descriptors and on-disk pg_attribute, so it makes sense to give it the job of resetting attcacheoff. This avoids having all the callers having to do so. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
* Set scan direction appropriately for SubPlans (bug #15336)Andrew Gierth2018-08-17
| | | | | | | | | | | | | | | | | | | | When executing a SubPlan in an expression, the EState's direction field was left alone, resulting in an attempt to execute the subplan backwards if it was encountered during a backwards scan of a cursor. Also, though much less likely, it was possible to reach the execution of an InitPlan while in backwards-scan state. Repair by saving/restoring estate->es_direction and forcing forward scan mode in the relevant places. Backpatch all the way, since this has been broken since 8.3 (prior to commit c7ff7663e, SubPlans had their own EStates rather than sharing the parent plan's, so there was no confusion over scan direction). Per bug #15336 reported by Vladimir Baranoff; analysis and patch by me, review by Tom Lane. Discussion: https://postgr.es/m/153449812167.1304.1741624125628126322@wrigleys.postgresql.org
* pg_upgrade: issue helpful error message for use on standbysBruce Momjian2018-08-17
| | | | | | | | | | | | | Commit 777e6ddf1723306bd2bf8fe6f804863f459b0323 checked for a shut down message from a standby and allowed it to continue. This patch reports a helpful error message in these cases, suggesting to use rsync as documented. Diagnosed-by: Martín Marqués Discussion: https://postgr.es/m/CAPdiE1xYCow-reLjrhJ9DqrMu-ppNq0ChUUEvVdxhdjGRD5_eA@mail.gmail.com Backpatch-through: 9.3
* Use the built-in float datatypes to implement geometric typesTomas Vondra2018-08-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch makes the geometric operators and functions use the exported function of the float4/float8 datatypes. The main reason of doing so is to check for underflow and overflow, and to handle NaNs consciously. The float datatypes consider NaNs values to be equal and greater than all non-NaN values. This change considers NaNs equal only for equality operators. The placement operators, contains, overlaps, left/right of etc. continue to return false when NaNs are involved. We don't need to worry about them being considered greater than any-NaN because there aren't any basic comparison operators like less/greater than for the geometric datatypes. The changes may be summarised as: * Check for underflow, overflow and division by zero * Consider NaN values to be equal * Return NULL when the distance is NaN for all closest point operators * Favour not-NaN over NaN where it makes sense The patch also replaces all occurrences of "double" as "float8". They are the same, but were used inconsistently in the same file. Author: Emre Hasegeli Reviewed-by: Kyotaro Horiguchi, Tomas Vondra Discussion: https://www.postgresql.org/message-id/CAE2gYzxF7-5djV6-cEvqQu-fNsnt%3DEqbOURx7ZDg%2BVv6ZMTWbg%40mail.gmail.com
* Remove remaining GEODEBUG references from geo_ops.cTomas Vondra2018-08-16
| | | | | | | | | | Commit a7dc63d904a6044d299aebdf59ad3199b6a9e99d removed most of the GEODEBUG occurrences, but there were a couple remaining. So remove them too, to get rid of the macro entirely. Author: Emre Hasegeli Discussion: https://www.postgresql.org/message-id/CAE2gYzxF7-5djV6-cEvqQu-fNsnt%3DEqbOURx7ZDg%2BVv6ZMTWbg%40mail.gmail.com
* Require a C99-compliant snprintf(), and remove related workarounds.Tom Lane2018-08-16
| | | | | | | | | | | | | | | | | | | | | | | | Since our substitute snprintf now returns a C99-compliant result, there's no need anymore to have complicated code to cope with pre-C99 behavior. We can just make configure substitute snprintf.c if it finds that the system snprintf() is pre-C99. (Note: I do not believe that there are any platforms where this test will trigger that weren't already being rejected due to our other C99-ish feature requirements for snprintf. But let's add the check for paranoia's sake.) Then, simplify the call sites that had logic to cope with the pre-C99 definition. I also dropped some stuff that was being paranoid about the possibility of snprintf overrunning the given buffer. The only reports we've ever heard of that being a problem were for Solaris 7, which is long dead, and we've sure not heard any reports of these assertions triggering in a long time. So let's drop that complexity too. Likewise, drop some code that wasn't trusting snprintf to set errno when it returns -1. That would be not-per-spec, and again there's no real reason to believe it is a live issue, especially not for snprintfs that pass all of configure's feature checks. Discussion: https://postgr.es/m/17245.1534289329@sss.pgh.pa.us
* Fix executor prune failure when plan already prunedAlvaro Herrera2018-08-16
| | | | | | | | | | | In a multi-layer partitioning setup, if at plan time all the sub-partitions are pruned but the intermediate one remains, the executor later throws a spurious error that there's nothing to prune. That is correct, but there's no reason to throw an error. Therefore, don't. Reported-by: Andreas Seltenreich <seltenreich@gmx.de> Author: David Rowley <david.rowley@2ndquadrant.com> Discussion: https://postgr.es/m/87in4h98i0.fsf@ansel.ydns.eu
* Close the file descriptor in ApplyLogicalMappingFileTomas Vondra2018-08-16
| | | | | | | | | | | | | | The function was forgetting to close the file descriptor, resulting in failures like this: ERROR: 53000: exceeded maxAllocatedDescs (492) while trying to open file "pg_logical/mappings/map-4000-4eb-1_60DE1E08-5376b5-537c6b" LOCATION: OpenTransientFile, fd.c:2161 Simply close the file at the end, and backpatch to 9.4 (where logical decoding was introduced). While at it, fix a nearby typo. Discussion: https://www.postgresql.org/message-id/flat/738a590a-2ce5-9394-2bef-7b1caad89b37%402ndquadrant.com
* Update comment in header of errcodes.txtMichael Paquier2018-08-16
| | | | | | | | This file mentions all the files generated from it, but missed that errcodes-list.sgml is no more, while errcodes-table.sgml is. Author: Noriyoshi Shinoda Discussion: https://postgr.es/m/TU4PR8401MB0430855D6B971E49EB55F328EE3E0@TU4PR8401MB0430.NAMPRD84.PROD.OUTLOOK.COM
* Improve comment in GetNewObjectId().Thomas Munro2018-08-16
| | | | | | | | | | | | The previous comment gave the impression that skipping OIDs before FirstNormalObjectId was merely an optimization to avoid likely collisions. In fact other parts of the system have been relying on this threshold to detect system-created objects since commit 8e18d04d4da, so adjust the wording. Author: Thomas Munro Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAEepm%3D33JASACeOayr_W3%3DCSjy2jiPxM-k89axu0akFbHdjnjA%40mail.gmail.com
* Update FSM on WAL replay of page all-visible/frozenAlvaro Herrera2018-08-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We aren't very strict about keeping FSM up to date on WAL replay, because per-page freespace values aren't critical in replicas (can't write to heap in a replica; and if the replica is promoted, the values would be updated by VACUUM anyway). However, VACUUM since 9.6 can skip processing pages marked all-visible or all-frozen, and if such pages are recorded in FSM with wrong values, those values are blindly propagated to FSM's upper layers by VACUUM's FreeSpaceMapVacuum. (This rationale assumes that crashes are not very frequent, because those would cause outdated FSM to occur in the primary.) Even when the FSM is outdated in standby, things are not too bad normally, because, most per-page FSM values will be zero (other than those propagated with the base-backup that created the standby); only once the remaining free space is less than 0.2*BLCKSZ the per-page value is maintained by WAL replay of heap ins/upd/del. However, if wal_log_hints=on causes complete FSM pages to be propagated to a standby via full-page images, many too-optimistic per-page values can end up being registered in the standby. Incorrect per-page values aren't critical in most cases, since an inserter that is given a page that doesn't actually contain the claimed free space will update FSM with the correct value, and retry until it finds a usable page. However, if there are many such updates to do, an inserter can spend a long time doing them before a usable page is found; in a heavily trafficked insert-only table with many concurrent inserters this has been observed to cause several second stalls, causing visible application malfunction. To fix this problem, it seems sufficient to have heap_xlog_visible (replay of setting all-visible and all-frozen VM bits for a heap page) update the FSM value for the page being processed. This fixes the per-page counters together with making the page skippable to vacuum, so when vacuum does FreeSpaceMapVacuum, the values propagated to FSM upper layers are the correct ones, avoiding the problem. While at it, apply the same fix to heap_xlog_clean (replay of tuple removal by HOT pruning and vacuum). This makes any space freed by the cleaning available earlier than the next vacuum in the promoted replica. Backpatch to 9.6, where this problem was diagnosed on an insert-only table with all-frozen pages, which were introduced as a concept in that release. Theoretically it could apply with all-visible pages to older branches, but there's been no report of that and it doesn't backpatch cleanly anyway. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/20180802172857.5skoexsilnjvgruk@alvherre.pgsql