aboutsummaryrefslogtreecommitdiff
path: root/src/backend/access
Commit message (Collapse)AuthorAge
* Fix lockmode initialization for custom relation optionsMichael Paquier2019-09-27
| | | | | | | | | | | | | The code was enforcing AccessExclusiveLock for all custom relation options, which is incorrect as the APIs allow a custom lock level to be set. While on it, fix a couple of inconsistencies in the tests and the README of dummy_index_am. Oversights in commit 773df88. Discussion: https://postgr.es/m/20190925234152.GA2115@paquier.xyz
* Fix comment in xlogreader.cMichael Paquier2019-09-26
| | | | | | | | | This has been introduced by 709d003, that has moved readSegNo, readOff and readPageTLI into a new structure called WALOpenSegment initialized separately. Author: Kyotaro Horiguchi Discussion: https://postgr.es/m/20190926.110809.248342687.horikyota.ntt@gmail.com
* Support reloptions of enum typeAlvaro Herrera2019-09-25
| | | | | | | | | | | | | | | | All our current in core relation options of type string (not many, admittedly) behave in reality like enums. But after seeing an implementation for enum reloptions, it's clear that strings are messier, so introduce the new reloption type. Switch all string options to be enums instead. Fortunately we have a recently introduced test module for reloptions, so we don't lose coverage of string reloptions, which may still be used by third-party modules. Authors: Nikolay Shaplov, Álvaro Herrera Reviewed-by: Nikita Glukhov, Aleksandr Parfenov Discussion: https://postgr.es/m/43332102.S2V5pIjXRx@x200m
* Allow definition of lock mode for custom reloptionsMichael Paquier2019-09-25
| | | | | | | | | | | Relation options can define a lock mode other than AccessExclusiveMode since 47167b7, but modules defining custom relation options did not really have a way to enforce that. Correct that by extending the current API set so as modules can define a custom lock mode. Author: Michael Paquier Reviewed-by: Kuntal Ghosh Discussion: https://postgr.es/m/20190920013831.GD1844@paquier.xyz
* Fix failure with lock mode used for custom relation optionsMichael Paquier2019-09-25
| | | | | | | | | | | | | | | | | | | | | In-core relation options can use a custom lock mode since 47167b7, that has lowered the lock available for some autovacuum parameters. However it forgot to consider custom relation options. This causes failures with ALTER TABLE SET when changing a custom relation option, as its lock is not defined. The existing APIs to define a custom reloption does not allow to define a custom lock mode, so enforce its initialization to AccessExclusiveMode which should be safe enough in all cases. An upcoming patch will extend the existing APIs to allow a custom lock mode to be defined. The problem can be reproduced with bloom indexes, so add a test there. Reported-by: Nikolay Sharplov Analyzed-by: Thomas Munro, Michael Paquier Author: Michael Paquier Reviewed-by: Kuntal Ghosh Discussion: https://postgr.es/m/20190920013831.GD1844@paquier.xyz Backpatch-through: 9.6
* Fix bug in pairingheap_SpGistSearchItem_cmp()Alexander Korotkov2019-09-25
| | | | | | | | | Our item contains only so->numberOfNonNullOrderBys of distances. Reflect that in the loop upper bound. Discussion: https://postgr.es/m/53536807-784c-e029-6e92-6da802ab8d60%40postgrespro.ru Author: Nikita Glukhov Backpatch-through: 12
* Rework WAL-reading supporting structsAlvaro Herrera2019-09-24
| | | | | | | | | | | | | | The state-tracking of WAL reading in various places was pretty messy, mostly because the ancient physical-replication WAL reading code wasn't using the XLogReader abstraction. This led to some untidy code. Make it prettier by creating two additional supporting structs, WALSegmentContext and WALOpenSegment which keep track of WAL-reading state. This makes code cleaner, as well as supports more future cleanup. Author: Antonin Houska Reviewed-by: Álvaro Herrera and (older versions) Robert Haas Discussion: https://postgr.es/m/14984.1554998742@spoje.net
* Speedup truncations of relation forks.Fujii Masao2019-09-24
| | | | | | | | | | | | | | | | | | When a relation is truncated, shared_buffers needs to be scanned so that any buffers for the relation forks are invalidated in it. Previously, shared_buffers was scanned for each relation forks, i.e., MAIN, FSM and VM, when VACUUM truncated off any empty pages at the end of relation or TRUNCATE truncated the relation in place. Since shared_buffers needed to be scanned multiple times, it could take a long time to finish those commands especially when shared_buffers was large. This commit changes the logic so that shared_buffers is scanned only one time for those three relation forks. Author: Kirk Jamison Reviewed-by: Masahiko Sawada, Thomas Munro, Alvaro Herrera, Takayuki Tsunakawa and Fujii Masao Discussion: https://postgr.es/m/D09B13F772D2274BB348A310EE3027C64E2067@g01jpexmbkw24
* Message style fixesPeter Eisentraut2019-09-23
|
* Fix freeing old values in index_store_float8_orderby_distances()Alexander Korotkov2019-09-20
| | | | | | | | | | | | | | | 6cae9d2c10 has added an error in freeing old values in index_store_float8_orderby_distances() function. It looks for old value in scan->xs_orderbynulls[i] after setting a new value there. This commit fixes that. Also it removes short-circuit in handling distances == NULL situation. Now distances == NULL will be treated the same way as array with all null distances. That is, previous values will be freed if any. Reported-by: Tom Lane, Nikita Glukhov Discussion: https://postgr.es/m/CAPpHfdu2wcoAVAm3Ek66rP%3Duo_C-D84%2B%2Buf1VEcbyi_caBXWCA%40mail.gmail.com Discussion: https://postgr.es/m/426580d3-a668-b9d1-7b8e-f74d1a6524e0%40postgrespro.ru Backpatch-through: 12
* Improve handling of NULLs in KNN-GiST and KNN-SP-GiSTAlexander Korotkov2019-09-19
| | | | | | | | | | | | | | | | | | | | | This commit improves subject in two ways: * It removes ugliness of 02f90879e7, which stores distance values and null flags in two separate arrays after GISTSearchItem struct. Instead we pack both distance value and null flag in IndexOrderByDistance struct. Alignment overhead should be negligible, because we typically deal with at most few "col op const" expressions in ORDER BY clause. * It fixes handling of "col op NULL" expression in KNN-SP-GiST. Now, these expression are not passed to support functions, which can't deal with them. Instead, NULL result is implicitly assumed. It future we may decide to teach support functions to deal with NULL arguments, but current solution is bugfix suitable for backpatch. Reported-by: Nikita Glukhov Discussion: https://postgr.es/m/826f57ee-afc7-8977-c44c-6111d18b02ec%40postgrespro.ru Author: Nikita Glukhov Reviewed-by: Alexander Korotkov Backpatch-through: 9.4
* Add some const decorations to array constantsPeter Eisentraut2019-09-17
| | | | | Author: Mark G <markg735@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAEeOP_YFVeFjq4zDZLDQbLSRFxBiTpwBQHxCNgGd%2Bp5VztTXyQ%40mail.gmail.com
* Fix nbtree page split rmgr desc routine.Peter Geoghegan2019-09-12
| | | | | | | | | | | | Include newitemoff in rmgr desc output for nbtree page split records. In passing, correct an obsolete comment that claimed that newitemoff is only logged for _L variant nbtree page split WAL records. Both issues were oversights in commit 2c03216d831, which revamped the WAL format. Author: Peter Geoghegan Backpatch: 9.5-, where the WAL format was revamped.
* Remove redundant _bt_truncate() comment paragraph.Peter Geoghegan2019-09-12
|
* Add _bt_binsrch() scantid assertion to nbtree.Peter Geoghegan2019-09-09
| | | | | | | | | | Assert that _bt_binsrch() binary searches with scantid set in insertion scankey cannot be performed on leaf pages. Leaf-level binary searches where scantid is set must use _bt_binsrch_insert() instead. _bt_binsrch_insert() is likely to have additional responsibilities in the future, such as searching within GIN-style posting lists using scantid. It seems like a good idea to tighten things up now.
* Fix handling of NULL distances in KNN-GiSTAlexander Korotkov2019-09-08
| | | | | | | | | | | | | | In order to implement NULL LAST semantic GiST previously assumed distance to the NULL value to be Inf. However, our distance functions can return Inf and NaN for non-null values. In such cases, NULL LAST semantic appears to be broken. This commit fixes that by introducing separate array of null flags for distances. Backpatch to all supported versions. Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Backpatch-through: 9.4
* Fix handling Inf and Nan values in GiST pairing heap comparatorAlexander Korotkov2019-09-08
| | | | | | | | | | | | | | | | Previously plain float comparison was used in GiST pairing heap. Such comparison doesn't provide proper ordering for value sets containing Inf and Nan values. This commit fixes that by usage of float8_cmp_internal(). Note, there is remaining problem with NULL distances, which are represented as Inf in pairing heap. It would be fixes in subsequent commit. Backpatch to all supported versions. Reported-by: Andrey Borodin Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Heikki Linnakangas Backpatch-through: 9.4
* Fix behavior of AND CHAIN outside of explicit transaction blocksPeter Eisentraut2019-09-08
| | | | | | | | | | | | | When using COMMIT AND CHAIN or ROLLBACK AND CHAIN not in an explicit transaction block, the previous implementation would leave a transaction block active in the ROLLBACK case but not the COMMIT case. To fix for now, error out when using these commands not in an explicit transaction block. This restriction could be lifted if a sensible definition and implementation is found. Bug: #15977 Author: fn ln <emuser20140816@gmail.com> Reviewed-by: Fabien COELHO <coelho@cri.ensmp.fr>
* Create an API for inserting and deleting rows in TOAST tables.Robert Haas2019-09-06
| | | | | | | | | | | | | | | This moves much of the non-heap-specific logic from toast_delete and toast_insert_or_update into a helper functions accessible via a new header, toast_helper.h. Using the functions in this module, a table AM can implement creation and deletion of TOAST table rows with much less code duplication than was possible heretofore. Some table AMs won't want to use the TOAST logic at all, but for those that do this will make that easier. Patch by me, reviewed and tested by Prabhat Sabu, Thomas Munro, Andres Freund, and Álvaro Herrera. Discussion: http://postgr.es/m/CA+TgmoZv-=2iWM4jcw5ZhJeL18HF96+W1yJeYrnGMYdkFFnEpQ@mail.gmail.com
* Make pg_promote() detect postmaster death while waiting for promotion to end.Fujii Masao2019-09-06
| | | | | | | | | | | | | | | | Previously even if postmaster died and WaitLatch() woke up with that event while pg_promote() was waiting for the standby promotion to finish, pg_promote() did nothing special and kept waiting until timeout occurred. This could cause a busy loop. This patch make pg_promote() return false immediately when postmaster dies, to avoid such a busy loop. Back-patch to v12 where pg_promote() was added. Author: Fujii Masao Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CAHGQGwEs9ROgSp+QF+YdDU+xP8W=CY1k-_Ov-d_Z3JY+to3eXA@mail.gmail.com
* Split tuptoaster.c into three separate files.Robert Haas2019-09-05
| | | | | | | | | | | | | | | | | | | | | | | | detoast.c/h contain functions required to detoast a datum, partially or completely, plus a few other utility functions for examining the size of toasted datums. toast_internals.c/h contain functions that are used internally to the TOAST subsystem but which (mostly) do not need to be accessed from outside. heaptoast.c/h contains code that is intrinsically specific to the heap AM, either because it operates on HeapTuples or is based on the layout of a heap page. detoast.c and toast_internals.c are placed in src/backend/access/common rather than src/backend/access/heap. At present, both files still have dependencies on the heap, but that will be improved in a future commit. Patch by me, reviewed and tested by Prabhat Sabu, Thomas Munro, Andres Freund, and Álvaro Herrera. Discussion: http://postgr.es/m/CA+TgmoZv-=2iWM4jcw5ZhJeL18HF96+W1yJeYrnGMYdkFFnEpQ@mail.gmail.com
* Make XLogReaderInvalReadState staticAlvaro Herrera2019-09-03
| | | | | | | | | | | | | This function is only used by xlogreader.c itself, so there's no need to export it. It was introduced by commit 3b02ea4f0780 with the apparent intention that it could be used externally, but I couldn't find any external code calling it. I (Álvaro) couldn't resist the urge to sort nearby function prototypes properly while at it. Author: Antonin Houska Discussion: https://postgr.es/m/14984.1554998742@spoje.net
* Remove 'msg' parameter from convert_tuples_by_nameAlvaro Herrera2019-09-03
| | | | | | | | | | The message was included as a parameter when this function was added in dcb2bda9b704, but I don't think it has ever served any useful purpose. Let's stop spreading it pointlessly. Reviewed by Amit Langote and Peter Eisentraut. Discussion: https://postgr.es/m/20190806224728.GA17233@alvherre.pgsql
* Better error messages for short reads/writes in SLRUPeter Eisentraut2019-09-03
| | | | | | | | | | | | This avoids getting a Could not read from file ...: Success. for a short read or write (since errno is not set in that case). Instead, report a more specific error messages. Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/5de61b6b-8be9-7771-0048-860328efe027%402ndquadrant.com
* Avoid touching replica identity index in ExtractReplicaIdentity().Tom Lane2019-09-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In what seems like a fit of misplaced optimization, ExtractReplicaIdentity() accessed the relation's replica-identity index without taking any lock on it. Usually, the surrounding query already holds some lock so this is safe enough ... but in the case of a previously-planned delete, there might be no existing lock. Given a suitable test case, this is exposed in v12 and HEAD by an assertion added by commit b04aeb0a0. The whole thing's rather poorly thought out anyway; rather than looking directly at the index, we should use the index-attributes bitmap that's held by the parent table's relcache entry, as the caller functions do. This is more consistent and likely a bit faster, since it avoids a cache lookup. Hence, change to doing it that way. While at it, rather than blithely assuming that the identity columns are non-null (with catastrophic results if that's wrong), add assertion checks that they aren't null. Possibly those should be actual test-and-elog, but I'll leave it like this for now. In principle, this is a bug that's been there since this code was introduced (in 9.4). In practice, the risk seems quite low, since we do have a lock on the index's parent table, so concurrent changes to the index's catalog entries seem unlikely. Given the precedent that commit 9c703c169 wasn't back-patched, I won't risk back-patching this further than v12. Per report from Hadi Moshayedi. Discussion: https://postgr.es/m/CAK=1=Wrek44Ese1V7LjKiQS-Nd-5LgLi_5_CskGbpggKEf3tKQ@mail.gmail.com
* Fix overflow check and comment in GIN posting list encoding.Heikki Linnakangas2019-08-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The comment did not match what the code actually did for integers with the 43rd bit set. You get an integer like that, if you have a posting list with two adjacent TIDs that are more than 2^31 blocks apart. According to the comment, we would store that in 6 bytes, with no continuation bit on the 6th byte, but in reality, the code encodes it using 7 bytes, with a continuation bit on the 6th byte as normal. The decoding routine also handled these 7-byte integers correctly, except for an overflow check that assumed that one integer needs at most 6 bytes. Fix the overflow check, and fix the comment to match what the code actually does. Also fix the comment that claimed that there are 17 unused bits in the 64-bit representation of an item pointer. In reality, there are 64-32-11=21. Fitting any item pointer into max 6 bytes was an important property when this was written, because in the old pre-9.4 format, item pointers were stored as plain arrays, with 6 bytes for every item pointer. The maximum of 6 bytes per integer in the new format guaranteed that we could convert any page from the old format to the new format after upgrade, so that the new format was never larger than the old format. But we hardly need to worry about that anymore, and running into that problem during upgrade, where an item pointer is expanded from 6 to 7 bytes such that the data doesn't fit on a page anymore, is implausible in practice anyway. Backpatch to all supported versions. This also includes a little test module to test these large distances between item pointers, without requiring a 16 TB table. It is not backpatched, I'm including it more for the benefit of future development of new posting list formats. Discussion: https://www.postgresql.org/message-id/33bfc20a-5c86-f50c-f5a5-58e9925d05ff%40iki.fi Reviewed-by: Masahiko Sawada, Alexander Korotkov
* Remove obsolete nbtree page deletion comment.Peter Geoghegan2019-08-27
| | | | | | Commit efada2b8e92, which made the nbtree page deletion algorithm more robust, removed the concept of a half-dead internal page. Remove a comment about half dead parent pages that was overlooked.
* Explain subtlety in nbtree locking protocol.Peter Geoghegan2019-08-23
| | | | | | | The Postgres approach to coupling locks during an ascent of the tree is slightly different to the approach taken by Lehman and Yao. Add a new paragraph to the "Differences to the Lehman & Yao algorithm" section of the nbtree README that explains the similarities and differences.
* Update comments on nbtree stack struct.Peter Geoghegan2019-08-21
| | | | | | | | Adjust the struct comment that describes how page splits use their descent stack to cascade up the tree from the leaf level. In passing, fix up some unrelated nbtree comments that had typos or were obsolete.
* Fix bogus commentAlvaro Herrera2019-08-20
| | | | | Author: Alexander Lakhin Discussion: https://postgr.es/m/20190819072244.GE18166@paquier.xyz
* Fix inconsistencies and typos in the tree, take 11Michael Paquier2019-08-19
| | | | | | | | This fixes various typos in docs and comments, and removes some orphaned definitions. Author: Alexander Lakhin Discussion: https://postgr.es/m/5da8e325-c665-da95-21e0-c8a99ea61fbf@gmail.com
* Remove fmgr.h includes from headers that don't really need it.Andres Freund2019-08-16
| | | | | | | | | Most of the fmgr.h includes were obsoleted by 352a24a1f9d6f7d4abb1. A few others can be obsoleted using the underlying struct type in an implementation detail. Author: Andres Freund Discussion: https://postgr.es/m/20190803193733.g3l3x3o42uv4qj7l@alap3.anarazel.de
* Remove block number field from nbtree stack.Peter Geoghegan2019-08-14
| | | | | | | | | | | | | | The initial value of the nbtree stack downlink block number field recorded during an initial descent of the tree wasn't actually used. Both _bt_getstackbuf() callers overwrote the value with their own value. Remove the block number field from the stack struct, and add a child block number argument to _bt_getstackbuf() in its place. This makes the overall design of _bt_getstackbuf() clearer. Author: Peter Geoghegan Reviewed-By: Anastasia Lubennikova Discussion: https://postgr.es/m/CAH2-Wzmx+UbXt2YNOUCZ-a04VdXU=S=OHuAuD7Z8uQq-PXTYUg@mail.gmail.com
* Remove obsolete nbtree README commentary.Peter Geoghegan2019-08-13
| | | | | | | | | | | | | Commit d2086b08b02 removed almost all cases where nbtree must release a read buffer lock and acquire a write buffer lock instead, so remaining cases in which that's still necessary are not notable enough to appear in the nbtree README. More importantly, holding on to a buffer pin in cases where nbtree must trade a read lock for a write lock is very unlikely to save any I/O. This seems to have been a long overlooked throwback to a time when nbtree cared about write-ordering dependencies, and performed synchronous buffer writes. It hasn't worked that way in many years.
* Use PageIndexTupleOverwrite() within nbtree.Peter Geoghegan2019-08-13
| | | | | | | | | | | | Use the PageIndexTupleOverwrite() bufpage.c routine within nbtree instead of deleting a tuple and re-inserting its replacement. This makes the intent of affected code slightly clearer. It also makes CREATE INDEX slightly faster, since there is no longer a need to shift every leaf page's line pointer array back and forth during index builds. Author: Peter Geoghegan, Anastasia Lubennikova Reviewed-By: Anastasia Lubennikova Discussion: https://postgr.es/m/CAH2-Wz=Zk=B9+Vwm376WuO7YTjFc2SSskifQm4Nme3RRRPtOSQ@mail.gmail.com
* Fix inconsistencies and typos in the tree, take 10Michael Paquier2019-08-13
| | | | | | | | | This addresses some issues with unnecessary code comments, fixes various typos in docs and comments, and removes some orphaned structures and definitions. Author: Alexander Lakhin Discussion: https://postgr.es/m/9aabc775-5494-b372-8bcb-4dfc0bd37c68@gmail.com
* Fix predicate-locking of HOT updated rows.Heikki Linnakangas2019-08-07
| | | | | | | | | | | | | | | | | | | | | | | In serializable mode, heap_hot_search_buffer() incorrectly acquired a predicate lock on the root tuple, not the returned tuple that satisfied the visibility checks. As explained in README-SSI, the predicate lock does not need to be copied or extended to other tuple versions, but for that to work, the correct, visible, tuple version must be locked in the first place. The original SSI commit had this bug in it, but it was fixed back in 2013, in commit 81fbbfe335. But unfortunately, it was reintroduced a few months later in commit b89e151054. Wising up from that, add a regression test to cover this, so that it doesn't get reintroduced again. Also, move the code that sets 't_self', so that it happens at the same time that the other HeapTuple fields are set, to make it more clear that all the code in the loop operate on the "current" tuple in the chain, not the root tuple. Bug spotted by Andres Freund, analysis and original fix by Thomas Munro, test case and some additional changes to the fix by Heikki Linnakangas. Backpatch to all supported versions (9.4). Discussion: https://www.postgresql.org/message-id/20190731210630.nqhszuktygwftjty%40alap3.anarazel.de
* Fix inconsistencies and typos in the tree, take 9Michael Paquier2019-08-05
| | | | | | | | This addresses more issues with code comments, variable names and unreferenced variables. Author: Alexander Lakhin Discussion: https://postgr.es/m/7ab243e0-116d-3e44-d120-76b3df7abefd@gmail.com
* Add error codes to some corruption log messagesPeter Eisentraut2019-08-01
| | | | | | | | | In some cases we have elog(ERROR) while corruption is certain and we can give a clear error code ERRCODE_DATA_CORRUPTED or ERRCODE_INDEX_CORRUPTED. Author: Andrey Borodin <x4mmm@yandex-team.ru> Discussion: https://www.postgresql.org/message-id/flat/25F6C686-6442-4A6B-BAF8-A6F7B84B16DE@yandex-team.ru
* Remove superfluous semicolon.Andres Freund2019-07-30
| | | | Author: Andres Freund
* Fix inconsistencies and typos in the treeMichael Paquier2019-07-29
| | | | | | | | This is numbered take 8, and addresses again a set of issues with code comments, variable names and unreferenced variables. Author: Alexander Lakhin Discussion: https://postgr.es/m/b137b5eb-9c95-9c2f-586e-38aba7d59788@gmail.com
* Use full 64-bit XID for checking if a deleted GiST page is old enough.Heikki Linnakangas2019-07-24
| | | | | | | | | | | Otherwise, after a deleted page gets even older, it becomes unrecyclable again. B-tree has the same problem, and has had since time immemorial, but let's at least fix this in GiST, where this is new. Backpatch to v12, where GiST page deletion was introduced. Reviewed-by: Andrey Borodin Discussion: https://www.postgresql.org/message-id/835A15A5-F1B4-4446-A711-BF48357EB602%40yandex-team.ru
* Refactor checks for deleted GiST pages.Heikki Linnakangas2019-07-24
| | | | | | | | | | | | The explicit check in gistScanPage() isn't currently really necessary, as a deleted page is always empty, so the loop would fall through without doing anything, anyway. But it's a marginal optimization, and it gives a nice place to attach a comment to explain how it works. Backpatch to v12, where GiST page deletion was introduced. Reviewed-by: Andrey Borodin Discussion: https://www.postgresql.org/message-id/835A15A5-F1B4-4446-A711-BF48357EB602%40yandex-team.ru
* Fix inconsistencies and typos in the treeMichael Paquier2019-07-22
| | | | | | | | This is numbered take 7, and addresses a set of issues with code comments, variable names and unreferenced variables. Author: Alexander Lakhin Discussion: https://postgr.es/m/dff75442-2468-f74f-568c-6006e141062f@gmail.com
* Fix nbtree metapage cache upgrade bug.Peter Geoghegan2019-07-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 857f9c36cda, which taught nbtree VACUUM to avoid unnecessary index scans, bumped the nbtree version number from 2 to 3, while adding the ability for nbtree indexes to be upgraded on-the-fly. Various assertions that assumed that an nbtree index was always on version 2 had to be changed to accept any supported version (version 2 or 3 on Postgres 11). However, a few assertions were missed in the initial commit, all of which were in code paths that cache a local copy of the metapage metadata, where the index had been expected to be on the current version (no longer version 2) as a generic sanity check. Rather than simply update the assertions, follow-up commit 0a64b45152b intentionally made the metapage caching code update the per-backend cached metadata version without changing the on-disk version at the same time. This could even happen when the planner needed to determine the height of a B-Tree for costing purposes. The assertions only fail on Postgres v12 when upgrading from v10, because they were adjusted to use the authoritative shared memory metapage by v12's commit dd299df8. To fix, remove the cache-only upgrade mechanism entirely, and update the assertions themselves to accept any supported version (go back to using the cached version in v12). The fix is almost a full revert of commit 0a64b45152b on the v11 branch. VACUUM only considers the authoritative metapage, and never bothers with a locally cached version, whereas everywhere else isn't interested in the metapage fields that were added by commit 857f9c36cda. It seems unlikely that this bug has affected any user on v11. Reported-By: Christoph Berg Bug: #15896 Discussion: https://postgr.es/m/15896-5b25e260fdb0b081%40postgresql.org Backpatch: 11-, where VACUUM was taught to avoid unnecessary index scans.
* Avoid using lcons and list_delete_first where it's easy to do so.Tom Lane2019-07-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Formerly, lcons was about the same speed as lappend, but with the new List implementation, that's not so; with a long List, data movement imposes an O(N) cost on lcons and list_delete_first, but not lappend. Hence, invent list_delete_last with semantics parallel to list_delete_first (but O(1) cost), and change various places to use lappend and list_delete_last where this can be done without much violence to the code logic. There are quite a few places that construct result lists using lcons not lappend. Some have semantic rationales for that; I added comments about it to a couple that didn't have them already. In many such places though, I think the coding is that way only because back in the dark ages lcons was faster than lappend. Hence, switch to lappend where this can be done without causing semantic changes. In ExecInitExprRec(), this results in aggregates and window functions that are in the same plan node being executed in a different order than before. Generally, the executions of such functions ought to be independent of each other, so this shouldn't result in visibly different query results. But if you push it, as one regression test case does, you can show that the order is different. The new order seems saner; it's closer to the order of the functions in the query text. And we never documented or promised anything about this, anyway. Also, in gistfinishsplit(), don't bother building a reverse-order list; it's easy now to iterate backwards through the original list. It'd be possible to go further towards removing uses of lcons and list_delete_first, but it'd require more extensive logic changes, and I'm not convinced it's worth it. Most of the remaining uses deal with queues that probably never get long enough to be worth sweating over. (Actually, I doubt that any of the changes in this patch will have measurable performance effects either. But better to have good examples than bad ones in the code base.) Patch by me, thanks to David Rowley and Daniel Gustafsson for review. Discussion: https://postgr.es/m/21272.1563318411@sss.pgh.pa.us
* Fix inconsistencies and typos in the treeMichael Paquier2019-07-16
| | | | | | | | | | | This is numbered take 7, and addresses a set of issues around: - Fixes for typos and incorrect reference names. - Removal of unneeded comments. - Removal of unreferenced functions and structures. - Fixes regarding variable name consistency. Author: Alexander Lakhin Discussion: https://postgr.es/m/10bfd4ac-3e7c-40ab-2b2e-355ed15495e8@gmail.com
* Correct nbtsplitloc.c comment.Peter Geoghegan2019-07-15
| | | | | | | | | | The logic just added by commit e3899ffd falls back on a 50:50 page split in the event of a new item that's just to the right of our provisional "many duplicates" split point. Fix a comment that incorrectly claimed that the new item had to be just to the left of our provisional split point. Backpatch: 12-, just like commit e3899ffd.
* Fix pathological nbtree split point choice issue.Peter Geoghegan2019-07-15
| | | | | | | | | | | | | | | | | | | | | | Specific ever-decreasing insertion patterns could cause successive unbalanced nbtree page splits. Problem cases involve a large group of duplicates to the left, and ever-decreasing insertions to the right. To fix, detect the situation by considering the newitem offset before performing a split using nbtsplitloc.c's "many duplicates" strategy. If the new item was inserted just to the right of our provisional "many duplicates" split point, infer ever-decreasing insertions and fall back on a 50:50 (space delta optimal) split. This seems to barely affect cases that already had acceptable space utilization. An alternative fix also seems possible. Instead of changing nbtsplitloc.c split choice logic, we could instead teach _bt_truncate() to generate a new value for new high keys by interpolating from the lastleft and firstright key values. That would certainly be a more elegant fix, but it isn't suitable for backpatching. Discussion: https://postgr.es/m/CAH2-WznCNvhZpxa__GqAa1fgQ9uYdVc=_apArkW2nc-K3O7_NA@mail.gmail.com Backpatch: 12-, where the nbtree page split enhancements were introduced.
* Represent Lists as expansible arrays, not chains of cons-cells.Tom Lane2019-07-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Originally, Postgres Lists were a more or less exact reimplementation of Lisp lists, which consist of chains of separately-allocated cons cells, each having a value and a next-cell link. We'd hacked that once before (commit d0b4399d8) to add a separate List header, but the data was still in cons cells. That makes some operations -- notably list_nth() -- O(N), and it's bulky because of the next-cell pointers and per-cell palloc overhead, and it's very cache-unfriendly if the cons cells end up scattered around rather than being adjacent. In this rewrite, we still have List headers, but the data is in a resizable array of values, with no next-cell links. Now we need at most two palloc's per List, and often only one, since we can allocate some values in the same palloc call as the List header. (Of course, extending an existing List may require repalloc's to enlarge the array. But this involves just O(log N) allocations not O(N).) Of course this is not without downsides. The key difficulty is that addition or deletion of a list entry may now cause other entries to move, which it did not before. For example, that breaks foreach() and sister macros, which historically used a pointer to the current cons-cell as loop state. We can repair those macros transparently by making their actual loop state be an integer list index; the exposed "ListCell *" pointer is no longer state carried across loop iterations, but is just a derived value. (In practice, modern compilers can optimize things back to having just one loop state value, at least for simple cases with inline loop bodies.) In principle, this is a semantics change for cases where the loop body inserts or deletes list entries ahead of the current loop index; but I found no such cases in the Postgres code. The change is not at all transparent for code that doesn't use foreach() but chases lists "by hand" using lnext(). The largest share of such code in the backend is in loops that were maintaining "prev" and "next" variables in addition to the current-cell pointer, in order to delete list cells efficiently using list_delete_cell(). However, we no longer need a previous-cell pointer to delete a list cell efficiently. Keeping a next-cell pointer doesn't work, as explained above, but we can improve matters by changing such code to use a regular foreach() loop and then using the new macro foreach_delete_current() to delete the current cell. (This macro knows how to update the associated foreach loop's state so that no cells will be missed in the traversal.) There remains a nontrivial risk of code assuming that a ListCell * pointer will remain good over an operation that could now move the list contents. To help catch such errors, list.c can be compiled with a new define symbol DEBUG_LIST_MEMORY_USAGE that forcibly moves list contents whenever that could possibly happen. This makes list operations significantly more expensive so it's not normally turned on (though it is on by default if USE_VALGRIND is on). There are two notable API differences from the previous code: * lnext() now requires the List's header pointer in addition to the current cell's address. * list_delete_cell() no longer requires a previous-cell argument. These changes are somewhat unfortunate, but on the other hand code using either function needs inspection to see if it is assuming anything it shouldn't, so it's not all bad. Programmers should be aware of these significant performance changes: * list_nth() and related functions are now O(1); so there's no major access-speed difference between a list and an array. * Inserting or deleting a list element now takes time proportional to the distance to the end of the list, due to moving the array elements. (However, it typically *doesn't* require palloc or pfree, so except in long lists it's probably still faster than before.) Notably, lcons() used to be about the same cost as lappend(), but that's no longer true if the list is long. Code that uses lcons() and list_delete_first() to maintain a stack might usefully be rewritten to push and pop at the end of the list rather than the beginning. * There are now list_insert_nth...() and list_delete_nth...() functions that add or remove a list cell identified by index. These have the data-movement penalty explained above, but there's no search penalty. * list_concat() and variants now copy the second list's data into storage belonging to the first list, so there is no longer any sharing of cells between the input lists. The second argument is now declared "const List *" to reflect that it isn't changed. This patch just does the minimum needed to get the new implementation in place and fix bugs exposed by the regression tests. As suggested by the foregoing, there's a fair amount of followup work remaining to do. Also, the ENABLE_LIST_COMPAT macros are finally removed in this commit. Code using those should have been gone a dozen years ago. Patch by me; thanks to David Rowley, Jesper Pedersen, and others for review. Discussion: https://postgr.es/m/11587.1550975080@sss.pgh.pa.us
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-02 23:28:39 +0000