- 09 May, 2019 10 commits
-
-
Michael Paquier authored
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
-
Tom Lane authored
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 3fc6e2d7 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
-
Bruce Momjian authored
Adjustments requested by reviewers. Reported-by: Amit Kapila, Thomas Munro, Andrew Gierth, Amit Langote, Oleg Bartunov, Michael Paquier, Alvaro Herrera, Tatsuo Ishii Discussion: https://postgr.es/m/20190506233029.ozwged67i7s4qd6c@momjian.us
-
Etsuro Fujita authored
In commit d50d172e, which added support for LIMIT/OFFSET pushdown in postgres_fdw, a new struct was introduced as the extra parameter of GetForeignUpperPaths() set for UPPERREL_FINAL, but I forgot to update the documentation to mention that. Author: Etsuro Fujita Discussion: https://postgr.es/m/CAPmGK17uSXQDe31oRb-z1nYyT6vVzkstZkA3_Wbq38U92b9BmQ%40mail.gmail.com
-
Etsuro Fujita authored
In commit 7012b132, which added support for aggregate pushdown in postgres_fdw, the expense of evaluating the final scan/join target computed by make_group_input_target() was not accounted for at all in costing aggregate pushdown paths with local statistics. The right fix for this would be to have a separate upper stage to adjust the final scan/join relation (see comments for apply_scanjoin_target_to_paths()); but for now, fix by adding the tlist eval cost when costing aggregate pushdown paths with local statistics. Apply this to HEAD only to avoid destabilizing existing plan choices. Author: Etsuro Fujita Reviewed-By: Antonin Houska Discussion: https://postgr.es/m/5C66A056.60007%40lab.ntt.co.jp
-
Thomas Munro authored
Commit bb16aba5 broke the code that maintains SxactGlobalXmin. It could get stuck when a well-timed READ ONLY transaction runs. If SxactGlobalXmin stops advancing, transactions on the FinishedSerializableTransactions queue are never cleaned up, so resources are effectively leaked. Revert that hunk of the commit. Also revert another similar hunk that was probably harmless, but unnecessary and unjustified, relating to the DOOMED flag in case of RO_SAFE early release. Author: Thomas Munro Reported-by: Tom Lane Discussion: https://postgr.es/m/16170.1557251214%40sss.pgh.pa.us
-
Peter Eisentraut authored
So it picks up strings in pg_log_* calls. This was forgotten when it was added to all other relevant subdirectories.
-
Peter Eisentraut authored
-
Tom Lane authored
The right way for IsCatalogRelation/Class to behave is to return true for OIDs less than FirstBootstrapObjectId (not FirstNormalObjectId), without any of the ad-hoc fooling around with schema membership. The previous code was wrong because (1) it claimed that information_schema tables were not catalog relations but their toast tables were, which is silly; and (2) if you dropped and recreated information_schema, which is a supported operation, the behavior changed. That's even sillier. With this definition, "catalog relations" are exactly the ones traceable to the postgres.bki data, which seems like what we want. With this simplification, we don't actually need access to the pg_class tuple to identify a catalog relation; we only need its OID. Hence, replace IsCatalogClass with "IsCatalogRelationOid(oid)". But keep IsCatalogRelation as a convenience function. This allows fixing some arguably-wrong semantics in contrib/sepgsql and ReindexRelationConcurrently, which were using an IsSystemNamespace test where what they really should be using is IsCatalogRelationOid. The previous coding failed to protect toast tables of system catalogs, and also was not on board with the general principle that user-created tables do not become catalogs just by virtue of being renamed into pg_catalog. We can also get rid of a messy hack in ReindexMultipleTables. While we're at it, also rename IsSystemNamespace to IsCatalogNamespace, because the previous name invited confusion with the more expansive semantics used by IsSystemRelation/Class. Also improve the comments in catalog.c. There are a few remaining places in replication-related code that are special-casing OIDs below FirstNormalObjectId. I'm inclined to think those are wrong too, and if there should be any special case it should just extend to FirstBootstrapObjectId. But first we need to debate whether a FOR ALL TABLES publication should include information_schema. Discussion: https://postgr.es/m/21697.1557092753@sss.pgh.pa.us Discussion: https://postgr.es/m/15150.1557257111@sss.pgh.pa.us
-
Michael Paquier authored
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
-
- 08 May, 2019 9 commits
-
-
Peter Geoghegan authored
Commit dd299df8, which added suffix truncation to nbtree, simplified the WAL record format used by page splits. It became necessary to explicitly WAL-log the new high key for the left half of a split in all cases, which relieved the REDO routine from having to reconstruct a new high key for the left page by copying the first item from the right page. Remove a comment that referred to the previous practice.
-
Alvaro Herrera authored
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 f56f8f8d).
-
Fujii Masao authored
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
-
Peter Eisentraut authored
REINDEX CONCURRENTLY locks tables with ShareUpdateExclusiveLock rather than the ShareLock used by a plain REINDEX. However, RangeVarCallbackForReindexIndex() was not updated for that and still used the ShareLock only. This would lead to lock upgrades later, leading to possible deadlocks. Reported-by: Andres Freund <andres@anarazel.de> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/20190430151735.wi52sxjvxsjvaxxt%40alap3.anarazel.de
-
Thomas Munro authored
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
-
Thomas Munro authored
Reported-by: Vik Fearing
-
Etsuro Fujita authored
-
Heikki Linnakangas authored
The flat file mechanism was removed in PostgreSQL 9.0.
-
Peter Geoghegan authored
It is no longer possible under any circumstances for nbtree code to reconstruct a strict lower bound key (parent page's pivot tuple key) for a right sibling page by retrieving the first item in the right sibling page.
-
- 07 May, 2019 9 commits
-
-
Alexander Korotkov authored
Reported-by: Tom Lane Discussion: https://postgr.es/m/14305.1557268259%40sss.pgh.pa.us
-
Alexander Korotkov authored
Word "singleton" is hard for user understanding, especially taking into account there is only one place it's used in the docs and there is even no definition. Use more evident wording instead. Discussion: https://postgr.es/m/23737.1556550645%40sss.pgh.pa.us
-
Alexander Korotkov authored
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
-
Fujii Masao authored
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
-
Magnus Hagander authored
Author: Daniel Gustafsson <daniel@yesql.se>
-
Tom Lane authored
On a 64-bit machine, if you set track_activity_query_size and max_connections such that their product exceeds 1GB, shared memory setup will still succeed (given enough RAM), but attempts to read pg_stat_activity fail with "invalid memory alloc request size". Work around that by using MemoryContextAllocHuge to allocate the local copy of the activity strings. Using the "huge" API costs us nothing extra in normal cases, and it seems better than throwing an error and/or explaining to people why they can't do this. This situation seems insanely profligate today, but who knows what people will consider normal in ten or twenty years? So let's fix it in HEAD but not worry about a back-patch. Per report from James Tomson. Discussion: https://postgr.es/m/1CFDCCD6-B268-48D8-85C8-400D2790B2C3@pushd.com
-
Peter Eisentraut authored
The SQL keywords table in the documentation had until now been generated by some ad hoc scripting outside the source tree once for each major release. This changes it to an automated process. We have the PostgreSQL keywords available in a parseable format in parser/kwlist.h. For the relevant SQL standard versions, keep the keyword lists in new text files. A new script generate-keywords-table.pl pulls it all together and produces a DocBook table. The final output in the documentation should be identical after this change. Discussion: https://www.postgresql.org/message-id/flat/07daeadd-8c82-0d95-5e19-e350502cb749%402ndquadrant.com
-
Amit Kapila authored
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: 06c8a509 Improve code comments in b0eaa4c5. 13e8643b During pg_upgrade, conditionally skip transfer of FSMs. 6f918159 Add more tests for FSM. 9c32e4c3 Clear the local map when not used. 29d108cd Update the documentation for FSM behavior.. 08ecdfe7 Make FSM test portable. b0eaa4c5 Avoid creation of the free space map for small heap relations. Discussion: https://postgr.es/m/20190416180452.3pm6uegx54iitbt5@alap3.anarazel.de
-
Michael Paquier authored
This code was broken as of 582edc36, 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
-
- 06 May, 2019 5 commits
-
-
Bruce Momjian authored
Still needs text markup, links, word wrap, and indenting.
-
Alvaro Herrera authored
... 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
-
Michael Paquier authored
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
-
Dean Rasheed authored
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 e2d4ef8d, 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 e2d4ef8d was. Dean Rasheed, reviewed by Jonathan Katz and Stephen Frost.
-
Dean Rasheed authored
In commit e2d4ef8d, 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
-
- 05 May, 2019 3 commits
-
-
Tom Lane authored
Noticed while reviewing nearby code. Given all the disclaimers about this not being meant as user-facing code, I wonder whether we should make these non-translatable? But in any case there's little excuse for them not to be good English.
-
Tom Lane authored
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 62148c35.
-
- 04 May, 2019 3 commits
-
-
Michael Paquier authored
Like the backend, the frontend has wrappers on top of malloc() and such whose use is recommended. Particularly, it is possible to do memory allocation without issuing an error. Some binaries missed the use of those wrappers, so let's fix the gap for consistency. This also fixes two latent bugs: - In pg_dump/pg_dumpall when parsing an ACL item, on an out-of-memory error for strdup(), the code considered the failure as a ACL parsing problem instead of an actual OOM. - In pg_waldump, an OOM when building the target directory string would cause a crash. Author: Daniel Gustafsson Discussion: https://postgr.es/m/gY0y9xenfoBPc-Tufsr2Zg-MmkrJslm0Tw_CMg4p_j58-k_PXNC0klMdkKQkg61BkXC9_uWo-DcUzfxnHqpkpoR5jjVZrPHqKYikcHIiONhg=@yesql.se
-
Noah Misch authored
Peifeng Qiu Discussion: https://postgr.es/m/CABmtVJiKXQjast0dQD-8KAtfm8XmyYxo-4Dc7+M+fBr8JRTqkw@mail.gmail.com
-
Noah Misch authored
Jeff Janes, reviewed by Michael Paquier. Discussion: https://postgr.es/m/CAMkU=1x8taZfsbPkv_MsWbTtzibW_yQHXoMhF_DTtm=z2hVHDg@mail.gmail.com
-
- 03 May, 2019 1 commit
-
-
Peter Geoghegan authored
Commit 3f342839 corrected obsolete comments about buffer locks at the main _bt_insert_parent() call site, but missed similar obsolete comments above _bt_insert_parent() itself. Both sets of comments were rendered obsolete by commit 40dae7ec, which made the nbtree page split algorithm more robust. Fix the comments that were missed the first time around now. In passing, refine a related _bt_insert_parent() comment about re-finding the parent page to insert new downlink.
-