4859 Commits (32fe7ee2dd2b2aa8620e69451f60b2b35989677c)

Author	SHA1	Message	Date
Michael Paquier	508300e2e1	Improve and fix some error handling for REINDEX INDEX/TABLE CONCURRENTLY ... This improves the user experience when it comes to restrict several flavors of REINDEX CONCURRENTLY. First, for INDEX, remove a restriction on shared relations as we already check after catalog relations. Then, for TABLE, add a proper error message when attempting to run the command on system catalogs. The code path of CREATE INDEX CONCURRENTLY already complains about that, but if a REINDEX is issued then then the error generated is confusing. While on it, add more tests to check restrictions on catalog indexes and on toast table/index for catalogs. Some error messages are improved, with wording suggestion coming from Tom Lane. Reported-by: Tom Lane Author: Michael Paquier Reviewed-by: Tom Lane Discussion: https://postgr.es/m/23694.1556806002@sss.pgh.pa.us	7 years ago
Tom Lane	24c19e9f66	Repair issues with faulty generation of merge-append plans. ... 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	7 years ago
Peter Eisentraut	02daece4ab	Fix grammar in error message	7 years ago
Alvaro Herrera	61639816b8	Fix error messages ... Some messages related to foreign servers were reporting the server name without quotes, or not at all; our style is to have all names be quoted, and the server name already appears quoted in a few other messages, so just add quotes and make them all consistent. Remove an extra "s" in other messages (typos introduced by myself in f56f8f8da6).	7 years ago
Thomas Munro	8efe710d9c	Probe only 127.0.0.1 when looking for ports on Unix. ... 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	7 years ago
Alexander Korotkov	e5f9786317	Add jsonpath_encoding_1.out changes missed in 29ceacc3f9 ... Reported-by: Tom Lane Discussion: https://postgr.es/m/14305.1557268259%40sss.pgh.pa.us	7 years ago
Alexander Korotkov	29ceacc3f9	Improve error reporting in jsonpath ... This commit contains multiple improvements to error reporting in jsonpath including but not limited to getting rid of following things: * definition of error messages in macros, * errdetail() when valueable information could fit to errmsg(), * word "singleton" which is not properly explained anywhere, * line breaks in error messages. Reported-by: Tom Lane Discussion: https://postgr.es/m/14890.1555523005%40sss.pgh.pa.us Author: Alexander Korotkov Reviewed-by: Tom Lane	7 years ago
Fujii Masao	b84dbc8eb8	Add TRUNCATE parameter to VACUUM. ... This commit adds new parameter to VACUUM command, TRUNCATE, which specifies that VACUUM should attempt to truncate off any empty pages at the end of the table and allow the disk space for the truncated pages to be returned to the operating system. This parameter, if specified, overrides the vacuum_truncate reloption. If neither the reloption nor the VACUUM option is used, the default is true, as before. Author: Fujii Masao Reviewed-by: Julien Rouhaud, Masahiko Sawada Discussion: https://postgr.es/m/CAD21AoD+qtrSDL=GSma4Wd3kLYLeRC0hPna-YAdkDeV4z156vg@mail.gmail.com	7 years ago
Amit Kapila	7db0cde6b5	Revert "Avoid the creation of the free space map for small heap relations". ... This feature was using a process local map to track the first few blocks in the relation. The map was reset each time we get the block with enough freespace. It was discussed that it would be better to track this map on a per-relation basis in relcache and then invalidate the same whenever vacuum frees up some space in the page or when FSM is created. The new design would be better both in terms of API design and performance. List of commits reverted, in reverse chronological order: 06c8a5090e Improve code comments in b0eaa4c51b. 13e8643bfc During pg_upgrade, conditionally skip transfer of FSMs. 6f918159a9 Add more tests for FSM. 9c32e4c350 Clear the local map when not used. 29d108cdec Update the documentation for FSM behavior.. 08ecdfe7e5 Make FSM test portable. b0eaa4c51b Avoid creation of the free space map for small heap relations. Discussion: https://postgr.es/m/20190416180452.3pm6uegx54iitbt5@alap3.anarazel.de	7 years ago
Michael Paquier	91248608a6	Add tests for error message generation in partition tuple routing ... This adds extra tests for the error message generated for partition tuple routing in the executor, using more than three levels of partitioning including partitioned tables with no partitions. These tests have been added to fix CVE-2019-10129 on REL_11_STABLE. HEAD has no active bugs in this area, but it lacked coverage. Author: Michael Paquier Reviewed-by: Noah Misch Security: CVE-2019-10129	7 years ago
Dean Rasheed	a0905056fd	Use checkAsUser for selectivity estimator checks, if it's set. ... 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.	7 years ago
Dean Rasheed	1aebfbea83	Fix security checks for selectivity estimation functions with RLS. ... 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	7 years ago
Andres Freund	809c9b48f4	Run catalog reindexing test from 3dbb317d32 serially, to avoid deadlocks. ... 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	7 years ago
Tom Lane	e03ff73969	Clean up handling of constraint_exclusion and enable_partition_pruning. ... 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	7 years ago
Alvaro Herrera	9f8b717a80	Message style fixes	7 years ago
Andres Freund	3dbb317d32	Fix potential assertion failure when reindexing a pg_class index. ... 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 5c1560606d 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	7 years ago
Tom Lane	e481d26285	Clean up minor warnings from buildfarm. ... Be more consistent about use of XXXGetDatum macros in new jsonpath code. This is mostly to avoid having code that looks randomly different from everyplace else that's doing the exact same thing. In pg_regress.c, avoid an unreferenced-function warning from compilers that don't understand pg_attribute_unused(). Putting the function inside the same #ifdef as its only caller is more straightforward coding anyway. In be-secure-openssl.c, avoid use of pg_attribute_unused() on a label. That's pretty creative, but there's no good reason to suppose that it's portable, and there's absolutely no need to use goto's here in the first place. (This wasn't actually causing any buildfarm complaints, but it's new code in v12 so it has no portability track record.)	7 years ago
Tom Lane	c01eb619a8	Apply stopgap fix for bug #15672. ... Fix DefineIndex so that it doesn't attempt to pass down a to-be-reused index relfilenode to a child index creation, and fix TryReuseIndex to not think that reuse is sensible for a partitioned index. In v11, this fixes a problem where ALTER TABLE on a partitioned table could assign the same relfilenode to several different child indexes, causing very nasty catalog corruption --- in fact, attempting to DROP the partitioned table then leads not only to a database crash, but to inability to restart because the same crash will recur during WAL replay. Either of these two changes would be enough to prevent the failure, but since neither action could possibly be sane, let's put in both changes for future-proofing. In HEAD, no such bug manifests, but that's just an accidental consequence of having changed the pg_class representation of partitioned indexes to have relfilenode = 0. Both of these changes still seem like smart future-proofing. This is only a stop-gap because the code for ALTER TABLE on a partitioned table with a no-op type change still leaves a great deal to be desired. As the added regression tests show, it gets things wrong for comments on child indexes/constraints, and it is regenerating child indexes it doesn't have to. However, fixing those problems will take more work which may not get back-patched into v11. We need a fix for the corruption problem now. Per bug #15672 from Jianing Yang. Patch by me, regression test cases based on work by Amit Langote, who also did a lot of the investigative work. Discussion: https://postgr.es/m/15672-b9fa7db32698269f@postgresql.org	7 years ago
Alvaro Herrera	05b38c7e63	Fix partitioned index attachment ... When an existing index in a partition is attached to a new index on its parent, we forgot to set the "relispartition" flag correctly, which meant that it was not possible to find the index in various operations, such as adding a foreign key constraint that references that partitioned table. One of four places that was assigning the parent index was forgetting to do that, so fix by shifting responsibility of updating the flag to the routine that changes the parent. Author: Amit Langote, Álvaro Herrera Reported-by: Hubert "depesz" Lubaczewski Discussion: https://postgr.es/m/CA+HiwqHMsRtRYRWYTWavKJ8x14AFsv7bmAV46mYwnfD3vy8goQ@mail.gmail.com	7 years ago
Alvaro Herrera	87259588d0	Fix tablespace inheritance for partitioned rels ... Commit ca4103025d left a few loose ends. The most important one (broken pg_dump output) is already fixed by virtue of commit 3b23552ad8, but some things remained: * When ALTER TABLE rewrites tables, the indexes must remain in the tablespace they were originally in. This didn't work because index recreation during ALTER TABLE runs manufactured SQL (yuck), which runs afoul of default_tablespace in competition with the parent relation tablespace. To fix, reset default_tablespace to the empty string temporarily, and add the TABLESPACE clause as appropriate. * Setting a partitioned rel's tablespace to the database default is confusing; if it worked, it would direct the partitions to that tablespace regardless of default_tablespace. But in reality it does not work, and making it work is a larger project. Therefore, throw an error when this condition is detected, to alert the unwary. Add some docs and tests, too. Author: Álvaro Herrera Discussion: https://postgr.es/m/CAKJS1f_1c260nOt_vBJ067AZ3JXptXVRohDVMLEBmudX1YEx-A@mail.gmail.com	7 years ago
Tom Lane	7ad1cd31bf	Repair assorted issues in locale data extraction. ... 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	7 years ago
Tom Lane	f4a3fdfbdc	Avoid order-of-execution problems with ALTER TABLE ADD PRIMARY KEY. ... Up to now, DefineIndex() was responsible for adding attnotnull constraints to the columns of a primary key, in any case where it hadn't been convenient for transformIndexConstraint() to mark those columns as is_not_null. It (or rather its minion index_check_primary_key) did this by executing an ALTER TABLE SET NOT NULL command for the target table. The trouble with this solution is that if we're creating the index due to ALTER TABLE ADD PRIMARY KEY, and the outer ALTER TABLE has additional sub-commands, the inner ALTER TABLE's operations executed at the wrong time with respect to the outer ALTER TABLE's operations. In particular, the inner ALTER would perform a validation scan at a point where the table's storage might be inconsistent with its catalog entries. (This is on the hairy edge of being a security problem, but AFAICS it isn't one because the inner scan would only be interested in the tuples' null bitmaps.) This can result in unexpected failures, such as the one seen in bug #15580 from Allison Kaptur. To fix, let's remove the attempt to do SET NOT NULL from DefineIndex(), reducing index_check_primary_key's role to verifying that the columns are already not null. (It shouldn't ever see such a case, but it seems wise to keep the check for safety.) Instead, make transformIndexConstraint() generate ALTER TABLE SET NOT NULL subcommands to be executed ahead of the ADD PRIMARY KEY operation in every case where it can't force the column to be created already-not-null. This requires only minor surgery in parse_utilcmd.c, and it makes for a much more satisfying spec for transformIndexConstraint(): it's no longer having to take it on faith that someone else will handle addition of NOT NULL constraints. To make that work, we have to move the execution of AT_SetNotNull into an ALTER pass that executes ahead of AT_PASS_ADD_INDEX. I moved it to AT_PASS_COL_ATTRS, and put that after AT_PASS_ADD_COL to avoid failure when the column is being added in the same command. This incidentally fixes a bug in the only previous usage of AT_PASS_COL_ATTRS, for AT_SetIdentity: it didn't work either for a newly-added column. Playing around with this exposed a separate bug in ALTER TABLE ONLY ... ADD PRIMARY KEY for partitioned tables. The intent of the ONLY modifier in that context is to prevent doing anything that would require holding lock for a long time --- but the implied SET NOT NULL would recurse to the child partitions, and do an expensive validation scan for any child where the column(s) were not already NOT NULL. To fix that, invent a new ALTER subcommand AT_CheckNotNull that just insists that a child column be already NOT NULL, and apply that, not AT_SetNotNull, when recursing to children in this scenario. This results in a slightly laxer definition of ALTER TABLE ONLY ... SET NOT NULL for partitioned tables, too: that command will now work as long as all children are already NOT NULL, whereas before it just threw up its hands if there were any partitions. In passing, clean up the API of generateClonedIndexStmt(): remove a useless argument, ensure that the output argument is not left undefined, update the header comment. A small side effect of this change is that no-such-column errors in ALTER TABLE ADD PRIMARY KEY now produce a different message that includes the table name, because they are now detected by the SET NOT NULL step which has historically worded its error that way. That seems fine to me, so I didn't make any effort to avoid the wording change. The basic bug #15580 is of very long standing, and these other bugs aren't new in v12 either. However, this is a pretty significant change in the way ALTER TABLE ADD PRIMARY KEY works. On balance it seems best not to back-patch, at least not till we get some more confidence that this patch has no new bugs. Patch by me, but thanks to Jie Zhang for a preliminary version. Discussion: https://postgr.es/m/15580-d1a6de5a3d65da51@postgresql.org Discussion: https://postgr.es/m/1396E95157071C4EBBA51892C5368521017F2E6E63@G08CNEXMBPEKD02.g08.fujitsu.local	7 years ago
Tom Lane	c06e3550dc	Don't request pretty-printed output from xmlNodeDump(). ... xml.c passed format = 1 to xmlNodeDump(), resulting in sometimes getting extra whitespace (newlines + spaces) in the output. We don't really want that, first because whitespace might be semantically significant in some XML uses, and second because it happens only very inconsistently. Only one case in our regression tests is affected. This potentially affects the results of xpath() and the XMLTABLE construct, when emitting nodeset values. Note that the older code in contrib/xml2 doesn't do this; it seems to have been an aboriginal bad decision in commit ea3b212fe. While this definitely seems like a bug to me, the small number of complaints to date argues against back-patching a behavioral change. Hence, fix in HEAD only, at least for now. Per report from Jean-Marc Voillequin. Discussion: https://postgr.es/m/1EC8157EB499BF459A516ADCF135ADCE3A23A9CA@LON-WGMSX712.ad.moodys.net	7 years ago
Michael Paquier	ccae190b91	Fix detection of passwords hashed with MD5 or SCRAM-SHA-256 ... This commit fixes a couple of issues related to the way password verifiers hashed with MD5 or SCRAM-SHA-256 are detected, leading to being able to store in catalogs passwords which do not follow the supported hash formats: - 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, as reported by Tom Lane. - A SCRAM-hashed entry was checked based on only its header, which should be "SCRAM-SHA-256$", but it never checked for any fields afterwards, as reported by Jonathan Katz. Backpatch down to v10, which is where SCRAM has been introduced, and where password verifiers in plain format have been removed. Author: Jonathan Katz Reviewed-by: Tom Lane, Michael Paquier Discussion: https://postgr.es/m/016deb6b-1f0a-8e9f-1833-a8675b170aa9@postgresql.org Backpatch-through: 10	7 years ago
Stephen Frost	eb882a1b71	GSSAPI: Improve documentation and tests ... The GSSAPI encryption patch neglected to update the protocol documentation to describe how to set up a GSSAPI encrypted connection from a client to the server, so fix that by adding the appropriate documentation to protocol.sgml. The tests added for encryption support were overly long and couldn't be run in parallel due to race conditions; this was largely because each test was setting up its own KDC to perform the tests. Instead, merge the authentication tests and the encryption tests into the original test, where we only create one KDC to run the tests with. Also, have the tests check what the server's opinion is of the connection and if it was GSS authenticated or encrypted using the pg_stat_gssapi view. In passing, fix the libpq label for GSSENC-Mode to be consistent with the "PGGSSENCMODE" environment variable. Missing protocol documentation pointed out by Michael Paquier. Issues with the tests pointed out by Tom Lane and Peter Eisentraut. Refactored tests and added documentation by me. Reviewed by Robbie Harwood (protocol documentation) and Michael Paquier (rework of the tests).	7 years ago
Michael Paquier	5a9323eab6	Remove dependency to pageinspect in recovery tests ... If contrib/pageinspect is not installed, this causes the test checking the minimum recovery point to fail. The point is that the dependency with pageinspect is not really necessary as the test does also all checks with an offline cluster by scanning directly the on-disk pages, which is enough for the purpose of the test. Per complaint from Tom Lane. Author: Michael Paquier Discussion: https://postgr.es/m/17806.1555566345@sss.pgh.pa.us	7 years ago
Peter Eisentraut	bb385c4fb0	Fix handling of temp and unlogged tables in FOR ALL TABLES publications ... If a FOR ALL TABLES publication exists, temporary and unlogged tables are ignored for publishing changes. But CheckCmdReplicaIdentity() would still check in that case that such a table has a replica identity set before accepting updates. To fix, have GetRelationPublicationActions() return that such a table publishes no actions. Discussion: https://www.postgresql.org/message-id/f3f151f7-c4dd-1646-b998-f60bd6217dd3@2ndquadrant.com	7 years ago
Alvaro Herrera	421a2c4832	Tie loose ends in psql's new \dP command ... * Remove one unnecessary pg_class join in SQL command. Not needed, because we use a regclass cast instead. * Doc: refer to "partitioned relations" rather than specifically tables, since indexes are also displayed. * Rename "On table" column to "Table", for consistency with \di. Author: Justin Pryzby Discussion: https://postgr.es/m/20190407212525.GB10080@telsasoft.com	7 years ago
Alvaro Herrera	b036982db7	psql: display tablespace for partitioned indexes ... Nothing was shown previously.	7 years ago
Magnus Hagander	252b707bc4	Return NULL for checksum failures if checksums are not enabled ... Returning 0 could falsely indicate that there is no problem. NULL correctly indicates that there is no information about potential problems. Also return 0 as numbackends instead of NULL for shared objects (as no connection can be made to a shared object only). Author: Julien Rouhaud <rjuju123@gmail.com> Reviewed-by: Robert Treat <rob@xzilla.net>	7 years ago
Michael Paquier	a6dcf9df4d	Rework handling of invalid indexes with REINDEX CONCURRENTLY ... Per discussion with others, allowing REINDEX INDEX CONCURRENTLY to work for invalid indexes when working directly on them can have a lot of value to unlock situations with invalid indexes without having to use a dance involving DROP INDEX followed by an extra CREATE INDEX CONCURRENTLY (which would not work for indexes with constraint dependency anyway). This also does not create extra bloat on the relation involved as this works on individual indexes, so let's enable it. Note that REINDEX TABLE CONCURRENTLY still bypasses invalid indexes as we don't want to bloat the number of indexes defined on a relation in the event of multiple and successive failures of REINDEX CONCURRENTLY. More regression tests are added to cover those behaviors, using an invalid index created with CREATE INDEX CONCURRENTLY. Reported-by: Dagfinn Ilmari Mannsåker, Álvaro Herrera Author: Michael Paquier Reviewed-by: Peter Eisentraut, Dagfinn Ilmari Mannsåker Discussion: https://postgr.es/m/20190411134947.GA22043@alvherre.pgsql	7 years ago
Noah Misch	e12a472612	Don't write to stdin of a test process that could have already exited. ... Instead, close that stdin. Per buildfarm member conchuela. Back-patch to 9.6, where the test was introduced. Discussion: https://postgr.es/m/26478.1555373328@sss.pgh.pa.us	7 years ago
Tomas Vondra	dbb984128e	Convert pre-existing stats_ext tests to new style ... The regression tests added in commit 7300a69950 test cardinality estimates using a function that extracts the interesting pieces from the EXPLAIN output, instead of testing the whole plan. That seems both easier to understand and less fragile, so this applies the same approach to pre-existing tests of ndistinct coefficients and functional dependencies. Discussion: https://postgr.es/m/dfdac334-9cf2-2597-fb27-f0fb3753f435@2ndquadrant.com	7 years ago
Tomas Vondra	3824ca30d1	Fix pg_mcv_list deserialization ... The memcpy() was copying type OIDs in the wrong direction, so the deserialized MCV list always had them as 0. This is mostly harmless except when printing the data in pg_mcv_list_items(), in which case it reported ERROR: cache lookup failed for type 0 Also added a simple regression test for pg_mcv_list_items() function, printing a single-item MCV list. Reported-By: Dean Rasheed Discussion: https://postgr.es/m/CAEZATCX6T0iDTTZrqyec4Cd6b4yuL7euu4=rQRXaVBAVrUi1Cg@mail.gmail.com	7 years ago
Tom Lane	4b40e44f07	Fix failure with textual partition hash keys. ... Commit 5e1963fb7 overlooked two places in partbounds.c that now need to pass a collation identifier to the hash functions for a partition key column. Amit Langote, per report from Jesper Pedersen Discussion: https://postgr.es/m/a620f85a-42ab-e0f3-3337-b04b97e2e2f5@redhat.com	7 years ago
Tom Lane	47169c2550	Avoid possible regression test instability in timestamp.sql. ... Concurrent autovacuum could result in a change in the order of the live rows in timestamp_tbl. While this would not happen with the default autovacuum parameters, it's fairly easy to hit if autovacuum_vacuum_threshold is made small enough to allow autovac to decide to process this table. That's a stumbling block for trying to exercise autovacuum aggressively using the core regression tests. To fix, replace an unqualified DELETE with a TRUNCATE. There's a similar DELETE just above (and no order-sensitive queries between), so this doesn't lose any test coverage and might indeed be argued to improve it. Discussion: https://postgr.es/m/17428.1555348950@sss.pgh.pa.us	7 years ago
Peter Eisentraut	abb9c63b2c	Unbreak index optimization for LIKE on bytea ... The same code is used to handle both text and bytea, but bytea is not collation-aware, so we shouldn't call get_collation_isdeterministic() in that case, since that will error out with an invalid collation. Reported-by: Jeevan Chalke <jeevan.chalke@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/CAM2%2B6%3DWaf3qJ1%3DyVTUH8_yG-SC0xcBMY%2BSFLhvKKNnWNXSUDBw%40mail.gmail.com	7 years ago
Michael Paquier	c34677fdaa	Fix SHOW ALL command for non-superusers with replication connection ... Since Postgres 10, SHOW commands can be triggered with replication connections in a WAL sender context, however it missed that a transaction context is needed for syscache lookups. This commit makes sure that the syscache lookups can happen correctly by setting a transaction context when running SHOW commands in a WAL sender. Superuser-only parameters can be displayed using SHOW commands not only to superusers, but also to members of system role pg_read_all_settings, which requires a syscache lookup to check if the connected role is a member of this system role or not, or the instance crashes. Superusers do not need to check the syscache so it worked correctly in this case. New tests are added to cover this issue. Reported-by: Alexander Kukushkin Author: Michael Paquier Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/15734-2daa8761eeed8e20@postgresql.org Backpatch-through: 10	7 years ago
Noah Misch	4ab02e8156	Test both 0.0.0.0 and 127.0.0.x addresses to find a usable port. ... Commit c098509927 changed PostgresNode::get_new_node() to probe 0.0.0.0 instead of 127.0.0.1, but the new test was less effective for Windows native Perl. This increased the failure rate of buildfarm members bowerbird and jacana. Instead, test 0.0.0.0 and concrete addresses. This restores the old level of defense, but the algorithm is still subject to its longstanding time of check to time of use race condition. Back-patch to 9.6, like the previous change. Discussion: https://postgr.es/m/GrdLgAdUK9FdyZg8VIcTDKVOkys122ZINEb3CjjoySfGj2KyPiMKTh1zqtRp0TAD7FJ27G-OBB3eplxIB5GhcQH5o8zzGZfp0MuJaXJxVxk=@yesql.se	7 years ago
Michael Paquier	d9f543e9e9	Switch TAP tests of pg_rewind to use non-superuser role, take two ... Up to now the tests of pg_rewind have been using a superuser for all its tests (which is the default of many tests actually, and something that ought to be reviewed) when involving an online source server, still it is possible to use a non-superuser role to do that as long as this role is granted permissions to execute all the source-side functions used for the rewind. This is possible since v11, and was already documented as of bfc8068. PostgresNode::init is extended so as callers of this routine can add extra options to configure the authentication of a new node, which gets used by this commit, and allows the tests to work properly on Windows where SSPI is used. This will allow to catch up easily any change in pg_rewind if the tool begins to use more backend-side functions, so as the properties introduced by v11 are kept. Per suggestion from Peter Eisentraut. Author: Michael Paquier Reviewed-by: Magnus Hagander Discussion: https://postgr.es/m/20190411041336.GM2728@paquier.xyz	7 years ago
Noah Misch	9daefff122	MSYS: Translate REGRESS_SHLIB to a Windows file name. ... Per buildfarm member jacana. Back-patch to v11; earlier branches skip the affected test under msys. Discussion: https://postgr.es/m/GrdLgAdUK9FdyZg8VIcTDKVOkys122ZINEb3CjjoySfGj2KyPiMKTh1zqtRp0TAD7FJ27G-OBB3eplxIB5GhcQH5o8zzGZfp0MuJaXJxVxk=@yesql.se	7 years ago
Noah Misch	947a35014f	When Perl "kill(9, ...)" fails, try "pg_ctl kill". ... Per buildfarm member jacana, the former fails under msys Perl 5.8.8. Back-patch to 9.6, like the code in question. Discussion: https://postgr.es/m/GrdLgAdUK9FdyZg8VIcTDKVOkys122ZINEb3CjjoySfGj2KyPiMKTh1zqtRp0TAD7FJ27G-OBB3eplxIB5GhcQH5o8zzGZfp0MuJaXJxVxk=@yesql.se	7 years ago
Noah Misch	c098509927	Consistently test for in-use shared memory. ... 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	7 years ago
Magnus Hagander	77bd49adba	Show shared object statistics in pg_stat_database ... This adds a row to the pg_stat_database view with datoid 0 and datname NULL for those objects that are not in a database. This was added particularly for checksums, but we were already tracking more satistics for these objects, just not returning it. Also add a checksum_last_failure column that holds the timestamptz of the last checksum failure that occurred in a database (or in a non-dataabase file), if any. Author: Julien Rouhaud <rjuju123@gmail.com>	7 years ago
Peter Eisentraut	ef6f30fe77	Fix REINDEX CONCURRENTLY of partitions ... In case of a partition index, when swapping the old and new index, we also need to attach the new index as a partition and detach the old one. Also, to handle partition indexes, we not only need to change dependencies referencing the index, but also dependencies of the index referencing something else. The previous code did this only specifically for a constraint, but we also need to do this for partitioned indexes. So instead write a generic function that does it for all dependencies. Author: Michael Paquier <michael@paquier.xyz> Author: Peter Eisentraut <peter.eisentraut@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/DF4PR8401MB11964EDB77C860078C343BEBEE5A0%40DF4PR8401MB1196.NAMPRD84.PROD.OUTLOOK.COM#154df1fedb735190a773481765f7b874	7 years ago
Michael Paquier	d527fda621	Fix more strcmp() calls using boolean-like comparisons for result checks ... Such calls can confuse the reader as strcmp() uses an integer as result. The places patched here have been spotted by Thomas Munro, David Rowley and myself. Author: Michael Paquier Reviewed-by: David Rowley Discussion: https://postgr.es/m/20190411021946.GG2728@paquier.xyz	7 years ago
Tom Lane	798070ec05	Re-order some regression test scripts for more parallelism. ... Move the strings, numerology, insert, insert_conflict, select and errors tests to be parts of nearby parallel groups, instead of executing by themselves. (Moving "select" required adjusting the constraints test, which uses a table named "tmp" as select also does. There don't seem to be any other conflicts.) Move psql and stats_ext to the next parallel group, where the rules test also has a long runtime. To make it safe to run stats_ext in parallel with rules, I adjusted the latter to only dump views/rules from the pg_catalog and public schemas, which was what it was doing anyway. stats_ext makes some views in a transient schema, which now will not affect rules. Reorder serial_schedule to match parallel_schedule. Discussion: https://postgr.es/m/735.1554935715@sss.pgh.pa.us	7 years ago
Tom Lane	5874c70557	Speed up sort-order-comparison tests in create_index_spgist. ... This test script verifies that KNN searches of an SP-GiST index produce the same sort order as a seqscan-and-sort. The FULL JOINs used for that are exceedingly slow, however. Investigation shows that the problem is that the initial join is on the rank() values, and we have a lot of duplicates due to the data set containing 1000 duplicate points. We're therefore going to produce 1000000 join rows that have to be thrown away again by the join filter. We can improve matters by using row_number() instead of rank(), so that the initial join keys are unique. The catch is that that makes the results sensitive to the sorting of rows with equal distances from the reference point. That doesn't matter for the actually-equal points, but as luck would have it, the data set also contains two distinct points that have identical distances to the origin. So those two rows could legitimately appear in either order, causing unwanted output from the check queries. However, it doesn't seem like it's the job of this test to check whether the <-> operator correctly computes distances; its charter is just to verify that SP-GiST emits the values in distance order. So we can dodge the indeterminacy problem by having the check only compare row numbers and distances not the actual point values. This change reduces the run time of create_index_spgist by a good three-quarters, on my machine, with ensuing beneficial effects on the runtime of create_index (thanks to interactions with CREATE INDEX CONCURRENTLY tests in the latter). I see a net improvement of more than 2X in the runtime of their parallel test group. Discussion: https://postgr.es/m/735.1554935715@sss.pgh.pa.us	7 years ago
Tom Lane	385d396b80	Split up a couple of long-running regression test scripts. ... The point of this change is to increase the potential for parallelism while running the core regression tests. Most people these days are using parallel testing modes on multi-core machines, so we might as well try a bit harder to keep multiple cores busy. Hence, a test that runs much longer than others in its parallel group is a candidate to be sub-divided. In this patch, create_index.sql and join.sql are split up. I haven't changed the content of the tests in any way, just moved them. I moved create_index.sql's SP-GiST-related tests into a new script create_index_spgist, and moved its btree multilevel page deletion test over to the existing script btree_index. (btree_index is a more natural home for that test, and it's shorter than others in its parallel group, so this doesn't hurt total runtime of that group.) There might be room for more aggressive splitting of create_index, but this is enough to improve matters considerably. Likewise, I moved join.sql's "exercises for the hash join code" into a new file join_hash. Those exercises contributed three-quarters of the script's runtime. Which might well be excessive ... but for the moment, I'm satisfied with shoving them into a different parallel group, where they can share runtime with the roughly-equally-lengthy gist test. (Note for anybody following along at home: there are interesting interactions between the runtimes of create_index and anything running in parallel with it, because the tests of CREATE INDEX CONCURRENTLY in that file will repeatedly block waiting for concurrent transactions to commit. As committed in this patch, create_index and create_index_spgist have roughly equal runtimes, but that's mostly an artifact of forced synchronization of the CONCURRENTLY tests; when run serially, create_index is much faster. A followup patch will reduce the runtime of create_index_spgist and thereby also create_index.) Discussion: https://postgr.es/m/735.1554935715@sss.pgh.pa.us	7 years ago
Tom Lane	6726d8d476	Move plpgsql error-trapping tests to a new module-specific test file. ... The test for statement timeout has a 2-second timeout, which was only moderately annoying when it was written, but nowadays it contributes a pretty significant chunk of the elapsed time needed to run the core regression tests on a fast machine. We can improve this situation by pushing the test into a plpgsql-specific test file instead of having it in a core regression test. That's a clean win when considering just the core tests. Even when considering check-world or a buildfarm test run, we should come out ahead because the core tests get run more times in those sequences. Furthermore, since the plpgsql tests aren't currently parallelized, it seems likely that the timing problems reflected in commit f1e671a0b (which increased that timeout from 1 sec to 2) will be much less severe in this context. Hence, let's try cutting the timeout back to 1 second in hopes of a further win for check-world. We can undo that if buildfarm experience proves it to be a bad idea. To give the new test file some modicum of intellectual coherency, I moved the surrounding tests related to error-trapping along with the statement timeout test proper. Those other tests don't run long enough to have any particular bearing on test-runtime considerations. The tests are the same as before, except with minor adjustments to not depend on an externally-created table. Discussion: https://postgr.es/m/735.1554935715@sss.pgh.pa.us	7 years ago