aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Fix variable initialization when using buffering build with GiSTMichael Paquier2019-07-10
| | | | | | | | | | | | | | | | 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
* Pass QueryEnvironment down to EvalPlanQual's EState.Thomas Munro2019-07-10
| | | | | | | | | | | Otherwise the executor can't see trigger transition tables during EPQ evaluation. Fixes bug #15900 and almost certainly also #15720. Back-patch to 10, where trigger transition tables landed. Author: Alex Aktsipetrov Reviewed-by: Thomas Munro, Tom Lane Discussion: https://postgr.es/m/15900-bc482754fe8d7415%40postgresql.org Discussion: https://postgr.es/m/15720-38c2b29e5d720187%40postgresql.org
* Propagate trigger arguments to partitionsAlvaro Herrera2019-07-09
| | | | | | | | | | | | | We were creating the cloned triggers with an empty list of arguments, losing the ones that had been specified by the user when creating the trigger in the partitioned table. Repair. This was forgotten in commit 86f575948c77. Author: Patrick McHardy Reviewed-by: Tomas Vondra Discussion: https://postgr.es/m/20190709130027.amr2cavjvo7rdvac@access1.trash.net Discussion: https://postgr.es/m/15752-123bc90287986de4@postgresql.org
* Force hash joins to be enabled in the hash join regression tests.Thomas Munro2019-07-09
| | | | | | | | Otherwise the regressplans.sh tests generate extremely slow nested loop joins. Back-patch to 11 where the hash join tests came in. Reported-by: Michael Paquier Discussion: https://postgr.es/m/20190708055256.GB2709%40paquier.xyz
* Ensure plpgsql result tuples have the right composite type marking.Tom Lane2019-07-03
| | | | | | | | | | | | | | | | | | | | | | | A function that is declared to return a named composite type must return tuple datums that are physically marked as having that type. The plpgsql code path that allowed directly returning an expanded-record datum forgot to check that, so that an expanded record marked as type RECORDOID could be returned if it had a physically-compatible tupdesc. This'd be harmless, I think, if the record value never escaped the current session --- but it's possible for it to get stored into a table, and then subsequent sessions can't interpret the anonymous record type. Fix by flattening the record into a tuple datum and overwriting its type/typmod fields, if its declared type doesn't match the function's declared type. (In principle it might be possible to just change the expanded record's stored type ID info, but there are enough tricky consequences that I didn't want to mess with that, especially not in a back-patched bug fix.) Per bug report from Steve Rogerson. Back-patch to v11 where the bug was introduced. Discussion: https://postgr.es/m/cbaecae6-7b87-584e-45f6-4d047b92ca2a@yewtc.demon.co.uk
* Don't remove surplus columns from GROUP BY for inheritance parentsDavid Rowley2019-07-03
| | | | | | | | | | | | | | | | | | | d4c3a156c added code to remove columns that were not part of a table's PRIMARY KEY constraint from the GROUP BY clause when all the primary key columns were present in the group by. This is fine to do since we know that there will only be one row per group coming from this relation. However, the logic failed to consider inheritance parent relations. These can have child relations without a primary key, but even if they did, they could duplicate one of the parent's rows or one from another child relation. In this case, those additional GROUP BY columns are required. Fix this by disabling the optimization for inheritance parent tables. In v11 and beyond, partitioned tables are fine since partitions cannot overlap and before v11 partitioned tables could not have a primary key. Reported-by: Manuel Rigger Discussion: http://postgr.es/m/CA+u7OA7VLKf_vEr6kLF3MnWSA9LToJYncgpNX2tQ-oWzYCBQAw@mail.gmail.com Backpatch-through: 9.6
* Add support for Visual Studio 2019 in build scriptsMichael Paquier2019-07-03
| | | | | | | | | | | 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
* Fix tab completion of "SET variable TO|=" to not offer bogus completions.Tom Lane2019-07-02
| | | | | | | | | | | | | | | | Don't think that the context "UPDATE tab SET var =" is a GUC-setting command. If we have "SET var =" but the "var" is not a known GUC variable, don't offer any completions. The most likely explanation is that we've misparsed the context and it's not really a GUC-setting command. Per gripe from Ken Tanzer. Back-patch to 9.6. The issue exists further back, but before 9.6 the code looks very different and it doesn't actually know whether the "var" name matches anything, so I desisted from trying to fix it. Discussion: https://postgr.es/m/CAD3a31XpXzrZA9TT3BqLSHghdTK+=cXjNCE+oL2Zn4+oWoc=qA@mail.gmail.com
* Don't read fields of a misaligned ExpandedObjectHeader or AnyArrayType.Noah Misch2019-06-30
| | | | | | | | | | | UBSan complains about this. Instead, cast to a suitable type requiring only 4-byte alignment. DatumGetAnyArrayP() already assumes one can cast between AnyArrayType and ArrayType, so this doesn't introduce a new assumption. Back-patch to 9.5, where AnyArrayType was introduced. Reviewed by Tom Lane. Discussion: https://postgr.es/m/20190629210334.GA1244217@rfd.leadboat.com
* Repair logic for reordering grouping sets optimization.Andrew Gierth2019-06-30
| | | | | | | | | | | | | | The logic in reorder_grouping_sets to order grouping set elements to match a pre-specified sort ordering was defective, resulting in unnecessary sort nodes (though the query output would still be correct). Repair, simplifying the code a little, and add a test. Per report from Richard Guo, though I didn't use their patch. Original bug seems to have been my fault. Backpatch back to 9.5 where grouping sets were introduced. Discussion: https://postgr.es/m/CAN_9JTzyjGcUjiBHxLsgqfk7PkdLGXiM=pwM+=ph2LsWw0WO1A@mail.gmail.com
* Fix for dropped columns in a partitioned table's default partitionAlvaro Herrera2019-06-28
| | | | | | | | | | | | | | | | | | We forgot to map column numbers to/from the default partition for various operations, leading to valid cases failing with spurious errors, such as ERROR: attribute N of type some_partition has been dropped It was also possible that the search for conflicting rows in the default partition when attaching another partition would fail to detect some. Secondarily, it was also possible that such a search should be skipped (because the constraint was implied) but wasn't. Fix all this by mapping column numbers when necessary. Reported by: Daniel Wilches Author: Amit Langote Discussion: https://postgr.es/m/15873-8c61945d6b3ef87c@postgresql.org
* Fix misleading comment in nodeIndexonlyscan.c.Thomas Munro2019-06-28
| | | | | | | | | | | 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
* Update reference to sampling algorithm in analyze.cTomas Vondra2019-06-27
| | | | | | | | | | 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
* Fix use-after-free introduced in 55ed3defc966Alvaro Herrera2019-06-27
| | | | | | | | Evidenced by failure under RELCACHE_FORCE_RELEASE (buildfarm member prion). Author: Amit Langote Discussion: https://postgr.es/m/CA+HiwqGV=k_Eh4jBiQw66ivvdG+EUkrEYeHTYL1SvDj_YOYV0g@mail.gmail.com
* Fix partitioned index creation with foreign partitionsAlvaro Herrera2019-06-26
| | | | | | | | | | | | | | | | | | | | | When a partitioned tables contains foreign tables as partitions, it is not possible to implement unique or primary key indexes -- but when regular indexes are created, there is no reason to do anything other than ignoring such partitions. We were raising errors upon encountering the foreign partitions, which is unfriendly and doesn't protect against any actual problems. Relax this restriction so that index creation is allowed on partitioned tables containing foreign partitions, becoming a no-op on them. (We may later want to redefine this so that the FDW is told to create the indexes on the foreign side.) This applies to CREATE INDEX, as well as ALTER TABLE / ATTACH PARTITION and CREATE TABLE / PARTITION OF. Backpatch to 11, where indexes on partitioned tables were introduced. Discussion: https://postgr.es/m/15724-d5a58fa9472eef4f@postgresql.org Author: Álvaro Herrera Reviewed-by: Amit Langote
* Add support for OpenSSL 1.1.0 and newer versions in MSVC scriptsMichael Paquier2019-06-26
| | | | | | | | | | | | | | | | | | | | | | 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
* Fix thinkos in LookupFuncName() for function name lookupsMichael Paquier2019-06-25
| | | | | | | | | | | | This could trigger valgrind failures when doing ambiguous function name lookups when no arguments are provided by the caller. The problem has been introduced in aefeb68, so backpatch to v10. HEAD is fine thanks to the refactoring done in bfb456c1. Reported-by: Alexander Lakhin Author: Alexander Lakhin, Michael Paquier Discussion: https://postgr.es/m/3d068be5-f617-a5ee-99f6-458a407bfd65@gmail.com Backpatch-through: 10
* Remove misleading comment from pathnodes.h.Thomas Munro2019-06-25
| | | | | | | | | | As of commit e5253fdc, it is no longer true that the leader always executes the subplan of a Gather Merge node. Remove comment to that effect. Back-patch to 11. Discussion: https://postgr.es/m/CA%2BhUKGJEaZJYezXAOutuiWT%2BfxCA44%2BoKtVPAND2ubLiigR%3D-w%40mail.gmail.com
* Further fix ALTER COLUMN TYPE's handling of indexes and index constraints.Tom Lane2019-06-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Fix spinlock assembly code for MIPS so it works on MIPS r6.Tom Lane2019-06-22
| | | | | | | | | | | | | | | | | | | | | | | | 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
* Consolidate methods for translating a Perl path to a Windows path.Noah Misch2019-06-21
| | | | | | | | | | | | This fixes some TAP suites when using msys Perl and a builddir located in an msys mount point other than "/". For example, builddir=/c/pg exhibited the problem, since /c/pg falls in mount point "/c". Back-patch to 9.6, where tests first started to perform such translations. In back branches, offer both new and old APIs. Reviewed by Andrew Dunstan. Discussion: https://postgr.es/m/20190610045838.GA238501@rfd.leadboat.com
* Remove obsolete comments about sempahores from proc.c.Thomas Munro2019-06-21
| | | | | | | | | | | Commit 6753333f switched from a semaphore-based wait to a latch-based wait for ProcSleep()/ProcWakeup(), but left behind some stray references to semaphores. Back-patch to 9.5. Reviewed-by: Daniel Gustafsson, Michael Paquier Discussion: https://postgr.es/m/CA+hUKGLs5H6zhmgTijZ1OaJvC1sG0=AFXc1aHuce32tKiQrdEA@mail.gmail.com
* Fix description of WAL record XLOG_BTREE_META_CLEANUPMichael Paquier2019-06-19
| | | | | | | | | | | | | | | This record uses one metadata buffer and registers some data associated to the buffer, but when parsing the record for its description a direct access to the record data was done, but there is none. This leads usually to an incorrect description, but can also cause crashes like in pg_waldump. Instead, fix things so as the parsing uses the data associated to the metadata block. This is an oversight from 3d92796, so backpatch down to 11. Author: Michael Paquier Description: https://postgr.es/m/20190617013059.GA3153@paquier.xyz Backpatch-through: 11
* Avoid spurious deadlocks when upgrading a tuple lockAlvaro Herrera2019-06-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This puts back reverted commit de87a084c0a5, with some bug fixes. When two (or more) transactions are waiting for transaction T1 to release a tuple-level lock, and transaction T1 upgrades its lock to a higher level, a spurious deadlock can be reported among the waiting transactions when T1 finishes. The simplest example case seems to be: T1: select id from job where name = 'a' for key share; Y: select id from job where name = 'a' for update; -- starts waiting for T1 Z: select id from job where name = 'a' for key share; T1: update job set name = 'b' where id = 1; Z: update job set name = 'c' where id = 1; -- starts waiting for T1 T1: rollback; At this point, transaction Y is rolled back on account of a deadlock: Y holds the heavyweight tuple lock and is waiting for the Xmax to be released, while Z holds part of the multixact and tries to acquire the heavyweight lock (per protocol) and goes to sleep; once T1 releases its part of the multixact, Z is awakened only to be put back to sleep on the heavyweight lock that Y is holding while sleeping. Kaboom. This can be avoided by having Z skip the heavyweight lock acquisition. As far as I can see, the biggest downside is that if there are multiple Z transactions, the order in which they resume after T1 finishes is not guaranteed. Backpatch to 9.6. The patch applies cleanly on 9.5, but the new tests don't work there (because isolationtester is not smart enough), so I'm not going to risk it. Author: Oleksii Kliukin Discussion: https://postgr.es/m/B9C9D7CD-EB94-4635-91B6-E558ACEC0EC3@hintbits.com Discussion: https://postgr.es/m/2815.1560521451@sss.pgh.pa.us
* Prevent Parallel Hash Join for JOIN_UNIQUE_INNER.Thomas Munro2019-06-19
| | | | | | | | | | | | | | WHERE EXISTS (...) queries cannot be executed by Parallel Hash Join with jointype JOIN_UNIQUE_INNER, because there is no way to make a partial plan totally unique. The consequence of allowing such plans was duplicate results from some EXISTS queries. Back-patch to 11. Bug #15857. Author: Thomas Munro Reviewed-by: Tom Lane Reported-by: Vladimir Kriukov Discussion: https://postgr.es/m/15857-d1ba2a64bce0795e%40postgresql.org
* Stamp 11.4.REL_11_4Tom Lane2019-06-17
|
* Translation updatesPeter Eisentraut2019-06-17
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 599a4bccd28710a88972e1a0ef6961c9bad816fc
* Fix buffer overflow when processing SCRAM final message in libpqMichael Paquier2019-06-17
| | | | | | | | | | | | | | When a client connects to a rogue server sending specifically-crafted messages, this can suffice to execute arbitrary code as the operating system account used by the client. While on it, fix one error handling when decoding an incorrect salt included in the first message received from server. Author: Michael Paquier Reviewed-by: Jonathan Katz, Heikki Linnakangas Security: CVE-2019-10164 Backpatch-through: 10
* Fix buffer overflow when parsing SCRAM verifiers in backendMichael Paquier2019-06-17
| | | | | | | | | | | | | | | | | | | Any authenticated user can overflow a stack-based buffer by changing the user's own password to a purpose-crafted value. This often suffices to execute arbitrary code as the PostgreSQL operating system account. This fix is contributed by multiple folks, based on an initial analysis from Tom Lane. This issue has been introduced by 68e61ee, so it was possible to make use of it at authentication time. It became more easily to trigger after ccae190 which has made the SCRAM parsing more strict when changing a password, in the case where the client passes down a verifier already hashed using SCRAM. Back-patch to v10 where SCRAM has been introduced. Reported-by: Alexander Lakhin Author: Jonathan Katz, Heikki Linnakangas, Michael Paquier Security: CVE-2019-10164 Backpatch-through: 10
* Revert "Avoid spurious deadlocks when upgrading a tuple lock"Alvaro Herrera2019-06-16
| | | | | | | | | | | This reverts commits 3da73d6839dc and de87a084c0a5. This code has some tricky corner cases that I'm not sure are correct and not properly tested anyway, so I'm reverting the whole thing for next week's releases (reintroducing the deadlock bug that we set to fix). I'll try again afterwards. Discussion: https://postgr.es/m/E1hbXKQ-0003g1-0C@gemulon.postgresql.org
* Prefer timezone name "UTC" over alternative spellings.Andrew Gierth2019-06-15
| | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* Silence compiler warningAlvaro Herrera2019-06-14
| | | | Introduced in de87a084c0a5.
* Attempt to identify system timezone by reading /etc/localtime symlink.Tom Lane2019-06-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Avoid spurious deadlocks when upgrading a tuple lockAlvaro Herrera2019-06-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When two (or more) transactions are waiting for transaction T1 to release a tuple-level lock, and transaction T1 upgrades its lock to a higher level, a spurious deadlock can be reported among the waiting transactions when T1 finishes. The simplest example case seems to be: T1: select id from job where name = 'a' for key share; Y: select id from job where name = 'a' for update; -- starts waiting for X Z: select id from job where name = 'a' for key share; T1: update job set name = 'b' where id = 1; Z: update job set name = 'c' where id = 1; -- starts waiting for X T1: rollback; At this point, transaction Y is rolled back on account of a deadlock: Y holds the heavyweight tuple lock and is waiting for the Xmax to be released, while Z holds part of the multixact and tries to acquire the heavyweight lock (per protocol) and goes to sleep; once X releases its part of the multixact, Z is awakened only to be put back to sleep on the heavyweight lock that Y is holding while sleeping. Kaboom. This can be avoided by having Z skip the heavyweight lock acquisition. As far as I can see, the biggest downside is that if there are multiple Z transactions, the order in which they resume after X finishes is not guaranteed. Backpatch to 9.6. The patch applies cleanly on 9.5, but the new tests don't work there (because isolationtester is not smart enough), so I'm not going to risk it. Author: Oleksii Kliukin Discussion: https://postgr.es/m/B9C9D7CD-EB94-4635-91B6-E558ACEC0EC3@hintbits.com
* Mark ReplicationSlotCtl as PGDLLIMPORT.Tom Lane2019-06-13
| | | | | | | | | | | | | 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
* Fix incorrect printing of queries with duplicated join names.Tom Lane2019-06-12
| | | | | | | | | | | | | | | | | 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
* In walreceiver, don't try to do ereport() in a signal handler.Tom Lane2019-06-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is quite unsafe, even for the case of ereport(FATAL) where we won't return control to the interrupted code, and despite this code's use of a flag to restrict the areas where we'd try to do it. It's possible for example that we interrupt malloc or free while that's holding a lock that's meant to protect against cross-thread interference. Then, any attempt to do malloc or free within ereport() will result in a deadlock, preventing the walreceiver process from exiting in response to SIGTERM. We hypothesize that this explains some hard-to-reproduce failures seen in the buildfarm. Hence, get rid of the immediate-exit code in WalRcvShutdownHandler, as well as the logic associated with WalRcvImmediateInterruptOK. Instead, we need to take care that potentially-blocking operations in the walreceiver's data transmission logic (libpqwalreceiver.c) will respond reasonably promptly to the process's latch becoming set and then call ProcessWalRcvInterrupts. Much of the needed code for that was already present in libpqwalreceiver.c. I refactored things a bit so that all the uses of PQgetResult use latch-aware waiting, but didn't need to do much more. These changes should be enough to ensure that libpqwalreceiver.c will respond promptly to SIGTERM whenever it's waiting to receive data. In principle, it could block for a long time while waiting to send data too, and this patch does nothing to guard against that. I think that that hazard is mostly theoretical though: such blocking should occur only if we fill the kernel's data transmission buffers, and we don't generally send enough data to make that happen without waiting for input. If we find out that the hazard isn't just theoretical, we could fix it by using PQsetnonblocking, but that would require more ticklish changes than I care to make now. Back-patch of commit a1a789eb5. This problem goes all the way back to the origins of walreceiver; but given the substantial reworking the module received during the v10 cycle, it seems unsafe to assume that our testing on HEAD validates this patch for pre-v10 branches. And we'd need to back-patch some prerequisite patches (at least 597a87ccc and its followups, maybe other things), increasing the risk of problems. Given the dearth of field reports matching this problem, it's not worth much risk. Hence back-patch to v10 and v11 only. Patch by me; thanks to Thomas Munro for review. Discussion: https://postgr.es/m/20190416070119.GK2673@paquier.xyz
* Fix ALTER COLUMN TYPE failure with a partial exclusion constraint.Tom Lane2019-06-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Fix handling of COMMENT for domain constraintsMichael Paquier2019-06-12
| | | | | | | | | | | | | | | | | | | | For a non-superuser, changing a comment on a domain constraint was leading to a cache lookup failure as the code tried to perform the ownership lookup on the constraint OID itself, thinking that it was a type, but this check needs to happen on the type the domain constraint relies on. As the type a domain constraint relies on can be guessed directly based on the constraint OID, first fetch its type OID and perform the ownership on it. This is broken since 7eca575, which has split the handling of comments for table constraints and domain constraints, so back-patch down to 9.5. Reported-by: Clemens Ladisch Author: Daniel Gustafsson, Michael Paquier Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/15833-808e11904835d26f@postgresql.org Backpatch-through: 9.5
* Fix conversion of JSON strings to JSON output columns in json_to_record().Tom Lane2019-06-11
| | | | | | | | | | | | | | | | | | | | | | | | | json_to_record(), when an output column is declared as type json or jsonb, should emit the corresponding field of the input JSON object. But it got this slightly wrong when the field is just a string literal: it failed to escape the contents of the string. That typically resulted in syntax errors if the string contained any double quotes or backslashes. jsonb_to_record() handles such cases correctly, but I added corresponding test cases for it too, to prevent future backsliding. Improve the documentation, as it provided only a very hand-wavy description of the conversion rules used by these functions. Per bug report from Robert Vollmert. Back-patch to v10 where the error was introduced (by commit cf35346e8). Note that PG 9.4 - 9.6 also get this case wrong, but differently so: they feed the de-escaped contents of the string literal to json[b]_in. That behavior is less obviously wrong, so possibly it's being depended on in the field, so I won't risk trying to make the older branches behave like the newer ones. Discussion: https://postgr.es/m/D6921B37-BD8E-4664-8D5F-DB3525765DCD@vllmrt.net
* Don't access catalogs to validate GUCs when not connected to a DB.Andres Freund2019-06-10
| | | | | | | | | | | | | | | | | | | | | 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-
* Make pg_dump emit ATTACH PARTITION instead of PARTITION OF (reprise)Alvaro Herrera2019-06-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Using PARTITION OF can result in column ordering being changed from the database being dumped, if the partition uses a column layout different from the parent's. It's not pg_dump's job to editorialize on table definitions, so this is not acceptable; back-patch all the way back to pg10, where partitioned tables where introduced. This change also ensures that partitions end up in the correct tablespace, if different from the parent's; this is an oversight in ca4103025dfe (in pg12 only). Partitioned indexes (in pg11) don't have this problem, because they're already created as independent indexes and attached to their parents afterwards. This change also has the advantage that the partition is restorable from the dump (as a standalone table) even if its parent table isn't restored. The original commits (3b23552ad8bb in branch master) failed to cover subsidiary column elements correctly, such as NOT NULL constraint and CHECK constraints, as reported by Rushabh Lathia (initially as a failure to restore serial columns). They were reverted. This recapitulation commit fixes those problems. Add some pg_dump tests to verify these things more exhaustively, including constraints with legacy-inheritance tables, which were not tested originally. In branches 10 and 11, add a local constraint to the pg_dump test partition that was added by commit 2d7eeb1b1492 to master. Author: Álvaro Herrera, David Rowley Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/CAKJS1f_1c260nOt_vBJ067AZ3JXptXVRohDVMLEBmudX1YEx-A@mail.gmail.com Discussion: https://postgr.es/m/20190423185007.GA27954@alvherre.pgsql Discussion: https://postgr.es/m/CAGPqQf0iQV=PPOv2Btog9J9AwOQp6HmuVd6SbGTR_v3Zp2XT1w@mail.gmail.com
* Fix inconsistency in comments atop ExecParallelEstimate.Amit Kapila2019-06-07
| | | | | | | | | | | | | When this code was initially introduced in commit d1b7c1ff, the structure used was SharedPlanStateInstrumentation, but later when it got changed to Instrumentation structure in commit b287df70, we forgot to update the comment. Reported-by: Wu Fei Author: Wu Fei Reviewed-by: Amit Kapila Backpatch-through: 9.6 Discussion: https://postgr.es/m/52E6E0843B9D774C8C73D6CF64402F0562215EB2@G08CNEXMBPEKD02.g08.fujitsu.local
* Fix unsafe memory management in CloneRowTriggersToPartition().Tom Lane2019-06-03
| | | | | | | | | | | | | It's not really supported to call systable_getnext() in a different memory context than systable_beginscan() was called in, and it's *definitely* not safe to do so and then reset that context between calls. I'm not very clear on how this code survived CLOBBER_CACHE_ALWAYS testing ... but Alexander Lakhin found a case that would crash it pretty reliably. Per bug #15828. Fix, and backpatch to v11 where this code came in. Discussion: https://postgr.es/m/15828-f6ddd7df4852f473@postgresql.org
* Fix C++ incompatibilities in plpgsql's header files.Tom Lane2019-05-31
| | | | | | | | | | | 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
* Make error logging in extended statistics more consistentTomas Vondra2019-05-30
| | | | | | | | | | | | | | | | Most errors reported in extended statistics are internal issues, and so should use elog(). The MCV list code was already following this rule, but the functional dependencies and ndistinct coefficients were using a mix of elog() and ereport(). Fix this by changing most places to elog(), with the exception of input functions. This is a mostly cosmetic change, it makes the life a little bit easier for translators, as elog() messages are not translated. So backpatch to PostgreSQL 10, where extended statistics were introduced. Author: Tomas Vondra Backpatch-through: 10 where extended statistics were added Discussion: https://postgr.es/m/20190503154404.GA7478@alvherre.pgsql
* In the pg_upgrade test suite, don't write to src/test/regress.Noah Misch2019-05-28
| | | | | | | | | | | | | | | | | | | | When this suite runs installcheck, redirect file creations from src/test/regress to src/bin/pg_upgrade/tmp_check/regress. This closes a race condition in "make -j check-world". If the pg_upgrade suite wrote to a given src/test/regress/results file in parallel with the regular src/test/regress invocation writing it, a test failed spuriously. Even without parallelism, in "make -k check-world", the suite finishing second overwrote the other's regression.diffs. This revealed test "largeobject" assuming @abs_builddir@ is getcwd(), so fix that, too. Buildfarm client REL_10, released fifty-four days ago, supports saving regression.diffs from its new location. When an older client reports a pg_upgradeCheck failure, it will no longer include regression.diffs. Back-patch to 9.5, where pg_upgrade moved to src/bin. Reviewed (in earlier versions) by Andrew Dunstan. Discussion: https://postgr.es/m/20181224034411.GA3224776@rfd.leadboat.com
* In the pg_upgrade test suite, remove and recreate "tmp_check".Noah Misch2019-05-28
| | | | | | | | This allows "vcregress upgradecheck" to pass twice in immediate succession, and it's more like how $(prove_check) works. Back-patch to 9.5, where pg_upgrade moved to src/bin. Discussion: https://postgr.es/m/20190520012436.GA1480421@rfd.leadboat.com
* pg_upgrade: Make test.sh's installcheck use to-be-upgraded version's bindir.Andres Freund2019-05-23
| | | | | | | | | | | | | | | | On master (after 700538) the old version's installed psql was used - even when the old version might not actually be installed / might be installed into a temporary directory. As commonly the case when just executing make check for pg_upgrade, as $oldbindir is just the current version's $bindir. In the back branches, with --install specified, psql from the new version's temporary installation was used, without --install (e.g for NO_TEMP_INSTALL, cf 47b3c26642), the new version's installed psql was used (which might or might not exist). Author: Andres Freund Discussion: https://postgr.es/m/20190522175150.c26f4jkqytahajdg@alap3.anarazel.de
* Fix array size allocation for HashAggregate hash keys.Andrew Gierth2019-05-23
| | | | | | | | | | | | | | | | | | | When there were duplicate columns in the hash key list, the array sizes could be miscomputed, resulting in access off the end of the array. Adjust the computation to ensure the array is always large enough. (I considered whether the duplicates could be removed in planning, but I can't rule out the possibility that duplicate columns might have different hash functions assigned. Simpler to just make sure it works at execution time regardless.) Bug apparently introduced in fc4b3dea2 as part of narrowing down the tuples stored in the hashtable. Reported by Colm McHugh of Salesforce, though I didn't use their patch. Backpatch back to version 10 where the bug was introduced. Discussion: https://postgr.es/m/CAFeeJoKKu0u+A_A9R9316djW-YW3-+Gtgvy3ju655qRHR3jtdA@mail.gmail.com