aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
* 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
* Doc: fix typo in pgbench random_zipfian() documentation.Tom Lane2019-05-24
| | | | | | Per bug #15819 from Koizumi Satoru. Discussion: https://postgr.es/m/15819-e6191bef1f7334c0@postgresql.org
* 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
* Fix ordering of GRANT commands in pg_dumpall for tablespacesMichael Paquier2019-05-23
| | | | | | | | | | | | | | | | | This uses a method similar to 68a7c24f and now b8c6014 (applied for database creation), which guarantees that GRANT commands using the WITH GRANT OPTION are dumped in a way so as cascading dependencies are respected. Note that tablespaces do not have support for initial privileges via pg_init_privs, so the same method needs to be applied again. It would be nice to merge all the logic generating ACL queries in dumps under the same banner, but this requires extending the support of pg_init_privs to objects that cannot use it yet, so this is left as future work. Discussion: https://postgr.es/m/20190522071555.GB1278@paquier.xyz Author: Michael Paquier Reviewed-by: Nathan Bossart Backpatch-through: 9.6
* Fix ordering of GRANT commands in pg_dump for database creationMichael Paquier2019-05-22
| | | | | | | | | | | | | | | | | This uses a method similar to 68a7c24f, which guarantees that GRANT commands using the WITH GRANT OPTION are dumped in a way so as cascading dependencies are respected. As databases do not have support for initial privileges via pg_init_privs, we need to repeat again the same ACL reordering method. ACL for databases have been moved from pg_dumpall to pg_dump in v11, so this impacts pg_dump for v11 and above, and pg_dumpall for v9.6 and v10. Discussion: https://postgr.es/m/15788-4e18847520ebcc75@postgresql.org Author: Nathan Bossart Reviewed-by: Haribabu Kommi Backpatch-through: 9.6
* Minimally fix partial aggregation for aggregates that don't have one argument.Andres Freund2019-05-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For partial aggregation combine steps, AggStatePerTrans->numTransInputs was set to the transition function's number of inputs, rather than the combine function's number of inputs (always 1). That lead to partial aggregates with strict combine functions to wrongly check for NOT NULL input as required by strictness. When the aggregate wasn't exactly passed one argument, the strictness check was either omitted (in the 0 args case) or too many arguments were checked. In the latter case we'd read beyond the end of FunctionCallInfoData->args (only in master). AggStatePerTrans->numTransInputs actually has been wrong since since 9.6, where partial aggregates were added. But it turns out to not be an active problem in 9.6 and 10, because numTransInputs wasn't used at all for combine functions: Before c253b722f6 there simply was no NULL check for the input to strict trans functions, and after that the check was simply hardcoded for the right offset in fcinfo, as it's done by code specific to combine functions. In bf6c614a2f2 (11) the strictness check was generalized, with common code doing the strictness checks for both plain and combine transition functions, based on numTransInputs. For combine functions this lead to not emitting an expression step to check for strict input in the 0 arguments case, and in the > 1 arguments case, we'd check too many arguments.Due to the fact that the relevant fcinfo->isnull[2..] was always zero-initialized (more or less by accident, by being part of the AggStatePerTrans struct, which is palloc0'ed), there was no observable damage in the latter case before a9c35cf85ca1f, we just checked too many array elements. Due to the changes in a9c35cf85ca1f, > 1 argument bug became visible, because these days fcinfo is a) dynamically allocated without being zeroed b) exactly the length required for the number of specified arguments (hardcoded to 2 in this case). This commit only contains a fairly minimal fix, setting numTransInputs to a hardcoded 1 when building a pertrans for a combine function. It seems likely that we'll want to clean this up further (e.g. the arguments build_pertrans_for_aggref() aren't particularly meaningful for combine functions). But the wrap date for 12 beta1 is coming up fast, so it seems good to have a minimal fix in place. Backpatch to 11. While AggStatePerTrans->numTransInputs was set wrongly before that, the value was not used for combine functions. Reported-By: Rajkumar Raghuwanshi Diagnosed-By: Kyotaro Horiguchi, Jeevan Chalke, Andres Freund, David Rowley Author: David Rowley, Kyotaro Horiguchi, Andres Freund Discussion: https://postgr.es/m/CAKcux6=uZEyWyLw0N7HtR9OBc-sWEFeByEZC7t-KDf15FKxVew@mail.gmail.com
* Fix some grammar in documentation of spgist and pgbenchMichael Paquier2019-05-20
| | | | | | | Discussion: https://postgr.es/m/92961161-9b49-e42f-0a72-d5d47e0ed4de@postgrespro.ru Author: Liudmila Mantrova Reviewed-by: Jonathan Katz, Tom Lane, Michael Paquier Backpatch-through: 9.4
* Revert "In the pg_upgrade test suite, don't write to src/test/regress."Noah Misch2019-05-19
| | | | | | | This reverts commit bd1592e8570282b1650af6b8eede0016496daecd. It had multiple defects. Discussion: https://postgr.es/m/12717.1558304356@sss.pgh.pa.us
* In the pg_upgrade test suite, don't write to src/test/regress.Noah Misch2019-05-19
| | | | | | | | | | | | | | | | | | | | 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 forty-five 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 by Andrew Dunstan. Discussion: https://postgr.es/m/20181224034411.GA3224776@rfd.leadboat.com
* Restructure creation of run-time pruning steps.Tom Lane2019-05-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, gen_partprune_steps() always built executor pruning steps using all suitable clauses, including those containing PARAM_EXEC Params. This meant that the pruning steps were only completely safe for executor run-time (scan start) pruning. To prune at executor startup, we had to ignore the steps involving exec Params. But this doesn't really work in general, since there may be logic changes needed as well --- for example, pruning according to the last operator's btree strategy is the wrong thing if we're not applying that operator. The rules embodied in gen_partprune_steps() and its minions are sufficiently complicated that tracking their incremental effects in other logic seems quite impractical. Short of a complete redesign, the only safe fix seems to be to run gen_partprune_steps() twice, once to create executor startup pruning steps and then again for run-time pruning steps. We can save a few cycles however by noting during the first scan whether we rejected any clauses because they involved exec Params --- if not, we don't need to do the second scan. In support of this, refactor the internal APIs in partprune.c to make more use of passing information in the GeneratePruningStepsContext struct, rather than as separate arguments. This is, I hope, the last piece of our response to a bug report from Alan Jackson. Back-patch to v11 where this code came in. Discussion: https://postgr.es/m/FAD28A83-AC73-489E-A058-2681FA31D648@tvsquared.com
* Fix bogus logic for combining range-partitioned columns during pruning.Tom Lane2019-05-16
| | | | | | | | | | | | gen_prune_steps_from_opexps's notion of how to do this was overly complicated and underly correct. Per discussion of a report from Alan Jackson (though this fixes only one aspect of that problem). Back-patch to v11 where this code came in. Amit Langote Discussion: https://postgr.es/m/FAD28A83-AC73-489E-A058-2681FA31D648@tvsquared.com
* Fix partition pruning to treat stable comparison operators properly.Tom Lane2019-05-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Cross-type comparison operators in a btree or hash opclass might be only stable not immutable (this is true of timestamp vs. timestamptz for example). partprune.c ignored this possibility and would perform plan-time pruning with them anyway, possibly leading to wrong answers if the environment changed between planning and execution. To fix, teach gen_partprune_steps() to do things differently when creating plan-time pruning steps vs. run-time pruning steps. analyze_partkey_exprs() also needs an extra check, which is rather annoying but now is not the time to restructure things enough to avoid that. While at it, simplify the logic for the plan-time case a little by insisting that the comparison value be a Const and nothing else. This relies on the assumption that eval_const_expressions will have reduced any immutable expression to a Const; which is not quite 100% true, but certainly any case that comes up often enough to be interesting should have simplification logic there. Also improve a bunch of inadequate/obsolete/wrong comments. Per discussion of a report from Alan Jackson (though this fixes only one aspect of that problem). Back-patch to v11 where this code came in. David Rowley, with some further hacking by me Discussion: https://postgr.es/m/FAD28A83-AC73-489E-A058-2681FA31D648@tvsquared.com
* Add isolation test for INSERT ON CONFLICT speculative insertion failure.Andres Freund2019-05-14
| | | | | | | | | | | | | | | | This path previously was not reliably covered. There was some heuristic coverage via insert-conflict-toast.spec, but that test is not deterministic, and only tested for a somewhat specific bug. Backpatch, as this is a complicated and otherwise untested code path. Unfortunately 9.5 cannot handle two waiting sessions, and thus cannot execute this test. Triggered by a conversion with Melanie Plageman. Author: Andres Freund Discussion: https://postgr.es/m/CAAKRu_a7hbyrk=wveHYhr4LbcRnRCG=yPUVoQYB9YO1CdUBE9Q@mail.gmail.com Backpatch: 9.5-
* Fix comment on when HOT update is possible.Heikki Linnakangas2019-05-14
| | | | | | | | The conditions listed in this comment have changed several times, and at some point the thing that the "if so" referred to was negated. The text was OK up to 9.6. It was differently wrong in v10, v11 and master, so fix in all those versions.
* Doc: Refer to line pointers as item identifiers.Peter Geoghegan2019-05-13
| | | | | | | | | | | An upcoming HEAD-only patch will standardize the terminology around ItemIdData variables/line pointers, ending the practice of referring to them as "item pointers". Make the "Database Page Layout" docs consistent with the new policy. The term "item identifier" is already used in the same section, so stick with that. Discussion: https://postgr.es/m/CAH2-Wz=c=MZQjUzde3o9+2PLAPuHTpVZPPdYxN=E4ndQ2--8ew@mail.gmail.com Backpatch: All supported branches.
* Fix logical replication's ideas about which type OIDs are built-in.Tom Lane2019-05-13
| | | | | | | | | | | | | | | | | | Only hand-assigned type OIDs should be presumed to match across different PG servers; those assigned during genbki.pl or during initdb are likely to change due to addition or removal of unrelated objects. This means that the cutoff should be FirstGenbkiObjectId (in HEAD) or FirstBootstrapObjectId (before that), not FirstNormalObjectId. Compare postgres_fdw's is_builtin() test. It's likely that this error has no observable consequence in a normally-functioning system, since ATM the only affected type OIDs are system catalog rowtypes and information_schema types, which would not typically be interesting for logical replication. But you could probably break it if you tried hard, so back-patch. Discussion: https://postgr.es/m/15150.1557257111@sss.pgh.pa.us
* Don't leave behind junk nbtree pages during split.Peter Geoghegan2019-05-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 8fa30f906be reduced the elevel of a number of "can't happen" _bt_split() errors from PANIC to ERROR. At the same time, the new right page buffer for the split could continue to be acquired well before the critical section. This was possible because it was relatively straightforward to make sure that _bt_split() could not throw an error, with a few specific exceptions. The exceptional cases were safe because they involved specific, well understood errors, making it possible to consistently zero the right page before actually raising an error using elog(). There was no danger of leaving around a junk page, provided _bt_split() stuck to this coding rule. Commit 8224de4f, which introduced INCLUDE indexes, added code to make _bt_split() truncate away non-key attributes. This happened at a point that broke the rule around zeroing the right page in _bt_split(). If truncation failed (perhaps due to palloc() failure), that would result in an errant right page buffer with junk contents. This could confuse VACUUM when it attempted to delete the page, and should be avoided on general principle. To fix, reorganize _bt_split() so that truncation occurs before the new right page buffer is even acquired. A junk page/buffer will not be left behind if _bt_nonkey_truncate()/_bt_truncate() raise an error. Discussion: https://postgr.es/m/CAH2-WzkcWT_-NH7EeL=Az4efg0KCV+wArygW8zKB=+HoP=VWMw@mail.gmail.com Backpatch: 11-, where INCLUDE indexes were introduced.
* Fix misuse of an integer as a bool.Tom Lane2019-05-13
| | | | | | | | | | | | | | | | | | | pgtls_read_pending is declared to return bool, but what the underlying SSL_pending function returns is a count of available bytes. This is actually somewhat harmless if we're using C99 bools, but in the back branches it's a live bug: if the available-bytes count happened to be a multiple of 256, it would get converted to a zero char value. On machines where char is signed, counts of 128 and up could misbehave as well. The net effect is that when using SSL, libpq might block waiting for data even though some has already been received. Broken by careless refactoring in commit 4e86f1b16, so back-patch to 9.5 where that came in. Per bug #15802 from David Binderman. Discussion: https://postgr.es/m/15802-f0911a97f0346526@postgresql.org
* postgres_fdw: Fix typo in comment.Etsuro Fujita2019-05-13
|
* Fix misoptimization of "{1,1}" quantifiers in regular expressions.Tom Lane2019-05-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Fail pgwin32_message_to_UTF16() for SQL_ASCII messages.Noah Misch2019-05-12
| | | | | | | | | | | | 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
* Rearrange pgstat_bestart() to avoid failures within its critical section.Tom Lane2019-05-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We long ago decided to design the shared PgBackendStatus data structure to minimize the cost of writing status updates, which means that writers just have to increment the st_changecount field twice. That isn't hooked into any sort of resource management mechanism, which means that if something were to throw error between the two increments, the st_changecount field would be left odd indefinitely. That would cause readers to lock up. Now, since it's also a bad idea to leave the field odd for longer than absolutely necessary (because readers will spin while we have it set), the expectation was that we'd treat these segments like spinlock critical sections, with only short, more or less straight-line, code in them. That was fine as originally designed, but commit 9029f4b37 broke it by inserting a significant amount of non-straight-line code into pgstat_bestart(), code that is very capable of throwing errors, not to mention taking a significant amount of time during which readers will spin. We have a report from Neeraj Kumar of readers actually locking up, which I suspect was due to an encoding conversion error in X509_NAME_to_cstring, though conceivably it was just a garden-variety OOM failure. Subsequent commits have loaded even more dubious code into pgstat_bestart's critical section (and commit fc70a4b0d deserves some kind of booby prize for managing to miss the critical section entirely, although the negative consequences seem minimal given that the PgBackendStatus entry should be seen by readers as inactive at that point). The right way to fix this mess seems to be to compute all these values into a local copy of the process' PgBackendStatus struct, and then just copy the data back within the critical section proper. This plan can't be implemented completely cleanly because of the struct's heavy reliance on out-of-line strings, which we must initialize separately within the critical section. But still, the critical section is far smaller and safer than it was before. In hopes of forestalling future errors of the same ilk, rename the macros for st_changecount management to make it more apparent that the writer-side macros create a critical section. And to prevent the worst consequences if we nonetheless manage to mess it up anyway, adjust those macros so that they really are a critical section, ie they now bump CritSectionCount. That doesn't add much overhead, and it guarantees that if we do somehow throw an error while the counter is odd, it will lead to PANIC and a database restart to reset shared memory. Back-patch to 9.5 where the problem was introduced. In HEAD, also fix an oversight in commit b0b39f72b: it failed to teach pgstat_read_current_status to copy st_gssstatus data from shared memory to local memory. Hence, subsequent use of that data within the transaction would potentially see changing data that it shouldn't see. Discussion: https://postgr.es/m/CAPR3Wj5Z17=+eeyrn_ZDG3NQGYgMEOY6JV6Y-WRRhGgwc16U3Q@mail.gmail.com
* Honor TEMP_CONFIG in TAP suites.Noah Misch2019-05-11
| | | | | | | | | | | | The buildfarm client uses TEMP_CONFIG to implement its extra_config setting. Except for stats_temp_directory, extra_config now applies to TAP suites; extra_config values seen in the past month are compatible with this. Back-patch to 9.6, where PostgresNode was introduced, so the buildfarm can rely on it sooner. Reviewed by Andrew Dunstan and Tom Lane. Discussion: https://postgr.es/m/20181229021950.GA3302966@rfd.leadboat.com
* Fix error reporting in reindexdbMichael Paquier2019-05-11
| | | | | | | | | | | | | When failing to reindex a table or an index, 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
* Cope with EINVAL and EIDRM shmat() failures in PGSharedMemoryAttach.Tom Lane2019-05-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Repair issues with faulty generation of merge-append plans.Tom Lane2019-05-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | create_merge_append_plan failed to honor the CP_EXACT_TLIST flag: it would generate the expected targetlist but then it felt free to add resjunk sort targets to it. This demonstrably leads to assertion failures in v11 and HEAD, and it's probably just accidental that we don't see the same in older branches. I've not looked into whether there would be any real-world consequences in non-assert builds. In HEAD, create_append_plan has sprouted the same problem, so fix that too (although we do not have any test cases that seem able to reach that bug). This is an oversight in commit 3fc6e2d7f which invented the CP_EXACT_TLIST flag, so back-patch to 9.6 where that came in. convert_subquery_pathkeys would create pathkeys for subquery output values if they match any EquivalenceClass known in the outer query and are available in the subquery's syntactic targetlist. However, the second part of that condition is wrong, because such values might not appear in the subquery relation's reltarget list, which would mean that they couldn't be accessed above the level of the subquery scan. We must check that they appear in the reltarget list, instead. This can lead to dropping knowledge about the subquery's sort ordering, but I believe it's okay, because any sort key that the outer query actually has any interest in would appear in the reltarget list. This second issue is of very long standing, but right now there's no evidence that it causes observable problems before 9.6, so I refrained from back-patching further than that. We can revisit that choice if somebody finds a way to make it cause problems in older branches. (Developing useful test cases for these issues is really problematic; fixing convert_subquery_pathkeys removes the only known way to exhibit the create_merge_append_plan bug, and neither of the test cases added by this patch causes a problem in all branches, even when considering the issues separately.) The second issue explains bug #15795 from Suresh Kumar R ("could not find pathkey item to sort" with nested DISTINCT queries). I stumbled across the first issue while investigating that. Discussion: https://postgr.es/m/15795-fadb56c8e44ee73c@postgresql.org
* Fix error status of vacuumdb when multiple jobs are usedMichael Paquier2019-05-09
| | | | | | | | | | | When running a batch of VACUUM or ANALYZE commands on a given database, there were cases where it is possible to have vacuumdb not report an error where it actually should, leading to incorrect status results. Author: Julien Rouhaud Reviewed-by: Amit Kapila, Michael Paquier Discussion: https://postgr.es/m/CAOBaU_ZuTwz7CtqLYJ1Ouuh272bTQPLN8b1bAPk0bCBm4PDMTQ@mail.gmail.com Backpatch-through: 9.5
* Fix documentation for the privileges required for replication functions.Fujii Masao2019-05-09
| | | | | | | | | | | | | | | | Previously it's documented that use of replication functions is restricted to superusers. This is true for the functions which use replication origin, but not for pg_logicl_emit_message() and functions which use replication slot. For example, not only superusers but also users with REPLICATION privilege is allowed to use the functions for replication slot. This commit fixes the documentation for the privileges required for those replication functions. Back-patch to 9.4 (all supported versions). Author: Matsumura Ryo Discussion: https://postgr.es/m/03040DFF97E6E54E88D3BFEE5F5480F74ABA6E16@G01JPEXMBYT04
* Probe only 127.0.0.1 when looking for ports on Unix.Thomas Munro2019-05-08
| | | | | | | | | | | | Commit c0985099, later adjusted by commit 4ab02e81, probed 0.0.0.0 in addition to 127.0.0.1, for the benefit of Windows build farm animals. It isn't really useful on Unix systems, and turned out to be a bit inconvenient to users of some corporate firewall software. Switch back to probing just 127.0.0.1 on non-Windows systems. Back-patch to 9.6, like the earlier changes. Discussion: https://postgr.es/m/CA%2BhUKG%2B21EPwfgs4m%2BtqyRtbVqkOUvP8QQ8sWk9%2Bh55Aub1H3A%40mail.gmail.com
* Remove leftover reference to old "flat file" mechanism in a comment.Heikki Linnakangas2019-05-08
| | | | The flat file mechanism was removed in PostgreSQL 9.0.
* Remove some code related to 7.3 and older servers from tools of src/bin/Michael Paquier2019-05-07
| | | | | | | | | | 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
* Stamp 11.3.REL_11_3Tom Lane2019-05-06
|
* Last-minute updates for release notes.Tom Lane2019-05-06
| | | | Security: CVE-2019-10129, CVE-2019-10130
* Revert "Make pg_dump emit ATTACH PARTITION instead of PARTITION OF"Alvaro Herrera2019-05-06
| | | | | | | | | ... and fallout (from branches 10, 11 and master). The change was ill-considered, and it broke a few normal use cases; since we don't have time to fix it, we'll try again after this week's minor releases. Reported-by: Rushabh Lathia Discussion: https://postgr.es/m/CAGPqQf0iQV=PPOv2Btog9J9AwOQp6HmuVd6SbGTR_v3Zp2XT1w@mail.gmail.com
* Translation updatesPeter Eisentraut2019-05-06
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 96d81aab04631d76c9ca90a3b12885100c061775
* Fix tuple printing in error message of tuple routing for partitionsMichael Paquier2019-05-06
| | | | | | | | | | | | With correctly crafted DDLs, this could lead to disclosure of arbitrary backend memory a user may have no right to access. This impacts only REL_11_STABLE, as the issue has been introduced by 34295b8. On HEAD, add regression tests to cover this issue in the future. Author: Michael Paquier Reviewed-by: Noah Misch Security: CVE-2019-10129
* Use checkAsUser for selectivity estimator checks, if it's set.Dean Rasheed2019-05-06
| | | | | | | | | | | | | | | | | | | | | | | | | 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.
* Fix security checks for selectivity estimation functions with RLS.Dean Rasheed2019-05-06
| | | | | | | | | | | | | | | | | | | | | | In commit e2d4ef8de8, security checks were added to prevent user-supplied operators from running over data from pg_statistic unless the user has table or column privileges on the table, or the operator is leakproof. For a table with RLS, however, checking for table or column privileges is insufficient, since that does not guarantee that the user has permission to view all of the column's data. Fix this by also checking for securityQuals on the RTE, and insisting that the operator be leakproof if there are any. Thus the leakproofness check will only be skipped if there are no securityQuals and the user has table or column privileges on the table -- i.e., only if we know that the user has access to all the data in the column. Back-patch to 9.5 where RLS was added. Dean Rasheed, reviewed by Jonathan Katz and Stephen Frost. Security: CVE-2019-10130
* Remove reindex_catalog test from test schedules.Andres Freund2019-05-05
| | | | | | | | | | | | | | | | | | | 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!)
* Release notes for 11.3, 10.8, 9.6.13, 9.5.17, 9.4.22.Tom Lane2019-05-05
|
* Fix style violations in syscache lookups.Tom Lane2019-05-05
| | | | | | | | | | | | | | | | | Project style is to check the success of SearchSysCacheN and friends by applying HeapTupleIsValid to the result. A tiny minority of calls creatively did it differently. Bring them into line with the rest. This is just cosmetic, since HeapTupleIsValid is indeed just a null check at the moment ... but that may not be true forever, and in any case it puts a mental burden on readers who may wonder why these call sites are not like the rest. Back-patch to v11 just to keep the branches in sync. (The bulk of these errors seem to have originated in v11 or v12, though a few are old.) Per searching to see if anyplace else had made the same error repaired in 62148c352.
* Add check for syscache lookup failure in update_relispartition().Tom Lane2019-05-05
| | | | | | | | Omitted in commit 05b38c7e6 (though it looks like the original blame belongs to 9e9befac4). A failure is admittedly unlikely, but if it did happen, SIGSEGV is not the approved method of reporting it. Per Coverity. Back-patch to v11 where the broken code originated.
* pg_verify_checksums: Fix message punctuationPeter Eisentraut2019-05-04
|
* pg_dump: Fix newline in error messagePeter Eisentraut2019-05-04
| | | | | The newline was incorrectly dropped in a98c48debcd0620ab07608d53ee08fdb0e7a1edb.
* First-draft release notes for 11.3.Tom Lane2019-05-03
| | | | | As usual, the release notes for other branches will be made by cutting these down, but put them up for community review first.
* Fix reindexing of pg_class indexes some more.Tom Lane2019-05-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Run catalog reindexing test from 3dbb317d32 serially, to avoid deadlocks.Andres Freund2019-04-30
| | | | | | | | | | | | | | | | | | | | | | | | | 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
* Fix unused variable compiler warning in !debug builds.Andres Freund2019-04-30
| | | | | | | | Introduced in 3dbb317d3. Fix by using the new local variable in more places. Reported-By: Bruce Momjian (off-list) Backpatch: 9.4-, like 3dbb317d3
* Clean up handling of constraint_exclusion and enable_partition_pruning.Tom Lane2019-04-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The interaction of these parameters was a bit confused/confusing, and in fact v11 entirely misses the opportunity to apply partition constraints when a partition is accessed directly (rather than indirectly from its parent). In HEAD, establish the principle that enable_partition_pruning controls partition pruning and nothing else. When accessing a partition via its parent, we do partition pruning (if enabled by enable_partition_pruning) and then there is no need to consider partition constraints in the constraint_exclusion logic. When accessing a partition directly, its partition constraints are applied by the constraint_exclusion logic, only if constraint_exclusion = on. In v11, we can't have such a clean division of these GUCs' effects, partly because we don't want to break compatibility too much in a released branch, and partly because the clean coding requires inheritance_planner to have applied partition pruning to a partitioned target table, which it doesn't in v11. However, we can tweak things enough to cover the missed case, which seems like a good idea since it's potentially a performance regression from v10. This patch keeps v11's previous behavior in which enable_partition_pruning overrides constraint_exclusion for an inherited target table, though. In HEAD, also teach relation_excluded_by_constraints that it's okay to use inheritable constraints when trying to prune a traditional inheritance tree. This might not be thought worthy of effort given that that feature is semi-deprecated now, but we have enough infrastructure that it only takes a couple more lines of code to do it correctly. Amit Langote and Tom Lane Discussion: https://postgr.es/m/9813f079-f16b-61c8-9ab7-4363cab28d80@lab.ntt.co.jp Discussion: https://postgr.es/m/29069.1555970894@sss.pgh.pa.us