aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* Make index_set_state_flags() transactionalMichael Paquier2020-09-14
| | | | | | | | | | | | | | | | | | | | | | | | 3c84046 is the original commit that introduced index_set_state_flags(), where the presence of SnapshotNow made necessary the use of an in-place update. SnapshotNow has been removed in 813fb03, so there is no actual reasons to not make this operation transactional. Note that while making the operation more robust, using a transactional operation in this routine was not strictly necessary as there was no use case for it yet. However, some future features are going to need a transactional behavior, like support for CREATE/DROP INDEX CONCURRENTLY with partitioned tables, where indexes in a partition tree need to have all their pg_index.indis* flags updated in the same transaction to make the operation stable to the end-user by keeping partition trees consistent, even with a failure mid-flight. REINDEX CONCURRENTLY uses already transactional updates when swapping the old and new indexes, making this change more consistent with the index-swapping logic. Author: Michael Paquier Reviewed-by: Anastasia Lubennikova Discussion: https://postgr.es/m/20200903080440.GA8559@paquier.xyz
* Message fixes and style improvementsPeter Eisentraut2020-09-14
|
* Use the properly transformed RangeVar for expandTableLikeClause().Tom Lane2020-09-13
| | | | | | | | | | | | | | | transformCreateStmt() adjusts the transformed statement's RangeVar to specify the target schema explicitly, for the express reason of making sure that auxiliary statements derived by parse transformation operate on the right table. But the refactoring I did in commit 502898192 got this wrong and passed the untransformed RangeVar to expandTableLikeClause(). This could lead to assertion failures or weird misbehavior if the wrong table was accessed. Per report from Alexander Lakhin. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/05051f9d-b32b-cb35-6735-0e9f2ab86b5f@gmail.com
* Fix inconsistency in determining the timestamp of the db statfile.Amit Kapila2020-09-12
| | | | | | | | | | | | | | | We use the timestamp of the global statfile if we are not able to determine it for a particular database in case the entry for that database doesn't exist. However, we were using it even when the statfile is corrupt. As there is no user reported issue and it is not clear if there is any impact of this on actual application so decided not to backpatch. Reported-by: Amit Kapila Author: Amit Kapila Reviewed-by: Sawada Masahiko, Magnus Hagander and Alvaro Herrera Discussion: https://postgr.es/m/CAA4eK1J3oTJKyVq6v7K4d3jD+vtnruG9fHRib6UuWWsrwAR6Aw@mail.gmail.com
* Remove unused function declaration in logicalproto.h.Amit Kapila2020-09-12
| | | | | | | | | In the passing, fix a typo in pgoutput.c. Reported-by: Tomas Vondra Author: Tomas Vondra Reviewed-by: Dilip Kumar Discussion: https://postgr.es/m/20200909084353.pncuclpbwlr7vylh@development
* logtape.c: do not preallocate for tapes when sortingJeff Davis2020-09-11
| | | | | | | | | | | The preallocation logic is only useful for HashAgg, so disable it when sorting. Also, adjust an out-of-date comment. Reviewed-by: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-Wzn_o7tE2+hRVvwSFghRb75AJ5g-nqGzDUqLYMexjOAe=g@mail.gmail.com Backpatch-through: 13
* Accept SIGQUIT during error recovery in auxiliary processes.Tom Lane2020-09-11
| | | | | | | | | | | | | | | | | | | | | | | | The bgwriter, checkpointer, walwriter, and walreceiver processes claimed to allow SIGQUIT "at all times". In reality SIGQUIT would get re-blocked during error recovery, because we didn't update the actual signal mask immediately, so sigsetjmp() would save and reinstate a mask that includes SIGQUIT. This appears to be simply a coding oversight. There's never a good reason to hold off SIGQUIT in these processes, because it's going to just call _exit(2) which should be safe enough, especially since the postmaster is going to tear down shared memory afterwards. Hence, stick in PG_SETMASK() calls to install the modified BlockSig mask immediately. Also try to improve the comments around sigsetjmp blocks. Most of them were just referencing postgres.c, which is misleading because actually postgres.c manages the signals differently. No back-patch, since there's no evidence that this is causing any problems in the field. Discussion: https://postgr.es/m/CALDaNm1d1hHPZUg3xU4XjtWBOLCrA+-2cJcLpw-cePZ=GgDVfA@mail.gmail.com
* Log a message when resorting to SIGKILL during shutdown/crash recovery.Tom Lane2020-09-11
| | | | | | | | | | Currently, no useful trace is left in the logs when the postmaster is forced to use SIGKILL to shut down children that failed to respond to SIGQUIT. Some questions were raised about how often that scenario happens in the buildfarm, so let's add a LOG-level message showing that it happened. Discussion: https://postgr.es/m/1850884.1599601164@sss.pgh.pa.us
* Don't run atexit callbacks during signal exits from ProcessStartupPacket.Tom Lane2020-09-11
| | | | | | | | | | | | | | | | | | | Although 58c6feccf fixed the case for SIGQUIT, we were still calling proc_exit() from signal handlers for SIGTERM and timeout failures in ProcessStartupPacket. Fortunately, at the point where that code runs, we haven't yet connected to shared memory in any meaningful way, so there is nothing we need to undo in shared memory. This means it should be safe to use _exit(1) here, ie, not run any atexit handlers but also inform the postmaster that it's not a crash exit. To make sure nobody breaks the "nothing to undo" expectation, add a cross-check that no on-shmem-exit or before-shmem-exit handlers have been registered yet when we finish using these signal handlers. This change is simple enough that maybe it could be back-patched, but I won't risk that right now. Discussion: https://postgr.es/m/1850884.1599601164@sss.pgh.pa.us
* Update copyright yearAlvaro Herrera2020-09-11
| | | | | | | Thinko in 40b3e2c201af. Reported-by: "Wang, Shenhao" <wangsh.fnst@cn.fujitsu.com> Discussion: https://postgr.es/m/ed98706b82694b57a8c0d339a10732aa@G08CNEXMBPEKD06.g08.fujitsu.local
* Print WAL logical message contents in pg_waldumpAlvaro Herrera2020-09-10
| | | | | | | | | This helps debuggability when looking at WAL streams containing logical messages. Author: Ashutosh Bapat <ashutosh.bapat@2ndquadrant.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CAExHW5sWx49rKmXbg5H1Xc1t+nRv9PaYKQmgw82HPt6vWDVmDg@mail.gmail.com
* Use _exit(2) for SIGQUIT during ProcessStartupPacket, too.Tom Lane2020-09-10
| | | | | | | | | | | | | | | | | | | | | Bring the signal handling for startup-packet collection into line with the policy established in commits bedadc732 and 8e19a8264, namely don't risk running atexit callbacks when handling SIGQUIT. Ideally, we'd not do so for SIGTERM or timeout interrupts either, but that change seems a bit too risky for the back branches. For now, just improve the comments in this area to describe the risk. Also relocate where BackendInitialize re-disables these interrupts, to minimize the code span where they're active. This doesn't buy a whole lot of safety, but it can't hurt. In passing, rename startup_die() to remove confusion about whether it is for the startup process. Like the previous commits, back-patch to all supported branches. Discussion: https://postgr.es/m/1850884.1599601164@sss.pgh.pa.us
* Clean up some code and comments in partbounds.c.Etsuro Fujita2020-09-10
| | | | | | | | | | | Do some minor cleanup for commit c8434d64c: 1) remove a useless assignment (in normal builds) and 2) improve comments a little. Back-patch to v13 where the aforementioned commit went in. Author: Etsuro Fujita Reviewed-by: Alvaro Herrera Discussion: https://postgr.es/m/CAPmGK16yCd2R4=bQ4g8N2dT9TtA5ZU+qNmJ3LPc_nypbNy4_2A@mail.gmail.com
* doc: Fix some grammar and inconsistenciesMichael Paquier2020-09-10
| | | | | | | | Some comments are fixed while on it. Author: Justin Pryzby Discussion: https://postgr.es/m/20200818171702.GK17022@telsasoft.com Backpatch-through: 9.6
* Fix rd_firstRelfilenodeSubid for nailed relations, in parallel workers.Noah Misch2020-09-09
| | | | | | | | | | | Move applicable code out of RelationBuildDesc(), which nailed relations bypass. Non-assert builds experienced no known problems. Back-patch to v13, where commit c6b92041d38512a4176ed76ad06f713d2e6c01a8 introduced rd_firstRelfilenodeSubid. Kyotaro Horiguchi. Reported by Justin Pryzby. Discussion: https://postgr.es/m/20200907023737.GA7158@telsasoft.com
* Make archiver's SIGQUIT handler exit via _exit().Tom Lane2020-09-09
| | | | | | | | | | | | | | | | | | Commit 8e19a8264 changed the SIGQUIT handlers of almost all server processes not to run atexit callbacks. The archiver process was skipped, perhaps because it's not connected to shared memory; but it's just as true here that running atexit callbacks in a signal handler is unsafe. So let's make it work like the rest. In HEAD and v13, we can use the common SignalHandlerForCrashExit handler. Before that, just tweak pgarch_exit to use _exit(2) explicitly. Like the previous commit, back-patch to all supported branches. Kyotaro Horiguchi, back-patching by me Discussion: https://postgr.es/m/1850884.1599601164@sss.pgh.pa.us
* Expose internal function for converting int64 to numericPeter Eisentraut2020-09-09
| | | | | | | | Existing callers had to take complicated detours via DirectFunctionCall1(). This simplifies a lot of code. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/42b73d2d-da12-ba9f-570a-420e0cce19d9@phystech.edu
* Minor fixes in docs and error messages.Tom Lane2020-09-09
| | | | | | Alexander Lakhin Discussion: https://postgr.es/m/ce7debdd-c943-d7a7-9b41-687107b27831@gmail.com
* Check default partitions constraints while descendingAlvaro Herrera2020-09-08
| | | | | | | | | | | | | | | | | | | | | | | Partitioning tuple route code assumes that the partition chosen while descending the partition hierarchy is always the correct one. This is true except when the partition is the default partition and another partition has been added concurrently: the partition constraint changes and we don't recheck it. This can lead to tuples mistakenly being added to the default partition that should have been rejected. Fix by rechecking the default partition constraint while descending the hierarchy. An isolation test based on the reproduction steps described by Hao Wu (with tweaks for extra coverage) is included. Backpatch to 12, where this bug came in with 898e5e3290a7. Reported by: Hao Wu <hawu@vmware.com> Author: Amit Langote <amitlangote09@gmail.com> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CA+HiwqFqBmcSSap4sFnCBUEL_VfOMmEKaQ3gwUhyfa4c7J_-nA@mail.gmail.com Discussion: https://postgr.es/m/DM5PR0501MB3910E97A9EDFB4C775CF3D75A42F0@DM5PR0501MB3910.namprd05.prod.outlook.com
* Install an error check into cancel_before_shmem_exit().Tom Lane2020-09-08
| | | | | | | | | | | | | | | | | | | | | | | Historically, cancel_before_shmem_exit() just silently did nothing if the specified callback wasn't the top-of-stack. The folly of ignoring this case was exposed by the bugs fixed in 303640199 and bab150045, so let's make it throw elog(ERROR) instead. There is a decent argument to be made that PG_ENSURE_ERROR_CLEANUP should use some separate infrastructure, so it wouldn't break if something inside the guarded code decides to register a new before_shmem_exit callback. However, a survey of the surviving uses of before_shmem_exit() and PG_ENSURE_ERROR_CLEANUP doesn't show any plausible conflicts of that sort today, so for now we'll forgo the extra complexity. (It will almost certainly become necessary if anyone ever wants to wrap PG_ENSURE_ERROR_CLEANUP around arbitrary user-defined actions, though.) No backpatch, since this is developer support not a production issue. Bharath Rupireddy, per advice from Andres Freund, Robert Haas, and myself Discussion: https://postgr.es/m/CALj2ACWk7j4F2v2fxxYfrroOF=AdFNPr1WsV+AGtHAFQOqm_pw@mail.gmail.com
* Fix autovacuum cancellation.Andres Freund2020-09-08
| | | | | | | | | | | | | The problem is caused by me (Andres) having ProcSleep() look at the wrong PGPROC entry in 5788e258bb2. Unfortunately it seems hard to write a reliable test for autovacuum cancellations. Perhaps somebody will come up with a good approach, but it seems worth fixing the issue even without a test. Reported-By: Jeff Janes <jeff.janes@gmail.com> Author: Jeff Janes <jeff.janes@gmail.com> Discussion: https://postgr.es/m/CAMkU=1wH2aUy+wDRDz+5RZALdcUnEofV1t9PzXS_gBJO9vZZ0Q@mail.gmail.com
* Use plain memset() in numeric.c, not MemSet and friends.Tom Lane2020-09-08
| | | | | | | | | | | | | | | | | | | | This essentially reverts a micro-optimization I made years ago, as part of the much larger commit d72f6c750. It's doubtful that there was any hard evidence for it being helpful even then, and the case is even more dubious now that modern compilers are so much smarter about inlining memset(). The proximate reason for undoing it is to get rid of the type punning inherent in MemSet, for fear that that may cause problems now that we're applying additional optimization switches to numeric.c. At the very least this'll silence some warnings from a few old buildfarm animals. (It's probably past time for another look at whether MemSet is still worth anything at all, but I do not propose to tackle that question right now.) Discussion: https://postgr.es/m/CAJ3gD9evtA_vBo+WMYMyT-u=keHX7-r8p2w7OSRfXf42LTwCZQ@mail.gmail.com
* Use <unnamed> for name of unnamed portal's memory contextPeter Eisentraut2020-09-08
| | | | | | | Otherwise just printing an empty string makes the memory context debug output slightly confusing. Discussion: https://www.postgresql.org/message-id/flat/ccb353ef-89ff-09b3-8046-1d2514624b9c%402ndquadrant.com
* Add support for partitioned tables and indexes in REINDEXMichael Paquier2020-09-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Until now, REINDEX was not able to work with partitioned tables and indexes, forcing users to reindex partitions one by one. This extends REINDEX INDEX and REINDEX TABLE so as they can accept a partitioned index and table in input, respectively, to reindex all the partitions assigned to them with physical storage (foreign tables, partitioned tables and indexes are then discarded). This shares some logic with schema and database REINDEX as each partition gets processed in its own transaction after building a list of relations to work on. This choice has the advantage to minimize the number of invalid indexes to one partition with REINDEX CONCURRENTLY in the event a cancellation or failure in-flight, as the only indexes handled at once in a single REINDEX CONCURRENTLY loop are the ones from the partition being working on. Isolation tests are added to emulate some cases I bumped into while developing this feature, particularly with the concurrent drop of a leaf partition reindexed. However, this is rather limited as LOCK would cause REINDEX to block in the first transaction building the list of partitions. Per its multi-transaction nature, this new flavor cannot run in a transaction block, similarly to REINDEX SCHEMA, SYSTEM and DATABASE. Author: Justin Pryzby, Michael Paquier Reviewed-by: Anastasia Lubennikova Discussion: https://postgr.es/m/db12e897-73ff-467e-94cb-4af03705435f.adger.lj@alibaba-inc.com
* Adjust cost model for HashAgg that spills to disk.Jeff Davis2020-09-07
| | | | | | | | | | | | Tomas Vondra observed that the IO behavior for HashAgg tends to be worse than for Sort. Penalize HashAgg IO costs accordingly. Also, account for the CPU effort of spilling the tuples and reading them back. Discussion: https://postgr.es/m/20200906212112.nzoy5ytrzjjodpfh@development Reviewed-by: Tomas Vondra Backpatch-through: 13
* Clarify comments in enforce_generic_type_consistency().Tom Lane2020-09-07
| | | | | | | | | | | | Some of the pre-existing comments were vague about whether they referred to all polymorphic types or only the old-style ones. Also be more consistent about using the "family 1" vs "family 2" terminology. Himanshu Upadhyaya and Tom Lane Discussion: https://postgr.es/m/CAPF61jBUg9XoMPNuLpoZ+h6UZ2VxKdNt3rQL1xw1GOBwjWzAXQ@mail.gmail.com
* Frob numeric.c loop so that clang will auto-vectorize it too.Tom Lane2020-09-07
| | | | | | | | | | Experimentation shows that clang will auto-vectorize the critical multiplication loop if the termination condition is written "i2 < limit" rather than "i2 <= limit". This seems unbelievably stupid, but I've reproduced it on both clang 9.0.1 (RHEL8) and 11.0.3 (macOS Catalina). gcc doesn't care, so tweak the code to do it that way. Discussion: https://postgr.es/m/CAJ3gD9evtA_vBo+WMYMyT-u=keHX7-r8p2w7OSRfXf42LTwCZQ@mail.gmail.com
* Skip unnecessary stat() calls in walkdir().Thomas Munro2020-09-07
| | | | | | | | | | | Some kernels can tell us the type of a "dirent", so we can avoid a call to stat() or lstat() in many cases. Define a new function get_dirent_type() to contain that logic, for use by the backend and frontend versions of walkdir(), and perhaps other callers in future. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Juan José Santamaría Flecha <juanjo.santamaria@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKG%2BFzxupGGN4GpUdbzZN%2Btn6FQPHo8w0Q%2BAPH5Wz8RG%2Bww%40mail.gmail.com
* Apply auto-vectorization to the inner loop of numeric multiplication.Tom Lane2020-09-06
| | | | | | | | | | | | | | | | | | | | | | | Compile numeric.c with -ftree-vectorize where available, and adjust the innermost loop of mul_var() so that it is amenable to being auto-vectorized. (Mainly, that involves making it process the arrays left-to-right not right-to-left.) Applying -ftree-vectorize actually makes numeric.o smaller, at least with my compiler (gcc 8.3.1 on x86_64), and it's a little faster too. Independently of that, fixing the inner loop to be vectorizable also makes things a bit faster. But doing both is a huge win for multiplications with lots of digits. For me, the numeric regression test is the same speed to within measurement noise, but numeric_big is a full 45% faster. We also looked into applying -funroll-loops, but that makes numeric.o bloat quite a bit, and the additional speed improvement is very marginal. Amit Khandekar, reviewed and edited a little by me Discussion: https://postgr.es/m/CAJ3gD9evtA_vBo+WMYMyT-u=keHX7-r8p2w7OSRfXf42LTwCZQ@mail.gmail.com
* Split Makefile symbol CFLAGS_VECTOR into two symbols.Tom Lane2020-09-06
| | | | | | | | | | | | | Replace CFLAGS_VECTOR with CFLAGS_UNROLL_LOOPS and CFLAGS_VECTORIZE, allowing us to distinguish whether we want to apply -funroll-loops, -ftree-vectorize, or both to a particular source file. Up to now the only consumer of the symbol has been checksum.c which wants both, so that there was no need to distinguish; but that's about to change. Amit Khandekar, reviewed and edited a little by me Discussion: https://postgr.es/m/CAJ3gD9evtA_vBo+WMYMyT-u=keHX7-r8p2w7OSRfXf42LTwCZQ@mail.gmail.com
* Refactor pg_get_line() to expose an alternative StringInfo-based API.Tom Lane2020-09-06
| | | | | | | | | | | | | | | | | Letting the caller provide a StringInfo to read into is helpful when the caller needs to merge lines or otherwise modify the data after it's been read. Notably, now the code added by commit 8f8154a50 can use pg_get_line_append() instead of having its own copy of that logic. A follow-on commit will also make use of this. Also, since StringInfo buffers are a minimum of 1KB long, blindly using pg_get_line() in a loop can eat a lot more memory than one would expect. I discovered for instance that commit e0f05cd5b caused initdb to consume circa 10MB to read postgres.bki, even though that's under 1MB worth of data. A less memory-hungry alternative is to re-use the same StringInfo for all lines and pg_strdup the results. Discussion: https://postgr.es/m/1315832.1599345736@sss.pgh.pa.us
* Fix typo in commentMagnus Hagander2020-09-06
| | | | Author: Hou, Zhijie
* Fix misleading error message about inconsistent moving-aggregate types.Tom Lane2020-09-06
| | | | | | | | | | | | | We reported the wrong types when complaining that an aggregate's moving-aggregate implementation is inconsistent with its regular implementation. This was wrong since the feature was introduced, so back-patch to all supported branches. Jeff Janes Discussion: https://postgr.es/m/CAMkU=1x808LH=LPhZp9mNSP0Xd1xDqEd+XeGcvEe48dfE6xV=A@mail.gmail.com
* Improve some ancient, crufty code in bootstrap + initdb.Tom Lane2020-09-05
| | | | | | | | | | | | | | | | | | | | | | | | At some point back in the last century, somebody felt that reading all of pg_type twice was cheaper, or at least easier, than using repalloc() to resize the Typ[] array dynamically. That seems like an entirely wacko proposition, so rewrite the code to do it the other way. (To add insult to injury, there were two not-quite-identical copies of said code.) initdb.c's readfile() function had the same disease of preferring to do double the I/O to avoid resizing its output array. Here, we can make things easier by using the just-invented pg_get_line() function to handle reading individual lines without a predetermined notion of how long they are. On my machine, it's difficult to detect any net change in the overall runtime of initdb from these changes; but they should help on slower buildfarm machines (especially since a buildfarm cycle involves a lot of initdb's these days). My attention was drawn to these places by scan-build complaints, but on inspection they needed a lot more work than just suppressing dead stores :-(
* Yet more elimination of dead stores and useless initializations.Tom Lane2020-09-05
| | | | | | | | | I'm not sure what tool Ranier was using, but the ones I contributed were found by using a newer version of scan-build than I tried before. Ranier Vilela and Tom Lane Discussion: https://postgr.es/m/CAEudQAo1+AcGppxDSg8k+zF4+Kv+eJyqzEDdbpDg58-=MQcerQ@mail.gmail.com
* Switch to multi-inserts when registering dependencies for many code pathsMichael Paquier2020-09-05
| | | | | | | | | | | | | | | | | This commit improves the dependency registrations by taking advantage of the preliminary work done in 63110c62, to group together the insertion of dependencies of the same type to pg_depend. With the current layer of routines available, and as only dependencies of the same type can be grouped, there are code paths still doing more than one multi-insert when it is necessary to register dependencies of multiple types (constraint and index creation are two cases doing that). While on it, this refactors some of the code to use ObjectAddressSet() when manipulating object addresses. Author: Daniel Gustafsson, Michael Paquier Reviewed-by: Andres Freund, Álvaro Herrera Discussion: https://postgr.es/m/20200807061619.GA23955@paquier.xyz
* Fix typo in commentPeter Eisentraut2020-09-05
|
* Use multi-inserts for pg_dependMichael Paquier2020-09-05
| | | | | | | | | | | | | | | | | | | | | | | This is a follow-up of the work done in e3931d01. This case is a bit different than pg_attribute and pg_shdepend: the maximum number of items to insert is known in advance, but there is no need to handle pinned dependencies. Hence, the base allocation for slots is done based on the number of items and the maximum allowed with a cap at 64kB. Slots are initialized once used to minimize the overhead of the operation. The insertions can be done for dependencies of the same type. More could be done by grouping the insertion of multiple dependency types in a single batch. This is left as future work. Some of the multi-insert logic is also simplified for pg_shdepend, as per the feedback discussed for this specific patch. This also moves to indexing.h the variable capping the maximum amount of data that can be used at once for a multi-insert, instead of having separate definitions for pg_attribute, pg_depend and pg_shdepend. Author: Daniel Gustafsson, Michael Paquier Reviewed-by: Andres Freund, Álvaro Herrera Discussion: https://postgr.es/m/20200807061619.GA23955@paquier.xyz
* Fix over-eager ping'ing in logical replication receiver.Tom Lane2020-09-04
| | | | | | | | | | | Commit 3f60f690f only partially fixed the broken-status-tracking issue in LogicalRepApplyLoop: we need ping_sent to have the same lifetime as last_recv_timestamp. The effects are much less serious than what that commit fixed, though. AFAICS this would just lead to extra ping requests being sent, once per second until the sender responds. Still, it's a bug, so backpatch to v10 as before. Discussion: https://postgr.es/m/959627.1599248476@sss.pgh.pa.us
* Remove still more useless assignments.Tom Lane2020-09-04
| | | | | | | | Fix some more things scan-build pointed to as dead stores. In some of these cases, rearranging the code a little leads to more readable code IMO. It's all cosmetic, though. Discussion: https://postgr.es/m/CAEudQAo1+AcGppxDSg8k+zF4+Kv+eJyqzEDdbpDg58-=MQcerQ@mail.gmail.com
* Fix bogus MaxAllocSize check in logtape.c.Jeff Davis2020-09-04
| | | | | | Reported-by: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-Wz=NZPZc3-fkdmvu=w2itx0PiB-G6QpxHXZOjuvFAzPdZw@mail.gmail.com Backpatch-through: 13
* Report expected contrecord length on mismatchAlvaro Herrera2020-09-04
| | | | | | | | When reading a WAL record fails to find continuation record(s) of the proper length, report what it expects, for clarity. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/20200903212152.GA15319@alvherre.pgsql
* Remove some more useless assignments.Tom Lane2020-09-04
| | | | | | | | | Found with clang's scan-build tool. It also whines about a lot of other dead stores that we should *not* change IMO, either as a matter of style or future-proofing. But these places seem like clear oversights. Discussion: https://postgr.es/m/CAEudQAo1+AcGppxDSg8k+zF4+Kv+eJyqzEDdbpDg58-=MQcerQ@mail.gmail.com
* Fix inline marking introduced in commit 464824323e.Amit Kapila2020-09-04
| | | | | | | | Forgot to add inline marking in changes_filename() declaration. In the passing, add inline marking for a similar function subxact_filename(). Reported-By: Nathan Bossart Discussion: https://postgr.es/m/E98FBE8F-B878-480D-A728-A60C6EED3047@amazon.com
* remove redundant initializationsBruce Momjian2020-09-03
| | | | | | | | | | Reported-by: Ranier Vilela Discussion: https://postgr.es/m/CAEudQAo1+AcGppxDSg8k+zF4+Kv+eJyqzEDdbpDg58-=MQcerQ@mail.gmail.com Author: Ranier Vilela Backpatch-through: master
* Remove variable "concurrent" from ReindexStmtMichael Paquier2020-09-04
| | | | | | | | | | This node already handles multiple options using a bitmask, so having a separate boolean flag is not necessary. This simplifies the code a bit with less arguments to give to the reindex routines, by replacing the boolean with an equivalent bitmask value. Reviewed-by: Julien Rouhaud Discussion: https://postgr.es/m/20200902110326.GA14963@paquier.xyz
* Remove arbitrary restrictions on password length.Tom Lane2020-09-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch started out with the goal of harmonizing various arbitrary limits on password length, but after awhile a better idea emerged: let's just get rid of those fixed limits. recv_password_packet() has an arbitrary limit on the packet size, which we don't really need, so just drop it. (Note that this doesn't really affect anything for MD5 or SCRAM password verification, since those will hash the user's password to something shorter anyway. It does matter for auth methods that require a cleartext password.) Likewise remove the arbitrary error condition in pg_saslprep(). The remaining limits are mostly in client-side code that prompts for passwords. To improve those, refactor simple_prompt() so that it allocates its own result buffer that can be made as big as necessary. Actually, it proves best to make a separate routine pg_get_line() that has essentially the semantics of fgets(), except that it allocates a suitable result buffer and hence will never return a truncated line. (pg_get_line has a lot of potential applications to replace randomly-sized fgets buffers elsewhere, but I'll leave that for another patch.) I built pg_get_line() atop stringinfo.c, which requires moving that code to src/common/; but that seems fine since it was a poor fit for src/port/ anyway. This patch is mostly mine, but it owes a good deal to Nathan Bossart who pressed for a solution to the password length problem and created a predecessor patch. Also thanks to Peter Eisentraut and Stephen Frost for ideas and discussion. Discussion: https://postgr.es/m/09512C4F-8CB9-4021-B455-EF4C4F0D55A0@amazon.com
* Avoid lockup of a parallel worker when reporting a long error message.Tom Lane2020-09-03
| | | | | | | | | | | | | | | | | | | | | Because sigsetjmp() will restore the initial state with signals blocked, the code path in bgworker.c for reporting an error and exiting would execute that way. Usually this is fairly harmless; but if a parallel worker had an error message exceeding the shared-memory communication buffer size (16K) it would lock up, because it would wait for a resume-sending signal from its parallel leader which it would never detect. To fix, just unblock signals at the appropriate point. This can be shown to fail back to 9.6. The lack of parallel query infrastructure makes it difficult to provide a simple test case for 9.5; but I'm pretty sure the issue exists in some form there as well, so apply the code change there too. Vignesh C, reviewed by Bharath Rupireddy, Robert Haas, and myself Discussion: https://postgr.es/m/CALDaNm1d1hHPZUg3xU4XjtWBOLCrA+-2cJcLpw-cePZ=GgDVfA@mail.gmail.com
* Allow records to span multiple lines in pg_hba.conf and pg_ident.conf.Tom Lane2020-09-03
| | | | | | | | | | | | | | | | | A backslash at the end of a line now causes the next line to be appended to the current one (effectively, the backslash and newline are discarded). This allows long HBA entries to be created without legibility problems. While we're here, get rid of the former hard-wired length limit on pg_hba.conf lines, by using an expansible StringInfo buffer instead of a fixed-size local variable. Since the same code is used to read the ident map file, these changes apply there as well. Fabien Coelho, reviewed by Justin Pryzby and David Zhang Discussion: https://postgr.es/m/alpine.DEB.2.21.2003251906140.15243@pseudo
* Add support for streaming to built-in logical replication.Amit Kapila2020-09-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | To add support for streaming of in-progress transactions into the built-in logical replication, we need to do three things: * Extend the logical replication protocol, so identify in-progress transactions, and allow adding additional bits of information (e.g. XID of subtransactions). * Modify the output plugin (pgoutput) to implement the new stream API callbacks, by leveraging the extended replication protocol. * Modify the replication apply worker, to properly handle streamed in-progress transaction by spilling the data to disk and then replaying them on commit. We however must explicitly disable streaming replication during replication slot creation, even if the plugin supports it. We don't need to replicate the changes accumulated during this phase, and moreover we don't have a replication connection open so we don't have where to send the data anyway. Author: Tomas Vondra, Dilip Kumar and Amit Kapila Reviewed-by: Amit Kapila, Kuntal Ghosh and Ajin Cherian Tested-by: Neha Sharma, Mahendra Singh Thalor and Ajin Cherian Discussion: https://postgr.es/m/688b0b7f-2f6c-d827-c27b-216a8e3ea700@2ndquadrant.com