aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Fix planner's test for case-foldable characters in ILIKE with ICU.Tom Lane2019-08-12
| | | | | | | | | | | | | As coded, the ICU-collation path in pattern_char_isalpha() failed to consider regular ASCII letters to be case-varying. This led to like_fixed_prefix treating too much of an ILIKE pattern as being a fixed prefix, so that indexscans derived from an ILIKE clause might miss entries that they should find. Per bug #15892 from James Inform. This is an oversight in the original ICU patch (commit eccfef81e), so back-patch to v10 where that came in. Discussion: https://postgr.es/m/15892-e5d2bea3e8a04a1b@postgresql.org
* Remove EState.es_range_table_array.Tom Lane2019-08-12
| | | | | | | | | | Now that list_nth is O(1), there's no good reason to maintain a separate array of RTE pointers rather than indexing into estate->es_range_table. Deleting the array doesn't save all that much either; but just on cleanliness grounds, it's better not to have duplicate representations of the identical information. Discussion: https://postgr.es/m/14960.1565384592@sss.pgh.pa.us
* Rationalize use of list_concat + list_copy combinations.Tom Lane2019-08-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | In the wake of commit 1cff1b95a, the result of list_concat no longer shares the ListCells of the second input. Therefore, we can replace "list_concat(x, list_copy(y))" with just "list_concat(x, y)". To improve call sites that were list_copy'ing the first argument, or both arguments, invent "list_concat_copy()" which produces a new list sharing no ListCells with either input. (This is a bit faster than "list_concat(list_copy(x), y)" because it makes the result list the right size to start with.) In call sites that were not list_copy'ing the second argument, the new semantics mean that we are usually leaking the second List's storage, since typically there is no remaining pointer to it. We considered inventing another list_copy variant that would list_free the second input, but concluded that for most call sites it isn't worth worrying about, given the relative compactness of the new List representation. (Note that in cases where such leakage would happen, the old code already leaked the second List's header; so we're only discussing the size of the leak not whether there is one. I did adjust two or three places that had been troubling to free that header so that they manually free the whole second List.) Patch by me; thanks to David Rowley for review. Discussion: https://postgr.es/m/11587.1550975080@sss.pgh.pa.us
* Fix string comparison in jsonpathAlexander Korotkov2019-08-12
| | | | | | | | | Take into account pg_server_to_any() may return input string "as is". Reported-by: Andrew Dunstan, Thomas Munro Discussion: https://postgr.es/m/0ed83a33-d900-466a-880a-70ef456c721f%402ndQuadrant.com Author: Alexander Korotkov, Thomas Munro Backpatch-through: 12
* Partially revert "Insert temporary debugging output in regression tests."Tom Lane2019-08-11
| | | | | | | | | | | | This reverts much of commit f03a9ca4366d064d89b7cf7ed75d4e43f2ed0667, but leaves the relpages/reltuples probe in select_parallel.sql. The pg_stat_all_tables probes are unstable enough to be annoying, and it no longer seems likely that they will teach us anything more about the underlying problem. I'd still like some more confirmation though that the observed plan instability is caused by VACUUM leaving relpages/reltuples as zero for one of these tables. Discussion: https://postgr.es/m/CA+hUKG+0CxrKRWRMf5ymN3gm+BECHna2B-q1w8onKBep4HasUw@mail.gmail.com
* Adjust string comparison in jsonpathAlexander Korotkov2019-08-11
| | | | | | | | | | | | | | | | | | | | We have implemented jsonpath string comparison using default database locale. However, standard requires us to compare Unicode codepoints. This commit implements that, but for performance reasons we still use per-byte comparison for "==" operator. Thus, for consistency other comparison operators do per-byte comparison if Unicode codepoints appear to be equal. In some edge cases, when same Unicode codepoints have different binary representations in database encoding, we diverge standard to achieve better performance of "==" operator. In future to implement strict standard conformance, we can do normalization of input JSON strings. Original patch was written by Nikita Glukhov, rewritten by me. Reported-by: Markus Winand Discussion: https://postgr.es/m/8B7FA3B4-328D-43D7-95A8-37B8891B8C78%40winand.at Author: Nikita Glukhov, Alexander Korotkov Backpatch-through: 12
* Fix "ANALYZE t, t" inside a transaction block.Tom Lane2019-08-10
| | | | | | | | | | | | | | | | | | | | | | | | | | This failed with either "tuple already updated by self" or "duplicate key value violates unique constraint", depending on whether the table had previously been analyzed or not. The reason is that ANALYZE tried to insert or update the same pg_statistic rows twice, and there was no CommandCounterIncrement between. So add one. The same case works fine outside a transaction block, because then there's a whole transaction boundary between, as a consequence of the way VACUUM works. This issue has been latent all along, but the problem was unreachable before commit 11d8d72c2 added the ability to specify multiple tables in ANALYZE. We could, perhaps, alternatively fix it by adding code to de-duplicate the list of VacuumRelations --- but that would add a lot of overhead to work around dumb commands, so it's not attractive. Per bug #15946 from Yaroslav Schekin. Back-patch to v11. (Note: in v11 I also back-patched the test added by commit 23224563d; otherwise the problem doesn't manifest in the test I added, because "vactst" is empty when the tests for multiple ANALYZE targets are reached. That seems like not a very good thing anyway, so I did this rather than rethinking the choice of test case.) Discussion: https://postgr.es/m/15946-5c7570a2884a26cf@postgresql.org
* Rename tuplesort.c's SortTuple.tupindex field.Peter Geoghegan2019-08-09
| | | | | | | | Rename the "tupindex" field from tuplesort.c's SortTuple struct to "srctape", since it can only ever be used to store a source/input tape number when merging external sort runs. This has been the case since commit 8b304b8b72b, which removed replacement selection sort from tuplesort.c.
* Fix SIGSEGV in pruning for ScalarArrayOp with constant-null array.Tom Lane2019-08-09
| | | | | | | | | | Not much to be said here: commit 9fdb675fc should have checked constisnull, didn't. Per report from Piotr Włodarczyk. Back-patch to v11 where bug was introduced. Discussion: https://postgr.es/m/CAP-dhMr+vRpwizEYjUjsiZ1vwqpohTm+3Pbdt6Pr7FEgPq9R0Q@mail.gmail.com
* Cosmetic improvements in setup of planner's per-RTE arrays.Tom Lane2019-08-09
| | | | | | | | | | | | | Merge setup_append_rel_array into setup_simple_rel_arrays. There's no particularly good reason to keep them separate, and it's inconsistent with the lack of separation in expand_planner_arrays. The only apparent benefit was that the fast path for trivial queries in query_planner() doesn't need to set up the append_rel_array; but all we're saving there is an if-test and NULL assignment, which surely ought to be negligible. Also improve some obsolete comments. Discussion: https://postgr.es/m/17220.1565301350@sss.pgh.pa.us
* Refactor logic to remove trailing CR/LF characters from stringsMichael Paquier2019-08-09
| | | | | | | | | | b654714 has reworked the way trailing CR/LF characters are removed from strings. This commit introduces a new routine in common/string.c and refactors the code so as the logic is in a single place, mostly. Author: Michael Paquier Reviewed-by: Bruce Momjian Discussion: https://postgr.es/m/20190801031820.GF29334@paquier.xyz
* Update obsolete tuplesort READTUP() comment.Peter Geoghegan2019-08-08
| | | | | | | | | READTUP() routines do not and cannot use the resettable "tuplecontext" memory context, since it is deleted when merging begins. Update an obsolete comment that claimed otherwise. This was an oversight in commit e94568ecc10. In passing, fix an unrelated tuplesort typo.
* Fix certificate subjects in ldap testAndrew Dunstan2019-08-08
| | | | | | | openssl doesn't like lower case subject attribute names. Error observed in buildfarm results. Backpatch to release 11.
* initdb: Use varargs macro for PG_CMD_PRINTFPeter Eisentraut2019-08-08
| | | | | | | | | I left PG_CMD_PUTS around even though it could be handled by PG_CMD_PRINTF since PG_CMD_PUTS is sometimes called with non-literal arguments, and so that would create a potential problem if such a string contained percent signs. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
* Remove unnecessary #include <limits.h>Alvaro Herrera2019-08-07
| | | | | | | | This include was probably copied from tuplestore.c, but it's not needed. Extracted from a larger patch submitted by vignesh C <vignesh21@gmail.com> Discussion: https://postgr.es/m/CALDaNm1B9naPDTm3ox1m_yZvOm3KA5S4kZQSWWAeLHAQ=3gV1Q@mail.gmail.com
* Add comment on no default partition with hash partitioningAlvaro Herrera2019-08-07
| | | | Discussion: https://postgr.es/m/20190806222735.GA9535@alvherre.pgsql
* Apply constraint exclusion more generally in partitioningAlvaro Herrera2019-08-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | We were applying constraint exclusion on the partition constraint when generating pruning steps for a clause, but only for the rather restricted situation of them being boolean OR operators; however it is possible to have differently shaped clauses that also benefit from constraint exclusion. This applies particularly to the default partition since their constraints are in essence a long list of OR'ed subclauses ... but it applies to other cases too. So in certain cases we're scanning partitions that we don't need to. Remove the specialized code in OR clauses, and add a generally applicable test of the clause refuting the partition constraint; mark the whole pruning operation as contradictory if it hits. This has the unwanted side-effect of testing some (most? all?) constraints more than once if constraint_exclusion=on. That seems unavoidable as far as I can tell without some additional work, but that's not the recommended setting for that parameter anyway. However, because this imposes additional processing cost for all queries using partitioned tables, I decided not to backpatch this change. Author: Amit Langote, Yuzuko Hosoya, Álvaro Herrera Reviewers: Shawn Wang, Thibaut Madeleine, Yoshikazu Imai, Kyotaro Horiguchi; they were also uncredited reviewers for commit 489247b0e615. Discussion: https://postgr.es/m/9bb31dfe-b0d0-53f3-3ea6-e64b811424cf@lab.ntt.co.jp
* Fix typos in comments.Etsuro Fujita2019-08-07
|
* 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 some incorrect parsing of time with time zone stringsMichael Paquier2019-08-07
| | | | | | | | | | | | | | | | | | | When parsing a timetz string with a dynamic timezone abbreviation or a timezone not specified, it was possible to generate incorrect timestamps based on a date which uses some non-initialized variables if the input string did not specify fully a date to parse. This is already checked when a full timezone spec is included in the input string, but the two other cases mentioned above missed the same checks. This gets fixed by generating an error as this input is invalid, or in short when a date is not fully specified. Valgrind was complaining about this problem. Bug: #15910 Author: Alexander Lakhin Discussion: https://postgr.es/m/15910-2eba5106b9aa0c61@postgresql.org Backpatch-through: 9.4
* Adjust tuple data lookup logic in multi-insert logical decodingMichael Paquier2019-08-07
| | | | | | | | | | | | | | | | | | | | As of now, logical decoding of a multi-insert has been scanning all xl_multi_insert_tuple entries only if XLH_INSERT_CONTAINS_NEW_TUPLE was getting set in the record. This is not an issue on HEAD as multi-insert records are not used for system catalogs, but the logical decoding logic includes all the code necessary to handle that properly, except that the code missed to iterate correctly over all xl_multi_insert_tuple entries when the flag is not set. Hence, when trying to use multi-insert for system catalogs, an assertion would be triggered. An upcoming patch is going to make use of multi-insert for system catalogs, and this fixes the logic to make sure that all entries are scanned correctly without softening the existing assertions. Reported-by: Daniel Gustafsson Author: Michael Paquier Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/CBFFD532-C033-49EB-9A5A-F67EAEE9EB0B@yesql.se
* Save Kerberos and LDAP daemon logs where the buildfarm can find them.Tom Lane2019-08-06
| | | | | | | | | | | | | src/test/kerberos and src/test/ldap try to run private authentication servers, which of course might fail. The logs from these servers were being dropped into the tmp_check/ subdirectory, but they should be put in tmp_check/log/, because the buildfarm will only capture log files in that subdirectory. Without the log output there's little hope of diagnosing buildfarm failures related to these servers. Backpatch to v11 where these test suites were added. Discussion: https://postgr.es/m/16017.1565047605@sss.pgh.pa.us
* Fix typo in pathnode.cMichael Paquier2019-08-06
| | | | | Author: Amit Langote Discussion: https://postgr.es/m/CA+HiwqFhZ6ABoz-i=JZ5wMMyz-orx4asjR0og9qBtgEwOww6Yg@mail.gmail.com
* Show specific OID suggestion in unused_oids output.Peter Geoghegan2019-08-05
| | | | | | | | | | | | | | | | | | | Commit a6417078 established a new project policy around OID assignment: new patches are encouraged to choose a random OID in the 8000..9999 range when a manually-assigned OID is required (if multiple OIDs are required, a consecutive block of OIDs starting from the random point should be used). Catalog entries added by committed patches that use OIDs from this "unstable" range are renumbered after feature freeze. This practice minimizes OID collisions among concurrently-developed patches. Show a specific random OID suggestion when the unused_oids script is run. This makes it easy for patch authors to use a random OID from the unstable range, per the new policy. Author: Julien Rouhaud, Peter Geoghegan Reviewed-By: Tom Lane Discussion: https://postgr.es/m/CAH2-WzkkRs2ScmuBQ7xWi7xzp7fC1B3w0Nt8X+n4rBw5k+Z=zA@mail.gmail.com
* Fix choice of comparison operators for cross-type hashed subplans.Tom Lane2019-08-05
| | | | | | | | | | | | | | | | | | | | | | Commit bf6c614a2 rearranged the lookup of the comparison operators needed in a hashed subplan, and in so doing, broke the cross-type case: it caused the original LHS-vs-RHS operator to be used to compare hash table entries too (which of course are all of the RHS type). This leads to C functions being passed a Datum that is not of the type they expect, with the usual hazards of crashes and unauthorized server memory disclosure. For the set of hashable cross-type operators present in v11 core Postgres, this bug is nearly harmless on 64-bit machines, which may explain why it escaped earlier detection. But it is a live security hazard on 32-bit machines; and of course there may be extensions that add more hashable cross-type operators, which would increase the risk. Reported by Andreas Seltenreich. Back-patch to v11 where the problem came in. Security: CVE-2019-10209
* Require the schema qualification in pg_temp.type_name(arg).Noah Misch2019-08-05
| | | | | | | | | | | | Commit aa27977fe21a7dfa4da4376ad66ae37cb8f0d0b5 introduced this restriction for pg_temp.function_name(arg); do likewise for types created in temporary schemas. Programs that this breaks should add "pg_temp." schema qualification or switch to arg::type_name syntax. Back-patch to 9.4 (all supported versions). Reviewed by Tom Lane. Reported by Tom Lane. Security: CVE-2019-10208
* Add safeguards in LSN, numeric and float calculation for custom errorsMichael Paquier2019-08-05
| | | | | | | | | | | | | | | | | | | | | Those data types use parsing and/or calculation wrapper routines which can generate some generic error messages in the event of a failure. The caller of these routines can also pass a pointer variable settable by the routine to track if an error has happened, letting the caller decide what to do in the event of an error and what error message to generate. Those routines have been slacking the initialization of the tracking flag, which can be confusing when reading the code, so add some safeguards against calls of these parsing routines which could lead to a dubious result. The LSN parsing gains an assertion to make sure that the tracking flag is set, while numeric and float paths initialize the flag to a saner state. Author: Jeevan Ladhe Reviewed-by: Álvaro Herrera, Michael Paquier Discussion: https://postgr.es/m/CAOgcT0NOM9oR0Hag_3VpyW0uF3iCU=BDUFSPfk9JrWXRcWQHqw@mail.gmail.com
* Fix tab completion for ALTER LANGUAGE in psqlMichael Paquier2019-08-05
| | | | | | | | | | | OWNER_TO was used for the completion, which is not a supported grammar, but OWNER TO is. This error has been introduced by d37b816, so backpatch down to 9.6. Author: Alexander Lakhin Discussion: https://postgr.es/m/7ab243e0-116d-3e44-d120-76b3df7abefd@gmail.com Backpatch-through: 9.6
* 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
* Revert "Add log_statement_sample_rate parameter"Tomas Vondra2019-08-04
| | | | | | | | | | | | | | This reverts commit 88bdbd3f746049834ae3cc972e6e650586ec3c9d. As committed, statement sampling used the existing duration threshold (log_min_duration_statement) when decide which statements to sample. The issue is that even the longest statements are subject to sampling, and so may not end up logged. An improvement was proposed, introducing a second duration threshold, but it would not be backwards compatible. So we've decided to revert this feature - the separate threshold should be part of the feature itself. Discussion: https://postgr.es/m/CAFj8pRDS8tQ3Wviw9%3DAvODyUciPSrGeMhJi_WPE%2BEB8%2B4gLL-Q%40mail.gmail.com
* Revert "Silence compiler warning"Tomas Vondra2019-08-04
| | | | | | | | | | | | | | This reverts commit 9dc122585551516309c9362e673effdbf3bd79bd. As committed, statement sampling used the existing duration threshold (log_min_duration_statement) when decide which statements to sample. The issue is that even the longest statements are subject to sampling, and so may not end up logged. An improvement was proposed, introducing a second duration threshold, but it would not be backwards compatible. So we've decided to revert this feature - the separate threshold should be part of the feature itself. Discussion: https://postgr.es/m/CAFj8pRDS8tQ3Wviw9%3DAvODyUciPSrGeMhJi_WPE%2BEB8%2B4gLL-Q%40mail.gmail.com
* Avoid picking already-bound TCP ports in kerberos and ldap test suites.Tom Lane2019-08-04
| | | | | | | | | | | | | | | | | | | | | | | src/test/kerberos and src/test/ldap need to run a private authentication server of the relevant type, for which they need a free TCP port. They were just picking a random port number in 48K-64K, which works except when something's already using the particular port. Notably, the probability of failure rises dramatically if one simply runs those tests in a tight loop, because each test cycle leaves behind a bunch of high ports that are transiently in TIME_WAIT state. To fix, split out the code that PostgresNode.pm already had for identifying a free TCP port number, so that it can be invoked to choose a port for the KDC or LDAP server. This isn't 100% bulletproof, since conceivably something else on the machine could grab the port between the time we check and the time we actually start the server. But that's a pretty short window, so in practice this should be good enough. Back-patch to v11 where these test suites were added. Patch by me, reviewed by Andrew Dunstan. Discussion: https://postgr.es/m/3397.1564872168@sss.pgh.pa.us
* Improve pruning of a default partitionAlvaro Herrera2019-08-04
| | | | | | | | | | | | | | | | | | | When querying a partitioned table containing a default partition, we were wrongly deciding to include it in the scan too early in the process, failing to exclude it in some cases. If we reinterpret the PruneStepResult.scan_default flag slightly, we can do a better job at detecting that it can be excluded. The change is that we avoid setting the flag for that pruning step unless the step absolutely requires the default partition to be scanned (in contrast with the previous arrangement, which was to set it unless the step was able to prune it). So get_matching_partitions() must explicitly check the partition that each returned bound value corresponds to in order to determine whether the default one needs to be included, rather than relying on the flag from the final step result. Author: Yuzuko Hosoya <hosoya.yuzuko@lab.ntt.co.jp> Reviewed-by: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> Discussion: https://postgr.es/m/00e601d4ca86$932b8bc0$b982a340$@lab.ntt.co.jp
* Refactor BuildIndexInfo() with the new makeIndexInfo()Michael Paquier2019-08-04
| | | | | | | | | This portion of the code got forgotten in 7cce159 which has introduced a new routine to build this node, and this finishes the unification of the places where IndexInfo is initialized. Author: Michael Paquier Discussion: https://postgr.es/m/20190801041322.GA3435@paquier.xyz
* Fix representation of hash keys in Hash/HashJoin nodes.Andres Freund2019-08-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In 5f32b29c1819 I changed the creation of HashState.hashkeys to actually use HashState as the parent (instead of HashJoinState, which was incorrect, as they were executed below HashState), to fix the problem of hashkeys expressions otherwise relying on slot types appropriate for HashJoinState, rather than HashState as would be correct. That reliance was only introduced in 12, which is why it previously worked to use HashJoinState as the parent (although I'd be unsurprised if there were problematic cases). Unfortunately that's not a sufficient solution, because before this commit, the to-be-hashed expressions referenced inner/outer as appropriate for the HashJoin, not Hash. That didn't have obvious bad consequences, because the slots containing the tuples were put into ecxt_innertuple when hashing a tuple for HashState (even though Hash doesn't have an inner plan). There are less common cases where this can cause visible problems however (rather than just confusion when inspecting such executor trees). E.g. "ERROR: bogus varno: 65000", when explaining queries containing a HashJoin where the subsidiary Hash node's hash keys reference a subplan. While normally hashkeys aren't displayed by EXPLAIN, if one of those expressions references a subplan, that subplan may be printed as part of the Hash node - which then failed because an inner plan was referenced, and Hash doesn't have that. It seems quite possible that there's other broken cases, too. Fix the problem by properly splitting the expression for the HashJoin and Hash nodes at plan time, and have them reference the proper subsidiary node. While other workarounds are possible, fixing this correctly seems easy enough. It was a pretty ugly hack to have ExecInitHashJoin put the expression into the already initialized HashState, in the first place. I decided to not just split inner/outer hashkeys inside make_hashjoin(), but also to separate out hashoperators and hashcollations at plan time. Otherwise we would have ended up having two very similar loops, one at plan time and the other during executor startup. The work seems to more appropriately belong to plan time, anyway. Reported-By: Nikita Glukhov, Alexander Korotkov Author: Andres Freund Reviewed-By: Tom Lane, in an earlier version Discussion: https://postgr.es/m/CAPpHfdvGVegF_TKKRiBrSmatJL2dR9uwFCuR+teQ_8tEXU8mxg@mail.gmail.com Backpatch: 12-
* Fix format truncation issue from ECPG testMichael Paquier2019-08-02
| | | | | | | | | This fixes one warning generated by GCC and present in the test case array part of ECPG. This likely got missed in past fixes like 3a4b891 because the compilation of those tests is not done by default. Reported-by: Sergei Kornilov Discussion: https://postgr.es/m/14951331562847675@sas2-a1efad875d04.qloud-c.yandex.net
* Allow simplehash to use already-calculated hash values.Jeff Davis2019-08-01
| | | | | | | | | | | | | | | Add _lookup_hash and _insert_hash functions for callers that have already calculated the hash value of the key. The immediate use case is for hash algorithms that write to disk in partitions. The hash value can be calculated once, used to perform a lookup, used to select the partition, then written to the partition along with the tuple. When the tuple is read back, the hash value does not need to be recalculated. Author: Jeff Davis Reviewed-by: Andres Freund Discussion: https://postgr.es/m/48abe675e1330f0c264ab2fe0d4ff23eb244f9ef.camel%40j-davis.com
* Allow functions-in-FROM to be pulled up if they reduce to constants.Tom Lane2019-08-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This allows simplification of the plan tree in some common usage patterns: we can get rid of a join to the function RTE. In principle we could pull up any immutable expression, but restricting it to Consts avoids the risk that multiple evaluations of the expression might cost more than we can save. (Possibly this could be improved in future --- but we've more or less promised people that putting a function in FROM guarantees single evaluation, so we'd have to tread carefully.) To do this, we need to rearrange when eval_const_expressions() happens for expressions in function RTEs. I moved it to inline_set_returning_functions(), which already has to iterate over every function RTE, and in consequence renamed that function to preprocess_function_rtes(). A useful consequence is that inline_set_returning_function() no longer has to do this for itself, simplifying that code. In passing, break out pull_up_simple_subquery's code that knows where everything that needs pullup_replace_vars() processing is, so that the new pull_up_constant_function() routine can share it. We'd gotten away with one-and-a-half copies of that code so far, since pull_up_simple_values() could assume that a lot of cases didn't apply to it --- but I don't think pull_up_constant_function() can make any simplifying assumptions. Might as well make pull_up_simple_values() use it too. (Possibly this refactoring should go further: maybe we could share some of the code to fill in the pullup_replace_vars_context struct? For now, I left it that the callers fill that completely.) Note: the one existing test case that this patch changes has to be changed because inlining its function RTEs would destroy the point of the test, namely to check join order. Alexander Kuzmenkov and Aleksandr Parfenov, reviewed by Antonin Houska and Anastasia Lubennikova, and whacked around some more by me Discussion: https://postgr.es/m/402356c32eeb93d4fed01f66d6c7fe2d@postgrespro.ru
* Bump catversion.Peter Geoghegan2019-08-01
| | | | Oversight in commit 71dcd743.
* Add sort support routine for the inet data type.Peter Geoghegan2019-08-01
| | | | | | | | | | Add sort support for inet, including support for abbreviated keys. Testing has shown that this reduces the time taken to sort medium to large inet/cidr inputs by ~50-60% in realistic cases. Author: Brandur Leach Reviewed-By: Peter Geoghegan, Edmund Horner Discussion: https://postgr.es/m/CABR_9B-PQ8o2MZNJ88wo6r-NxW2EFG70M96Wmcgf99G6HUQ3sw@mail.gmail.com
* Add an isolation test to exercise parallel-worker deadlock resolution.Tom Lane2019-08-01
| | | | | | | | | | | | | | | | | | | | | | Commit a1c1af2a1 added logic in the deadlock checker to handle lock grouping, but it was very poorly tested, as evidenced by the bug fixed in 3420851a2. Add a test case that exercises that a bit better (and catches the bug --- if you revert 3420851a2, this will hang). Since it's pretty hard to get parallel workers to take exclusive regular locks that their parents don't already have, this test operates by creating a deadlock among advisory locks taken in parallel workers. To make that happen, we must override the parallel-safety labeling of the advisory-lock functions, which we do by putting them in mislabeled, non-inlinable wrapper functions. We also have to remove the redundant PreventAdvisoryLocksInParallelMode checks in lockfuncs.c. That seems fine though; if some user accidentally does what this test is intentionally doing, not much harm will ensue. (If there are any remaining bugs that are reachable that way, they're probably reachable in other ways too.) Discussion: https://postgr.es/m/3243.1564437314@sss.pgh.pa.us
* Mark advisory-lock functions as parallel restricted, not parallel unsafe.Tom Lane2019-08-01
| | | | | | | | | There seems no good reason not to allow a parallel leader to execute these functions. (The workers still can't, though. Although the code would work, any such lock would go away at worker exit, which is not the documented behavior of advisory locks.) Discussion: https://postgr.es/m/11847.1564496844@sss.pgh.pa.us
* 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
* Fix pg_dump's handling of dependencies for custom opclasses.Tom Lane2019-07-31
| | | | | | | | | | | | | | | | | | | | | | | | Since pg_dump doesn't treat the member operators and functions of operator classes/families (that is, the pg_amop and pg_amproc entries, not the underlying operators/functions) as separate dumpable objects, it missed their dependency information. I think this was safe when the code was designed, because the default object sorting rule emits operators and functions before opclasses, and there were no dependency types that could mess that up. However, the introduction of range types in 9.2 broke it: now a type can have a dependency on an opclass, allowing dependency rules to push the opclass before the type and hence before custom operators. Lacking any information showing that it shouldn't do so, pg_dump emitted the objects in the wrong order. Fix by teaching getDependencies() to translate pg_depend entries for pg_amop/amproc rows to look like dependencies for their parent opfamily. I added a regression test for this in HEAD/v12, but not further back; life is too short to fight with 002_pg_dump.pl. Per bug #15934 from Tom Gottfried. Back-patch to all supported branches. Discussion: https://postgr.es/m/15934-58b8c8ab7a09ea15@postgresql.org
* Run UTF8-requiring collation tests by defaultPeter Eisentraut2019-07-31
| | | | | | | | | | | | | | | | | | | | | The tests collate.icu.utf8 and collate.linux.utf8 were previously only run when explicitly selected via EXTRA_TESTS. They require a UTF8 database, because the error messages in the expected files refer to that, and they use some non-ASCII characters in the tests. Since users can select any locale and encoding for the regression test run, it was not possible to include these tests automatically. To fix, use psql's \if facility to check various prerequisites such as platform and the server encoding and quit the tests at the very beginning if the configuration is not adequate. We then need to maintain alternative expected files for these tests, but they are very tiny and never need to change after this. These two tests are now run automatically as part of the regression tests. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/052295c2-a2e1-9a21-bd36-8fbff8686cf3%402ndquadrant.com
* Remove superfluous newlines in function prototypes.Andres Freund2019-07-31
| | | | | | | | | | | | | | | These were introduced by pgindent due to fixe to broken indentation (c.f. 8255c7a5eeba8). Previously the mis-indentation of function prototypes was creatively used to reduce indentation in a few places. As that formatting only exists in master and REL_12_STABLE, it seems better to fix it in both, rather than having some odd indentation in v12 that somebody might copy for future patches or such. Author: Andres Freund Discussion: https://postgr.es/m/20190728013754.jwcbe5nfyt3533vx@alap3.anarazel.de Backpatch: 12-
* Remove superfluous semicolon.Andres Freund2019-07-30
| | | | Author: Andres Freund
* Allow table AM's to use rd_amcache, too.Heikki Linnakangas2019-07-30
| | | | | | | | | | | The rd_amcache allows an index AM to cache arbitrary information in a relcache entry. This commit moves the cleanup of rd_amcache so that it can also be used by table AMs. Nothing takes advantage of that yet, but I'm sure it'll come handy for anyone writing new table AMs. Backpatch to v12, where table AM interface was introduced. Reviewed-by: Julien Rouhaud
* Print WAL position correctly in pg_rewind error message.Heikki Linnakangas2019-07-30
| | | | | | | | | | This has been wrong ever since pg_rewind was added. The if-branch just above this, where we print the same error with an extra message supplied by XLogReadRecord() got this right, but the variable name was wrong in the else-branch. As a consequence, the error printed the WAL position as 0/0 if there was an error reading a WAL file. Backpatch to 9.5, where pg_rewind was added.
* Don't build extended statistics on inheritance treesTomas Vondra2019-07-30
| | | | | | | | | | | | | | | | | | | | | | | | | | When performing ANALYZE on inheritance trees, we collect two samples for each relation - one for the relation alone, and one for the inheritance subtree (relation and its child relations). And then we build statistics on each sample, so for each relation we get two sets of statistics. For regular (per-column) statistics this works fine, because the catalog includes a flag differentiating statistics built from those two samples. But we don't have such flag in the extended statistics catalogs, and we ended up updating the same row twice, triggering this error: ERROR: tuple already updated by self The simplest solution is to disable extended statistics on inheritance trees, which is what this commit is doing. In the future we may need to do something similar to per-column statistics, but that requires adding a flag to the catalog - and that's not backpatchable. Moreover, the current selectivity estimation code only works with individual relations, so building statistics on inheritance trees would be pointless anyway. Author: Tomas Vondra Backpatch-to: 10- Discussion: https://postgr.es/m/20190618231233.GA27470@telsasoft.com Reported-by: Justin Pryzby