aboutsummaryrefslogtreecommitdiff
path: root/contrib
Commit message (Collapse)AuthorAge
* Allow pg_stat_statements to track WAL usage statistics.Amit Kapila2020-04-05
| | | | | | | | | | | | This commit adds three new columns in pg_stat_statements output to display WAL usage statistics added by commit df3b181499. This commit doesn't bump the version of pg_stat_statements as the same is done for this release in commit 17e0328224. Author: Kirill Bychik and Julien Rouhaud Reviewed-by: Julien Rouhaud, Fujii Masao, Dilip Kumar and Amit Kapila Discussion: https://postgr.es/m/CAB-hujrP8ZfUkvL5OYETipQwA=e3n7oqHFU=4ZLxWS_Cza3kQQ@mail.gmail.com
* Skip WAL for new relfilenodes, under wal_level=minimal.Noah Misch2020-04-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Until now, only selected bulk operations (e.g. COPY) did this. If a given relfilenode received both a WAL-skipping COPY and a WAL-logged operation (e.g. INSERT), recovery could lose tuples from the COPY. See src/backend/access/transam/README section "Skipping WAL for New RelFileNode" for the new coding rules. Maintainers of table access methods should examine that section. To maintain data durability, just before commit, we choose between an fsync of the relfilenode and copying its contents to WAL. A new GUC, wal_skip_threshold, guides that choice. If this change slows a workload that creates small, permanent relfilenodes under wal_level=minimal, try adjusting wal_skip_threshold. Users setting a timeout on COMMIT may need to adjust that timeout, and log_min_duration_statement analysis will reflect time consumption moving to COMMIT from commands like COPY. Internally, this requires a reliable determination of whether RollbackAndReleaseCurrentSubTransaction() would unlink a relation's current relfilenode. Introduce rd_firstRelfilenodeSubid. Amend the specification of rd_createSubid such that the field is zero when a new rel has an old rd_node. Make relcache.c retain entries for certain dropped relations until end of transaction. Bump XLOG_PAGE_MAGIC, since this introduces XLOG_GIST_ASSIGN_LSN. Future servers accept older WAL, so this bump is discretionary. Kyotaro Horiguchi, reviewed (in earlier, similar versions) by Robert Haas. Heikki Linnakangas and Michael Paquier implemented earlier designs that materially clarified the problem. Reviewed, in earlier designs, by Andrew Dunstan, Andres Freund, Alvaro Herrera, Tom Lane, Fujii Masao, and Simon Riggs. Reported by Martijn van Oosterhout. Discussion: https://postgr.es/m/20150702220524.GA9392@svana.org
* Fix bogus CALLED_AS_TRIGGER() defenses.Tom Lane2020-04-03
| | | | | | | | | | | | | | | | | | | | | contrib/lo's lo_manage() thought it could use trigdata->tg_trigger->tgname in its error message about not being called as a trigger. That naturally led to a core dump. unique_key_recheck() figured it could Assert that fcinfo->context is a TriggerData node in advance of having checked that it's being called as a trigger. That's harmless in production builds, and perhaps not that easy to reach in any case, but it's logically wrong. The first of these per bug #16340 from William Crowell; the second from manual inspection of other CALLED_AS_TRIGGER call sites. Back-patch the lo.c change to all supported branches, the other to v10 where the thinko crept in. Discussion: https://postgr.es/m/16340-591c7449dc7c8c47@postgresql.org
* Fix whitespacePeter Eisentraut2020-04-02
|
* Allow pg_stat_statements to track planning statistics.Fujii Masao2020-04-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit makes pg_stat_statements support new GUC pg_stat_statements.track_planning. If this option is enabled, pg_stat_statements tracks the planning statistics of the statements, e.g., the number of times the statement was planned, the total time spent planning the statement, etc. This feature is useful to check the statements that it takes a long time to plan. Previously since pg_stat_statements tracked only the execution statistics, we could not use that for the purpose. The planning and execution statistics are stored at the end of each phase separately. So there are not always one-to-one relationship between them. For example, if the statement is successfully planned but fails in the execution phase, only its planning statistics are stored. This may cause the users to be able to see different pg_stat_statements results from the previous version. To avoid this, pg_stat_statements.track_planning needs to be disabled. This commit bumps the version of pg_stat_statements to 1.8 since it changes the definition of pg_stat_statements function. Author: Julien Rouhaud, Pascal Legrand, Thomas Munro, Fujii Masao Reviewed-by: Sergei Kornilov, Tomas Vondra, Yoshikazu Imai, Haribabu Kommi, Tom Lane Discussion: https://postgr.es/m/CAHGQGwFx_=DO-Gu-MfPW3VQ4qC7TfVdH2zHmvZfrGv6fQ3D-Tw@mail.gmail.com Discussion: https://postgr.es/m/CAEepm=0e59Y_6Q_YXYCTHZkqOc6H2pJ54C_Xe=VFu50Aqqp_sA@mail.gmail.com Discussion: https://postgr.es/m/DB6PR0301MB21352F6210E3B11934B0DCC790B00@DB6PR0301MB2135.eurprd03.prod.outlook.com
* Clean up parsing of ltree and lquery some more.Tom Lane2020-04-01
| | | | | | | | | | | | | | | | Fix lquery parsing to handle repeated flag characters correctly, and to enforce the max label length correctly in some cases where it did not before, and to detect empty labels in some cases where it did not before. In a more cosmetic vein, use a switch rather than if-then chains to handle the different states, and avoid unnecessary checks on charlen when looking for ASCII characters, and factor out multiple copies of the label length checking code. Tom Lane and Dmitry Belyavsky Discussion: https://postgr.es/m/CADqLbzLVkBuPX0812o+z=c3i6honszsZZ6VQOSKR3VPbB56P3w@mail.gmail.com
* Add support for binary I/O of ltree, lquery, and ltxtquery types.Tom Lane2020-04-01
| | | | | | | | | | | | | | | | | | Not much to say here --- does what it says on the tin. The "binary" representation in each case is really just the same as the text format, though we prefix a version-number byte in case anyone ever feels motivated to change that. Thus, there's not any expectation of improved speed or reduced space; the point here is just to allow clients to use binary format for all columns of a query result or COPY data. This makes use of the recently added ALTER TYPE support to add binary I/O functions to an existing data type. As in commit a80818605, we can piggy-back on there already being a new-for-v13 version of the ltree extension, so we don't need a new update script file. Nino Floris, reviewed by Alexander Korotkov and myself Discussion: https://postgr.es/m/CANmj9Vxx50jOo1L7iSRxd142NyTz6Bdcgg7u9P3Z8o0=HGkYyQ@mail.gmail.com
* Improve selectivity estimation for assorted match-style operators.Tom Lane2020-04-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Quite a few matching operators such as JSONB's @> used "contsel" and "contjoinsel" as their selectivity estimators. That was a bad idea, because (a) contsel is only a stub, yielding a fixed default estimate, and (b) that default is 0.001, meaning we estimate these operators as five times more selective than equality, which is surely pretty silly. There's a good model for improving this in ltree's ltreeparentsel(): for any "var OP constant" query, we can try applying the operator to all of the column's MCV and histogram values, taking the latter as being a random sample of the non-MCV values. That code is actually 100% generic, except for the question of exactly what default selectivity ought to be plugged in when we don't have stats. Hence, migrate the guts of ltreeparentsel() into the core code, provide wrappers "matchingsel" and "matchingjoinsel" with a more-appropriate default estimate, and use those for the non-geometric operators that formerly used contsel (mostly JSONB containment operators and tsquery matching). Also apply this code to some match-like operators in hstore, ltree, and pg_trgm, including the former users of ltreeparentsel as well as ones that improperly used contsel. Since commit 911e70207 just created new versions of those extensions that we haven't released yet, we can sneak this change into those new versions instead of having to create an additional generation of update scripts. Patch by me, reviewed by Alexey Bashtanov Discussion: https://postgr.es/m/12237.1582833074@sss.pgh.pa.us
* Remove header noise from test_decoding testAlvaro Herrera2020-03-31
| | | | | | | | Use psql's expanded output to avoid a pointless header. Kyotaro Horiguchi, after an idea of Michael Paquier Discussion: https://postgr.es/m/20181120050744.GJ4400@paquier.xyz
* Fix lquery's NOT handling, and add ability to quantify non-'*' items.Tom Lane2020-03-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The existing implementation of the ltree ~ lquery match operator is sufficiently complex and undocumented that it's hard to tell exactly what it does. But one thing it clearly gets wrong is the combination of NOT symbols (!) and '*' symbols. A pattern such as '*.!foo.*' should, by any ordinary understanding of regular expression behavior, match any ltree that has at least one label that's not "foo". As best we can tell by experimentation, what it's actually matching is any ltree in which *no* label is "foo". That's surprising, and not at all what the documentation says. Now, that's arguably a useful behavior, so if we rewrite to fix the bug we should provide some other way to get it. To do so, add the ability to attach lquery quantifiers to non-'*' items as well as '*'s. Then the pattern '!foo{,}' expresses "any ltree in which no label is foo". For backwards compatibility, the default quantifier for non-'*' items has to be "{1}", although the default for '*' items is '{,}'. I wouldn't have done it like that in a green field, but it's not totally horrible. Armed with that, rewrite checkCond() from scratch. Treating '*' and non-'*' items alike makes it simpler, not more complicated, so that the function actually gets a lot shorter than it was. Filip Rembiałkowski, Tom Lane, Nikita Glukhov, per a very ancient bug report from M. Palm Discussion: https://postgr.es/m/CAP_rww=waX2Oo6q+MbMSiZ9ktdj6eaJj0cQzNu=Ry2cCDij5fw@mail.gmail.com
* Improve error messages in ltree_in and lquery_in.Tom Lane2020-03-31
| | | | | | | | | | | | | | | Ensure that the type name is mentioned in all cases (bare "syntax error" isn't that helpful). Avoid using the term "level", since that's not used in the documentation. Phrase error position reports as "at character N" not "in position N"; the latter seems ambiguous, and it's certainly not how we say it elsewhere. For the same reason, make the character position values be 1-based not 0-based. Provide a position in more cases. (I continued to leave that out of messages complaining about end-of-input, where it seemed pointless, as well as messages complaining about overall input complexity, where fingering any one part of the input would be arbitrary.) Discussion: https://postgr.es/m/15582.1585529626@sss.pgh.pa.us
* Implement operator class parametersAlexander Korotkov2020-03-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | PostgreSQL provides set of template index access methods, where opclasses have much freedom in the semantics of indexing. These index AMs are GiST, GIN, SP-GiST and BRIN. There opclasses define representation of keys, operations on them and supported search strategies. So, it's natural that opclasses may be faced some tradeoffs, which require user-side decision. This commit implements opclass parameters allowing users to set some values, which tell opclass how to index the particular dataset. This commit doesn't introduce new storage in system catalog. Instead it uses pg_attribute.attoptions, which is used for table column storage options but unused for index attributes. In order to evade changing signature of each opclass support function, we implement unified way to pass options to opclass support functions. Options are set to fn_expr as the constant bytea expression. It's possible due to the fact that opclass support functions are executed outside of expressions, so fn_expr is unused for them. This commit comes with some examples of opclass options usage. We parametrize signature length in GiST. That applies to multiple opclasses: tsvector_ops, gist__intbig_ops, gist_ltree_ops, gist__ltree_ops, gist_trgm_ops and gist_hstore_ops. Also we parametrize maximum number of integer ranges for gist__int_ops. However, the main future usage of this feature is expected to be json, where users would be able to specify which way to index particular json parts. Catversion is bumped. Discussion: https://postgr.es/m/d22c3a18-31c7-1879-fc11-4c1ce2f5e5af%40postgrespro.ru Author: Nikita Glukhov, revised by me Reviwed-by: Nikolay Shaplov, Robert Haas, Tom Lane, Tomas Vondra, Alvaro Herrera
* Expose BufferUsageAccumDiff().Fujii Masao2020-03-30
| | | | | | | | | | | | | | | | Previously pg_stat_statements calculated the difference of buffer counters by its own code even while BufferUsageAccumDiff() had the same code. This commit expose BufferUsageAccumDiff() and makes pg_stat_statements use it for the calculation, in order to simply the code. This change also would be useful for the upcoming patch for the planning counters in pg_stat_statements because the patch will add one more code for the calculation of difference of buffer counters and that can easily be done by using BufferUsageAccumDiff(). Author: Julien Rouhaud Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/bdfee4e0-a304-2498-8da5-3cb52c0a193e@oss.nttdata.com
* Cosmetic improvements in ltree code.Tom Lane2020-03-29
| | | | | | | | | Add more comments in ltree.h, and correct a misstatement or two. Use a symbol, rather than hardwired constants, for the maximum length of an ltree label. The max length is still hardwired in the associated error messages, but I want to clean that up as part of a separate patch to improve the error messages.
* Fix lquery's behavior for consecutive '*' items.Tom Lane2020-03-28
| | | | | | | | | | | | | | | | | | | | | | Something like "*{2}.*{3}" should presumably mean the same as "*{5}", but it didn't. Improve that. Get rid of an undocumented and remarkably ugly (though not, as far as I can tell, actually unsafe) static variable in favor of passing more arguments to checkCond(). Reverse-engineer some commentary. This function, like all of ltree, is still far short of what I would consider the minimum acceptable level of internal documentation, but at least now it has more than zero comments. Although this certainly seems like a bug fix, people might not thank us for changing query behavior in stable branches, so no back-patch. Nikita Glukhov, with cosmetic improvements by me Discussion: https://postgr.es/m/CAP_rww=waX2Oo6q+MbMSiZ9ktdj6eaJj0cQzNu=Ry2cCDij5fw@mail.gmail.com
* Protect against overflow of ltree.numlevel and lquery.numlevel.Tom Lane2020-03-28
| | | | | | | | | | | | | | | | | | | | | | These uint16 fields could be overflowed by excessively long input, producing strange results. Complain for invalid input. Likewise check for out-of-range values of the repeat counts in lquery. (We don't try too hard on that one, notably not bothering to detect if atoi's result has overflowed.) Also detect length overflow in ltree_concat. In passing, be more consistent about whether "syntax error" messages include the type name. Also, clarify the documentation about what the size limit is. This has been broken for a long time, so back-patch to all supported branches. Nikita Glukhov, reviewed by Benjie Gillam and Tomas Vondra Discussion: https://postgr.es/m/CAP_rww=waX2Oo6q+MbMSiZ9ktdj6eaJj0cQzNu=Ry2cCDij5fw@mail.gmail.com
* Revert "Skip WAL for new relfilenodes, under wal_level=minimal."Noah Misch2020-03-22
| | | | | | | | This reverts commit cb2fd7eac285b1b0a24eeb2b8ed4456b66c5a09f. Per numerous buildfarm members, it was incompatible with parallel query, and a test case assumed LP64. Back-patch to 9.5 (all supported versions). Discussion: https://postgr.es/m/20200321224920.GB1763544@rfd.leadboat.com
* Skip WAL for new relfilenodes, under wal_level=minimal.Noah Misch2020-03-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Until now, only selected bulk operations (e.g. COPY) did this. If a given relfilenode received both a WAL-skipping COPY and a WAL-logged operation (e.g. INSERT), recovery could lose tuples from the COPY. See src/backend/access/transam/README section "Skipping WAL for New RelFileNode" for the new coding rules. Maintainers of table access methods should examine that section. To maintain data durability, just before commit, we choose between an fsync of the relfilenode and copying its contents to WAL. A new GUC, wal_skip_threshold, guides that choice. If this change slows a workload that creates small, permanent relfilenodes under wal_level=minimal, try adjusting wal_skip_threshold. Users setting a timeout on COMMIT may need to adjust that timeout, and log_min_duration_statement analysis will reflect time consumption moving to COMMIT from commands like COPY. Internally, this requires a reliable determination of whether RollbackAndReleaseCurrentSubTransaction() would unlink a relation's current relfilenode. Introduce rd_firstRelfilenodeSubid. Amend the specification of rd_createSubid such that the field is zero when a new rel has an old rd_node. Make relcache.c retain entries for certain dropped relations until end of transaction. Back-patch to 9.5 (all supported versions). This introduces a new WAL record type, XLOG_GIST_ASSIGN_LSN, without bumping XLOG_PAGE_MAGIC. As always, update standby systems before master systems. This changes sizeof(RelationData) and sizeof(IndexStmt), breaking binary compatibility for affected extensions. (The most recent commit to affect the same class of extensions was 089e4d405d0f3b94c74a2c6a54357a84a681754b.) Kyotaro Horiguchi, reviewed (in earlier, similar versions) by Robert Haas. Heikki Linnakangas and Michael Paquier implemented earlier designs that materially clarified the problem. Reviewed, in earlier designs, by Andrew Dunstan, Andres Freund, Alvaro Herrera, Tom Lane, Fujii Masao, and Simon Riggs. Reported by Martijn van Oosterhout. Discussion: https://postgr.es/m/20150702220524.GA9392@svana.org
* Add missing errcode() in a few ereport calls.Amit Kapila2020-03-18
| | | | | | | | | | This will allow to specifying SQLSTATE error code for the errors in the missing places. Reported-by: Sawada Masahiko Author: Sawada Masahiko Backpatch-through: 9.5 Discussion: https://postgr.es/m/CA+fd4k6N8EjNvZpM8nme+y+05mz-SM8Z_BgkixzkA34R+ej0Kw@mail.gmail.com
* Remove useless pfree()s at the ends of various ValuePerCall SRFs.Tom Lane2020-03-16
| | | | | | | | | | | | | | | | | | | | We don't need to manually clean up allocations in a SRF's multi_call_memory_ctx, because the SRF_RETURN_DONE infrastructure takes care of that (and also ensures that it will happen even if the function never gets a final call, which simple manual cleanup cannot do). Hence, the code removed by this patch is a waste of code and cycles. Worse, it gives the impression that cleaning up manually is a thing, which can lead to more serious errors such as those fixed in commits 085b6b667 and b4570d33a. So we should get rid of it. These are not quite actual bugs though, so I couldn't muster the enthusiasm to back-patch. Fix in HEAD only. Justin Pryzby Discussion: https://postgr.es/m/20200308173103.GC1357@telsasoft.com
* Avoid holding a directory FD open across assorted SRF calls.Tom Lane2020-03-16
| | | | | | | | | | | | | | | | | This extends the fixes made in commit 085b6b667 to other SRFs with the same bug, namely pg_logdir_ls(), pgrowlocks(), pg_timezone_names(), pg_ls_dir(), and pg_tablespace_databases(). Also adjust various comments and documentation to warn against expecting to clean up resources during a ValuePerCall SRF's final call. Back-patch to all supported branches, since these functions were all born broken. Justin Pryzby, with cosmetic tweaks by me Discussion: https://postgr.es/m/20200308173103.GC1357@telsasoft.com
* Improve checking of child pages in contrib/amcheck.Alexander Korotkov2020-03-11
| | | | | | | | | | | | | | | This commit eliminates lossiness in check for missing parent downlinks in B-tree. Instead of collecting lossy bitmap, we check for missing downlinks while visiting child pages referenced by downlinks of target level. We traverse from previous child page to the subsequent child page by right links. Intermediate pages are candidates to have lost parent downlinks. Also this commit introduces matching of child high key to the pivot key of it's parent. Discussion: https://postgr.es/m/CAPpHfduoF-c4RhOyOm%3D4-Y367%2B8txq9Q6iM_ty0OYc8si1Abww%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan
* Preserve integer and float values accurately in (de)serialize_deflist.Tom Lane2020-03-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, this code just smashed all types of DefElem values to strings, cavalierly reasoning that nobody would care. But in point of fact, most of the defGetFoo functions do distinguish among different input syntaxes; for instance defGetBoolean will accept 1 as an integer but not "1" as a string. This led to CREATE/ALTER TEXT SEARCH DICTIONARY accepting 0 and 1 as values for boolean dictionary properties, only to have the dictionary fail at runtime. We can upgrade this behavior by teaching serialize_deflist that it does not need to quote T_Integer or T_Float nodes' values on output, and then teaching deserialize_deflist to restore unquoted integer or float values as the appropriate node type. This should not break anything using pg_ts_dict.dictinitoption, since that field is just defined as being something valid to include in CREATE TEXT SEARCH DICTIONARY. deserialize_deflist is also used to parse the options arguments for the ts_headline family of functions, but so far as I can see this won't cause any problems there either: the only consumer of that output is prsd_headline which always uses defGetString. (Really that's a bad idea, but I won't risk changing it here.) This is surely a bug fix, but given the lack of field complaints I don't think it's necessary to back-patch. Discussion: https://postgr.es/m/CAMkU=1xRcs_BUPzR0+V3WndaCAv0E_m3h6aUEJ8NF-sY1nnHsw@mail.gmail.com
* Remove utils/acl.h from catalog/objectaddress.hPeter Eisentraut2020-03-10
| | | | | | | | | | | | | | | | | | The need for this was removed by 8b9e9644dc6a9bd4b7a97950e6212f63880cf18b. A number of files now need to include utils/acl.h or parser/parse_node.h explicitly where they previously got it indirectly somehow. Since parser/parse_node.h already includes nodes/parsenodes.h, the latter is then removed where the former was added. Also, remove nodes/pg_list.h from objectaddress.h, since that's included via nodes/parsenodes.h. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/7601e258-26b2-8481-36d0-dc9dca6f28f1%402ndquadrant.com
* Add tg_updatedcols to TriggerDataPeter Eisentraut2020-03-09
| | | | | | | | | | This allows a trigger function to determine for an UPDATE trigger which columns were actually updated. This allows some optimizations in generic trigger functions such as lo_manage and tsvector_update_trigger. Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://www.postgresql.org/message-id/flat/11c5f156-67a9-0fb5-8200-2a8018eb2e0c@2ndquadrant.com
* Add an "absval" parameter to allow contrib/dict_int to ignore signs.Tom Lane2020-03-08
| | | | | | Jeff Janes Discussion: https://postgr.es/m/CAMkU=1xRcs_BUPzR0+V3WndaCAv0E_m3h6aUEJ8NF-sY1nnHsw@mail.gmail.com
* Add an explicit test to catch changes in checksumming calculations.Tom Lane2020-03-08
| | | | | | | | Seems like a good idea in view of 006517432 and addd034ae. Michael Paquier, Tom Lane Discussion: https://postgr.es/m/20200306075230.GA118430@paquier.xyz
* pageinspect: Fix types used for bt_metap() columns.Peter Geoghegan2020-03-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The data types that contrib/pageinspect's bt_metap() function were declared to return as OUT arguments were wrong in some cases. For example, the oldest_xact column (a TransactionId/xid field) was declared integer/int4 within the pageinspect extension's sql file. This led to errors when an oldest_xact value that exceeded 2^31-1 was encountered. Some of the other columns were defined incorrectly ever since pageinspect was first introduced, though they were far less likely to produce problems in practice. Fix these issues by changing the declaration of bt_metap() to consistently use data types that can reliably represent all possible values. This fixes things on HEAD only. No backpatch, since it doesn't seem like there is a safe way to fix the issue without including a new version of the pageinspect extension (HEAD/Postgres 13 already introduced a new version of the extension). Besides, the oldest_xact issue has been around since the release of Postgres 11, and we haven't heard any complaints about it before now. Also, throw an error when we detect a bt_metap() declaration that must be from an old version of the pageinspect extension by examining the number of attributes from the tuple descriptor for the return tuples. It seems better to throw an error in a reliable and obvious way following a Postgres upgrade, rather than letting bt_metap() fail unpredictably. The problem is fundamentally with the CREATE FUNCTION declared data types themselves, so I see no sensible alternative. Reported-By: Victor Yegorov Bug: #16285 Discussion: https://postgr.es/m/16285-df8fc1000ab3d5fc@postgresql.org
* Create contrib/bool_plperl to provide a bool transform for PL/Perl[U].Tom Lane2020-03-06
| | | | | | | | | | | | | | | | | plperl's default handling of bool arguments or results is not terribly satisfactory, since Perl doesn't consider the string 'f' to be false. Ideally we'd just fix that, but the backwards-compatibility hazard would be substantial. Instead, build a TRANSFORM module that can be optionally applied to provide saner semantics. Perhaps usefully, this is also about the minimum possible skeletal example of a plperl transform module; so it might be a better starting point for user-written transform modules than hstore_plperl or jsonb_plperl. Ivan Panchenko Discussion: https://postgr.es/m/1583013317.881182688@f390.i.mail.ru
* Introduce macros for typalign and typstorage constants.Tom Lane2020-03-04
| | | | | | | | | | | | | | | | | | | | | Our usual practice for "poor man's enum" catalog columns is to define macros for the possible values and use those, not literal constants, in C code. But for some reason lost in the mists of time, this was never done for typalign/attalign or typstorage/attstorage. It's never too late to make it better though, so let's do that. The reason I got interested in this right now is the need to duplicate some uses of the TYPSTORAGE constants in an upcoming ALTER TYPE patch. But in general, this sort of change aids greppability and readability, so it's a good idea even without any specific motivation. I may have missed a few places that could be converted, and it's even more likely that pending patches will re-introduce some hard-coded references. But that's not fatal --- there's no expectation that we'd actually change any of these values. We can clean up stragglers over time. Discussion: https://postgr.es/m/16457.1583189537@sss.pgh.pa.us
* pg_standby: Don't use HAVE_WORKING_LINKPeter Eisentraut2020-03-03
| | | | | | | | | HAVE_WORKING_LINK is meant to indicate support for hard links, mainly for Windows. Here it is used for soft links on Unix, and the functionality is optional anyway, so we can just make it error out normally if needed. Discussion: https://www.postgresql.org/message-id/flat/72fff73f-dc9c-4ef4-83e8-d2e60c98df48%402ndquadrant.com
* Fix possibly-uninitialized variable.Tom Lane2020-03-02
| | | | Thinko in 2f9661311. Per buildfarm, as well as warning seen locally.
* Represent command completion tags as structsAlvaro Herrera2020-03-02
| | | | | | | | | | | | | | | | | | | | | | The backend was using strings to represent command tags and doing string comparisons in multiple places, but that's slow and unhelpful. Create a new command list with a supporting structure to use instead; this is stored in a tag-list-file that can be tailored to specific purposes with a caller-definable C macro, similar to what we do for WAL resource managers. The first first such uses are a new CommandTag enum and a CommandTagBehavior struct. Replace numerous occurrences of char *completionTag with a QueryCompletion struct so that the code no longer stores information about completed queries in a cstring. Only at the last moment, in EndCommand(), does this get converted to a string. EventTriggerCacheItem no longer holds an array of palloc’d tag strings in sorted order, but rather just a Bitmapset over the CommandTags. Author: Mark Dilger, with unsolicited help from Álvaro Herrera Reviewed-by: John Naylor, Tom Lane Discussion: https://postgr.es/m/981A9DB4-3F0C-4DA5-88AD-CB9CFF4D6CAD@enterprisedb.com
* Teach pageinspect about nbtree deduplication.Peter Geoghegan2020-02-29
| | | | | | | | | | | | | | | | | | | | | | | | | Add a new bt_metap() column to display the metapage's allequalimage field. Also add three new columns to contrib/pageinspect's bt_page_items() function: * Add a boolean column ("dead") that displays the LP_DEAD bit value for each non-pivot tuple. * Add a TID column ("htid") that displays a single heap TID value for each tuple. This is the TID that is returned by BTreeTupleGetHeapTID(), so comparable values are shown for pivot tuples, plain non-pivot tuples, and posting list tuples. * Add a TID array column ("tids") that displays TIDs from each tuple's posting list, if any. This works just like the "tids" column from pageinspect's gin_leafpage_items() function. No version bump for the pageinspect extension, since there hasn't been a stable Postgres release since the last version bump (the last bump was part of commit 58b4cb30). Author: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-WzmSMmU2eNvY9+a4MNP+z02h6sa-uxZvN3un6jY02ZVBSw@mail.gmail.com
* Add PostgreSQL home page to --help outputPeter Eisentraut2020-02-28
| | | | | | | | Per emerging standard in GNU programs and elsewhere. Autoconf already has support for specifying a home page, so we can just that. Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://www.postgresql.org/message-id/flat/8d389c5f-7fb5-8e48-9a4a-68cec44786fa%402ndquadrant.com
* Refer to bug report address by symbol rather than hardcodingPeter Eisentraut2020-02-28
| | | | | | | | | | Use the PACKAGE_BUGREPORT macro that is created by Autoconf for referring to the bug reporting address rather than hardcoding it everywhere. This makes it easier to change the address and it reduces translation work. Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://www.postgresql.org/message-id/flat/8d389c5f-7fb5-8e48-9a4a-68cec44786fa%402ndquadrant.com
* Move src/backend/utils/hash/hashfn.c to src/commonRobert Haas2020-02-27
| | | | | | | | | | | | | | This also involves renaming src/include/utils/hashutils.h, which becomes src/include/common/hashfn.h. Perhaps an argument can be made for keeping the hashutils.h name, but it seemed more consistent to make it match the name of the file, and also more descriptive of what is actually going on here. Patch by me, reviewed by Suraj Kharage and Mark Dilger. Off-list advice on how not to break the Windows build from Davinder Singh and Amit Kapila. Discussion: http://postgr.es/m/CA+TgmoaRiG4TXND8QuM6JXFRkM_1wL2ZNhzaUKsuec9-4yrkgw@mail.gmail.com
* Add deduplication to nbtree.Peter Geoghegan2020-02-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Deduplication reduces the storage overhead of duplicates in indexes that use the standard nbtree index access method. The deduplication process is applied lazily, after the point where opportunistic deletion of LP_DEAD-marked index tuples occurs. Deduplication is only applied at the point where a leaf page split would otherwise be required. New posting list tuples are formed by merging together existing duplicate tuples. The physical representation of the items on an nbtree leaf page is made more space efficient by deduplication, but the logical contents of the page are not changed. Even unique indexes make use of deduplication as a way of controlling bloat from duplicates whose TIDs point to different versions of the same logical table row. The lazy approach taken by nbtree has significant advantages over a GIN style eager approach. Most individual inserts of index tuples have exactly the same overhead as before. The extra overhead of deduplication is amortized across insertions, just like the overhead of page splits. The key space of indexes works in the same way as it has since commit dd299df8 (the commit that made heap TID a tiebreaker column). Testing has shown that nbtree deduplication can generally make indexes with about 10 or 15 tuples for each distinct key value about 2.5X - 4X smaller, even with single column integer indexes (e.g., an index on a referencing column that accompanies a foreign key). The final size of single column nbtree indexes comes close to the final size of a similar contrib/btree_gin index, at least in cases where GIN's posting list compression isn't very effective. This can significantly improve transaction throughput, and significantly reduce the cost of vacuuming indexes. A new index storage parameter (deduplicate_items) controls the use of deduplication. The default setting is 'on', so all new B-Tree indexes automatically use deduplication where possible. This decision will be reviewed at the end of the Postgres 13 beta period. There is a regression of approximately 2% of transaction throughput with synthetic workloads that consist of append-only inserts into a table with several non-unique indexes, where all indexes have few or no repeated values. The underlying issue is that cycles are wasted on unsuccessful attempts at deduplicating items in non-unique indexes. There doesn't seem to be a way around it short of disabling deduplication entirely. Note that deduplication of items in unique indexes is fairly well targeted in general, which avoids the problem there (we can use a special heuristic to trigger deduplication passes in unique indexes, since we're specifically targeting "version bloat"). Bump XLOG_PAGE_MAGIC because xl_btree_vacuum changed. No bump in BTREE_VERSION, since the representation of posting list tuples works in a way that's backwards compatible with version 4 indexes (i.e. indexes built on PostgreSQL 12). However, users must still REINDEX a pg_upgrade'd index to use deduplication, regardless of the Postgres version they've upgraded from. This is the only way to set the new nbtree metapage flag indicating that deduplication is generally safe. Author: Anastasia Lubennikova, Peter Geoghegan Reviewed-By: Peter Geoghegan, Heikki Linnakangas Discussion: https://postgr.es/m/55E4051B.7020209@postgrespro.ru https://postgr.es/m/4ab6e2db-bcee-f4cf-0916-3a06e6ccbb55@postgrespro.ru
* Fix compile failure.Tom Lane2020-02-24
| | | | | | I forgot that some compilers won't handle #if constructs within ereport() calls. Duplicating most of the call is annoying but simple. Per buildfarm.
* Account explicitly for long-lived FDs that are allocated outside fd.c.Tom Lane2020-02-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The comments in fd.c have long claimed that all file allocations should go through that module, but in reality that's not always practical. fd.c doesn't supply APIs for invoking some FD-producing syscalls like pipe() or epoll_create(); and the APIs it does supply for non-virtual FDs are mostly insistent on releasing those FDs at transaction end; and in some cases the actual open() call is in code that can't be made to use fd.c, such as libpq. This has led to a situation where, in a modern server, there are likely to be seven or so long-lived FDs per backend process that are not known to fd.c. Since NUM_RESERVED_FDS is only 10, that meant we had *very* few spare FDs if max_files_per_process is >= the system ulimit and fd.c had opened all the files it thought it safely could. The contrib/postgres_fdw regression test, in particular, could easily be made to fall over by running it under a restrictive ulimit. To improve matters, invent functions Acquire/Reserve/ReleaseExternalFD that allow outside callers to tell fd.c that they have or want to allocate a FD that's not directly managed by fd.c. Add calls to track all the fixed FDs in a standard backend session, so that we are honestly guaranteeing that NUM_RESERVED_FDS FDs remain unused below the EMFILE limit in a backend's idle state. The coding rules for these functions say that there's no need to call them in code that just allocates one FD over a fairly short interval; we can dip into NUM_RESERVED_FDS for such cases. That means that there aren't all that many places where we need to worry. But postgres_fdw and dblink must use this facility to account for long-lived FDs consumed by libpq connections. There may be other places where it's worth doing such accounting, too, but this seems like enough to solve the immediate problem. Internally to fd.c, "external" FDs are limited to max_safe_fds/3 FDs. (Callers can choose to ignore this limit, but of course it's unwise to do so except for fixed file allocations.) I also reduced the limit on "allocated" files to max_safe_fds/3 FDs (it had been max_safe_fds/2). Conceivably a smarter rule could be used here --- but in practice, on reasonable systems, max_safe_fds should be large enough that this isn't much of an issue, so KISS for now. To avoid possible regression in the number of external or allocated files that can be opened, increase FD_MINFREE and the lower limit on max_files_per_process a little bit; we now insist that the effective "ulimit -n" be at least 64. This seems like pretty clearly a bug fix, but in view of the lack of field complaints, I'll refrain from risking a back-patch. Discussion: https://postgr.es/m/E1izCmM-0005pV-Co@gemulon.postgresql.org
* Remove support for upgrading extensions from "unpackaged" state.Tom Lane2020-02-19
| | | | | | | | | | | | | | | | | | | | | | | | | Andres Freund pointed out that allowing non-superusers to run "CREATE EXTENSION ... FROM unpackaged" has security risks, since the unpackaged-to-1.0 scripts don't try to verify that the existing objects they're modifying are what they expect. Just attaching such objects to an extension doesn't seem too dangerous, but some of them do more than that. We could have resolved this, perhaps, by still requiring superuser privilege to use the FROM option. However, it's fair to ask just what we're accomplishing by continuing to lug the unpackaged-to-1.0 scripts forward. None of them have received any real testing since 9.1 days, so they may not even work anymore (even assuming that one could still load the previous "loose" object definitions into a v13 database). And an installation that's trying to go from pre-9.1 to v13 or later in one jump is going to have worse compatibility problems than whether there's a trivial way to convert their contrib modules into extension style. Hence, let's just drop both those scripts and the core-code support for "CREATE EXTENSION ... FROM". Discussion: https://postgr.es/m/20200213233015.r6rnubcvl4egdh5r@alap3.anarazel.de
* Stop demanding that top xact must be seen before subxact in decoding.Amit Kapila2020-02-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | Manifested as ERROR: subtransaction logged without previous top-level txn record this check forbids legit behaviours like - First xl_xact_assignment record is beyond reading, i.e. earlier restart_lsn. - After restart_lsn there is some change of a subxact. - After that, there is second xl_xact_assignment (for another subxact) revealing the relationship between top and first subxact. Such a transaction won't be streamed anyway because we hadn't seen it in full. Saying for sure whether xact of some record encountered after the snapshot was deserialized can be streamed or not requires to know whether it wrote something before deserialization point --if yes, it hasn't been seen in full and can't be decoded. Snapshot doesn't have such info, so there is no easy way to relax the check. Reported-by: Hsu, John Diagnosed-by: Arseny Sher Author: Arseny Sher, Amit Kapila Reviewed-by: Amit Kapila, Dilip Kumar Backpatch-through: 9.5 Discussion: https://postgr.es/m/AB5978B2-1772-4FEE-A245-74C91704ECB0@amazon.com
* Remove some dead code in contrib/adminpack/Michael Paquier2020-02-14
| | | | | | | | | | Since its introduction in fe59e56, the code in charge of validating and converting a file path includes some extra handling for absolute paths pointing to an external log_directory, but this has never been used. Author: Antonin Houska Reviewed-by: Julien Rouhaud, Michael Paquier Discussion: https://postgr.es/m/32663.1581592539@antos
* Mark some contrib modules as "trusted".Tom Lane2020-02-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This allows these modules to be installed into a database without superuser privileges (assuming that the DBA or sysadmin has installed the module's files in the expected place). You only need CREATE privilege on the current database, which by default would be available to the database owner. The following modules are marked trusted: btree_gin btree_gist citext cube dict_int earthdistance fuzzystrmatch hstore hstore_plperl intarray isn jsonb_plperl lo ltree pg_trgm pgcrypto seg tablefunc tcn tsm_system_rows tsm_system_time unaccent uuid-ossp In the future we might mark some more modules trusted, but there seems to be no debate about these, and on the whole it seems wise to be conservative with use of this feature to start out with. Discussion: https://postgr.es/m/32315.1580326876@sss.pgh.pa.us
* Use pg_pwrite() in more places.Thomas Munro2020-02-11
| | | | | | | | This removes some lseek() system calls. Author: Thomas Munro Reviewed-by: Andres Freund Discussion: https://postgr.es/m/CA%2BhUKGJ%2BoHhnvqjn3%3DHro7xu-YDR8FPr0FL6LF35kHRX%3D_bUzg%40mail.gmail.com
* In jsonb_plpython.c, suppress warning message from gcc 10.Tom Lane2020-01-30
| | | | | | | | | | | | Very recent gcc complains that PLyObject_ToJsonbValue could return a pointer to a local variable. I think it's wrong; but the coding is fragile enough, and the savings of one palloc() minimal enough, that it seems better to just do a palloc() all the time. (My other idea of tweaking the if-condition doesn't suppress the warning.) Back-patch to v11 where this code was introduced. Discussion: https://postgr.es/m/21547.1580170366@sss.pgh.pa.us
* Clean up newlines following left parenthesesAlvaro Herrera2020-01-30
| | | | | | | | | | | | We used to strategically place newlines after some function call left parentheses to make pgindent move the argument list a few chars to the left, so that the whole line would fit under 80 chars. However, pgindent no longer does that, so the newlines just made the code vertically longer for no reason. Remove those newlines, and reflow some of those lines for some extra naturality. Reviewed-by: Michael Paquier, Tom Lane Discussion: https://postgr.es/m/20200129200401.GA6303@alvherre.pgsql
* Remove excess parens in ereport() callsAlvaro Herrera2020-01-30
| | | | | | | Cosmetic cleanup, not worth backpatching. Discussion: https://postgr.es/m/20200129200401.GA6303@alvherre.pgsql Reviewed-by: Tom Lane, Michael Paquier
* Move jsonapi.c and jsonapi.h to src/common.Robert Haas2020-01-29
| | | | | | | | | | | To make this work, (1) makeJsonLexContextCstringLen now takes the encoding to be used as an argument; (2) check_stack_depth() is made to do nothing in frontend code, and (3) elog(ERROR, ...) is changed to pg_log_fatal + exit in frontend code. Mark Dilger, reviewed and slightly revised by me. Discussion: http://postgr.es/m/CA+TgmoYfOXhd27MUDGioVh6QtpD0C1K-f6ObSA10AWiHBAL5bA@mail.gmail.com
* Add connection parameters to control SSL protocol min/max in libpqMichael Paquier2020-01-28
| | | | | | | | | | | | | | | These two new parameters, named sslminprotocolversion and sslmaxprotocolversion, allow to respectively control the minimum and the maximum version of the SSL protocol used for the SSL connection attempt. The default setting is to allow any version for both the minimum and the maximum bounds, causing libpq to rely on the bounds set by the backend when negotiating the protocol to use for an SSL connection. The bounds are checked when the values are set at the earliest stage possible as this makes the checks independent of any SSL implementation. Author: Daniel Gustafsson Reviewed-by: Michael Paquier, Cary Huang Discussion: https://postgr.es/m/4F246AE3-A7AE-471E-BD3D-C799D3748E03@yesql.se