summaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2019-07-25Fix failures to ignore \r when reading Windows-style newlines.Tom Lane
libpq failed to ignore Windows-style newlines in connection service files. This normally wasn't a problem on Windows itself, because fgets() would convert \r\n to just \n. But if libpq were running inside a program that changes the default fopen mode to binary, it would see the \r's and think they were data. In any case, it's project policy to ignore \r in text files unconditionally, because people sometimes try to use files with DOS-style newlines on Unix machines, where the C library won't hide that from us. Hence, adjust parseServiceFile() to ignore \r as well as \n at the end of the line. In HEAD, go a little further and make it ignore all trailing whitespace, to match what it's always done with leading whitespace. In HEAD, also run around and fix up everyplace where we have newline-chomping code to make all those places look consistent and uniformly drop \r. It is not clear whether any of those changes are fixing live bugs. Most of the non-cosmetic changes are in places that are reading popen output, and the jury is still out as to whether popen on Windows can return \r\n. (The Windows-specific code in pipe_read_line seems to think so, but our lack of support for this elsewhere suggests maybe it's not a problem in practice.) Hence, I desisted from applying those changes to back branches, except in run_ssl_passphrase_command() which is new enough and little-tested enough that we'd probably not have heard about any problems there. Tom Lane and Michael Paquier, per bug #15827 from Jorge Gustavo Rocha. Back-patch the parseServiceFile() change to all supported branches, and the run_ssl_passphrase_command() change to v11 where that was added. Discussion: https://postgr.es/m/15827-e6ba53a3a7ed543c@postgresql.org
2019-07-25Honor MSVC WindowsSDKVersion if setAndrew Dunstan
Add a line to the project file setting the target SDK. Otherwise, in for example VS2017, if the default but optional 8.1 SDK is not installed the build will fail. Patch from Peifeng Qiu, slightly edited by me. Discussion: https://postgr.es/m/CABmtVJhw1boP_bd4=b3Qv5YnqEdL696NtHFi2ruiyQ6mFHkeQQ@mail.gmail.com Backpatch to all live branches.
2019-07-19Silence compiler warning, hopefully.Tom Lane
Absorb commit e5e04c962a5d12eebbf867ca25905b3ccc34cbe0 from upstream IANA code, in hopes of silencing warnings from MSVC about negating a bool value. Discussion: https://postgr.es/m/20190719035347.GJ1859@paquier.xyz
2019-07-18Fix error in commit e6feef57.Jeff Davis
I was careless passing a datum directly to DATE_NOT_FINITE without calling DatumGetDateADT() first. Backpatch-through: 9.4
2019-07-18Fix daterange canonicalization for +/- infinity.Jeff Davis
The values 'infinity' and '-infinity' are a part of the DATE type itself, so a bound of the date 'infinity' is not the same as an unbounded/infinite range. However, it is still wrong to try to canonicalize such values, because adding or subtracting one has no effect. Fix by treating 'infinity' and '-infinity' the same as unbounded ranges for the purposes of canonicalization (but not other purposes). Backpatch to all versions because it is inconsistent with the documented behavior. Note that this could be an incompatibility for applications relying on the behavior contrary to the documentation. Author: Laurenz Albe Reviewed-by: Thomas Munro Discussion: https://postgr.es/m/77f24ea19ab802bc9bc60ddbb8977ee2d646aec1.camel%40cybertec.at Backpatch-through: 9.4
2019-07-17Update time zone data files to tzdata release 2019b.Tom Lane
Brazil no longer observes DST. Historical corrections for Palestine, Hong Kong, and Italy.
2019-07-17Sync our copy of the timezone library with IANA release tzcode2019b.Tom Lane
A large fraction of this diff is just due to upstream's somewhat random decision to rename a bunch of internal variables and struct fields. However, there is an interesting new feature in zic: it's grown a "-b slim" option that emits zone files without 32-bit data and other backwards-compatibility hacks. We should consider whether we wish to enable that.
2019-07-16Fix thinko in construction of old_conpfeqop list.Tom Lane
This should lappend the OIDs, not lcons them; the existing code produced a list in reversed order. This is harmless for single-key FKs or FKs where all the key columns are of the same type, which probably explains how it went unnoticed. But if those conditions are not met, ATAddForeignKeyConstraint would make the wrong decision about whether an existing FK needs to be revalidated. I think it would almost always err in the safe direction by revalidating a constraint that didn't need it. You could imagine scenarios where the pfeqop check was fooled by swapping the types of two FK columns in one ALTER TABLE, but that case would probably be rejected by other tests, so it might be impossible to get to the worst-case scenario where an FK should be revalidated and isn't. (And even then, it's likely to be fine, unless there are weird inconsistencies in the equality behavior of the replacement types.) However, this is a performance bug at least. Noted while poking around to see whether lcons calls could be converted to lappend. This bug is old, dating to commit cb3a7c2b9, so back-patch to all supported branches.
2019-07-10Fix variable initialization when using buffering build with GiSTMichael Paquier
This can cause valgrind to complain, as the flag marking a buffer as a temporary copy was not getting initialized. While on it, fill in with zeros newly-created buffer pages. This does not matter when loading a block from a temporary file, but it makes the push of an index tuple into a new buffer page safer. This has been introduced by 1d27dcf, so backpatch all the way down to 9.4. Author: Alexander Lakhin Discussion: https://postgr.es/m/15899-0d24fb273b3dd90c@postgresql.org Backpatch-through: 9.4
2019-07-03Add support for Visual Studio 2019 in build scriptsMichael Paquier
This adjusts the documentation and the scripts related to the versions of Windows SDK supported. Author: Haribabu Kommi Reviewed-by: Andrew Dunstan, Juan José Santamaría Flecha, Michael Paquier Discussion: https://postgr.es/m/CAJrrPGcfqXhfPyMrny9apoDU7M1t59dzVAvoJ9AeAh5BJi+UzA@mail.gmail.com Backpatch-through: 9.4
2019-06-28Fix misleading comment in nodeIndexonlyscan.c.Thomas Munro
The stated reason for acquiring predicate locks on heap pages hasn't existed since commit c01262a8, so fix the comment. Perhaps in a later release we'll also be able to change the code to use tuple locks. Back-patch all the way. Reviewed-by: Ashwin Agrawal Discussion: https://postgr.es/m/CAEepm%3D2GK3FVdnt5V3d%2Bh9njWipCv_fNL%3DwjxyUhzsF%3D0PcbNg%40mail.gmail.com
2019-06-27Update reference to sampling algorithm in analyze.cTomas Vondra
Commit 83e176ec1 moved row sampling functions from analyze.c to utils/misc/sampling.c, but failed to update comment referring to the sampling algorithm from Jeff Vitter's paper. Correct the comment by pointing to utils/misc/sampling.c. Author: Etsuro Fujita Discussion: https://postgr.es/m/CAPmGK154gp%2BQd%3DcorQOv%2BPmbyVyZBjp_%2Bhb766UJeD1e_ie6XQ%40mail.gmail.com
2019-06-26Add support for OpenSSL 1.1.0 and newer versions in MSVC scriptsMichael Paquier
Up to now, the MSVC build scripts are able to support only one fixed version of OpenSSL, and they lacked logic to detect the version of OpenSSL a given compilation of Postgres is linking to (currently 1.0.2, the latest LTS of upstream which will be EOL'd at the end of 2019). This commit adds more logic to detect the version of OpenSSL used by a build and makes use of it to add support for compilation with OpenSSL 1.1.0 which requires a new set of compilation flags to work properly. The supported OpenSSL installers have changed their library layer with various library renames with the upgrade to 1.1.0, making the logic a bit more complicated. The scripts are now able to adapt to the new world order. Reported-by: Sergey Pashkov Author: Juan José Santamaría Flecha, Michael Paquier Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/15789-8fc75dea3c5a17c8@postgresql.org Backpatch-through: 9.4
2019-06-24Further fix ALTER COLUMN TYPE's handling of indexes and index constraints.Tom Lane
This patch reverts all the code changes of commit e76de8861, which turns out to have been seriously misguided. We can't wait till later to compute the definition string for an index; we must capture that before applying the data type change for any column it depends on, else ruleutils.c will deliverr wrong/misleading results. (This fine point was documented nowhere, of course.) I'd also managed to forget that ATExecAlterColumnType executes once per ALTER COLUMN TYPE clause, not once per statement; which resulted in the code being basically completely broken for any case in which multiple ALTER COLUMN TYPE clauses are applied to a table having non-constraint indexes that must be rebuilt. Through very bad luck, none of the existing test cases nor the ones added by e76de8861 caught that, but of course it was soon found in the field. The previous patch also had an implicit assumption that if a constraint's index had a dependency on a table column, so would the constraint --- but that isn't actually true, so it didn't fix such cases. Instead of trying to delete unneeded index dependencies later, do the is-there-a-constraint lookup immediately on seeing an index dependency, and switch to remembering the constraint if so. In the unusual case of multiple column dependencies for a constraint index, this will result in duplicate constraint lookups, but that's not that horrible compared to all the other work that happens here. Besides, such cases did not work at all before, so it's hard to argue that they're performance-critical for anyone. Per bug #15865 from Keith Fiske. As before, back-patch to all supported branches. Discussion: https://postgr.es/m/15865-17940eacc8f8b081@postgresql.org
2019-06-22Fix spinlock assembly code for MIPS so it works on MIPS r6.Tom Lane
Original MIPS-I processors didn't have the LL/SC instructions (nor any other userland synchronization primitive). If the build toolchain targets that ISA variant by default, as an astonishingly large fraction of MIPS platforms still do, the assembler won't take LL/SC without coercion in the form of a ".set mips2" instruction. But we issued that unconditionally, making it an ISA downgrade for chips later than MIPS2. That breaks things for the latest MIPS r6 ISA, which encodes these instructions differently. Adjust the code so we don't change ISA level if it's >= 2. Note that this patch doesn't change what happens on an actual MIPS-I processor: either the kernel will emulate these instructions transparently, or you'll get a SIGILL failure. That tradeoff seemed fine in 2002 when this code was added (cf 3cbe6b247), and it's even more so today when MIPS-I is basically extinct. But let's add a comment about that. YunQiang Su (with cosmetic adjustments by me). Back-patch to all supported branches. Discussion: https://postgr.es/m/15844-8f62fe7e163939b3@postgresql.org
2019-06-17Stamp 9.4.23.REL9_4_23Tom Lane
2019-06-17Translation updatesPeter Eisentraut
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 75e94214ab3f87bfd37ad417ed6349bf0d1fdb89
2019-06-15Prefer timezone name "UTC" over alternative spellings.Andrew Gierth
tzdb 2019a made "UCT" a link to the "UTC" zone rather than a separate zone with its own abbreviation. Unfortunately, our code for choosing a timezone in initdb has an arbitrary preference for names earlier in the alphabet, and so it would choose the spelling "UCT" over "UTC" when the system is running on a UTC zone. Commit 23bd3cec6 was backpatched in order to address this issue, but that code helps only when /etc/localtime exists as a symlink, and does nothing to help on systems where /etc/localtime is a copy of a zone file (as is the standard setup on FreeBSD and probably some other platforms too) or when /etc/localtime is simply absent (giving UTC as the default). Accordingly, add a preference for the spelling "UTC", such that if multiple zone names have equally good content matches, we prefer that name before applying the existing arbitrary rules. Also add a slightly lower preference for "Etc/UTC"; lower because that preserves the previous behaviour of choosing the shorter name, but letting us still choose "Etc/UTC" over "Etc/UCT" when both exist but "UTC" does not (not common, but I've seen it happen). Backpatch all the way, because the tzdb change that sparked this issue is in those branches too.
2019-06-14Attempt to identify system timezone by reading /etc/localtime symlink.Tom Lane
On many modern platforms, /etc/localtime is a symlink to a file within the IANA database. Reading the symlink lets us find out the name of the system timezone directly, without going through the brute-force search embodied in scan_available_timezones(). This shortens the runtime of initdb by some tens of ms, which is helpful for the buildfarm, and it also allows us to reliably select the same zone name the system was actually configured for, rather than possibly choosing one of IANA's many zone aliases. (For example, in a system configured for "Asia/Tokyo", the brute-force search would not choose that name but its alias "Japan", on the grounds of the latter string being shorter. More surprisingly, "Navajo" is preferred to either "America/Denver" or "US/Mountain", as seen in an old complaint from Josh Berkus.) If /etc/localtime doesn't exist, or isn't a symlink, or we can't make sense of its contents, or the contents match a zone we know but that zone doesn't match the observed behavior of localtime(), fall back to the brute-force search. Also, tweak initdb so that it prints the zone name it selected. In passing, replace the last few references to the "Olson" database in code comments with "IANA", as that's been our preferred term since commit b2cbced9e. Back-patch of commit 23bd3cec6. The original intention was to not back-patch, since this can result in cosmetic behavioral changes --- for example, on my own workstation initdb now chooses "America/New_York", where it used to prefer "US/Eastern" which is equivalent and shorter. However, our hand has been more or less forced by tzdb update 2019a, which made the "UCT" zone fully equivalent to "UTC". Our old code now prefers "UCT" on the grounds of it being alphabetically first, and that's making nobody happy. Choosing the alias indicated by /etc/localtime is a more defensible behavior. (Users who don't like the results can always force the decision by setting the TZ environment variable before running initdb.) Patch by me, per a suggestion from Robert Haas; review by Michael Paquier Discussion: https://postgr.es/m/7408.1525812528@sss.pgh.pa.us Discussion: https://postgr.es/m/20190604085735.GD24018@msg.df7cb.de
2019-06-13Mark ReplicationSlotCtl as PGDLLIMPORT.Tom Lane
Also MyReplicationSlot, in branches where it wasn't already. This was discussed in the thread that resulted in c572599c6, but for some reason nobody pulled the trigger. Now that we have another request for the same thing, we should just do it. Craig Ringer Discussion: https://postgr.es/m/CAMsr+YFTsq-86MnsNng=mPvjjh5EAbzfMK0ptJPvzyvpFARuRg@mail.gmail.com Discussion: https://postgr.es/m/345138875.20190611151943@cybertec.at
2019-06-12Fix incorrect printing of queries with duplicated join names.Tom Lane
Given a query in which multiple JOIN nodes used the same alias (which'd necessarily be in different sub-SELECTs), ruleutils.c would assign the JOIN nodes distinct aliases for clarity ... but then it forgot to print the modified aliases when dumping the JOIN nodes themselves. This results in a dump/reload hazard for views, because the emitted query is flat-out incorrect: Vars will be printed with table names that have no referent. This has been wrong for a long time, so back-patch to all supported branches. Philip Dubé Discussion: https://postgr.es/m/CY4PR2101MB080246F2955FF58A6ED1FEAC98140@CY4PR2101MB0802.namprd21.prod.outlook.com
2019-06-12Fix ALTER COLUMN TYPE failure with a partial exclusion constraint.Tom Lane
ATExecAlterColumnType failed to consider the possibility that an index that needs to be rebuilt might be a child of a constraint that needs to be rebuilt. We missed this so far because usually a constraint index doesn't have a direct dependency on its table, just on the constraint object. But if there's a WHERE clause, then dependency analysis of the WHERE clause results in direct dependencies on the column(s) mentioned in WHERE. This led to trying to drop and rebuild both the constraint and its underlying index. In v11/HEAD, we successfully drop both the index and the constraint, and then try to rebuild both, and of course the second rebuild hits a duplicate-index-name problem. Before v11, it fails with obscure messages about a missing relation OID, due to trying to drop the index twice. This is essentially the same kind of problem noted in commit 20bef2c31: the possible dependency linkages are broader than what ATExecAlterColumnType was designed for. It was probably OK when written, but it's certainly been broken since the introduction of partial exclusion constraints. Fix by adding an explicit check for whether any of the indexes-to-be-rebuilt belong to any of the constraints-to-be-rebuilt, and ignoring any that do. In passing, fix a latent bug introduced by commit 8b08f7d48: in get_constraint_index() we must "continue" not "break" when rejecting a relation of a wrong relkind. This is harmless today because we don't expect that code path to be taken anyway; but if there ever were any relations to be ignored, the existing coding would have an extremely undesirable dependency on the order of pg_depend entries. Also adjust a couple of obsolete comments. Per bug #15835 from Yaroslav Schekin. Back-patch to all supported branches. Discussion: https://postgr.es/m/15835-32d9b7a76c06a7a9@postgresql.org
2019-06-10Don't access catalogs to validate GUCs when not connected to a DB.Andres Freund
Vignesh found this bug in the check function for default_table_access_method's check hook, but that was just copied from older GUCs. Investigation by Michael and me then found the bug in further places. When not connected to a database (e.g. in a walsender connection), we cannot perform (most) GUC checks that need database access. Even when only shared tables are needed, unless they're nailed (c.f. RelationCacheInitializePhase2()), they cannot be accessed without pg_class etc. being present. Fix by extending the existing IsTransactionState() checks to also check for MyDatabaseOid. Reported-By: Vignesh C, Michael Paquier, Andres Freund Author: Vignesh C, Andres Freund Discussion: https://postgr.es/m/CALDaNm1KXK9gbZfY-p_peRFm_XrBh1OwQO1Kk6Gig0c0fVZ2uw%40mail.gmail.com Backpatch: 9.4-
2019-05-31Fix C++ incompatibilities in plpgsql's header files.Tom Lane
Rename some exposed parameters so that they don't conflict with C++ reserved words. Back-patch to all supported versions. George Tarasov Discussion: https://postgr.es/m/b517ec3918d645eb950505eac8dd434e@gaz-is.ru
2019-05-12Fix misoptimization of "{1,1}" quantifiers in regular expressions.Tom Lane
A bounded quantifier with m = n = 1 might be thought a no-op. But according to our documentation (which traces back to Henry Spencer's original man page) it still imposes greediness, or non-greediness in the case of the non-greedy variant "{1,1}?", on whatever it's attached to. This turns out not to work though, because parseqatom() optimizes away the m = n = 1 case without regard for whether it's supposed to change the greediness of the argument RE. We can fix this by just not applying the optimization when the greediness needs to change; the subsequent general cases handle it fine. The three cases in which we can still apply the optimization are (a) no quantifier, or quantifier does not impose a preference; (b) atom has no greediness property, implying it cannot match a variable amount of text anyway; or (c) quantifier's greediness is same as atom's. Note that in most cases where one of these applies, we'd have exited earlier in the "not a messy case" fast path. I think it's now only possible to get to the optimization when the atom involves capturing parentheses or a non-top-level backref. Back-patch to all supported branches. I'd ordinarily be hesitant to put a subtle behavioral change into back branches, but in this case it's very hard to see a reason why somebody would write "{1,1}?" unless they're trying to get the documented change-of-greediness behavior. Discussion: https://postgr.es/m/5bb27a41-350d-37bf-901e-9d26f5592dd0@charter.net
2019-05-12Fail pgwin32_message_to_UTF16() for SQL_ASCII messages.Noah Misch
The function had been interpreting SQL_ASCII messages as UTF8, throwing an error when they were invalid UTF8. The new behavior is consistent with pg_do_encoding_conversion(). This affects LOG_DESTINATION_STDERR and LOG_DESTINATION_EVENTLOG, which will send untranslated bytes to write() and ReportEventA(). On buildfarm member bowerbird, enabling log_connections caused an error whenever the role name was not valid UTF8. Back-patch to 9.4 (all supported versions). Discussion: https://postgr.es/m/20190512015615.GD1124997@rfd.leadboat.com
2019-05-11Fix error reporting in reindexdbMichael Paquier
When failing to reindex a table, reindexdb would generate an extra error message related to a database failure, which is misleading. Backpatch all the way down, as this has been introduced by 85e9a5a0. Discussion: https://postgr.es/m/CAOBaU_Yo61RwNO3cW6WVYWwH7EYMPuexhKqufb2nFGOdunbcHw@mail.gmail.com Author: Julien Rouhaud Reviewed-by: Daniel Gustafsson, Álvaro Herrera, Tom Lane, Michael Paquier Backpatch-through: 9.4
2019-05-10Cope with EINVAL and EIDRM shmat() failures in PGSharedMemoryAttach.Tom Lane
There's a very old race condition in our code to see whether a pre-existing shared memory segment is still in use by a conflicting postmaster: it's possible for the other postmaster to remove the segment in between our shmctl() and shmat() calls. It's a narrow window, and there's no risk unless both postmasters are using the same port number, but that's possible during parallelized "make check" tests. (Note that while the TAP tests take some pains to choose a randomized port number, pg_regress doesn't.) If it does happen, we treated that as an unexpected case and errored out. To fix, allow EINVAL to be treated as segment-not-present, and the same for EIDRM on Linux. AFAICS, the considerations here are basically identical to the checks for acceptable shmctl() failures, so I documented and coded it that way. While at it, adjust PGSharedMemoryAttach's API to remove its undocumented dependency on UsedShmemSegAddr in favor of passing the attach address explicitly. This makes it easier to be sure we're using a null shmaddr when probing for segment conflicts (thus avoiding questions about what EINVAL means). I don't think there was a bug there, but it required fragile assumptions about the state of UsedShmemSegAddr during PGSharedMemoryIsInUse. Commit c09850992 may have made this failure more probable by applying the conflicting-segment tests more often. Hence, back-patch to all supported branches, as that was. Discussion: https://postgr.es/m/22224.1557340366@sss.pgh.pa.us
2019-05-08Remove leftover reference to old "flat file" mechanism in a comment.Heikki Linnakangas
The flat file mechanism was removed in PostgreSQL 9.0.
2019-05-07Remove some code related to 7.3 and older servers from tools of src/bin/Michael Paquier
This code was broken as of 582edc3, and is most likely not used anymore. Note that pg_dump supports servers down to 8.0, and psql has code to support servers down to 7.4. Author: Julien Rouhaud Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAOBaU_Y5y=zo3+2gf+2NJC1pvMYPcbRXoQaPXx=U7+C8Qh4CzQ@mail.gmail.com
2019-05-06Stamp 9.4.22.REL9_4_22Tom Lane
2019-05-06Translation updatesPeter Eisentraut
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 07a92cdb0f05a078521d8464c99cd654409cd0af
2019-05-06Use checkAsUser for selectivity estimator checks, if it's set.Dean Rasheed
In examine_variable() and examine_simple_variable(), when checking the user's table and column privileges to determine whether to grant access to the pg_statistic data, use checkAsUser for the privilege checks, if it's set. This will be the case if we're accessing the table via a view, to indicate that we should perform privilege checks as the view owner rather than the current user. This change makes this planner check consistent with the check in the executor, so the planner will be able to make use of statistics if the table is accessible via the view. This fixes a performance regression introduced by commit e2d4ef8de8, which affects queries against non-security barrier views in the case where the user doesn't have privileges on the underlying table, but the view owner does. Note that it continues to provide the same safeguards controlling access to pg_statistic for direct table access (in which case checkAsUser won't be set) and for security barrier views, because of the nearby checks on rte->security_barrier and rte->securityQuals. Back-patch to all supported branches because e2d4ef8de8 was. Dean Rasheed, reviewed by Jonathan Katz and Stephen Frost.
2019-05-05Remove reindex_catalog test from test schedules.Andres Freund
As the test currently causes occasional deadlocks (due to the schema cleanup from previous sessions potentially still running), and the patch from f912d7dec2 has gotten a fair bit of buildfarm coverage, remove the test from the test schedules. There's a set of minor releases coming up. Leave the tests in place, so it can manually be run using EXTRA_TESTS. For now also leave it in master, as there's no imminent release, and there's plenty (re-)index related work in 12. But we'll have to disable it before long there too, unless somebody comes up with simple enough fixes for the deadlock (I'm about to post a vague idea to the list). Discussion: https://postgr.es/m/4622.1556982247@sss.pgh.pa.us Backpatch: 9.4-11 (no master!)
2019-05-02Fix reindexing of pg_class indexes some more.Tom Lane
Commits 3dbb317d3 et al failed under CLOBBER_CACHE_ALWAYS testing. Investigation showed that to reindex pg_class_oid_index, we must suppress accesses to the index (via SetReindexProcessing) before we call RelationSetNewRelfilenode, or at least before we do CommandCounterIncrement therein; otherwise, relcache reloads happening within the CCI may try to fetch pg_class rows using the index's new relfilenode value, which is as yet an empty file. Of course, the point of 3dbb317d3 was that that ordering didn't work either, because then RelationSetNewRelfilenode's own update of the index's pg_class row cannot access the index, should it need to. There are various ways we might have got around that, but Andres Freund came up with a brilliant solution: for a mapped index, we can really just skip the pg_class update altogether. The only fields it was actually changing were relpages etc, but it was just setting them to zeroes which is useless make-work. (Correct new values will be installed at the end of index build.) All pg_class indexes are mapped and probably always will be, so this eliminates the problem by removing work rather than adding it, always a pleasant outcome. Having taught RelationSetNewRelfilenode to do it that way, we can revert the code reordering in reindex_index. (But I left the moved setup code where it was; there seems no reason why it has to run without use of the old index. If you're trying to fix a busted pg_class index, you'll have had to disable system index use altogether to get this far.) Moreover, this means we don't need RelationSetIndexList at all, because reindex_relation's hacking to make "REINDEX TABLE pg_class" work is likewise now unnecessary. We'll leave that code in place in the back branches, but a follow-on patch will remove it in HEAD. In passing, do some minor cleanup for commit 5c1560606 (in HEAD only), notably removing a duplicate newrnode assignment. Patch by me, using a core idea due to Andres Freund. Back-patch to all supported branches, as 3dbb317d3 was. Discussion: https://postgr.es/m/28926.1556664156@sss.pgh.pa.us
2019-04-30Run catalog reindexing test from 3dbb317d32 serially, to avoid deadlocks.Andres Freund
The tests turn out to cause deadlocks in some circumstances. Fairly reproducibly so with -DRELCACHE_FORCE_RELEASE -DCATCACHE_FORCE_RELEASE. Some of the deadlocks may be hard to fix without disproportionate measures, but others probably should be fixed - but not in 12. We discussed removing the new tests until we can fix the issues underlying the deadlocks, but results from buildfarm animal markhor (which runs with CLOBBER_CACHE_ALWAYS) indicates that there might be a more severe, as of yet undiagnosed, issue (including on stable branches) with reindexing catalogs. The failure is: ERROR: could not read block 0 in file "base/16384/28025": read only 0 of 8192 bytes Therefore it seems advisable to keep the tests. It's not certain that running the tests in isolation removes the risk of deadlocks. It's possible that additional locks are needed to protect against a concurrent auto-analyze or such. Per discussion with Tom Lane. Discussion: https://postgr.es/m/28926.1556664156@sss.pgh.pa.us Backpatch: 9.4-, like 3dbb317d3
2019-04-30Fix unused variable compiler warning in !debug builds.Andres Freund
Introduced in 3dbb317d3. Fix by using the new local variable in more places. Reported-By: Bruce Momjian (off-list) Backpatch: 9.4-, like 3dbb317d3
2019-04-29Fix potential assertion failure when reindexing a pg_class index.Andres Freund
When reindexing individual indexes on pg_class it was possible to either trigger an assertion failure: TRAP: FailedAssertion("!(!ReindexIsProcessingIndex(((index)->rd_id))) That's because reindex_index() called SetReindexProcessing() - which enables an asserts ensuring no index insertions happen into the index - before calling RelationSetNewRelfilenode(). That not correct for indexes on pg_class, because RelationSetNewRelfilenode() updates the relevant pg_class row, which needs to update the indexes. The are two reasons this wasn't noticed earlier. Firstly the bug doesn't trigger when reindexing all of pg_class, as reindex_relation has code "hiding" all yet-to-be-reindexed indexes. Secondly, the bug only triggers when the the update to pg_class doesn't turn out to be a HOT update - otherwise there's no index insertion to trigger the bug. Most of the time there's enough space, making this bug hard to trigger. To fix, move RelationSetNewRelfilenode() to before the SetReindexProcessing() (and, together with some other code, to outside of the PG_TRY()). To make sure the error checking intended by SetReindexProcessing() is more robust, modify CatalogIndexInsert() to check ReindexIsProcessingIndex() even when the update is a HOT update. Also add a few regression tests for REINDEXing of system catalogs. The last two improvements would have prevented some of the issues fixed in 5c1560606dc4c from being introduced in the first place. Reported-By: Michael Paquier Diagnosed-By: Tom Lane and Andres Freund Author: Andres Freund Reviewed-By: Tom Lane Discussion: https://postgr.es/m/20190418011430.GA19133@paquier.xyz Backpatch: 9.4-, the bug is present in all branches
2019-04-26Portability fix for zic.c.Tom Lane
Missed an inttypes.h dependency in previous patch. Per buildfarm.
2019-04-26Sync our copy of the timezone library with IANA release tzcode2019a.Tom Lane
This corrects a small bug in zic that caused it to output an incorrect year-2440 transition in the Africa/Casablanca zone. More interestingly, zic has grown a "-r" option that limits the range of zone transitions that it will put into the output files. That might be useful to people who don't like the weird GMT offsets that tzdb likes to use for very old dates. It appears that for dates before the cutoff time specified with -r, zic will use the zone's standard-time offset as of the cutoff time. So for example one might do make install ZIC_OPTIONS='-r @-1893456000' to cause all dates before 1910-01-01 to be treated as though 1910 standard time prevailed indefinitely far back. (Don't blame me for the unfriendly way of specifying the cutoff time --- it's seconds since or before the Unix epoch. You can use extract(epoch ...) to calculate it.) As usual, back-patch to all supported branches.
2019-04-26Update time zone data files to tzdata release 2019a.Tom Lane
DST law changes in Palestine and Metlakatla. Historical corrections for Israel. Etc/UCT is now a backward-compatibility link to Etc/UTC, instead of being a separate zone that generates the abbreviation "UCT", which nowadays is typically a typo. Postgres will still accept "UCT" as an input zone name, but it won't output it.
2019-04-24Fix some minor postmaster-state-machine issues.Tom Lane
In sigusr1_handler, don't ignore PMSIGNAL_ADVANCE_STATE_MACHINE based on pmState. The restriction is unnecessary (PostmasterStateMachine should work in any state), not future-proof (since it makes too many assumptions about why the signal might be sent), and broken even today because a race condition can make it necessary to respond to the signal in PM_WAIT_READONLY state. The race condition seems unlikely, but if it did happen, a hot-standby postmaster could fail to shut down after receiving a smart-shutdown request. In MaybeStartWalReceiver, don't clear the WalReceiverRequested flag if the fork attempt fails. Leaving it set allows us to try again in future iterations of the postmaster idle loop. (The startup process would eventually send a fresh request signal, but this change may allow us to retry the fork sooner.) Remove an obsolete comment and unnecessary test in PostmasterStateMachine's handling of PM_SHUTDOWN_2 state. It's not possible to have a live walreceiver in that state, and AFAICT has not been possible since commit 5e85315ea. This isn't a live bug, but the false comment is quite confusing to readers. In passing, rearrange sigusr1_handler's CheckPromoteSignal tests so that we don't uselessly perform stat() calls that we're going to ignore the results of. Add some comments clarifying the behavior of MaybeStartWalReceiver; I very nearly rearranged it in a way that'd reintroduce the race condition fixed in e5d494d78. Mea culpa for not commenting that properly at the time. Back-patch to all supported branches. The PMSIGNAL_ADVANCE_STATE_MACHINE change is the only one of even minor significance, but we might as well keep this code in sync across branches. Discussion: https://postgr.es/m/9001.1556046681@sss.pgh.pa.us
2019-04-24Fix detection of passwords hashed with MD5Michael Paquier
This commit fixes an issue related to the way password verifiers hashed with MD5 are detected, leading to possibly detect that plain passwords are legit MD5 hashes. A MD5-hashed entry was checked based on if its header uses "md5" and if the string length matches what is expected. Unfortunately the code never checked if the hash only used hexadecimal characters after the three-character prefix. Fix 9.6 down to 9.4, where this code is present. This area of the code has changed in 10 and upwards with the introduction of SCRAM, which led to a different fix committed as of ccae190. Reported-by: Tom Lane Author: Michael Paquier Reviewed-by: Jonathan Katz Discussion: https://postgr.es/m/016deb6b-1f0a-8e9f-1833-a8675b170aa9@postgresql.org Backpatch-through: 9.4
2019-04-23Repair assorted issues in locale data extraction.Tom Lane
cache_locale_time (extraction of LC_TIME-related info) had never been taught the lessons we previously learned about extraction of info related to LC_MONETARY and LC_NUMERIC. Specifically, commit 95a777c61 taught PGLC_localeconv() that data coming out of localeconv() was in an encoding determined by the relevant locale, but we didn't realize that there's a similar issue with strftime(). And commit a4930e7ca hardened PGLC_localeconv() against errors occurring partway through, but failed to do likewise for cache_locale_time(). So, rearrange the latter function to perform encoding conversion and not risk failure while it's got the locales set to temporary values. This time around I also changed PGLC_localeconv() to treat it as FATAL if it can't restore the previous settings of the locale values. There is no reason (except possibly OOM) for that to fail, and proceeding with the wrong locale values seems like a seriously bad idea --- especially on Windows where we have to also temporarily change LC_CTYPE. Also, protect against the possibility that we can't identify the codeset reported for LC_MONETARY or LC_NUMERIC; rather than just failing, try to validate the data without conversion. The user-visible symptom this fixes is that if LC_TIME is set to a locale name that implies an encoding different from the database encoding, non-ASCII localized day and month names would be retrieved in the wrong encoding, leading to either unexpected encoding-conversion error reports or wrong output from to_char(). The other possible failure modes are unlikely enough that we've not seen reports of them, AFAIK. The encoding conversion problems do not manifest on Windows, since we'd already created special-case code to handle that issue there. Per report from Juan José Santamaría Flecha. Back-patch to all supported versions. Juan José Santamaría Flecha and Tom Lane Discussion: https://postgr.es/m/CAC+AXB22So5aZm2vZe+MChYXec7gWfr-n-SK-iO091R0P_1Tew@mail.gmail.com
2019-04-17postgresql.conf.sample: add proper defaults for include actionsBruce Momjian
Previously, include actions include_dir, include_if_exists, and include listed commented-out values which were not the defaults, which is inconsistent with other entries. Instead, replace them with '', which is the default value. Reported-by: Emanuel Araújo Discussion: https://postgr.es/m/CAMuTAkYMx6Q27wpELDR3_v9aG443y7ZjeXu15_+1nGUjhMWOJA@mail.gmail.com Backpatch-through: 9.4
2019-04-12Consistently test for in-use shared memory.Noah Misch
postmaster startup scrutinizes any shared memory segment recorded in postmaster.pid, exiting if that segment matches the current data directory and has an attached process. When the postmaster.pid file was missing, a starting postmaster used weaker checks. Change to use the same checks in both scenarios. This increases the chance of a startup failure, in lieu of data corruption, if the DBA does "kill -9 `head -n1 postmaster.pid` && rm postmaster.pid && pg_ctl -w start". A postmaster will no longer stop if shmat() of an old segment fails with EACCES. A postmaster will no longer recycle segments pertaining to other data directories. That's good for production, but it's bad for integration tests that crash a postmaster and immediately delete its data directory. Such a test now leaks a segment indefinitely. No "make check-world" test does that. win32_shmem.c already avoided all these problems. In 9.6 and later, enhance PostgresNode to facilitate testing. Back-patch to 9.4 (all supported versions). Reviewed (in earlier versions) by Daniel Gustafsson and Kyotaro HORIGUCHI. Discussion: https://postgr.es/m/20190408064141.GA2016666@rfd.leadboat.com
2019-04-11Fix off-by-one check that can lead to a memory overflow in ecpg.Michael Meskes
Patch by Liu Huailing <liuhuailing@cn.fujitsu.com>
2019-04-09Define WIN32_STACK_RLIMIT throughout win32 and cygwin builds.Noah Misch
The MSVC build system already did this, and commit 617dc6d299c957e2784320382b3277ede01d9c63 used it in a second file. Back-patch to 9.4, like that commit. Discussion: https://postgr.es/m/CAA8=A7_1SWc3+3Z=-utQrQFOtrj_DeohRVt7diA2tZozxsyUOQ@mail.gmail.com
2019-04-08Avoid "could not reattach" by providing space for concurrent allocation.Noah Misch
We've long had reports of intermittent "could not reattach to shared memory" errors on Windows. Buildfarm member dory fails that way when PGSharedMemoryReAttach() execution overlaps with creation of a thread for the process's "default thread pool". Fix that by providing a second region to receive asynchronous allocations that would otherwise intrude into UsedShmemSegAddr. In pgwin32_ReserveSharedMemoryRegion(), stop trying to free reservations landing at incorrect addresses; the caller's next step has been to terminate the affected process. Back-patch to 9.4 (all supported versions). Reviewed by Tom Lane. He also did much of the prerequisite research; see commit bcbf2346d69f6006f126044864dd9383d50d87b4. Discussion: https://postgr.es/m/20190402135442.GA1173872@rfd.leadboat.com
2019-04-08Fix improper interaction of FULL JOINs with lateral references.Tom Lane
join_is_legal() needs to reject forming certain outer joins in cases where that would lead the planner down a blind alley. However, it mistakenly supposed that the way to handle full joins was to treat them as applying the same constraints as for left joins, only to both sides. That doesn't work, as shown in bug #15741 from Anthony Skorski: given a lateral reference out of a join that's fully enclosed by a full join, the code would fail to believe that any join ordering is legal, resulting in errors like "failed to build any N-way joins". However, we don't really need to consider full joins at all for this purpose, because we effectively force them to be evaluated in syntactic order, and that order is always legal for lateral references. Hence, get rid of this broken logic for full joins and just ignore them instead. This seems to have been an oversight in commit 7e19db0c0. Back-patch to all supported branches, as that was. Discussion: https://postgr.es/m/15741-276f1f464b3f40eb@postgresql.org