aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* Fix incorrect variable datatype.Fujii Masao2020-07-08
| | | | | | | | | | | Since slot_keep_segs indicates the number of WAL segments not LSN, its datatype should not be XLogRecPtr. Back-patch to v13 where this issue was added. Reported-by: Atsushi Torikoshi Author: Atsushi Torikoshi, tweaked by Fujii Masao Discussion: https://postgr.es/m/ebd0d674f3e050222238a960cac5251a@oss.nttdata.com
* Fix typoMagnus Hagander2020-07-08
| | | | Author: Daniel Gustafsson
* Fix function name in comment.Fujii Masao2020-07-08
| | | | | Author: Masahiro Ikeda Discussion: https://postgr.es/m/0043eee90b38351ea199d7e3294c10c4@oss.nttdata.com
* Don't create pg_type entries for sequences or toast tables.Tom Lane2020-07-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | Commit f7f70d5e2 left one inconsistency behind: we're still creating pg_type entries for the composite types of sequences and toast tables, but not arrays over those composites. But there seems precious little reason to have named composite types for toast tables, and not much more to have them for sequences (especially given the thought that sequences may someday not be standalone relations at all). So, let's close that inconsistency by removing these composite types, rather than adding arrays for them. This buys back a little bit of the initial pg_type bloat added by the previous patch, and could be a significant savings in a large database with many toast tables. Aside from a small logic rearrangement in heap_create_with_catalog, this patch mostly needs to clean up some places that were assuming that pg_class.reltype always has a valid value. Those are really pre-existing bugs, given that it's documented otherwise; notably, the plpgsql changes fix code that gives "cache lookup failed for type 0" on indexes today. But none of these seem interesting enough to back-patch. Also, remove the pg_dump/pg_upgrade infrastructure for propagating a toast table's pg_type OID into the new database, since we no longer need that. Discussion: https://postgr.es/m/761F1389-C6A8-4C15-80CE-950C961F5341@gmail.com
* Morph pg_replication_slots.min_safe_lsn to safe_wal_sizeAlvaro Herrera2020-07-07
| | | | | | | | | | | | | | | The previous definition of the column was almost universally disliked, so provide this updated definition which is more useful for monitoring purposes: a large positive value is good, while zero or a negative value means danger. This should be operationally more convenient. Backpatch to 13, where the new column to pg_replication_slots (and the feature it represents) were added. Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reported-by: Fujii Masao <masao.fujii@oss.nttdata.com> Discussion: https://postgr.es/m/9ddfbf8c-2f67-904d-44ed-cf8bc5916228@oss.nttdata.com
* Check ssl_in_use flag when reporting statisticsMagnus Hagander2020-07-07
| | | | | | | | | | Previously we checked that the ssl pointer was not null, but this puts a requirement on there being such a pointer which may not be true in future multi-ssl-library supporting times. This seems to have been an oversight in 9029f4b3740, but hasn't really had any effect since we only have one library. Author: Daniel Gustafsson
* Remove unnecessary PageIsEmpty() nbtree build check.Peter Geoghegan2020-07-06
| | | | | | | | | | | | | | | nbtree index builds cannot write out an empty page. That would mean that there was no way to create a pivot tuple pointing to the page one level up, since _bt_truncate() generates one based on page's firstright tuple. Replace the unnecessary PageIsEmpty() check with an assertion that checks that the page has space for at least two line pointers (the would-be high key line pointer, plus at least one valid "data item" tuple line pointer). The PageIsEmpty() check was added by commit 5d9f146c over 20 years ago. It looks like it has always been unnecessary.
* Create composite array types for initdb-created relations.Tom Lane2020-07-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | When we invented arrays of composite types (commit bc8036fc6), we excluded system catalogs, basically just on the grounds of not wanting to bloat pg_type. However, it's definitely inconsistent that catalogs' composite types can't be put into arrays when others can. Another problem is that the exclusion is done by checking IsUnderPostmaster in heap_create_with_catalog, which means that (1) If a user tries to create a table in single-user mode, it doesn't get an array type. That's bad in itself, plus it breaks pg_upgrade. (2) If someone drops and recreates a system view or information_schema view (as we occasionally recommend doing), it will now have an array type where it did not before, making for still more inconsistency. So this is all pretty messy. Let's just get rid of the inconsistency and decree that system-created relations should have array types if similar user-created ones would, i.e. it only depends on the relkind. As of HEAD, that means that the initial contents of pg_type grow from 411 rows to 605, which is a lot of growth percentage-wise, but it's still quite a small catalog compared to others. Wenjing Zeng, reviewed by Shawn Wang, further hacking by me Discussion: https://postgr.es/m/761F1389-C6A8-4C15-80CE-950C961F5341@gmail.com
* Refactor routines for name lookups of procedures and operatorsMichael Paquier2020-07-06
| | | | | | | | | | | | | | | | | | | This introduces a new set of extended routines for procedure and operator name lookups, with a flag bitmask argument that can modify the result. The following options are available: - Force schema qualification, ignoring search_path. This is similar to the existing option for format_{operator|procedure}_qualified(). - Force NULL as result instead of a numeric OID for an undefined object. This option is new. This is a refactoring similar to 1185c78, that will be used for a future patch to improve the SQL functions providing information using object addresses for undefined objects. Author: Michael Paquier Reviewed-by: Aleksander Alekseev, Dmitry Dolgov, Daniel Gustafsson, Álvaro Herrera Discussion: https://postgr.es/m/CAB7nPqSZxrSmdHK-rny7z8mi=EAFXJ5J-0RbzDw6aus=wB5azQ@mail.gmail.com
* Remove extra whitespace in comments atop ReorderBufferCheckMemoryLimit.Amit Kapila2020-07-06
| | | | Backpatch-through: 13, where it was introduced
* Add new flag to format_type_extended() to get NULL for undefined typeMichael Paquier2020-07-06
| | | | | | | | | | | | | | | | | | | | | | If a type scanned is undefined, type format routines have two behaviors depending on if FORMAT_TYPE_ALLOW_INVALID is used by the caller or not: - Issue a cache lookup error - Return an undefined type name "???", "???[]" or "-" The current interface is not really helpful for callers willing to format properly a type name, but still make sure that the type is defined as there could be types matching the strings generated when looking for an undefined type, even if that should not be a problem in practice. In order to counter that, add a new flag called FORMAT_TYPE_INVALID_AS_NULL that returns a NULL result instead of "??? or "-" which does not generate an error. This flag will be used in a follow-up patch improving the set of SQL functions showing information for object addresses when it comes to undefined objects. Author: Michael Paquier Reviewed-by: Aleksander Alekseev, Dmitry Dolgov, Daniel Gustafsson, Álvaro Herrera Discussion: https://postgr.es/m/CAB7nPqSZxrSmdHK-rny7z8mi=EAFXJ5J-0RbzDw6aus=wB5azQ@mail.gmail.com
* Remove unused function parameter in end_parallel_vacuum.Amit Kapila2020-07-06
| | | | | | | Author: Vignesh C Reviewed-by: Sawada Masahiko Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/CALDaNm3Ppt71NafGY5mk3V2i3Q+mm93pVibDq-0NpW7WU67Jcg@mail.gmail.com
* Rename enable_incrementalsort for clarityPeter Eisentraut2020-07-05
| | | | | Author: James Coleman <jtc331@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/df652910-e985-9547-152c-9d4357dc3979%402ndquadrant.com
* Fix "ignoring return value" complaints from commit 96d1f423f9Joe Conway2020-07-04
| | | | | | | | | | | | | | | The cfbot and some BF animals are complaining about the previous read_binary_file commit because of ignoring return value of ‘fread’. So let's make everyone happy by testing the return value even though not strictly needed. Reported by Justin Pryzby, and suggested patch by Tom Lane. Backpatched to v11 same as the previous commit. Reported-By: Justin Pryzby Reviewed-By: Tom Lane Discussion: https://postgr.es/m/flat/969b8d82-5bb2-5fa8-4eb1-f0e685c5d736%40joeconway.com Backpatch-through: 11
* Read until EOF vice stat-reported size in read_binary_fileJoe Conway2020-07-04
| | | | | | | | | | | | | | | | | | read_binary_file(), used by SQL functions pg_read_file() and friends, uses stat to determine file length to read, when not passed an explicit length as an argument. This is problematic, for example, if the file being read is a virtual file with a stat-reported length of zero. Arrange to read until EOF, or StringInfo data string lenth limit, is reached instead. Original complaint and patch by me, with significant review, corrections, advice, and code optimizations by Tom Lane. Backpatched to v11. Prior to that only paths relative to the data and log dirs were allowed for files, so no "zero length" files were reachable anyway. Reviewed-By: Tom Lane Discussion: https://postgr.es/m/flat/969b8d82-5bb2-5fa8-4eb1-f0e685c5d736%40joeconway.com Backpatch-through: 11
* Clamp total-tuples estimates for foreign tables to ensure planner sanity.Tom Lane2020-07-03
| | | | | | | | | | | | | | | | | | | | | After running GetForeignRelSize for a foreign table, adjust rel->tuples to be at least as large as rel->rows. This prevents bizarre behavior in estimate_num_groups() and perhaps other places, especially in the scenario where rel->tuples is zero because pg_class.reltuples is (suggesting that ANALYZE has never been run for the table). As things stood, we'd end up estimating one group out of any GROUP BY on such a table, whereas the default group-count estimate is more likely to result in a sane plan. Also, clarify in the documentation that GetForeignRelSize has the option to override the rel->tuples value if it has a better idea of what to use than what is in pg_class.reltuples. Per report from Jeff Janes. Back-patch to all supported branches. Patch by me; thanks to Etsuro Fujita for review Discussion: https://postgr.es/m/CAMkU=1xNo9cnan+Npxgz0eK7394xmjmKg-QEm8wYG9P5-CcaqQ@mail.gmail.com
* Fix temporary tablespaces for shared filesets some more.Tom Lane2020-07-03
| | | | | | | | | | | | | | | | | | | | | | | | Commit ecd9e9f0b fixed the problem in the wrong place, causing unwanted side-effects on the behavior of GetNextTempTableSpace(). Instead, let's make SharedFileSetInit() responsible for subbing in the value of MyDatabaseTableSpace when the default tablespace is called for. The convention about what is in the tempTableSpaces[] array is evidently insufficiently documented, so try to improve that. It also looks like SharedFileSetInit() is doing the wrong thing in the case where temp_tablespaces is empty. It was hard-wiring use of the pg_default tablespace, but it seems like using MyDatabaseTableSpace is more consistent with what happens for other temp files. Back-patch the reversion of PrepareTempTablespaces()'s behavior to 9.5, as ecd9e9f0b was. The changes in SharedFileSetInit() go back to v11 where that was introduced. (Note there is net zero code change before v11 from these two patch sets, so nothing to release-note.) Magnus Hagander and Tom Lane Discussion: https://postgr.es/m/CABUevExg5YEsOvqMxrjoNvb3ApVyH+9jggWGKwTDFyFCVWczGQ@mail.gmail.com
* Fix temporary tablespaces for shared filesetsMagnus Hagander2020-07-03
| | | | | | | | | | | | | | A likely copy/paste error in 98e8b480532 from back in 2004 would cause temp tablespace to be reset to InvalidOid if temp_tablespaces was set to the same value as the primary tablespace in the database. This would cause shared filesets (such as for parallel hash joins) to ignore them, putting the temporary files in the default tablespace instead of the configured one. The bug is in the old code, but it appears to have been exposed only once we had shared filesets. Reviewed-By: Daniel Gustafsson Discussion: https://postgr.es/m/CABUevExg5YEsOvqMxrjoNvb3ApVyH+9jggWGKwTDFyFCVWczGQ@mail.gmail.com Backpatch-through: 9.5
* Initialize work_mem using current guc.c default.Peter Geoghegan2020-07-02
| | | | | | | Do the same for the maintenance_work_mem global variable. Oversight in commit 848ae330a49, which increased the previous defaults for work_mem and maintenance_work_mem by 4X.
* nbtree: Rename _bt_search() variables.Peter Geoghegan2020-07-02
| | | | | | | Make some of the variable names in _bt_search() consistent with corresponding variables within _bt_getstackbuf(). This naming scheme is clearer because the variable names always express a relationship between the currently locked buffer/page and some other page.
* Move description of libpqwalreceiver hooks out of the replication's READMEMichael Paquier2020-07-02
| | | | | | | | | | | | | | | | | | | | src/backend/replication/README includes since 32bc08b a basic description of the WAL receiver hooks available in walreceiver.h for a module like libpqwalreceiver, but the README has never been updated to reflect changes done to the hooks, so the contents of the README have rotten with the time. This commit moves the description from the README to walreceiver.h, where it will be hard to miss that a description update or addition is needed depending on the modifications done to the hooks. Each hook now includes a description of what it does in walreceiver.h, and the replication's README mentions walreceiver.h. Thanks also to Amit Kapila for the discussion. Author: Michael Paquier Reviewed-by: Peter Eisentraut Discussion: https://postgr.es/m/20200502024606.GA471944@paquier.xyz
* Refactor ObjectAddress field assignments in more placesMichael Paquier2020-07-01
| | | | | | | | | | | This is a follow-up commit similar to 68de144, with more places in the backend code simplified with the macros able to assign values to the fields of ObjectAddress. The code paths changed here could be transitioned later into using more grouping when inserting dependency records, simplifying this future work. Author: Daniel Gustafsson, Michael Paquier Discussion: https://postgr.es/m/20190213182737.mxn6hkdxwrzgxk35@alap3.anarazel.de
* Improve vacuum error context handling.Amit Kapila2020-07-01
| | | | | | | | | | Use separate functions to save and restore error context information as that made code easier to understand.  Also, make it clear that the index information required for error context is sane. Author: Andres Freund, Justin Pryzby, Amit Kapila Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/CAA4eK1LWo+v1OWu=Sky27GTGSCuOmr7iaURNbc5xz6jO+SaPeA@mail.gmail.com
* Refactor creation of normal dependency records when creating extensionMichael Paquier2020-07-01
| | | | | | | | | | | | | | | When creating an extension, the same type of dependency is used when registering a dependency to a schema and required extensions. This improves the code so as those dependencies are not recorded one-by-one, but grouped together. Note that this has as side effect to remove duplicate dependency entries, even if it should not happen in practice as extensions listed as required in a control file should be listed only once. Extracted from a larger patch by the same author. Author: Daniel Dustafsson Discussion: https://postgr.es/m/20200629065535.GA183079@paquier.xyz
* Further adjustments to Hashagg EXPLAIN ANALYZE outputDavid Rowley2020-07-01
| | | | | | | | | | | | | | | | | | | The "Disk Usage" and "HashAgg Batches" properties in the EXPLAIN ANALYZE output for HashAgg were previously only shown if the number of batches was greater than 0. Here we change this so that these properties are always shown for EXPLAIN ANALYZE formats other than "text". The idea here is that since the HashAgg could have spilled to disk if there had been more data or groups to aggregate, then it's relevant that we're clear in the EXPLAIN ANALYZE output when no spilling occurred in this particular execution of the given plan. For the "text" EXPLAIN format, we still hide these properties when no spilling occurs. This EXPLAIN format is designed to be easy for humans to read. To maintain the readability we have a higher threshold for which properties we display for this format. Discussion: https://postgr.es/m/CAApHDvo_dmNozQQTmN-2jGp1vT%3Ddxx7Q0vd%2BMvD1cGpv2HU%3DSg%40mail.gmail.com Backpatch-through: 13, where the hashagg spilling code was added.
* Add +(pg_lsn,numeric) and -(pg_lsn,numeric) operators.Fujii Masao2020-06-30
| | | | | | | | | | | By using these operators, the number of bytes can be added into and subtracted from LSN. Bump catalog version. Author: Fujii Masao Reviewed-by: Kyotaro Horiguchi, Michael Paquier, Asif Rehman Discussion: https://postgr.es/m/ed9f7f74-e996-67f8-554a-52ebd3779b3b@oss.nttdata.com
* Mop up some no-longer-necessary hacks around printf %.*s format.Tom Lane2020-06-29
| | | | | | | | | | | | | | | | | | Commit 54cd4f045 added some kluges to work around an old glibc bug, namely that %.*s could misbehave if glibc thought any characters in the supplied string were incorrectly encoded. Now that we use our own snprintf.c implementation, we need not worry about that bug (even if it still exists in the wild). Revert a couple of particularly ugly hacks, and remove or improve assorted comments. Note that there can still be encoding-related hazards here: blindly clipping at a fixed length risks producing wrongly-encoded output if the clip splits a multibyte character. However, code that's doing correct multibyte-aware clipping doesn't really need a comment about that, while code that isn't needs an explanation why not, rather than a red-herring comment about an obsolete bug. Discussion: https://postgr.es/m/279428.1593373684@sss.pgh.pa.us
* nbtree: Correct inaccurate split location comment.Peter Geoghegan2020-06-29
| | | | Minor oversight in commit fab25024338.
* Avoid using %c printf format for potentially non-ASCII characters.Tom Lane2020-06-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since %c only passes a C "char" to printf, it's incapable of dealing with multibyte characters. Passing just the first byte of such a character leads to an output string that is visibly not correctly encoded, resulting in undesirable behavior such as encoding conversion failures while sending error messages to clients. We've lived with this issue for a long time because it was inconvenient to avoid in a portable fashion. However, now that we always use our own snprintf code, it's reasonable to use the %.*s format to print just one possibly-multibyte character in a string. (We previously avoided that obvious-looking answer in order to work around glibc's bug #6530, cf commits 54cd4f045 and ed437e2b2.) Hence, run around and fix a bunch of places that used %c to report a character found in a user-supplied string. For simplicity, I did not touch places that were emitting non-user-facing debug messages, or reporting catalog data that should always be ASCII. (It's also unclear how useful this approach could be in frontend code, where it's less certain that we know what encoding we're dealing with.) In passing, improve a couple of poorly-written error messages in pageinspect/heapfuncs.c. This is a longstanding issue, but I'm hesitant to back-patch because of the impact on translatable message strings. In any case this fix would not work reliably before v12. Tom Lane and Quan Zongliang Discussion: https://postgr.es/m/a120087c-4c88-d9d4-1ec5-808d7a7f133d@gmail.com
* Add current substring regular expression syntaxPeter Eisentraut2020-06-29
| | | | | | | | | | | | | | | | | | | SQL:1999 had syntax SUBSTRING(text FROM pattern FOR escapechar) but this was replaced in SQL:2003 by the more clear SUBSTRING(text SIMILAR pattern ESCAPE escapechar) but this was never implemented in PostgreSQL. This patch adds that new syntax as an alternative in the parser, and updates documentation and tests to indicate that this is the preferred alternative now. Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com> Reviewed-by: Vik Fearing <vik@postgresfriends.org> Reviewed-by: Fabien COELHO <coelho@cri.ensmp.fr> Discussion: https://www.postgresql.org/message-id/flat/a15db31c-d0f8-8ce0-9039-578a31758adb%402ndquadrant.com
* Clean up grammar a bitPeter Eisentraut2020-06-29
| | | | | | | | | | Simplify the grammar specification of substring() and overlay() a bit, simplify and update some comments. Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com> Reviewed-by: Vik Fearing <vik@postgresfriends.org> Reviewed-by: Fabien COELHO <coelho@cri.ensmp.fr> Discussion: https://www.postgresql.org/message-id/flat/a15db31c-d0f8-8ce0-9039-578a31758adb%402ndquadrant.com
* Refactor ObjectAddress field assignments for type dependenciesMichael Paquier2020-06-29
| | | | | | | | | | | | | | | | The logic used to build the set of dependencies needed for a type is rather repetitive with direct assignments for each ObjectAddress field. This refactors the code to use the macro ObjectAddressSet() instead, to do the same work. There are more areas of the backend code that could use this macro, but these are left for a follow-up patch that will partially rework the way dependencies are recorded as well. Type dependencies are left out of the follow-up patch, so they are refactored separately here. Extracted from a larger patch by the same author. Author: Daniel Gustafsson Discussion: https://potgr.es/m/20190213182737.mxn6hkdxwrzgxk35@alap3.anarazel.de
* Fix list of SSL error codes for older OpenSSL versions.Tom Lane2020-06-27
| | | | | Apparently 1.0.1 lacks SSL_R_VERSION_TOO_HIGH and SSL_R_VERSION_TOO_LOW. Per buildfarm.
* Add hints about protocol-version-related SSL connection failures.Tom Lane2020-06-27
| | | | | | | | | | | | | | | | | OpenSSL's native reports about problems related to protocol version restrictions are pretty opaque and inconsistent. When we get an SSL error that is plausibly due to this, emit a hint message that includes the range of SSL protocol versions we (think we) are allowing. This should at least get the user thinking in the right direction to resolve the problem, even if the hint isn't totally accurate, which it might not be for assorted reasons. Back-patch to v13 where we increased the default minimum protocol version, thereby increasing the risk of this class of failure. Patch by me, reviewed by Daniel Gustafsson Discussion: https://postgr.es/m/a9408304-4381-a5af-d259-e55d349ae4ce@2ndquadrant.com
* Remove duplicate check added by commit b2a5545bd6.Amit Kapila2020-06-27
| | | | | | | As this doesn't cause any harm so we decided to this clean up in HEAD only. Author: Ádám Balogh Discussion: https://postgr.es/m/VI1PR0702MB36631BD67559461AFDE1FEEE81920@VI1PR0702MB3663.eurprd07.prod.outlook.com
* Persist slot invalidation correctlyAlvaro Herrera2020-06-26
| | | | | | | | | | | | | | | We failed to save slot to disk after invalidating it, so the state was lost in case of server restart or crash. Fix by marking it dirty and flushing. Also, if the slot is known invalidated we don't need to reason about the LSN at all -- it's known invalidated. Only test the LSN if the slot is known not invalidated. Author: Fujii Masao <masao.fujii@oss.nttdata.com> Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/17a69cfe-f1c1-a416-ee25-ae15427c69eb@oss.nttdata.com
* Fix misuse of table_index_fetch_tuple_check().Peter Geoghegan2020-06-25
| | | | | | | | | | | | | Commit 0d861bbb, which added deduplication to nbtree, had _bt_check_unique() pass a TID to table_index_fetch_tuple_check() that isn't safe to mutate. table_index_fetch_tuple_check()'s tid argument is modified when the TID in question is not the latest visible tuple in a hot chain, though this wasn't documented. To fix, go back to using a local copy of the TID in _bt_check_unique(), and update comments above table_index_fetch_tuple_check(). Backpatch: 13-, where B-Tree deduplication was introduced.
* Remove erroneous assertion from pg_copy_logical_replication_slot().Fujii Masao2020-06-25
| | | | | | | | | | | | | | | | | | | | | | | | If restart_lsn of logical replication slot gets behind more than max_slot_wal_keep_size from the current LSN, the logical replication slot would be invalidated and its restart_lsn is reset to an invalid LSN. If this logical replication slot with an invalid restart_lsn was specified as the source slot in pg_copy_logical_replication_slot(), the function caused the assertion failure unexpectedly. This assertion was added because restart_lsn should not be invalid before. But in v13, it can be invalid thanks to max_slot_wal_keep_size. So since this assertion is no longer useful, this commit removes it. This commit also changes the errcode in the error message that pg_copy_logical_replication_slot() emits when the slot with an invalid restart_lsn is specified, to more appropriate one. Back-patch to v13 where max_slot_wal_keep_size was added and the assertion was no longer valid. Author: Fujii Masao Reviewed-by: Alvaro Herrera, Kyotaro Horiguchi Discussion: https://postgr.es/m/f91de4fb-a7ab-b90e-8132-74796e049d51@oss.nttdata.com
* Adjust max_slot_wal_keep_size behavior per reviewAlvaro Herrera2020-06-24
| | | | | | | | | | | | | | | | | | | | | | | | | | In pg_replication_slot, change output from normal/reserved/lost to reserved/extended/unreserved/ lost, which better expresses the possible states particularly near the time where segments are no longer safe but checkpoint has not run yet. Under the new definition, reserved means the slot is consuming WAL that's still under the normal WAL size constraints; extended means it's consuming WAL that's being protected by wal_keep_segments or the slot itself, whose size is below max_slot_wal_keep_size; unreserved means the WAL is no longer safe, but checkpoint has not yet removed those files. Such as slot is in imminent danger, but can still continue for a little while and may catch up to the reserved WAL space. Also, there were some bugs in the calculations used to report the status; fixed those. Backpatch to 13. Reported-by: Fujii Masao <masao.fujii@oss.nttdata.com> Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reviewed-by: Fujii Masao <masao.fujii@oss.nttdata.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/20200616.120236.1809496990963386593.horikyota.ntt@gmail.com
* Save slot's restart_lsn when invalidated due to sizeAlvaro Herrera2020-06-24
| | | | | | | | | | | | | We put it aside as invalidated_at, which let us show "lost" in pg_replication slot. Prior to this change, the state value was reported as NULL. Backpatch to 13. Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/20200617.101707.1735599255100002667.horikyota.ntt@gmail.com Discussion: https://postgr.es/m/20200407.120905.1507671100168805403.horikyota.ntt@gmail.com
* Add parens to ConvertToXSegs macroAlvaro Herrera2020-06-24
| | | | | | | The current definition is dangerous. No bugs exist in our code at present, but backpatch to 11 nonetheless where it was introduced. Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
* Fix comment in heap.cMichael Paquier2020-06-24
| | | | | | | | | | The description of InsertPgAttributeTuple() does not match its handling of pg_attribute contents with NULL values for a long time, with 911e702 making things more inconsistent. This adjusts the description to match the reality. Author: Daniel Gustafsson Discussion: https://postgr.es/m/4E4E4B33-9FDF-4D21-B77A-642D027AEAD9@yesql.se
* Undo double-quoting of index names in non-text EXPLAIN output formats.Tom Lane2020-06-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | explain_get_index_name() applied quote_identifier() to the index name. This is fine for text output, but the non-text output formats all have their own quoting conventions and would much rather start from the actual index name. For example in JSON you'd get something like "Index Name": "\"My Index\"", which is surely not desirable, especially when the same does not happen for table names. Hence, move the responsibility for applying quoting out to the callers, where it can go into already-existing special code paths for text format. This changes the API spec for users of explain_get_index_name_hook: before, they were supposed to apply quote_identifier() if necessary, now they should not. Research suggests that the only publicly available user of the hook is hypopg, and it actually forgot to apply quoting anyway, so it's fine. (In any case, there's no behavioral change for the output of a hook as seen in non-text EXPLAIN formats, so this won't break any case that programs should be relying on.) Digging in the commit logs, it appears that quoting was included in explain_get_index_name's duties when commit 604ffd280 invented it; and that was fine at the time because we only had text output format. This should have been rethought when non-text formats were invented, but it wasn't. This is a fairly clear bug for users of non-text EXPLAIN formats, so back-patch to all supported branches. Per bug #16502 from Maciek Sakrejda. Patch by me (based on investigation by Euler Taveira); thanks to Julien Rouhaud for review. Discussion: https://postgr.es/m/16502-57bd1c9f913ed1d1@postgresql.org
* Fix masking of SP-GiST pages during xlog consistency checkAlexander Korotkov2020-06-20
| | | | | | | | | | spg_mask() didn't take into account that pd_lower equal to SizeOfPageHeaderData is still valid value. This commit fixes that. Backpatch to 11, where spg_mask() pg_lower check was introduced. Reported-by: Michael Paquier Discussion: https://postgr.es/m/20200615131405.GM52676%40paquier.xyz Backpatch-through: 11
* Remove dead forceSync parameter of XactLogCommitRecord().Noah Misch2020-06-20
| | | | | | | | | | | | The function has been reading global variable forceSyncCommit, mirroring the intent of the caller that passed forceSync=forceSyncCommit. The other caller, RecordTransactionCommitPrepared(), passed false. Since COMMIT PREPARED can't share a transaction with any command, it certainly doesn't share a transaction with a command that sets forceSyncCommit. Reviewed by Michael Paquier. Discussion: https://postgr.es/m/20200617032615.GC2916904@rfd.leadboat.com
* Removal unused function parameter in CopyReadBinaryAttribute.Amit Kapila2020-06-20
| | | | | | | | | | | | The function parameter column_no is not used in CopyReadBinaryAttribute, this can be removed. Commit 0e319c7ad7 removed the usage of column_no parameter in function CopyReadBinaryAttribute but forgot to remove the parameter. Reported-by: Vignesh C Author: Vignesh C Discussion: https://postgr.es/m/CALDaNm1TYSNTfqx_jfz9_mwEZ2Er=dZnu++duXpC1uQo1cG=WA@mail.gmail.com
* Fix deduplication "single value" strategy bug.Peter Geoghegan2020-06-19
| | | | | | | | | | | | | | | | It was possible for deduplication's single value strategy to mistakenly believe that a very small duplicate tuple counts as one of the six large tuples that it aims to leave behind after the page finally splits. This could cause slightly suboptimal space utilization with very low cardinality indexes, though only under fairly narrow conditions. To fix, be particular about what kind of tuple counts as a maxpostingsize-capped tuple. This avoids confusion in the event of a small tuple that gets "wedged" between two large tuples, where all tuples on the page are duplicates of the same value. Discussion: https://postgr.es/m/CAH2-Wz=Y+sgSFc-O3LpiZX-POx2bC+okec2KafERHuzdVa7-rQ@mail.gmail.com Backpatch: 13-, where deduplication was introduced (by commit 0d861bbb)
* Fix issues in invalidation of obsolete replication slots.Fujii Masao2020-06-19
| | | | | | | | | | | | | | | | | | | | | | This commit fixes the following issues. 1. There is the case where the slot is dropped while trying to invalidate it. InvalidateObsoleteReplicationSlots() did not handle this case, and which could cause checkpoint to fail. 2. InvalidateObsoleteReplicationSlots() could emit the same log message multiple times unnecessary. It should be logged only once. 3. When marking the slot as used, we always searched the target slot from all the replication slots even if we already found it. This could cause useless waste of cycles. Back-patch to v13 where these issues were added as a part of max_slot_wal_keep_size code. Author: Fujii Masao Reviewed-by: Kyotaro Horiguchi, Alvaro Herrera Discussion: https://postgr.es/m/66c05b67-3396-042c-1b41-bfa6c3ddcf82@oss.nttdata.com
* Fix EXPLAIN ANALYZE for parallel HashAgg plansDavid Rowley2020-06-19
| | | | | | | | | | | | | | | Since 1f39bce02, HashAgg nodes have had the ability to spill to disk when memory consumption exceeds work_mem. That commit added new properties to EXPLAIN ANALYZE to show the maximum memory usage and disk usage, however, it didn't quite go as far as showing that information for parallel workers. Since workers may have experienced something very different from the main process, we should show this information per worker, as is done in Sort. Reviewed-by: Justin Pryzby Reviewed-by: Jeff Davis Discussion: https://postgr.es/m/CAApHDvpEKbfZa18mM1TD7qV6PG+w97pwCWq5tVD0dX7e11gRJw@mail.gmail.com Backpatch-through: 13, where the hashagg spilling code was added.
* Clean up includes of s_lock.h.Andres Freund2020-06-18
| | | | | | | Users of spinlocks should use spin.h, not s_lock.h. And lwlock.h hasn't utilized spinlocks for quite a while. Discussion: https://postgr.es/m/20200618183041.upyrd25eosecyf3x@alap3.anarazel.de