aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* Support subscripting of arbitrary types, not only arrays.Tom Lane2020-12-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch generalizes the subscripting infrastructure so that any data type can be subscripted, if it provides a handler function to define what that means. Traditional variable-length (varlena) arrays all use array_subscript_handler(), while the existing fixed-length types that support subscripting use raw_array_subscript_handler(). It's expected that other types that want to use subscripting notation will define their own handlers. (This patch provides no such new features, though; it only lays the foundation for them.) To do this, move the parser's semantic processing of subscripts (including coercion to whatever data type is required) into a method callback supplied by the handler. On the execution side, replace the ExecEvalSubscriptingRef* layer of functions with direct calls to callback-supplied execution routines. (Thus, essentially no new run-time overhead should be caused by this patch. Indeed, there is room to remove some overhead by supplying specialized execution routines. This patch does a little bit in that line, but more could be done.) Additional work is required here and there to remove formerly hard-wired assumptions about the result type, collation, etc of a SubscriptingRef expression node; and to remove assumptions that the subscript values must be integers. One useful side-effect of this is that we now have a less squishy mechanism for identifying whether a data type is a "true" array: instead of wiring in weird rules about typlen, we can look to see if pg_type.typsubscript == F_ARRAY_SUBSCRIPT_HANDLER. For this to be bulletproof, we have to forbid user-defined types from using that handler directly; but there seems no good reason for them to do so. This patch also removes assumptions that the number of subscripts is limited to MAXDIM (6), or indeed has any hard-wired limit. That limit still applies to types handled by array_subscript_handler or raw_array_subscript_handler, but to discourage other dependencies on this constant, I've moved it from c.h to utils/array.h. Dmitry Dolgov, reviewed at various times by Tom Lane, Arthur Zakirov, Peter Eisentraut, Pavel Stehule Discussion: https://postgr.es/m/CA+q6zcVDuGBv=M0FqBYX8DPebS3F_0KQ6OVFobGJPM507_SZ_w@mail.gmail.com Discussion: https://postgr.es/m/CA+q6zcVovR+XY4mfk-7oNk-rF91gH0PebnNfuUjuuDsyHjOcVA@mail.gmail.com
* Change get_constraint_index() to use pg_constraint.conindidPeter Eisentraut2020-12-09
| | | | | | | | | | | | | It was still using a scan of pg_depend instead of using the conindid column that has been added since. Since it is now just a catalog lookup wrapper and not related to pg_depend, move from pg_depend.c to lsyscache.c. Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/4688d55c-9a2e-9a5a-d166-5f24fe0bf8db%40enterprisedb.com
* jit: Reference function pointer types via llvmjit_types.c.Andres Freund2020-12-08
| | | | | | | | | | It is error prone (see 5da871bfa1b) and verbose to manually create function types. Add a helper that can reference a function pointer type via llvmjit_types.c and and convert existing instances of manual creation. Author: Andres Freund <andres@anarazel.de> Reviewed-By: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/20201207212142.wz5tnbk2jsaqzogb@alap3.anarazel.de
* Teach contain_leaked_vars that assignment SubscriptingRefs are leaky.Tom Lane2020-12-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | array_get_element and array_get_slice qualify as leakproof, since they will silently return NULL for bogus subscripts. But array_set_element and array_set_slice throw errors for such cases, making them clearly not leakproof. contain_leaked_vars was evidently written with only the former case in mind, as it gave the wrong answer for assignment SubscriptingRefs (nee ArrayRefs). This would be a live security bug, were it not that assignment SubscriptingRefs can only occur in INSERT and UPDATE target lists, while we only care about leakproofness for qual expressions; so the wrong answer can't occur in practice. Still, that's a rather shaky answer for a security-related question; and maybe in future somebody will want to ask about leakproofness of a tlist. So it seems wise to fix and even back-patch this correction. (We would need some change here anyway for the upcoming generic-subscripting patch, since extensions might make different tradeoffs about whether to throw errors. Commit 558d77f20 attempted to lay groundwork for that by asking check_functions_in_node whether a SubscriptingRef contains leaky functions; but that idea fails now that the implementation methods of a SubscriptingRef are not SQL-visible functions that could be marked leakproof or not.) Back-patch to 9.6. While 9.5 has the same issue, the code's a bit different. It seems quite unlikely that we'd introduce any actual bug in the short time 9.5 has left to live, so the work/risk/reward balance isn't attractive for changing 9.5. Discussion: https://postgr.es/m/3143742.1607368115@sss.pgh.pa.us
* Remove operator_precedence_warning.Tom Lane2020-12-08
| | | | | | | | | | | This GUC was always intended as a temporary solution to help with finding 9.4-to-9.5 migration issues. Now that all pre-9.5 branches are out of support, and 9.5 will be too before v14 is released, it seems like it's okay to drop it. Doing so allows removal of several hundred lines of poorly-tested code in parse_expr.c, which have been a fertile source of bugs when people did use this. Discussion: https://postgr.es/m/2234320.1607117945@sss.pgh.pa.us
* Improve estimation of ANDs under ORs using extended statistics.Dean Rasheed2020-12-08
| | | | | | | | | | | | Formerly, extended statistics only handled clauses that were RestrictInfos. However, the restrictinfo machinery doesn't create sub-AND RestrictInfos for AND clauses underneath OR clauses. Therefore teach extended statistics to handle bare AND clauses, looking for compatible RestrictInfo clauses underneath them. Dean Rasheed, reviewed by Tomas Vondra. Discussion: https://postgr.es/m/CAEZATCW=J65GUFm50RcPv-iASnS2mTXQbr=CfBvWRVhFLJ_fWA@mail.gmail.com
* Improve estimation of OR clauses using multiple extended statistics.Dean Rasheed2020-12-08
| | | | | | | | | | | | When estimating an OR clause using multiple extended statistics objects, treat the estimates for each set of clauses for each statistics object as independent of one another. The overlap estimates produced for each statistics object do not apply to clauses covered by other statistics objects. Dean Rasheed, reviewed by Tomas Vondra. Discussion: https://postgr.es/m/CAEZATCW=J65GUFm50RcPv-iASnS2mTXQbr=CfBvWRVhFLJ_fWA@mail.gmail.com
* Speed up rechecking if relation needs to be vacuumed or analyze in autovacuum.Fujii Masao2020-12-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | After autovacuum collects the relations to vacuum or analyze, it rechecks whether each relation still needs to be vacuumed or analyzed before actually doing that. Previously this recheck could be a significant overhead especially when there were a very large number of relations. This was because each recheck forced the statistics to be refreshed, and the refresh of the statistics for a very large number of relations could cause heavy overhead. There was the report that this issue caused autovacuum workers to have gotten “stuck” in a tight loop of table_recheck_autovac() that rechecks whether a relation needs to be vacuumed or analyzed. This commit speeds up the recheck by making autovacuum worker reuse the previously-read statistics for the recheck if possible. Then if that "stale" statistics says that a relation still needs to be vacuumed or analyzed, autovacuum refreshes the statistics and does the recheck again. The benchmark shows that the more relations exist and autovacuum workers are running concurrently, the more this change reduces the autovacuum execution time. For example, when there are 20,000 tables and 10 autovacuum workers are running, the benchmark showed that the change improved the performance of autovacuum more than three times. On the other hand, even when there are only 1000 tables and only a single autovacuum worker is running, the benchmark didn't show any big performance regression by the change. Firstly POC patch was proposed by Jim Nasby. As the result of discussion, we used Tatsuhito Kasahara's version of the patch using the approach suggested by Tom Lane. Reported-by: Jim Nasby Author: Tatsuhito Kasahara Reviewed-by: Masahiko Sawada, Fujii Masao Discussion: https://postgr.es/m/3FC6C2F2-8A47-44C0-B997-28830B5716D0@amazon.com
* jit: Correct parameter type for generated expression evaluation functions.Andres Freund2020-12-07
| | | | | | | | | | | | | clang only uses the 'i1' type for scalar booleans, not for pointers to booleans (as the pointer might be pointing into a larger memory allocation). Therefore a pointer-to-bool needs to the "storage" boolean. There's no known case of wrong code generation due to this, but it seems quite possible that it could cause problems (see e.g. 72559438f92). Author: Andres Freund Discussion: https://postgr.es/m/20201207212142.wz5tnbk2jsaqzogb@alap3.anarazel.de Backpatch: 11-, where jit support was added
* Avoid using tuple from syscache for update of pg_database.datfrozenxidMichael Paquier2020-12-08
| | | | | | | | | | | | | | | | | | | pg_database.datfrozenxid gets updated using an in-place update at the end of vacuum or autovacuum. Since 96cdeae, as pg_database has a toast relation, it is possible for a pg_database tuple to have toast values if there is a large set of ACLs in place. In such a case, the in-place update would fail because of the flattening of the toast values done for the catcache entry fetched. Instead of using a copy from the catcache, this changes the logic to fetch the copy of the tuple by directly scanning pg_database. Per the lack of complaints on the matter, no backpatch is done. Note that before 96cdeae, attempting to insert such a tuple to pg_database would cause a "row is too big" error, so the end-of-vacuum problem was not reachable. Author: Ashwin Agrawal, Junfeng Yang Discussion: https://postgr.es/m/DM5PR0501MB38800D9E4605BCA72DD35557CCE10@DM5PR0501MB3880.namprd05.prod.outlook.com
* Fix missed step in removal of useless RESULT RTEs in the planner.Tom Lane2020-12-05
| | | | | | | | | | | | Commit 4be058fe9 forgot that the append_rel_list would already be populated at the time we remove useless result RTEs, and it might contain PlaceHolderVars that need to be adjusted like the ones in the main parse tree. This could lead to "no relation entry for relid N" failures later on, when the planner tries to do something with an unadjusted PHV. Per report from Tom Ellis. Back-patch to v12 where the bug came in. Discussion: https://postgr.es/m/20201205173056.GF30712@cloudinit-builder
* Convert elog(LOG) calls to ereport() where appropriatePeter Eisentraut2020-12-04
| | | | | | | | | | | User-visible log messages should go through ereport(), so they are subject to translation. Many remaining elog(LOG) calls are really debugging calls. Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Noah Misch <noah@leadboat.com> Discussion: https://www.postgresql.org/message-id/flat/92d6f545-5102-65d8-3c87-489f71ea0a37%40enterprisedb.com
* Remove unnecessary grammar symbolsPeter Eisentraut2020-12-04
| | | | | | | | | Instead of publication_name_list, we can use name_list. We already refer to publications everywhere else by the 'name' or 'name_list' symbols, so this only improves consistency. Reviewed-by: https://www.postgresql.org/message-id/flat/3e3ccddb-41bd-ecd8-29fe-195e34d9886f%40enterprisedb.com Discussion: Tom Lane <tgl@sss.pgh.pa.us>
* Remove incorrect assertion in reorderbuffer.c.Amit Kapila2020-12-04
| | | | | | | | | | | | | | | | | We start recording changes in ReorderBufferTXN even before we reach SNAPBUILD_CONSISTENT state so that if the commit is encountered after reaching that we should be able to send the changes of the entire transaction. Now, while recording changes if the reorder buffer memory has exceeded logical_decoding_work_mem then we can start streaming if it is allowed and we haven't yet streamed that data. However, we must not allow streaming to start unless the snapshot has reached SNAPBUILD_CONSISTENT state. In passing, improve the comments atop ReorderBufferResetTXN to mention the case when we need to continue streaming after getting an error. Author: Amit Kapila Reviewed-by: Dilip Kumar Discussion: https://postgr.es/m/CAA4eK1KoOH0byboyYY40NBcC7Fe812trwTa+WY3jQF7WQWZbQg@mail.gmail.com
* Rename cryptohashes.c to cryptohashfuncs.cMichael Paquier2020-12-04
| | | | | | | | | | | | | 87ae969 has created two new files called cryptohash{_openssl}.c in src/common/, whose names overlap with the existing backend file called cryptohashes.c dedicated to the SQL wrappers for SHA2 and MD5. This file is renamed to cryptohashfuncs.c to be more consistent with the surroundings and reduce the confusion with the new cryptohash interface of src/common/. Author: Michael Paquier Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/X8hHhaQgbMbW+aGU@paquier.xyz
* Change SHA2 implementation based on OpenSSL to use EVP digest routinesMichael Paquier2020-12-04
| | | | | | | | | | | | | | | | | | | | | | | | The use of low-level hash routines is not recommended by upstream OpenSSL since 2000, and pgcrypto already switched to EVP as of 5ff4a67. This takes advantage of the refactoring done in 87ae969 that has introduced the allocation and free routines for cryptographic hashes. Since 1.1.0, OpenSSL does not publish the contents of the cryptohash contexts, forcing any consumers to rely on OpenSSL for all allocations. Hence, the resource owner callback mechanism gains a new set of routines to track and free cryptohash contexts when using OpenSSL, preventing any risks of leaks in the backend. Nothing is needed in the frontend thanks to the refactoring of 87ae969, and the resowner knowledge is isolated into cryptohash_openssl.c. Note that this also fixes a failure with SCRAM authentication when using FIPS in OpenSSL, but as there have been few complaints about this problem and as this causes an ABI breakage, no backpatch is done. Author: Michael Paquier Reviewed-by: Daniel Gustafsson, Heikki Linnakangas Discussion: https://postgr.es/m/20200924025314.GE7405@paquier.xyz Discussion: https://postgr.es/m/20180911030250.GA27115@paquier.xyz
* Small code simplificationsPeter Eisentraut2020-12-03
| | | | | strVal() can be used in a couple of places instead of coding the same thing by hand.
* Improve estimation of OR clauses using extended statistics.Dean Rasheed2020-12-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Formerly we only applied extended statistics to an OR clause as part of the clauselist_selectivity() code path for an OR clause appearing in an implicitly-ANDed list of clauses. This meant that it could only use extended statistics if all sub-clauses of the OR clause were covered by a single extended statistics object. Instead, teach clause_selectivity() how to apply extended statistics to an OR clause by handling its ORed list of sub-clauses in a similar manner to an implicitly-ANDed list of sub-clauses, but with different combination rules. This allows one or more extended statistics objects to be used to estimate all or part of the list of sub-clauses. Any remaining sub-clauses are then treated as if they are independent. Additionally, to avoid double-application of extended statistics, this introduces "extended" versions of clause_selectivity() and clauselist_selectivity(), which include an option to ignore extended statistics. This replaces the old clauselist_selectivity_simple() function which failed to completely ignore extended statistics when called from the extended statistics code. A known limitation of the current infrastructure is that an AND clause under an OR clause is not treated as compatible with extended statistics (because we don't build RestrictInfos for such sub-AND clauses). Thus, for example, "(a=1 AND b=1) OR (a=2 AND b=2)" will currently be treated as two independent AND clauses (each of which may be estimated using extended statistics), but extended statistics will not currently be used to account for any possible overlap between those clauses. Improving that is left as a task for the future. Original patch by Tomas Vondra, with additional improvements by me. Discussion: https://postgr.es/m/20200113230008.g67iyk4cs3xbnjju@development
* Refactor CLUSTER and REINDEX grammar to use DefElem for option listsMichael Paquier2020-12-03
| | | | | | | | | | | | | | This changes CLUSTER and REINDEX so as a parenthesized grammar becomes possible for options, while unifying the grammar parsing rules for option lists with the existing ones. This is a follow-up of the work done in 873ea9e for VACUUM, ANALYZE and EXPLAIN. This benefits REINDEX for a potential backend-side filtering for collatable-sensitive indexes and TABLESPACE, while CLUSTER would benefit from the latter. Author: Alexey Kondratov, Justin Pryzby Discussion: https://postgr.es/m/8a8f5f73-00d3-55f8-7583-1375ca8f6a91@postgrespro.ru
* Add GSS information to connection authorized log messageStephen Frost2020-12-02
| | | | | | | | | | | | | | | GSS information (if used) such as if the connection was authorized using GSS or if it was encrypted using GSS, and perhaps most importantly, what the GSS principal used for the authentication was, is extremely useful but wasn't being included in the connection authorized log message. Therefore, add to the connection authorized log message that information, in a similar manner to how we log SSL information when SSL is used for a connection. Author: Vignesh C Reviewed-by: Bharath Rupireddy Discussion: https://www.postgresql.org/message-id/CALDaNm2N1385_Ltoo%3DS7VGT-ESu_bRQa-sC1wg6ikrM2L2Z49w%40mail.gmail.com
* Track total number of WAL records, FPIs and bytes generated in the cluster.Fujii Masao2020-12-02
| | | | | | | | | | | | | | | Commit 6b466bf5f2 allowed pg_stat_statements to track the number of WAL records, full page images and bytes that each statement generated. Similarly this commit allows us to track the cluster-wide WAL statistics counters. New columns wal_records, wal_fpi and wal_bytes are added into the pg_stat_wal view, and reports the total number of WAL records, full page images and bytes generated in the , respectively. Author: Masahiro Ikeda Reviewed-by: Amit Kapila, Movead Li, Kyotaro Horiguchi, Fujii Masao Discussion: https://postgr.es/m/35ef960128b90bfae3b3fdf60a3a860f@oss.nttdata.com
* Allow restore_command parameter to be changed with reload.Fujii Masao2020-12-02
| | | | | | | | | | | | | | | | | | | | | | This commit changes restore_command from PGC_POSTMASTER to PGC_SIGHUP. As the side effect of this commit, restore_command can be reset to empty during archive recovery. In this setting, archive recovery tries to replay only WAL files available in pg_wal directory. This is the same behavior as when the command that always fails is specified in restore_command. Note that restore_command still must be specified (not empty) when starting archive recovery, even after applying this commit. This is necessary as the safeguard to prevent users from forgetting to specify restore_command and starting archive recovery. Thanks to Peter Eisentraut, Michael Paquier, Andres Freund, Robert Haas and Anastasia Lubennikova for discussion. Author: Sergei Kornilov Reviewed-by: Kyotaro Horiguchi, Fujii Masao Discussion: https://postgr.es/m/2317771549527294@sas2-985f744271ca.qloud-c.yandex.net
* Move SHA2 routines to a new generic API layer for crypto hashesMichael Paquier2020-12-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Two new routines to allocate a hash context and to free it are created, as these become necessary for the goal behind this refactoring: switch the all cryptohash implementations for OpenSSL to use EVP (for FIPS and also because upstream does not recommend the use of low-level cryptohash functions for 20 years). Note that OpenSSL hides the internals of cryptohash contexts since 1.1.0, so it is necessary to leave the allocation to OpenSSL itself, explaining the need for those two new routines. This part is going to require more work to properly track hash contexts with resource owners, but this not introduced here. Still, this refactoring makes the move possible. This reduces the number of routines for all SHA2 implementations from twelve (SHA{224,256,386,512} with init, update and final calls) to five (create, free, init, update and final calls) by incorporating the hash type directly into the hash context data. The new cryptohash routines are moved to a new file, called cryptohash.c for the fallback implementations, with SHA2 specifics becoming a part internal to src/common/. OpenSSL specifics are part of cryptohash_openssl.c. This infrastructure is usable for more hash types, like MD5 or HMAC. Any code paths using the internal SHA2 routines are adapted to report correctly errors, which are most of the changes of this commit. The zones mostly impacted are checksum manifests, libpq and SCRAM. Note that e21cbb4 was a first attempt to switch SHA2 to EVP, but it lacked the refactoring needed for libpq, as done here. This patch has been tested on Linux and Windows, with and without OpenSSL, and down to 1.0.1, the oldest version supported on HEAD. Author: Michael Paquier Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/20200924025314.GE7405@paquier.xyz
* Ensure that expandTableLikeClause() re-examines the same table.Tom Lane2020-12-01
| | | | | | | | | | | | | | | | | | | | | | As it stood, expandTableLikeClause() re-did the same relation_openrv call that transformTableLikeClause() had done. However there are scenarios where this would not find the same table as expected. We hold lock on the LIKE source table, so it can't be renamed or dropped, but another table could appear before it in the search path. This explains the odd behavior reported in bug #16758 when cloning a table as a temp table of the same name. This case worked as expected before commit 502898192 introduced the need to open the source table twice, so we should fix it. To make really sure we get the same table, let's re-open it by OID not name. That requires adding an OID field to struct TableLikeClause, which is a little nervous-making from an ABI standpoint, but as long as it's at the end I don't think there's any serious risk. Per bug #16758 from Marc Boeren. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/16758-840e84a6cfab276d@postgresql.org
* Avoid memcpy() with a NULL source pointer and count == 0Alvaro Herrera2020-12-01
| | | | | | | | | | | | | | | | When memcpy() is called on a pointer, the compiler is entitled to assume that the pointer is not null, which can lead to optimizing nearby code in potentially undesirable ways. We still want such optimizations (gcc's -fdelete-null-pointer-checks) in cases where they're valid. Related: commit 13bba02271dc. Backpatch to pg11, where this particular instance appeared. Reported-by: Ranier Vilela <ranier.vf@gmail.com> Reported-by: Zhihong Yu <zyu@yugabyte.com> Discussion: https://postgr.es/m/CAEudQApUndmQkr5fLrCKXQ7+ib44i7S+Kk93pyVThS85PnG3bQ@mail.gmail.com Discussion: https://postgr.es/m/CALNJ-vSdhwSM5f4tnNn1cdLHvXMVe_S+V3nR5GwNrmCPNB2VtQ@mail.gmail.com
* Use truncate(2) where appropriate.Thomas Munro2020-12-01
| | | | | | | When truncating files by name, use truncate(2). Windows hasn't got it, so keep our previous coding based on ftruncate(2) as a fallback. Discussion: https://postgr.es/m/16663-fe97ccf9932fc800%40postgresql.org
* Free disk space for dropped relations on commit.Thomas Munro2020-12-01
| | | | | | | | | | | | | | | | | | | | | When committing a transaction that dropped a relation, we previously truncated only the first segment file to free up disk space (the one that won't be unlinked until the next checkpoint). Truncate higher numbered segments too, even though we unlink them on commit. This frees the disk space immediately, even if other backends have open file descriptors and might take a long time to get around to handling shared invalidation events and closing them. Also extend the same behavior to the first segment, in recovery. Back-patch to all supported releases. Bug: #16663 Reported-by: Denis Patron <denis.patron@previnet.it> Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com> Reviewed-by: Neil Chen <carpenter.nail.cz@gmail.com> Reviewed-by: David Zhang <david.zhang@highgo.ca> Discussion: https://postgr.es/m/16663-fe97ccf9932fc800%40postgresql.org
* Fix missing outfuncs.c support for IncrementalSortPath.Tom Lane2020-11-30
| | | | | | | | | | | | | | For debugging purposes, Path nodes are supposed to have outfuncs support, but this was overlooked in the original incremental sort patch. While at it, clean up a couple other minor oversights, as well as bizarre choice of return type for create_incremental_sort_path(). (All the existing callers just cast it to "Path *" immediately, so they don't care, but some future caller might care.) outfuncs.c fix by Zhijie Hou, the rest by me Discussion: https://postgr.es/m/324c4d81d8134117972a5b1f6cdf9560@G08CNEXMBPEKD05.g08.fujitsu.local
* Prevent parallel index build in a standalone backend.Tom Lane2020-11-30
| | | | | | | | | | | | | | | | | This can't work if there's no postmaster, and indeed the code got an assertion failure trying. There should be a check on IsUnderPostmaster gating the use of parallelism, as the planner has for ordinary parallel queries. Commit 40d964ec9 got this right, so follow its model of checking IsUnderPostmaster at the same place where we check for max_parallel_maintenance_workers == 0. In general, new code implementing parallel utility operations should do the same. Report and patch by Yulin Pei, cosmetically adjusted by me. Back-patch to v11 where this code came in. Discussion: https://postgr.es/m/HK0PR01MB22747D839F77142D7E76A45DF4F50@HK0PR01MB2274.apcprd01.prod.exchangelabs.com
* Fix miscomputation of direct_lateral_relids for join relations.Tom Lane2020-11-30
| | | | | | | | | | | | | | If a PlaceHolderVar is to be evaluated at a join relation, but its value is only needed there and not at higher levels, we neglected to update the joinrel's direct_lateral_relids to include the PHV's source rel. This causes problems because join_is_legal() then won't allow joining the joinrel to the PHV's source rel at all, leading to "failed to build any N-way joins" planner failures. Per report from Andreas Seltenreich. Back-patch to 9.5 where the problem originated. Discussion: https://postgr.es/m/87blfgqa4t.fsf@aurora.ydns.eu
* Refactor parsing rules for option lists of EXPLAIN, VACUUM and ANALYZEMichael Paquier2020-11-30
| | | | | | | | | | | Those three commands have been using the same grammar rules to handle a a list of parenthesized options. This refactors the code so as they use the same parsing rules, shaving some code. A future commit will make use of those option parsing rules for more utility commands, like REINDEX and CLUSTER. Author: Alexey Kondratov, Justin Pryzby Discussion: https://postgr.es/m/8a8f5f73-00d3-55f8-7583-1375ca8f6a91@postgrespro.ru
* Remove leftover comments, left behind by removal of WITH OIDS.Heikki Linnakangas2020-11-30
| | | | | Author: Amit Langote Discussion: https://www.postgresql.org/message-id/CA%2BHiwqGaRoF3XrhPW-Y7P%2BG7bKo84Z_h%3DkQHvMh-80%3Dav3wmOw%40mail.gmail.com
* Improve log message about termination of background workers.Fujii Masao2020-11-30
| | | | | | | | | | | | | | | | Previously the shutdown of a background worker that uses die() as SIGTERM signal handler produced the log message "terminating connection due to administrator command". This log message was confusing because a background worker is not a connection. This commit improves that log message to "terminating background worker XXX due to administrator command" (XXX is replaced with the name of the background worker). This is the same log message as another SIGTERM signal handler bgworker_die() for a background worker reports. Author: Bharath Rupireddy Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/3f292fbb-f155-9a01-7cb2-7ccc9007ab3f@oss.nttdata.com
* Fix a recently-introduced race condition in LISTEN/NOTIFY handling.Tom Lane2020-11-28
| | | | | | | | | | | | | | | | | | | | | | | | | | Commit 566372b3d fixed some race conditions involving concurrent SimpleLruTruncate calls, but it introduced new ones in async.c. A newly-listening backend could attempt to read Notify SLRU pages that were in process of being truncated, possibly causing an error. Also, the QUEUE_TAIL pointer could become set to a value that's not equal to the queue position of any backend. While that's fairly harmless in v13 and up (thanks to commit 51004c717), in older branches it resulted in near-permanent disabling of the queue truncation logic, so that continued use of NOTIFY led to queue-fill warnings and eventual inability to send any more notifies. (A server restart is enough to make that go away, but it's still pretty unpleasant.) The core of the problem is confusion about whether QUEUE_TAIL represents the "logical" tail of the queue (i.e., the oldest still-interesting data) or the "physical" tail (the oldest data we've not yet truncated away). To fix, split that into two variables. QUEUE_TAIL regains its definition as the logical tail, and we introduce a new variable to track the oldest un-truncated page. Per report from Mikael Gustavsson. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/1b8561412e8a4f038d7a491c8b922788@smhi.se
* Fix CLUSTER progress reporting of number of blocks scanned.Fujii Masao2020-11-27
| | | | | | | | | | | | | | | | | | | | | | | | | Previously pg_stat_progress_cluster view reported the current block number in heap scan as the number of heap blocks scanned (i.e., heap_blks_scanned). This reported number could be incorrect when synchronize_seqscans is enabled, because it allowed the heap scan to start at block in middle. This could result in wraparounds in the heap_blks_scanned column when the heap scan wrapped around. This commit fixes the bug by calculating the number of blocks from the block that the heap scan starts at to the current block in scan, and reporting that number in the heap_blks_scanned column. Also, in pg_stat_progress_cluster view, previously heap_blks_scanned could not reach heap_blks_total at the end of heap scan phase if the last pages scanned were empty. This commit fixes the bug by manually updating heap_blks_scanned to the same value as heap_blks_total when the heap scan phase finishes. Back-patch to v12 where pg_stat_progress_cluster view was introduced. Reported-by: Matthias van de Meent Author: Matthias van de Meent Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CAEze2WjCBWSGkVfYag001Rc4+-nNLDpWM7QbyD6yPvuhKs-gYQ@mail.gmail.com
* Fix replication of in-progress transactions in tablesync worker.Amit Kapila2020-11-27
| | | | | | | | | | | | | | | | | Tablesync worker runs under a single transaction but in streaming mode, we were committing the transaction on stream_stop, stream_abort, and stream_commit. We need to avoid committing the transaction in a streaming mode in tablesync worker. In passing move the call to process_syncing_tables in apply_handle_stream_commit after clean up of stream files. This will allow clean up of files to happen before the exit of tablesync worker which would otherwise be handled by one of the proc exit routines. Author: Dilip Kumar Reviewed-by: Amit Kapila and Peter Smith Tested-by: Peter Smith Discussion: https://postgr.es/m/CAHut+Pt4PyKQCwqzQ=EFF=bpKKJD7XKt_S23F6L20ayQNxg77A@mail.gmail.com
* Restore lock level to update statusFlagsAlvaro Herrera2020-11-26
| | | | | | | | | | | | | | Reverts 27838981be9d (some comments are kept). Per discussion, it does not seem safe to relax the lock level used for this; in order for it to be safe, there would have to be memory barriers between the point we set the flag and the point we set the trasaction Xid, which perhaps would not be so bad; but there would also have to be barriers at the readers' side, which from a performance perspective might be bad. Now maybe this analysis is wrong and it *is* safe for some reason, but proof of that is not trivial. Discussion: https://postgr.es/m/20201118190928.vnztes7c2sldu43a@alap3.anarazel.de
* Use Enums for logical replication message types at more places.Amit Kapila2020-11-26
| | | | | | | | | Commit 644f0d7cc9 added logical replication message type enums to use instead of character literals but some char substitutions were overlooked. Author: Peter Smith Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CAHut+PsTG=Vrv8hgrvOnAvCNR21jhqMdPk2n0a1uJPoW0p+UfQ@mail.gmail.com
* Avoid spurious waits in concurrent indexingAlvaro Herrera2020-11-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | In the various waiting phases of CREATE INDEX CONCURRENTLY (CIC) and REINDEX CONCURRENTLY (RC), we wait for other processes to release their snapshots; this is necessary in general for correctness. However, processes doing CIC in other tables cannot possibly affect CIC or RC done in "this" table, so we don't need to wait for those. This commit adds a flag in MyProc->statusFlags to indicate that the current process is doing CIC, so that other processes doing CIC or RC can ignore it when waiting. Note that this logic is only valid if the index does not access other tables. For simplicity we avoid setting the flag if the index has a column that's an expression, or has a WHERE predicate. (It is possible to have expressional or partial indexes that do not access other tables, but figuring that out would require more work.) This flag can potentially also be used by processes doing REINDEX CONCURRENTLY to be skipped; and by VACUUM to ignore processes in CIC or RC for the purposes of computing an Xmin. That's left for future commits. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Author: Dimitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/20200810233815.GA18970@alvherre.pgsql
* Avoid spamming the client with multiple ParameterStatus messages.Tom Lane2020-11-25
| | | | | | | | | | | | | | | | | | Up to now, we sent a ParameterStatus message to the client immediately upon any change in the active value of any GUC_REPORT variable. This was only barely okay when the feature was designed; now that we have things like function SET clauses, there are very plausible use-cases where a GUC_REPORT variable might change many times within a query --- and even end up back at its original value, perhaps. Fortunately most of our GUC_REPORT variables are unlikely to be changed often; but there are proposals in play to enlarge that set, or even make it user-configurable. Hence, let's fix things to not generate more than one ParameterStatus message per variable per query, and to not send any message at all unless the end-of-query value is different from what we last reported. Discussion: https://postgr.es/m/5708.1601145259@sss.pgh.pa.us
* Make error hint from bind() failure more accuratePeter Eisentraut2020-11-25
| | | | | | | | | | | | | | | | | The hint "Is another postmaster already running ..." should only be printed for errors that are really about something else already using the address. In other cases it is misleading. So only show that hint if errno == EADDRINUSE. Also, since Unix-domain sockets in the file-system namespace never report EADDRINUSE for an existing file (they would just overwrite it), the part of the hint saying "If not, remove socket file \"%s\" and retry." can never happen, so remove it. Unix-domain sockets in the abstract namespace can report EADDRINUSE, but in that case there is no file to remove, so the hint doesn't work there either. Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/6dee8574-b0ad-fc49-9c8c-2edc796f0033@2ndquadrant.com
* Add support for abstract Unix-domain socketsPeter Eisentraut2020-11-25
| | | | | | | | | | This is a variant of the normal Unix-domain sockets that don't use the file system but a separate "abstract" namespace. At the user interface, such sockets are represented by names starting with "@". Supported on Linux and Windows right now. Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/6dee8574-b0ad-fc49-9c8c-2edc796f0033@2ndquadrant.com
* Fix WaitLatch(NULL) on Windows.Thomas Munro2020-11-25
| | | | | | | | | | Further to commit 733fa9aa, on Windows when a latch is triggered but we aren't currently waiting for it, we need to locate the latch's HANDLE rather than calling ResetEvent(NULL). Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reported-by: Ranier Vilela <ranier.vf@gmail.com> Discussion: https://postgr.es/m/CAEudQArTPi1YBc%2Bn1fo0Asy3QBFhVjp_QgyKG-8yksVn%2ByRTiw%40mail.gmail.com
* Remove obsolete comment atop ri_PlanCheck.Amit Kapila2020-11-25
| | | | | | | | | Commit 5b7ba75f7f removed the unused parameter but forgot to update the nearby comments. Author: Li Japin Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/0E2F62A2-B2F1-4052-83AE-F0BEC8A75789@hotmail.com
* Remove catalog function currtid()Michael Paquier2020-11-25
| | | | | | | | | | | | | | | | | | | | | | | | | currtid() and currtid2() are an undocumented set of functions whose sole known user is the Postgres ODBC driver, able to retrieve the latest TID version for a tuple given by the caller of those functions. As used by Postgres ODBC, currtid() is a shortcut able to retrieve the last TID loaded into a backend by passing an OID of 0 (magic value) after a tuple insertion. This is removed in this commit, as it became obsolete after the driver began using "RETURNING ctid" with inserts, a clause supported since Postgres 8.2 (using RETURNING is better for performance anyway as it reduces the number of round-trips to the backend). currtid2() is still used by the driver, so this remains around for now. Note that this function is kept in its original shape for backward compatibility reasons. Per discussion with many people, including Andres Freund, Peter Eisentraut, Álvaro Herrera, Hiroshi Inoue, Tom Lane and myself. Bump catalog version. Discussion: https://postgr.es/m/20200603021448.GB89559@paquier.xyz
* Properly check index mark/restore in ExecSupportsMarkRestore.Andrew Gierth2020-11-24
| | | | | | | | | | | | | | | | Previously this code assumed that all IndexScan nodes supported mark/restore, which is not true since it depends on optional index AM support functions. This could lead to errors about missing support functions in rare edge cases of mergejoins with no sort keys, where an unordered non-btree index scan was placed on the inner path without a protecting Materialize node. (Normally, the fact that merge join requires ordered input would avoid this error.) Backpatch all the way since this bug is ancient. Per report from Eugen Konkov on irc. Discussion: https://postgr.es/m/87o8jn50be.fsf@news-spur.riddles.org.uk
* Put "inline" marker on declarations of inline functions.Tom Lane2020-11-24
| | | | | | I'm having a hard time telling whether the letter of the C standard requires this, but we do have a couple of buildfarm members that throw warnings when this is not done. Oversight in c532d15dd.
* Move per-agg and per-trans duplicate finding to the planner.Heikki Linnakangas2020-11-24
| | | | | | | | | | This has the advantage that the cost estimates for aggregates can count the number of calls to transition and final functions correctly. Bump catalog version, because views can contain Aggrefs. Reviewed-by: Andres Freund Discussion: https://www.postgresql.org/message-id/b2e3536b-1dbc-8303-c97e-89cb0b4a9a48%40iki.fi
* Use macros instead of hardcoded offsets for LWLock initializationMichael Paquier2020-11-24
| | | | | | | | | This makes the code slightly easier to follow, as the initialization relies on an offset that overlapped with an equivalent set of macros defined, which are used in other places already. Author: Japin Li Discussion: https://postgr.es/m/MEYP282MB1669FB410006758402F2C3A2B6E00@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
* Centralize logic for skipping useless ereport/elog calls.Tom Lane2020-11-23
| | | | | | | | | | | | | | | | | | | | | While ereport() and elog() themselves are quite cheap when the error message level is too low to be printed, some places need to do substantial work before they can call those macros at all. To allow optimizing away such setup work when nothing is to be printed, make elog.c export a new function message_level_is_interesting(elevel) that reports whether ereport/elog will do anything. Make use of that in various places that had ad-hoc direct tests of log_min_messages etc. Also teach ProcSleep to use it to avoid some work. (There may well be other places that could usefully use this; I didn't search hard.) Within elog.c, refactor a little bit to avoid having duplicate copies of the policy-setting logic. When that code was written, we weren't relying on the availability of inline functions; so it had some duplications in the name of efficiency, which I got rid of. Alvaro Herrera and Tom Lane Discussion: https://postgr.es/m/129515.1606166429@sss.pgh.pa.us