aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* Fix equalfuncs for accessMethod addition in 8586bf7ed8.Andres Freund2019-03-06
| | | | | | | In a complete brown paper bag moment, I forgot to include equalfuncs in my previous fix of copy/out/readfuncs. Thanks Tom for noticing. Discussion: https://postgr.es/m/1659.1551903210@sss.pgh.pa.us
* Don't log incomplete startup packet if it's emptyAndrew Dunstan2019-03-06
| | | | | | | | | | This will stop logging cases where, for example, a monitor opens a connection and immediately closes it. If the packet contains any data an incomplete packet will still be logged. Author: Tom Lane Discussion: https://postgr.es/m/a1379a72-2958-1ed0-ef51-09a21219b155@2ndQuadrant.com
* Fix copy/out/readfuncs for accessMethod addition in 8586bf7ed8.Andres Freund2019-03-06
| | | | | | | | This includes a catversion bump, as IntoClause is theoretically speaking part of storable rules. In practice I don't think that can happen, but there's no reason to be stingy here. Per buildfarm member calliphoridae.
* tableam: introduce table AM infrastructure.Andres Freund2019-03-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This introduces the concept of table access methods, i.e. CREATE ACCESS METHOD ... TYPE TABLE and CREATE TABLE ... USING (storage-engine). No table access functionality is delegated to table AMs as of this commit, that'll be done in following commits. Subsequent commits will incrementally abstract table access functionality to be routed through table access methods. That change is too large to be reviewed & committed at once, so it'll be done incrementally. Docs will be updated at the end, as adding them incrementally would likely make them less coherent, and definitely is a lot more work, without a lot of benefit. Table access methods are specified similar to index access methods, i.e. pg_am.amhandler returns, as INTERNAL, a pointer to a struct with callbacks. In contrast to index AMs that struct needs to live as long as a backend, typically that's achieved by just returning a pointer to a constant struct. Psql's \d+ now displays a table's access method. That can be disabled with HIDE_TABLEAM=true, which is mainly useful so regression tests can be run against different AMs. It's quite possible that this behaviour still needs to be fine tuned. For now it's not allowed to set a table AM for a partitioned table, as we've not resolved how partitions would inherit that. Disallowing allows us to introduce, if we decide that's the way forward, such a behaviour without a compatibility break. Catversion bumped, to add the heap table AM and references to it. Author: Haribabu Kommi, Andres Freund, Alvaro Herrera, Dimitri Golgov and others Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de https://postgr.es/m/20160812231527.GA690404@alvherre.pgsql https://postgr.es/m/20190107235616.6lur25ph22u5u5av@alap3.anarazel.de https://postgr.es/m/20190304234700.w5tmhducs5wxgzls@alap3.anarazel.de
* Fix bug in clearing of virtual tuple slot.Andres Freund2019-03-06
| | | | | | | | I broke/typoed this in 4da597edf1bae0c. Astonishingly this mostly doesn't cause breakage, except when trying to change the tuple descriptor of a slot (because TTS_FLAG_FIXED is assumed to be set). Author: Andres Freund
* Removed unused variable, openLogOff.Robert Haas2019-03-06
| | | | | | Antonin Houska Discussion: http://postgr.es/m/30413.1551870730@localhost
* Increase the default vacuum_cost_limit from 200 to 2000Andrew Dunstan2019-03-06
| | | | | | | | | | | | | | | | | | | | | | | | The original 200 default value was set back in f425b605f4e when the cost delay settings were first added. Hardware has improved quite a bit since then and we've also made improvements such as sorting buffers during checkpoints (9cd00c457e6) which should result in less random writes. This low default value was reportedly causing problems with badly configured servers and in the absence of a native method to remove excessive bloat from tables without incurring an AccessExclusiveLock, this often made cleaning up the damage caused by badly configured auto-vacuums difficult. It seems more likely that someone will notice that auto-vacuum is running too quickly than too slowly, so let's go all out and multiple the default value for the setting by 10. With the default vacuum_cost_page_dirty and autovacuum_vacuum_cost_delay (assuming a page size of 8192 bytes), this allows autovacuum a theoretical maximum dirty write rate of around 39MB/s instead of just 3.9MB/s. Author: David Rowley Discussion: https://postgr.es/m/CAKJS1f_YbXC2qTMPyCbmsPiKvZYwpuQNQMohiRXLj1r=8_rYvw@mail.gmail.com
* Scan GiST indexes in physical order during VACUUM.Heikki Linnakangas2019-03-05
| | | | | | | | | | | | Scanning an index in physical order is faster than walking it in logical order, because sequential I/O is faster than random I/O. The idea and code structure is borrowed from B-tree vacuum code. Patch by Andrey Borodin, with changes by me. Based on early work by Konstantin Kuznetsov, although the patch has been rewritten multiple times since his original version. Discussion: https://www.postgresql.org/message-id/1B9FAC6F-FA19-4A24-8C1B-F4F574844892%40yandex-team.ru
* Note case where nbtree VACUUM finishes splits.Peter Geoghegan2019-03-04
| | | | | | | | The nbtree README claims that VACUUM can never finish interrupted page splits by design. That isn't entirely accurate, though. Note an exception to the general rule. Discussion: https://postgr.es/m/CAH2-Wz=_Xvv8byzK_LvY4ci76OgsHCQzoKF7We8yG9waO7j6rA@mail.gmail.com
* Correct obsolete nbtree page split WAL comment.Peter Geoghegan2019-03-04
| | | | | Commit 2c03216d831, which revamped the WAL record format, failed to update a comment referencing the old API. Update the comment.
* pg_partition_ancestorsAlvaro Herrera2019-03-04
| | | | | | | | Adds another introspection feature for partitioning, necessary for further psql patches. Reviewed-by: Michaël Paquier Discussion: https://postgr.es/m/20190226222757.GA31622@alvherre.pgsql
* Remove volatile from latch APIPeter Eisentraut2019-03-04
| | | | | | | | This was no longer useful since the latch functions use memory barriers already, which are also compiler barriers, and volatile does not help with cross-process access. Discussion: https://www.postgresql.org/message-id/flat/20190218202511.qsfpuj5sy4dbezcw%40alap3.anarazel.de#18783c27d73e9e40009c82f6e0df0974
* Improve performance of index-only scans with many index columns.Tom Lane2019-03-03
| | | | | | | | | | | | | | | | | | | | StoreIndexTuple was a loop over index_getattr, which is O(N^2) if the index columns are variable-width, and the performance impact is already quite visible at ten columns. The obvious move is to replace that with a call to index_deform_tuple ... but that's *also* a loop over index_getattr. Improve it to be essentially a clone of heap_deform_tuple. (There are a few other places that loop over all index columns with index_getattr, and perhaps should be changed likewise, but most of them don't seem performance-critical. Anyway, the rest would mostly only be interested in the index key columns, which there aren't likely to be so many of. Wide index tuples are a new thing with INCLUDE.) Konstantin Knizhnik Discussion: https://postgr.es/m/e06b2d27-04fc-5c0e-bb8c-ecd72aa24959@postgrespro.ru
* Further fixing for multi-row VALUES lists for updatable views.Dean Rasheed2019-03-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, rewriteTargetListIU() generated a list of attribute numbers from the targetlist, which were passed to rewriteValuesRTE(), which expected them to contain the same number of entries as there are columns in the VALUES RTE, and to be in the same order. That was fine when the target relation was a table, but for an updatable view it could be broken in at least three different ways --- rewriteTargetListIU() could insert additional targetlist entries for view columns with defaults, the view columns could be in a different order from the columns of the underlying base relation, and targetlist entries could be merged together when assigning to elements of an array or composite type. As a result, when recursing to the base relation, the list of attribute numbers generated from the rewritten targetlist could no longer be relied upon to match the columns of the VALUES RTE. We got away with that prior to 41531e42d3 because it used to always be the case that rewriteValuesRTE() did nothing for the underlying base relation, since all DEFAULTS had already been replaced when it was initially invoked for the view, but that was incorrect because it failed to apply defaults from the base relation. Fix this by examining the targetlist entries more carefully and picking out just those that are simple Vars referencing the VALUES RTE. That's sufficient for the purposes of rewriteValuesRTE(), which is only responsible for dealing with DEFAULT items in the VALUES RTE. Any DEFAULT item in the VALUES RTE that doesn't have a matching simple-Var-assignment in the targetlist is an error which we complain about, but in theory that ought to be impossible. Additionally, move this code into rewriteValuesRTE() to give a clearer separation of concerns between the 2 functions. There is no need for rewriteTargetListIU() to know about the details of the VALUES RTE. While at it, fix the comment for rewriteValuesRTE() which claimed that it doesn't support array element and field assignments --- that hasn't been true since a3c7a993d5 (9.6 and later). Back-patch to all supported versions, with minor differences for the pre-9.6 branches, which don't support array element and field assignments to the same column in multi-row VALUES lists. Reviewed by Amit Langote. Discussion: https://postgr.es/m/15623-5d67a46788ec8b7f@postgresql.org
* Consider only relations part of partition trees in partition functionsMichael Paquier2019-03-02
| | | | | | | | | | | | | | | | | | | | This changes the partition functions so as tables and indexes which are not part of partition trees are handled the same way as what is done for undefined objects and unsupported relkinds: pg_partition_tree() returns no rows and pg_partition_root() returns a NULL result. Hence, partitioned tables, partitioned indexes and relations whose flag pg_class.relispartition is set are considered as valid objects to process. Previously, tables and indexes not included in a partition tree were processed the same way as a partition or a partitioned table, which caused the functions to return inconsistent results for inherited tables, especially when inheriting from multiple tables. Reported-by: Álvaro Herrera Author: Amit Langote, Michael Paquier Reviewed-by: Tom Lane Discussion: https://postgr.es/m/20190228193203.GA26151@alvherre.pgsql
* Use a virtual rather than a heap slot in two places where that suffices.Andres Freund2019-03-01
| | | | | Author: Andres Freund Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
* Teach optimizer's predtest.c more things about ScalarArrayOpExpr.Tom Lane2019-03-01
| | | | | | | | | | | | | | | | | | | | | | | In particular, make it possible to prove/refute "x IS NULL" and "x IS NOT NULL" predicates from a clause involving a ScalarArrayOpExpr even when we are unable or unwilling to deconstruct the expression into an AND/OR tree. This avoids a former unexpected degradation of plan quality when the size of an ARRAY[] expression or array constant exceeded the arbitrary MAX_SAOP_ARRAY_SIZE limit. For IS-NULL proofs, we don't really care about the values of the individual array elements; at most, we care whether there are any, and for some common cases we needn't even know that. The main user-visible effect of this is to let the optimizer recognize applicability of partial indexes with "x IS NOT NULL" predicates to queries with "x IN (array)" clauses in some cases where it previously failed to recognize that. The structure of predtest.c is such that a bunch of related proofs will now also succeed, but they're probably much less useful in the wild. James Coleman, reviewed by David Rowley Discussion: https://postgr.es/m/CAAaqYe8yKSvzbyu8w-dThRs9aTFMwrFxn_BkTYeXgjqe3CbNjg@mail.gmail.com
* Store tuples for EvalPlanQual in slots, rather than as HeapTuples.Andres Freund2019-03-01
| | | | | | | | | | | | | | | | | | | | | | | | | | For the upcoming pluggable table access methods it's quite inconvenient to store tuples as HeapTuples, as that'd require converting tuples from a their native format into HeapTuples. Instead use slots to manage epq tuples. To fit into that scheme, change the foreign data wrapper callback RefetchForeignRow, to store the tuple in a slot. Insist on using the caller provided slot, so it conveniently can be stored in the corresponding EPQ slot. As there is no in core user of RefetchForeignRow, that change was done blindly, but we plan to test that soon. To avoid duplicating that work for row locks, move row locks to just directly use the EPQ slots - it previously temporarily stored tuples in LockRowsState.lr_curtuples, but that doesn't seem beneficial, given we'd possibly end up with a significant number of additional slots. The behaviour of es_epqTupleSet[rti -1] is now checked by es_epqTupleSlot[rti -1] != NULL, as that is distinguishable from a slot containing an empty tuple. Author: Andres Freund, Haribabu Kommi, Ashutosh Bapat Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
* Clear the local map when not used.Amit Kapila2019-03-01
| | | | | | | | | | | | | | | | | After commit b0eaa4c51b, we use a local map of pages to find the required space for small relations. We do clear this map when we have found a block with enough free space, when we extend the relation, or on transaction abort so that it can be used next time. However, we miss to clear it when we didn't find any pages to try from the map which leads to an assertion failure when we later tried to use it after relation extension. In the passing, I have improved some comments in this area. Reported-by: Tom Lane based on buildfarm results Author: Amit Kapila Reviewed-by: John Naylor Tested-by: Kuntal Ghosh Discussion: https://postgr.es/m/32368.1551114120@sss.pgh.pa.us
* Make pg_partition_tree return no rows on unsupported and undefined objectsMichael Paquier2019-03-01
| | | | | | | | | | | The function was tweaked so as it returned one row full of NULLs when working on an unsupported relkind or an undefined object as of cc53123, and after discussion with Amit and Álvaro it looks more natural to make it return no rows. Author: Michael Paquier Reviewed-by: Álvaro Herrera, Amit Langote Discussion: https://postgr.es/m/20190227184808.GA17357@alvherre.pgsql
* Don't superfluously materialize slot after DELETE from an FDW.Andres Freund2019-02-28
| | | | | | | Previously that was needed to safely store the table oid, but after b8d71745eac0a127 that's not necessary anymore. Author: Andres Freund
* Don't force materializing when copying a buffer tuple table slot.Andres Freund2019-02-28
| | | | | | | | | | After 5408e233f0667478 it's not necessary to force materializing the target slot when copying from one buffer slot to another. Previously that was required because the HeapTupleData portion of the source slot wasn't guaranteed to stay valid long enough, but now we can simply copy that part into the destination slot's tupdata. Author: Andres Freund
* Allow buffer tuple table slots to materialize after ExecStoreVirtualTuple().Andres Freund2019-02-28
| | | | | | | | | | | | | | | | While not common, it can be useful to store a virtual tuple into a buffer tuple table slot, and then materialize that slot. So far we've asserted out, which surprisingly wasn't a problem for anything in core. But that seems fragile, and it also breaks redis_fdw after ff11e7f4b9. Thus, allow materializing a virtual tuple stored in a buffer tuple table slot. Author: Andres Freund Discussion: https://postgr.es/m/20190227181621.xholonj7ff7ohxsg@alap3.anarazel.de https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
* Merge near-duplicate code in RI triggersPeter Eisentraut2019-02-28
| | | | | | | | | | | Merge ri_setnull and ri_setdefault into one function ri_set. These functions were to a large part identical. This is a continuation in spirit of 4797f9b519995ceca5d6b8550b5caa2ff6d19347. Author: Corey Huinker <corey.huinker@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/0ccdd3e1-10b0-dd05-d8a7-183507c11eb1%402ndquadrant.com
* Standardize some more loops that chase down parallel lists.Tom Lane2019-02-28
| | | | | | | | | | | | | | | | | | | | | | | | | We have forboth() and forthree() macros that simplify iterating through several parallel lists, but not everyplace that could reasonably use those was doing so. Also invent forfour() and forfive() macros to do the same for four or five parallel lists, and use those where applicable. The immediate motivation for doing this is to reduce the number of ad-hoc lnext() calls, to reduce the footprint of a WIP patch. However, it seems like good cleanup and error-proofing anyway; the places that were combining forthree() with a manually iterated loop seem particularly illegible and bug-prone. There was some speculation about restructuring related parsetree representations to reduce the need for parallel list chasing of this sort. Perhaps that's a win, or perhaps not, but in any case it would be considerably more invasive than this patch; and it's not particularly related to my immediate goal of improving the List infrastructure. So I'll leave that question for another day. Patch by me; thanks to David Rowley for review. Discussion: https://postgr.es/m/11587.1550975080@sss.pgh.pa.us
* Clean up some variable names in ri_triggers.cPeter Eisentraut2019-02-28
| | | | | | | | There was a mix of old_slot/oldslot, new_slot/newslot. Since we've changed everything from row to slot, we might as well take this opportunity to clean this up. Also update some more comments for the slot change.
* Compact for loopsPeter Eisentraut2019-02-28
| | | | | | | Declare loop variable in for loop, for readability and to save space. Reviewed-by: Corey Huinker <corey.huinker@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/0ccdd3e1-10b0-dd05-d8a7-183507c11eb1%402ndquadrant.com
* Reduce commentsPeter Eisentraut2019-02-28
| | | | | | | | | Reduce the vertical space used by comments in ri_triggers.c, making the file longer and more tedious to read than it needs to be. Update some comments to use a more common style. Reviewed-by: Corey Huinker <corey.huinker@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/0ccdd3e1-10b0-dd05-d8a7-183507c11eb1%402ndquadrant.com
* Remove unnecessary unused MATCH PARTIAL codePeter Eisentraut2019-02-28
| | | | | | | | | | | | | | | | | | | | | | ri_triggers.c spends a lot of space catering to a not-yet-implemented MATCH PARTIAL option. An actual implementation would probably not use the existing code structure anyway, so let's just simplify this for now. First, have ri_FetchConstraintInfo() check that riinfo->confmatchtype is valid. Then we don't have to repeat that everywhere. In the various referential action functions, we don't need to pay attention to the match type at all right now, so remove all that code. A future MATCH PARTIAL implementation would probably have some conditions added to the present code, but it won't need an entirely separate switch branch in each case. In RI_FKey_fk_upd_check_required(), reorganize the code to make it much simpler. Reviewed-by: Corey Huinker <corey.huinker@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/0ccdd3e1-10b0-dd05-d8a7-183507c11eb1%402ndquadrant.com
* Update commentPeter Eisentraut2019-02-28
| | | | for ff11e7f4b9ae017585c3ba146db7ba39c31f209a
* Improve documentation of data_sync_retryMichael Paquier2019-02-28
| | | | | | | | Reflecting an updated parameter value requires a server restart, which was not mentioned in the documentation and in postgresql.conf.sample. Reported-by: Thomas Poty Discussion: https://postgr.es/m/15659-0cd812f13027a2d8@postgresql.org
* Initialize variable to silence compiler warning.Andres Freund2019-02-27
| | | | | | | | | After ff11e7f4b9ae Tom's compiler warns about accessing a potentially uninitialized rInfo. That's not actually possible, but it's understandable the compiler would get this wrong. NULL initialize too. Reported-By: Tom Lane Discussion: https://postgr.es/m/11199.1551285318@sss.pgh.pa.us
* Set fallback_application_name for a walreceiver to cluster_namePeter Eisentraut2019-02-27
| | | | | | | | | | | | | | | | By default, the fallback_application_name for a physical walreceiver is "walreceiver". This means that multiple standbys cannot be distinguished easily on a primary, for example in pg_stat_activity or synchronous_standby_names. If cluster_name is set, use that for fallback_application_name in the walreceiver. (If it's not set, it remains "walreceiver".) If someone set cluster_name to identify their instance, we might as well use that by default to identify the node remotely as well. It's still possible to specify another application_name in primary_conninfo explicitly. Reviewed-by: Euler Taveira <euler@timbira.com.br> Discussion: https://www.postgresql.org/message-id/flat/1257eaee-4874-e791-e83a-46720c72cac7@2ndquadrant.com
* Fix memory leak when inserting tuple at relation creation for CTASMichael Paquier2019-02-27
| | | | | | | | | | The leak has been introduced by 763f2ed which has addressed the problem for transient tables, and forgot CREATE TABLE AS which shares a similar logic when receiving a new tuple to store into the newly-created relation. Author: Jeff Janes Discussion: https://postgr.es/m/CAMkU=1xZXtz3mziPEPD2Fubbas4G2RWkZm5HHABtfKVcbu1=Sg@mail.gmail.com
* Use slots in trigger infrastructure, except for the actual invocation.Andres Freund2019-02-26
| | | | | | | | | | | | | | | | | | | | | In preparation for abstracting table storage, convert trigger.c to track tuples in slots. Which also happens to make code calling triggers simpler. As the calling interface for triggers themselves is not changed in this patch, HeapTuples still are extracted from the slot at that time. But that's handled solely inside trigger.c, not visible to callers. It's quite likely that we'll want to revise the external trigger interface, but that's a separate large project. As part of this work the slots used for old/new/return tuples are moved from EState into ResultRelInfo, as different updated tables might need different slots. The slots are now also now created on-demand, which is good both from an efficiency POV, but also makes the modifying code simpler. Author: Andres Freund, Amit Khandekar and Ashutosh Bapat Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
* Store table oid and tuple's tid in tuple slots directly.Andres Freund2019-02-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | After the introduction of tuple table slots all table AMs need to support returning the table oid of the tuple stored in a slot created by said AM. It does not make sense to re-implement that in every AM, therefore move handling of table OIDs into the TupleTableSlot structure itself. It's possible that we, at a later date, might want to get rid of HeapTupleData.t_tableOid entirely, but doing so before the abstractions for table AMs are integrated turns out to be too hard, so delay that for now. Similarly, every AM needs to support the concept of a tuple identifier (tid / item pointer) for its tuples. It's quite possible that we'll generalize the exact form of a tid at a future point (to allow for things like index organized tables), but for now many parts of the code know about tids, so there's not much point in abstracting tids away. Therefore also move into slot (rather than providing API to set/get the tid associated with the tuple in a slot). Once table AM includes insert/updating/deleting tuples, the responsibility to set the correct tid after such an action will move into that. After that change, code doing such modifications, should not have to deal with HeapTuples directly anymore. Author: Andres Freund, Haribabu Kommi and Ashutosh Bapat Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
* Add ExecStorePinnedBufferHeapTuple.Andres Freund2019-02-26
| | | | | | | | | | | | This allows to avoid an unnecessary pin/unpin cycle when storing a tuple in an already pinned buffer into a slot, when the pin isn't further needed at the call site. Only a single caller for now (to ensure coverage), but upcoming patches will increase use of the new function. Author: Andres Freund Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
* Change lock acquisition order in expand_inherited_rtentry.Robert Haas2019-02-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, this function acquired locks in the order using find_all_inheritors(), which locks the children of each table that it processes in ascending OID order, and which processes the inheritance hierarchy as a whole in a breadth-first fashion. Now, it processes the inheritance hierarchy in a depth-first fashion, and at each level it proceeds in the order in which tables appear in the PartitionDesc. If table inheritance rather than table partitioning is used, the old order is preserved. This change moves the locking of any given partition much closer to the code that actually expands that partition. This seems essential if we ever want to allow concurrent DDL to add or remove partitions, because if the set of partitions can change, we must use the same data to decide which partitions to lock as we do to decide which partitions to expand; otherwise, we might expand a partition that we haven't locked. It should hopefully also facilitate efforts to postpone inheritance expansion or locking for performance reasons, because there's really no way to postpone locking some partitions if we're blindly locking them all using find_all_inheritors(). The only downside of this change which is known to me is that it further deviates from the principle that we should always lock the inheritance hierarchy in find_all_inheritors() order to avoid deadlock risk. However, we've already crossed that bridge in commit 9eefba181f7782d27d85d7e94e6028371e7ab2d7 and there are futher patches pending that make similar changes, so this isn't really giving up anything that we haven't surrendered already -- and it seems entirely worth it, given the performance benefits some of those changes seem likely to bring. Patch by me; thanks to David Rowley for discussion of these issues. Discussion: http://postgr.es/m/CAKJS1f_eEYVEq5tM8sm1k-HOwG0AyCPwX54XG9x4w0zy_N4Q_Q@mail.gmail.com Discussion: http://postgr.es/m/CA+TgmoZUwPf_uanjF==gTGBMJrn8uCq52XYvAEorNkLrUdoawg@mail.gmail.com
* Remove unneeded argument from _bt_getstackbuf().Peter Geoghegan2019-02-25
| | | | | | | | | _bt_getstackbuf() is called at exactly two points following commit efada2b8e92 (one call site is concerned with page splits, while the other is concerned with page deletion). The parent buffer returned by _bt_getstackbuf() is write-locked in both cases. Remove the 'access' argument and make _bt_getstackbuf() assume that callers require a write-lock.
* Correct obsolete nbtree page deletion comment.Peter Geoghegan2019-02-25
| | | | | | | | | Commit efada2b8e92, which made the nbtree page deletion algorithm more robust, removed _bt_getstackbuf() calls from _bt_pagedel(). It failed to update a comment that referenced the earlier approach. Update the comment to explain that the _bt_getstackbuf() page deletion call site mirrors the only other remaining _bt_getstackbuf() call site, which is reached during page splits.
* Remove unnecessary use of PROCEDURALPeter Eisentraut2019-02-25
| | | | | | | | | | | Remove some unnecessary, legacy-looking use of the PROCEDURAL keyword before LANGUAGE. We mostly don't use this anymore, so some of these look a bit old. There is still some use in pg_dump, which is harder to remove because it's baked into the archive format, so I'm not touching that. Discussion: https://www.postgresql.org/message-id/2330919b-62d9-29ac-8de3-58c024fdcb96@2ndquadrant.com
* Make release of 2PC identifier and locks consistent in COMMIT PREPAREDMichael Paquier2019-02-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When preparing a transaction in two-phase commit, a dummy PGPROC entry holding the GID used for the transaction is registered, which gets released once COMMIT PREPARED is run. Prior releasing its shared memory state, all the locks taken in the prepared transaction are released using a dedicated set of callbacks (pgstat and multixact having similar callbacks), which may cause the locks to be released before the GID is set free. Hence, there is a small window where lock conflicts could happen, for example: - Transaction A releases its locks, still holding its GID in shared memory. - Transaction B held a lock which conflicted with locks of transaction A. - Transaction B continues its processing, reusing the same GID as transaction A. - Transaction B fails because of a conflicting GID, already in use by transaction A. This commit changes the shared memory state release so as post-commit callbacks and predicate lock cleanup happen consistently with the shared memory state cleanup for the dummy PGPROC entry. The race window is small and 2PC had this issue from the start, so no backpatch is done. On top if that fixes discussed involved ABI breakages, which are not welcome in stable branches. Reported-by: Oleksii Kliukin, Ildar Musin Diagnosed-by: Oleksii Kliukin, Ildar Musin Author: Michael Paquier Reviewed-by: Masahiko Sawada, Oleksii Kliukin Discussion: https://postgr.es/m/BF9B38A4-2BFF-46E8-BA87-A2D00A8047A6@hintbits.com
* Fix inconsistent out-of-memory error reporting in dsa.c.Thomas Munro2019-02-25
| | | | | | | | | | | | | | | | | | | | | | | | | | Commit 16be2fd1 introduced the flag DSA_ALLOC_NO_OOM to control whether the DSA allocator would raise an error or return InvalidDsaPointer on failure to allocate. One edge case was not handled correctly: if we fail to allocate an internal "span" object for a large allocation, we would always return InvalidDsaPointer regardless of the flag; a caller not expecting that could then dereference a null pointer. This is a plausible explanation for a one-off report of a segfault. Remove a redundant pair of braces so that all three stanzas that handle DSA_ALLOC_NO_OOM match in style, for visual consistency. While fixing inconsistencies, if FreePageManagerGet() can't supply the pages that our book-keeping says it should be able to supply, then we should always report a FATAL error. Previously we treated that as a regular allocation failure in one code path, but as a FATAL condition in another. Back-patch to 10, where dsa.c landed. Author: Thomas Munro Reported-by: Jakub Glapa Discussion: https://postgr.es/m/CAEepm=2oPqXxyWQ-1o60tpOLrwkw=VpgNXqqF1VN2EyO9zKGQw@mail.gmail.com
* Fix ecpg bugs caused by missing semicolons in the backend grammar.Tom Lane2019-02-24
| | | | | | | | | | | | | | | | | | | | | | | | | The Bison documentation clearly states that a semicolon is required after every grammar rule, and our scripts that generate ecpg's grammar from the backend's implicitly assumed this is true. But it turns out that only ancient versions of Bison actually enforce that. There have been a couple of rules without trailing semicolons in gram.y for some time, and as a consequence, ecpg's grammar was faulty and produced wrong output for the affected statements. To fix, add the missing semis, and add some cross-checks to ecpg's scripts so that they'll bleat if we mess this up again. The cases that were broken were: * "SET variable = DEFAULT" (but not "SET variable TO DEFAULT"), as well as allied syntaxes such as ALTER SYSTEM SET ... DEFAULT. These produced syntactically invalid output that the server would reject. * Multiple type names in DROP TYPE/DOMAIN commands. Only the first type name would be listed in the emitted command. Per report from Daisuke Higuchi. Back-patch to all supported versions. Discussion: https://postgr.es/m/1803D792815FC24D871C00D17AE95905DB51CE@g01jpexmbkw24
* Tolerate EINVAL when calling fsync() on a directory.Thomas Munro2019-02-24
| | | | | | | | | | | Previously, we tolerated EBADF as a way for the operating system to indicate that it doesn't support fsync() on a directory. Tolerate EINVAL too, for older versions of Linux CIFS. Bug #15636. Back-patch all the way. Reported-by: John Klann Discussion: https://postgr.es/m/15636-d380890dafd78fc6@postgresql.org
* Tolerate ENOSYS failure from sync_file_range().Thomas Munro2019-02-24
| | | | | | | | | | | | | | | | | | | | | One unintended consequence of commit 9ccdd7f6 was that Windows WSL users started getting a panic whenever we tried to initiate data flushing with sync_file_range(), because WSL does not implement that system call. Previously, they got a stream of periodic warnings, which was also undesirable but at least ignorable. Prevent the panic by handling ENOSYS specially and skipping the panic promotion with data_sync_elevel(). Also suppress future attempts after the first such failure so that the pre-existing problem of noisy warnings is improved. Back-patch to 9.6 (older branches were not affected in this way by 9ccdd7f6). Author: Thomas Munro and James Sewell Tested-by: James Sewell Reported-by: Bruce Klein Discussion: https://postgr.es/m/CA+mCpegfOUph2U4ZADtQT16dfbkjjYNJL1bSTWErsazaFjQW9A@mail.gmail.com
* Fix incorrect function reference in comment of twophase.cMichael Paquier2019-02-23
| | | | | | | | The header block of TwoPhaseGetDummyBackendId mentioned incorrectly TwoPhaseGetDummyProc. Reported-by: Oleksii Kliukin Discussion: https://postgr.es/m/D8336E40-BBE1-4954-98BB-7830D3F5CB36@hintbits.com
* Fix plan created for inherited UPDATE/DELETE with all tables excluded.Tom Lane2019-02-22
| | | | | | | | | | | | | | | | | | | | | | In the case where inheritance_planner() finds that every table has been excluded by constraints, it thought it could get away with making a plan consisting of just a dummy Result node. While certainly there's no updating or deleting to be done, this had two user-visible problems: the plan did not report the correct set of output columns when a RETURNING clause was present, and if there were any statement-level triggers that should be fired, it didn't fire them. Hence, rather than only generating the dummy Result, we need to stick a valid ModifyTable node on top, which requires a tad more effort here. It's been broken this way for as long as inheritance_planner() has known about deleting excluded subplans at all (cf commit 635d42e9c), so back-patch to all supported branches. Amit Langote and Tom Lane, per a report from Petr Fedorov. Discussion: https://postgr.es/m/5da6f0f0-1364-1876-6978-907678f89a3e@phystech.edu
* Report correct name in autovacuum "work items" activityAlvaro Herrera2019-02-22
| | | | | | | | We were reporting the database name instead of the relation name to pg_stat_activity. Repair. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20190220185552.GR28750@telsasoft.com
* Move estimate_hashagg_tablesize to selfuncs.c, and widen result to double.Tom Lane2019-02-21
| | | | | | | | | | | | | | | | | It seems to make more sense for this to be in selfuncs.c, since it's largely a statistical-estimation thing, and it's related to other functions like estimate_hash_bucket_stats that are there. While at it, change the result type from Size to double. Perhaps at one point it was impossible for the result to overflow an integer, but I've got no confidence in that proposition anymore. Nothing's actually done with the result except to compare it to a work_mem-based limit, so as long as we don't get an overflow on the way to that comparison, things should be fine even with very large dNumGroups. Code movement proposed by Antonin Houska, type change by me Discussion: https://postgr.es/m/25767.1549359615@localhost