summaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2021-07-12Remove dead assignment to local variable.Heikki Linnakangas
This should have been removed in commit 7e30c186da, which split the loop into two. Only the first loop uses the 'from' variable; updating it in the second loop is bogus. It was never read after the first loop, so this was harmless and surely optimized away by the compiler, but let's be tidy. Backpatch to all supported versions. Author: Ranier Vilela Discussion: https://www.postgresql.org/message-id/CAEudQAoWq%2BAL3BnELHu7gms2GN07k-np6yLbukGaxJ1vY-zeiQ%40mail.gmail.com
2021-07-12Revert "Fix issues with Windows' stat() for files pending on deletion"Michael Paquier
This reverts commit 54fb8c7, as per the issues reported by fairywren when it comes to MinGW because of the lack of microsoft_native_stat() there. Using just stat() for MSVC is not sufficient to take care of the concurrency problems with files pending on deletion. It may be possible to paint some __MINGW64__ in the code to switch to a different implementation of stat() in this build context, but I am not sure either if relying on the implementation of stat() in MinGW to take care of the problems we are trying to fix is enough or not. So this needs more study. Discussion: https://postgr.es/m/YOvOlfRrIO0yGtgw@paquier.xyz Backpatch-through: 14
2021-07-12Fix issues with Windows' stat() for files pending on deletionMichael Paquier
The code introduced by bed9075 to enhance the stat() implementation on Windows for file sizes larger than 4GB fails to properly detect files pending for deletion with its method based on NtQueryInformationFile() or GetFileInformationByHandleEx(), as proved by Alexander Lakhin in a custom TAP test of his own. The method used in the implementation of open() to sleep and loop when when failing on ERROR_ACCESS_DENIED (EACCES) is showing much more stability, so switch to this method. This could still lead to issues if the permission problem stays around for much longer than the timeout of 1 second used, but that should (hopefully) never happen in performance-critical paths. Still, there could be a point in increasing the timeouts for the sake of machines that handle heavy loads. Note that WIN32's open() now uses microsoft_native_stat() as it should be similar to stat() when working around issues with concurrent file deletions. I have spent some time testing this patch with pgbench in combination of the SQL functions from genfile.c, as well as running the TAP test provided on the thread with MSVC builds, and this looks much more stable than the previous method. Author: Alexander Lakhin Reviewed-by: Tom Lane, Michael Paquier, Justin Pryzby Discussion: https://postgr.es/m/c3427edf-d7c0-ff57-90f6-b5de3bb62709@gmail.com Backpatch-through: 14
2021-07-12pageinspect: Improve page_header() for pages of 32kBMichael Paquier
ld_upper, ld_lower, pd_special and the page size have been using smallint as return type, which could cause those fields to return negative values in certain cases for builds configures with a page size of 32kB. Bump pageinspect to 1.10. page_header() is able to handle the correct return type of those fields at runtime when using an older version of the extension, with some tests are added to cover that. Author: Quan Zongliang Reviewed-by: Michael Paquier, Bharath Rupireddy Discussion: https://postgr.es/m/8b8ec36e-61fe-14f9-005d-07bc85aa4eed@yeah.net
2021-07-11Lock the extension during ALTER EXTENSION ADD/DROP.Tom Lane
Although we were careful to lock the object being added or dropped, we failed to get any sort of lock on the extension itself. This allowed the ALTER to proceed in parallel with a DROP EXTENSION, which is problematic for a couple of reasons. If both commands succeeded we'd be left with a dangling link in pg_depend, which would cause problems later. Also, if the ALTER failed for some reason, it might try to print the extension's name, and that could result in a crash or (in older branches) a silly error message complaining about extension "(null)". Per bug #17098 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/17098-b960f3616c861f83@postgresql.org
2021-07-11Fix pgbench timestamp bugs.Thomas Munro
Commit 547f04e changed pgbench to use microsecond accounting, but introduced a couple of logging and aggregation bugs: 1. We print Unix epoch timestamps so that you can correlate them with other logs, but these were inadvertently changed to use a system-dependent reference epoch. Compute Unix timestamps, and begin aggregation intervals on the boundaries of whole Unix epoch seconds, as before. 2. The user-supplied aggregation interval needed to be scaled. Back-patch to 14. Author: Fabien COELHO <coelho@cri.ensmp.fr> Author: Yugo NAGATA <nagata@sraoss.co.jp> Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reported-by: YoungHwan Joo <rulyox@gmail.com> Reported-by: Gregory Smith <gregsmithpgsql@gmail.com> Discussion: https://postgr.es/m/CAF7igB1r6wRfSCEAB-iZBKxkowWY6%2BdFF2jObSdd9%2BiVK%2BvHJg%40mail.gmail.com Discussion: https://postgr.es/m/CAHLJuCW_8Vpcr0%3Dt6O_gozrg3wXXWXZXDioYJd3NhvKriqgpfQ%40mail.gmail.com
2021-07-10Fix assign_record_type_typmod().Jeff Davis
If an error occurred in the wrong place, it was possible to leave an unintialized entry in the hash table, leading to a crash. Fixed. Also, be more careful about the order of operations so that an allocation error doesn't leak memory in CacheMemoryContext or unnecessarily advance NextRecordTypmod. Backpatch through version 11. Earlier versions (prior to 35ea75632a5) do not exhibit the problem, because an uninitialized hash entry contains a valid empty list. Author: Sait Talha Nisanci <Sait.Nisanci@microsoft.com> Reviewed-by: Andres Freund Discussion: https://postgr.es/m/HE1PR8303MB009069D476225B9A9E194B8891779@HE1PR8303MB0090.EURPRD83.prod.outlook.com Backpatch-through: 11
2021-07-10Fix busted test for ldap_initialize.Tom Lane
Sigh ... I was expecting AC_CHECK_LIB to do something it didn't, namely update LIBS. This led to not finding ldap_initialize. Fix by moving the probe for ldap_initialize. In some sense this is more correct anyway, since (at least for now) we care about whether ldap_initialize exists in libldap not libldap_r. Per buildfarm member elver and local testing. Discussion: https://postgr.es/m/17083-a19190d9591946a7@postgresql.org
2021-07-10Add more sanity checks in SASL exchangesMichael Paquier
The following checks are added, to make the SASL infrastructure more aware of defects when implementing new mechanisms: - Detect that no output is generated by a mechanism if an exchange fails in the backend, failing if there is a message waiting to be sent. - Handle zero-length messages in the frontend. The backend handles that already, and SCRAM would complain if sending empty messages as this is not authorized for this mechanism, but other mechanisms may want this capability (the SASL specification allows that). - Make sure that a mechanism generates a message in the middle of the exchange in the frontend. SCRAM, as implemented, respects all these requirements already, and the recent refactoring of SASL done in 9fd8557 helps in documenting that in a cleaner way. Analyzed-by: Jacob Champion Author: Michael Paquier Reviewed-by: Jacob Champion Discussion: https://postgr.es/m/3d2a6f5d50e741117d6baf83eb67ebf1a8a35a11.camel@vmware.com
2021-07-10Fix numeric_mul() overflow due to too many digits after decimal point.Dean Rasheed
This fixes an overflow error when using the numeric * operator if the result has more than 16383 digits after the decimal point by rounding the result. Overflow errors should only occur if the result has too many digits *before* the decimal point. Discussion: https://postgr.es/m/CAEZATCUmeFWCrq2dNzZpRj5+6LfN85jYiDoqm+ucSXhb9U2TbA@mail.gmail.com
2021-07-09Un-break AIX build, take 2.Tom Lane
I incorrectly diagnosed the reason why hoverfly is unhappy. Looking closer, it appears that it fails to link libldap unless libssl is also present; so the problem was my idea of clearing LIBS before making the check. Revert to essentially the original coding, except that instead of failing when libldap_r isn't there, use libldap. Per buildfarm member hoverfly. Discussion: https://postgr.es/m/17083-a19190d9591946a7@postgresql.org
2021-07-09libpq: Fix sending queries in pipeline aborted stateAlvaro Herrera
When sending queries in pipeline mode, we were careless about leaving the connection in the right state so that PQgetResult would behave correctly; trying to read further results after sending a query after having read a result with an error would sometimes hang. Fix by ensuring internal libpq state is changed properly. All the state changes were being done by the callers of pqAppendCmdQueueEntry(); it would have become too repetitious to have this logic in each of them, so instead put it all in that function and relieve callers of the responsibility. Add a test to verify this case. Without the code fix, this new test hangs sometimes. Also, document that PQisBusy() would return false when no queries are pending result. This is not intuitively obvious, and NULL would be obtained by calling PQgetResult() at that point, which is confusing. Wording by Boris Kolpackov. In passing, fix bogus use of "false" to mean "0", per Ranier Vilela. Backpatch to 14. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reported-by: Boris Kolpackov <boris@codesynthesis.com> Discussion: https://postgr.es/m/boris.20210624103805@codesynthesis.com
2021-07-09Eliminate replication protocol error related to IDENTIFY_SYSTEM.Jeff Davis
The requirement that IDENTIFY_SYSTEM be run before START_REPLICATION was both undocumented and unnecessary. Remove the error and ensure that ThisTimeLineID is initialized in START_REPLICATION. Elect not to backport because this requirement was expected behavior (even if inconsistently enforced), and is not likely to cause any major problem. Author: Jeff Davis Reviewed-by: Andres Freund Discussion: https://postgr.es/m/de4bbf05b7cd94227841c433ea6ff71d2130c713.camel%40j-davis.com
2021-07-09Un-break AIX build.Tom Lane
In commit d0a02bdb8, I'd supposed that uniformly probing for ldap_bind would make the intent clearer. However, that seems not to work on AIX, for obscure reasons (maybe it's a macro there?). Revert to the former behavior of probing ldap_simple_bind for thread-safe cases and ldap_bind otherwise. Per buildfarm member hoverfly. Discussion: https://postgr.es/m/17083-a19190d9591946a7@postgresql.org
2021-07-09Avoid creating a RESULT RTE that's marked LATERAL.Tom Lane
Commit 7266d0997 added code to pull up simple constant function results, converting the RTE_FUNCTION RTE to a dummy RTE_RESULT RTE since it no longer need be scanned. But I forgot to clear the LATERAL flag if the RTE has it set. If the function reduced to a constant, it surely contains no lateral references so this simplification is logically OK. It's needed because various other places will Assert that RESULT RTEs aren't LATERAL. Per bug #17097 from Yaoguang Chen. Back-patch to v13 where the faulty code came in. Discussion: https://postgr.es/m/17097-3372ef9f798fc94f@postgresql.org
2021-07-09Update configure's probe for libldap to work with OpenLDAP 2.5.Tom Lane
The separate libldap_r is gone and libldap itself is now always thread-safe. Unfortunately there seems no easy way to tell by inspection whether libldap is thread-safe, so we have to take it on faith that libldap is thread-safe if there's no libldap_r. That should be okay, as it appears that libldap_r was a standard part of the installation going back at least 20 years. Report and patch by Adrian Ho. Back-patch to all supported branches, since people might try to build any of them with a newer OpenLDAP. Discussion: https://postgr.es/m/17083-a19190d9591946a7@postgresql.org
2021-07-09Reject cases where a query in WITH rewrites to just NOTIFY.Tom Lane
Since the executor can't cope with a utility statement appearing as a node of a plan tree, we can't support cases where a rewrite rule inserts a NOTIFY into an INSERT/UPDATE/DELETE command appearing in a WITH clause of a larger query. (One can imagine ways around that, but it'd be a new feature not a bug fix, and so far there's been no demand for it.) RewriteQuery checked for this, but it missed the case where the DML command rewrites to *only* a NOTIFY. That'd lead to crashes later on in planning. Add the missed check, and improve the level of testing of this area. Per bug #17094 from Yaoguang Chen. It's been busted since WITH was introduced, so back-patch to all supported branches. Discussion: https://postgr.es/m/17094-bf15dff55eaf2e28@postgresql.org
2021-07-09Teach pg_size_pretty and pg_size_bytes about petabytesDavid Rowley
There was talk about adding units all the way up to yottabytes but it seems quite far-fetched that anyone would need those. Since such large units are not exactly commonplace, it seems unlikely that having pg_size_pretty outputting unit any larger than petabytes would actually be helpful to anyone. Since petabytes are on the horizon, let's just add those only. Maybe one day we'll get to add additional units, but it will likely be a while before we'll need to think beyond petabytes in regards to the size of a database. Author: David Christensen Discussion: https://postgr.es/m/CAOxo6XKmHc_WZip-x5QwaOqFEiCq_SVD0B7sbTZQk+qqcn2qaw@mail.gmail.com
2021-07-09Add forgotten LSN_FORMAT_ARGS() in xlogreader.cMichael Paquier
These should have been part of 4035cd5, that introduced LZ4 support for wal_compression.
2021-07-09Remove more obsolete comments about semaphores.Thomas Munro
Commit 6753333f stopped using semaphores as the sleep/wake mechanism for heavyweight locks, but some obsolete references to that scheme remained in comments. As with similar commit 25b93a29, back-patch all the way. Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/CA%2BhUKGLafjB1uzXcy%3D%3D2L3cy7rjHkqOVn7qRYGBjk%3D%3DtMJE7Yg%40mail.gmail.com
2021-07-09Use a lookup table for units in pg_size_pretty and pg_size_bytesDavid Rowley
We've grown 2 versions of pg_size_pretty over the years, one for BIGINT and one for NUMERIC. Both should output the same, but keeping them in sync is harder than needed due to neither function sharing a source of truth about which units to use and how to transition to the next largest unit. Here we add a static array which defines the units that we recognize and have both pg_size_pretty and pg_size_pretty_numeric use it. This will make adding any units in the future a very simple task. The table contains all information required to allow us to also modify pg_size_bytes to use the lookup table, so adjust that too. There are no behavioral changes here. Author: David Rowley Reviewed-by: Dean Rasheed, Tom Lane, David Christensen Discussion: https://postgr.es/m/CAApHDvru1F7qsEVL-iOHeezJ+5WVxXnyD_Jo9nht+Eh85ekK-Q@mail.gmail.com
2021-07-09Fix incorrect return value in pg_size_pretty(bigint)David Rowley
Due to how pg_size_pretty(bigint) was implemented, it's possible that when given a negative number of bytes that the returning value would not match the equivalent positive return value when given the equivalent positive number of bytes. This was due to two separate issues. 1. The function used bit shifting to convert the number of bytes into larger units. The rounding performed by bit shifting is not the same as dividing. For example -3 >> 1 = -2, but -3 / 2 = -1. These two operations are only equivalent with positive numbers. 2. The half_rounded() macro rounded towards positive infinity. This meant that negative numbers rounded towards zero and positive numbers rounded away from zero. Here we fix #1 by dividing the values instead of bit shifting. We fix #2 by adjusting the half_rounded macro always to round away from zero. Additionally, adjust the pg_size_pretty(numeric) function to be more explicit that it's using division rather than bit shifting. A casual observer might have believed bit shifting was used due to a static function being named numeric_shift_right. However, that function was calculating the divisor from the number of bits and performed division. Here we make that more clear. This change is just cosmetic and does not affect the return value of the numeric version of the function. Here we also add a set of regression tests both versions of pg_size_pretty() which test the values directly before and after the function switches to the next unit. This bug was introduced in 8a1fab36a. Prior to that negative values were always displayed in bytes. Author: Dean Rasheed, David Rowley Discussion: https://postgr.es/m/CAEZATCXnNW4HsmZnxhfezR5FuiGgp+mkY4AzcL5eRGO4fuadWg@mail.gmail.com Backpatch-through: 9.6, where the bug was introduced.
2021-07-08Fix typos in pgstat.c, reorderbuffer.c and pathnodes.hDaniel Gustafsson
Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/50250765-5B87-4AD7-9770-7FCED42A6175@yesql.se
2021-07-08Improve error messages about mismatching relkindPeter Eisentraut
Most error messages about a relkind that was not supported or appropriate for the command was of the pattern "relation \"%s\" is not a table, foreign table, or materialized view" This style can become verbose and tedious to maintain. Moreover, it's not very helpful: If I'm trying to create a comment on a TOAST table, which is not supported, then the information that I could have created a comment on a materialized view is pointless. Instead, write the primary error message shorter and saying more directly that what was attempted is not possible. Then, in the detail message, explain that the operation is not supported for the relkind the object was. To simplify that, add a new function errdetail_relkind_not_supported() that does this. In passing, make use of RELKIND_HAS_STORAGE() where appropriate, instead of listing out the relkinds individually. Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/flat/dc35a398-37d0-75ce-07ea-1dd71d98f8ec@2ndquadrant.com
2021-07-07Fix crash in postgres_fdw for provably-empty remote UPDATE/DELETE.Tom Lane
In 86dc90056, I'd written find_modifytable_subplan with the assumption that if the immediate child of a ModifyTable is a Result, it must be a projecting Result with a subplan. However, if the UPDATE or DELETE has a provably-constant-false WHERE clause, that's not so: we'll generate a dummy subplan with a childless Result. Add the missing null-check so we don't crash on such cases. Per report from Alexander Pyhalov. Discussion: https://postgr.es/m/b9a6f53549456b2f3e2fd150dcd79d72@postgrespro.ru
2021-07-07doc: Fix description about pg_stat_statements.track_planning.Fujii Masao
This commit fixes wrong wording like "a fewer kinds" in the description about track_planning option. Back-patch to v13 where pg_stat_statements.track_planning was added. Author: Justin Pryzby Reviewed-by: Julien Rouhaud, Fujii Masao Discussion: https://postgr.es/m/20210418233615.GB7256@telsasoft.com
2021-07-07Use a hash table to speed up NOT IN(values)David Rowley
Similar to 50e17ad28, which allowed hash tables to be used for IN clauses with a set of constants, here we add the same feature for NOT IN clauses. NOT IN evaluates the same as: WHERE a <> v1 AND a <> v2 AND a <> v3. Obviously, if we're using a hash table we must be exactly equivalent to that and return the same result taking into account that either side of the condition could contain a NULL. This requires a little bit of special handling to make work with the hash table version. When processing NOT IN, the ScalarArrayOpExpr's operator will be the <> operator. To be able to build and lookup a hash table we must use the <>'s negator operator. The planner checks if that exists and is hashable and sets the relevant fields in ScalarArrayOpExpr to instruct the executor to use hashing. Author: David Rowley, James Coleman Reviewed-by: James Coleman, Zhihong Yu Discussion: https://postgr.es/m/CAApHDvoF1mum_FRk6D621edcB6KSHBi2+GAgWmioj5AhOu2vwQ@mail.gmail.com
2021-07-07postgres_fdw: Tighten up allowed values for batch_size, fetch_size options.Fujii Masao
Previously the values such as '100$%$#$#', '9,223,372,' were accepted and treated as valid integers for postgres_fdw options batch_size and fetch_size. Whereas this is not the case with fdw_startup_cost and fdw_tuple_cost options for which an error is thrown. This was because endptr was not used while converting strings to integers using strtol. This commit changes the logic so that it uses parse_int function instead of strtol as it serves the purpose by returning false in case if it is unable to convert the string to integer. Note that this function also rounds off the values such as '100.456' to 100 and '100.567' or '100.678' to 101. While on this, use parse_real for fdw_startup_cost and fdw_tuple_cost options. Since parse_int and parse_real are being used for reloptions and GUCs, it is more appropriate to use in postgres_fdw rather than using strtol and strtod directly. Back-patch to v14. Author: Bharath Rupireddy Reviewed-by: Ashutosh Bapat, Tom Lane, Kyotaro Horiguchi, Fujii Masao Discussion: https://postgr.es/m/CALj2ACVMO6wY5Pc4oe1OCgUOAtdjHuFsBDw8R5uoYR86eWFQDA@mail.gmail.com
2021-07-07Refactor SASL code with a generic interface for its mechanismsMichael Paquier
The code of SCRAM and SASL have been tightly linked together since SCRAM exists in the core code, making hard to apprehend the addition of new SASL mechanisms, but these are by design different facilities, with SCRAM being an option for SASL. This refactors the code related to both so as the backend and the frontend use a set of callbacks for SASL mechanisms, documenting while on it what is expected by anybody adding a new SASL mechanism. The separation between both layers is neat, using two sets of callbacks for the frontend and the backend to mark the frontier between both facilities. The shape of the callbacks is now directly inspired from the routines used by SCRAM, so the code change is straight-forward, and the SASL code is moved into its own set of files. These will likely change depending on how and if new SASL mechanisms get added in the future. Author: Jacob Champion Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/3d2a6f5d50e741117d6baf83eb67ebf1a8a35a11.camel@vmware.com
2021-07-06Allow CustomScan providers to say whether they support projections.Tom Lane
Previously, all CustomScan providers had to support projections, but there may be cases where this is inconvenient. Add a flag bit to say if it's supported. Important item for the release notes: this is non-backwards-compatible since the default is now to assume that CustomScan providers can't project, instead of assuming that they can. It's fail-soft, but could result in visible performance penalties due to adding unnecessary Result nodes. Sven Klemm, reviewed by Aleksander Alekseev; some cosmetic fiddling by me. Discussion: https://postgr.es/m/CAMCrgp1kyakOz6c8aKhNDJXjhQ1dEjEnp+6KNT3KxPrjNtsrDg@mail.gmail.com
2021-07-06Improve TestLib::system_or_bail error reportingAlvaro Herrera
The original coding was not quoting the complete failing command, and it wasn't printing the reason for the failure either. Do both. This is cosmetic only, so no backpatch. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/202106301524.eq5pblzstapj@alvherre.pgsql
2021-07-06Reduce the cost of planning deeply-nested views.Tom Lane
Joel Jacobson reported that deep nesting of trivial (flattenable) views results in O(N^3) growth of planning time for N-deep nesting. It turns out that a large chunk of this cost comes from copying around the "subquery" sub-tree of each view's RTE_SUBQUERY RTE. But once we have successfully flattened the subquery, we don't need that anymore, because the planner isn't going to do anything else interesting with that RTE. We already zap the subquery pointer during setrefs.c (cf. add_rte_to_flat_rtable), but it's useless baggage earlier than that too. Clearing the pointer as soon as pull_up_simple_subquery is done with the RTE reduces the cost from O(N^3) to O(N^2); which is still not great, but it's quite a lot better. Further improvements will require rethinking of the RTE data structure, which is being considered in another thread. Patch by me; thanks to Dean Rasheed for review. Discussion: https://postgr.es/m/797aff54-b49b-4914-9ff9-aa42564a4d7d@www.fastmail.com
2021-07-06Avoid doing catalog lookups in postgres_fdw's conversion_error_callback.Tom Lane
As in 50371df26, this is a bad idea since the callback can't really know what error is being thrown and thus whether or not it is safe to attempt catalog accesses. Rather than pushing said accesses into the mainline code where they'd usually be a waste of cycles, we can look at the query's rangetable instead. This change does mean that we'll be printing query aliases (if any were used) rather than the table or column's true name. But that doesn't seem like a bad thing: it's certainly a more useful definition in self-join cases, for instance. In any case, it seems unlikely that any applications would be depending on this detail, so it seems safe to change. Patch by me. Original complaint by Andres Freund; Bharath Rupireddy noted the connection to conversion_error_callback. Discussion: https://postgr.es/m/20210106020229.ne5xnuu6wlondjpe@alap3.anarazel.de
2021-07-06Doc: add info about timestamps with fractional-minute UTC offsets.Tom Lane
Our code has supported fractional-minute UTC offsets for ages, but there was no mention of the possibility in the main docs, and only a very indirect reference in Appendix B. Improve that. Discussion: https://postgr.es/m/162543102827.697.5755498651217979813@wrigleys.postgresql.org
2021-07-06Refactor function parse_subscription_options.Amit Kapila
Instead of using multiple parameters in parse_subscription_options function signature, use the struct SubOpts that encapsulate all the subscription options and their values. It will be useful for future work where we need to add other options in the subscription. Also, use bitmaps to pass the supported and retrieve the specified options much like the way it is done in the commit a3dc926009. Author: Bharath Rupireddy Reviewed-By: Peter Smith, Amit Kapila, Alvaro Herrera Discussion: https://postgr.es/m/CALj2ACXtoQczfNsDQWobypVvHbX2DtgEHn8DawS0eGFwuo72kw@mail.gmail.com
2021-07-06Fix typo in commentDavid Rowley
Author: James Coleman Discussion: https://postgr.es/m/CAAaqYe8f8ENA0i1PdBtUNWDd2sxHSMgscNYbjhaXMuAdfBrZcg@mail.gmail.com
2021-07-06Reduce the number of pallocs when building partition boundsDavid Rowley
In each of the create_*_bound() functions for LIST, RANGE and HASH partitioning, there were a large number of palloc calls which could be reduced down to a much smaller number. In each of these functions, an array was built so that we could qsort it before making the PartitionBoundInfo. For LIST and HASH partitioning, an array of pointers was allocated then each element was allocated within that array. Since the number of items of each dimension is known beforehand, we can just allocate a single chunk of memory for this. Similarly, with all partition strategies, we're able to reduce the number of allocations to build the ->datums field. This is an array of Datum pointers, but there's no need for the Datums that each element points to to be singly allocated. One big chunk will do. For RANGE partitioning, the PartitionBoundInfo->kind field can get the same treatment. We can apply the same optimizations to partition_bounds_copy(). Doing this might have a small effect on cache performance when searching for the correct partition during partition pruning or DML on a partitioned table. However, that's likely to be small and this is mostly about reducing palloc overhead. Author: Nitin Jadhav, Justin Pryzby, David Rowley Reviewed-by: Justin Pryzby, Zhihong Yu Discussion: https://postgr.es/m/flat/CAMm1aWYFTqEio3bURzZh47jveiHRwgQTiSDvBORczNEz2duZ1Q@mail.gmail.com
2021-07-06Use WaitLatch() instead of pg_usleep() at the end of backupsMichael Paquier
This concerns pg_stop_backup() and BASE_BACKUP, when waiting for the WAL segments required for a backup to be archived. This simplifies a bit the handling of the wait event used in this code path. Author: Bharath Rupireddy Reviewed-by: Michael Paquier, Stephen Frost Discussion: https://postgr.es/m/CALj2ACU4AdPCq6NLfcA-ZGwX7pPCK5FgEj-CAU0xCKzkASSy_A@mail.gmail.com
2021-07-05Reduce overhead of cache-clobber testing in LookupOpclassInfo().Tom Lane
Commit 03ffc4d6d added logic to bypass all caching behavior in LookupOpclassInfo when CLOBBER_CACHE_ALWAYS is enabled. It doesn't look like I stopped to think much about what that would cost, but recent investigation shows that the cost is enormous: it roughly doubles the time needed for cache-clobber test runs. There does seem to be value in this behavior when trying to test the opclass-cache loading logic itself, but for other purposes the cost is excessive. Hence, let's back off to doing this only when debug_invalidate_system_caches_always is at least 3; or in older branches, when CLOBBER_CACHE_RECURSIVELY is defined. While here, clean up some other minor issues in LookupOpclassInfo. Re-order the code so we aren't left with broken cache entries (leading to later core dumps) in the unlikely case that we suffer OOM while trying to allocate space for a new entry. (That seems to be my oversight in 03ffc4d6d.) Also, in >= v13, stop allocating one array entry too many. That's evidently left over from sloppy reversion in 851b14b0c. Back-patch to all supported branches, mainly to reduce the runtime of cache-clobbering buildfarm animals. Discussion: https://postgr.es/m/1370856.1625428625@sss.pgh.pa.us
2021-07-05Rethink blocking annotations in detach-partition-concurrently-[34].Tom Lane
In 741d7f104, I tried to make the reports from canceled steps come out after the pg_cancel_backend() steps, since that was the most common ordering before. However, that doesn't ensure that a canceled step doesn't report even later, as shown in a recent failure on buildfarm member idiacanthus. Rather than complicating things even more with additional annotations, let's just force the cancel's effect to be reported first. It's not *that* unnatural-looking. Back-patch to v14 where these test cases appeared. Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=idiacanthus&dt=2021-07-02%2001%3A40%3A04
2021-07-05Prevent numeric overflows in parallel numeric aggregates.Dean Rasheed
Formerly various numeric aggregate functions supported parallel aggregation by having each worker convert partial aggregate values to Numeric and use numeric_send() as part of serializing their state. That's problematic, since the range of Numeric is smaller than that of NumericVar, so it's possible for it to overflow (on either side of the decimal point) in cases that would succeed in non-parallel mode. Fix by serializing NumericVars instead, to avoid the overflow risk and ensure that parallel and non-parallel modes work the same. A side benefit is that this improves the efficiency of the serialization/deserialization code, which can make a noticeable difference to performance with large numbers of parallel workers. No back-patch due to risk from changing the binary format of the aggregate serialization states, as well as lack of prior field complaints and low probability of such overflows in practice. Patch by me. Thanks to David Rowley for review and performance testing, and Ranier Vilela for an additional suggestion. Discussion: https://postgr.es/m/CAEZATCUmeFWCrq2dNzZpRj5+6LfN85jYiDoqm+ucSXhb9U2TbA@mail.gmail.com
2021-07-05doc: Fix quoting markupPeter Eisentraut
2021-07-05Doc: Hash Indexes.Amit Kapila
A new chapter for Hash Indexes, designed to help users understand how they work and when to use them. Backpatch-through 10 where we have made hash indexes durable. Author: Simon Riggs Reviewed-By: Justin Pryzby, Amit Kapila Discussion: https://postgr.es/m/CANbhV-HRjNPYgHo--P1ewBrFJ-GpZPb9_25P7=Wgu7s7hy_sLQ@mail.gmail.com
2021-07-04doc: Mention requirement to --enable-tap-tests on section for TAP testsMichael Paquier
Author: Greg Sabino Mullane Discussion: https://postgr.es/m/CAKAnmmJYH2FBn_+Vwd2FD5SaKn8hjhAXOCHpZc6n4wXaUaW_SA@mail.gmail.com Backpatch-through: 9.6
2021-07-04Doc: mention that VACUUM can't utilize over 1GB of RAMDavid Rowley
Document that setting maintenance_work_mem to values over 1GB has no effect on VACUUM. Reported-by: Martín Marqués Author: Laurenz Albe Discussion: https://postgr.es/m/CABeG9LsZ2ozUMcqtqWu_-GiFKB17ih3p8wBHXcpfnHqhCnsc7A%40mail.gmail.com Backpatch-through: 9.6, oldest supported release
2021-07-04Cleanup some aggregate code in the executorDavid Rowley
Here we alter the code that calls build_pertrans_for_aggref() so that the function no longer needs to special-case whether it's dealing with an aggtransfn or an aggcombinefn. This allows us to reuse the build_aggregate_transfn_expr() function and just get rid of the build_aggregate_combinefn_expr() completely. All of the special case code that was in build_pertrans_for_aggref() has been moved up to the calling functions. This saves about a dozen lines of code in nodeAgg.c and a few dozen more in parse_agg.c Also, rename a few variables in nodeAgg.c to try to make it more clear that we're working with either a aggtransfn or an aggcombinefn. Some of the old names would have you believe that we were always working with an aggtransfn. Discussion: https://postgr.es/m/CAApHDvptMQ9FmF0D67zC_w88yVnoNVR2+kkOQGUrCmdxWxLULQ@mail.gmail.com
2021-07-03Further restrict the scope of no-exit()-in-libpq test.Tom Lane
Disable this check altogether in --enable-coverage builds, because newer versions of gcc insert exit() as well as abort() calls for that. Also disable it on AIX and Solaris, because those platforms tend to provide facilities such as libldap as static libraries, which then get included in libpq's shlib. We can't expect such libraries to honor our coding rules. (That platform list might need additional tweaking, but I think this is enough to keep the buildfarm happy.) Per reports from Jacob Champion and Noah Misch. Discussion: https://postgr.es/m/3128896.1624742969@sss.pgh.pa.us
2021-07-02doc: adjust "cities" example to be consistent with other SQLBruce Momjian
Reported-by: tom@crystae.net Discussion: https://postgr.es/m/162345756191.14472.9754568432103008703@wrigleys.postgresql.org Backpatch-through: 9.6
2021-07-02docs: clarify new aggressive vacuum mode for multi-xactsBruce Momjian
Reported-by: eric.mutta@gmail.com Discussion: https://postgr.es/m/162395467510.686.11947486273299446208@wrigleys.postgresql.org Backpatch-through: 14
2021-07-02Don't try to print data type names in slot_store_error_callback().Tom Lane
The existing code tried to do syscache lookups in an already-failed transaction, which is problematic to say the least. After some consideration of alternatives, the best fix seems to be to just drop type names from the error message altogether. The table and column names seem like sufficient localization. If the user is unsure what types are involved, she can check the local and remote table definitions. Having done that, we can also discard the LogicalRepTypMap hash table, which had no other use. Arguably, LOGICAL_REP_MSG_TYPE replication messages are now obsolete as well; but we should probably keep them in case some other use emerges. (The complexity of removing something from the replication protocol would likely outweigh any savings anyhow.) Masahiko Sawada and Bharath Rupireddy, per complaint from Andres Freund. Back-patch to v10 where this code originated. Discussion: https://postgr.es/m/20210106020229.ne5xnuu6wlondjpe@alap3.anarazel.de