aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* Fix comment related to concurrent index swapping in index.cMichael Paquier2020-03-19
| | | | | | | | | | | | A comment about switching indisvalid of the new and old indexes swapped in REINDEX CONCURRENTLY got this backwards. Issue introduced by 5dc92b8, the original commit of REINDEX CONCURRENTLY. Author: Julien Rouhaud Discussion: https://postgr.es/m/20200318143340.GA46897@nol Backpatch-through: 12
* Disk-based Hash Aggregation.Jeff Davis2020-03-18
| | | | | | | | | | | | | | | | | | | | | | While performing hash aggregation, track memory usage when adding new groups to a hash table. If the memory usage exceeds work_mem, enter "spill mode". In spill mode, new groups are not created in the hash table(s), but existing groups continue to be advanced if input tuples match. Tuples that would cause a new group to be created are instead spilled to a logical tape to be processed later. The tuples are spilled in a partitioned fashion. When all tuples from the outer plan are processed (either by advancing the group or spilling the tuple), finalize and emit the groups from the hash table. Then, create new batches of work from the spilled partitions, and select one of the saved batches and process it (possibly spilling recursively). Author: Jeff Davis Reviewed-by: Tomas Vondra, Adam Lee, Justin Pryzby, Taylor Vesely, Melanie Plageman Discussion: https://postgr.es/m/507ac540ec7c20136364b5272acbcd4574aa76ef.camel@j-davis.com
* Specialize MemoryContextMemAllocated().Jeff Davis2020-03-18
| | | | | | | | | | | An AllocSet doubles the size of allocated blocks (up to maxBlockSize), which means that the current block can represent half of the total allocated space for the memory context. But the free space in the current block may never have been touched, so don't count the untouched memory as allocated for the purposes of MemoryContextMemAllocated(). Discussion: https://postgr.es/m/ec63d70b668818255486a83ffadc3aec492c1f57.camel@j-davis.com
* Enable BEFORE row-level triggers for partitioned tablesAlvaro Herrera2020-03-18
| | | | | | | | ... with the limitation that the tuple must remain in the same partition. Reviewed-by: Ashutosh Bapat Discussion: https://postgr.es/m/20200227165158.GA2071@alvherre.pgsql
* Refactor nbtree fastpath optimization.Peter Geoghegan2020-03-18
| | | | | | | | | | | | | | | | | | Commit 2b272734, which added the fastpath rightmost leaf page cache insert optimization, added code to _bt_doinsert() to handle using and invalidating the backend local block cache. It doesn't seem like a good place to handle these low level details, though. _bt_doinsert() is supposed to be a high level function -- it is the main entry point to nbtinsert.c. Restructure the code by placing handling of the rightmost block cache at the start of a new _bt_search() shim function, _bt_search_insert(). The new function is called from _bt_doinsert(), which uses it as a _bt_search() variant that conveniently accepts its BTInsertState state as an argument. _bt_doinsert() no longer needs to directly consider the fastpath optimization. Discussion: https://postgr.es/m/CAH2-Wzk59cxKJRd=rfbyub6-V4yWRjsOYRkUNHBLT1P1GdtCQQ@mail.gmail.com
* Implement type regcollationPeter Eisentraut2020-03-18
| | | | | | | | | This will be helpful for a following commit and it's also just generally useful, like the other reg* types. Author: Julien Rouhaud Reviewed-by: Thomas Munro and Michael Paquier Discussion: https://postgr.es/m/CAEepm%3D0uEQCpfq_%2BLYFBdArCe4Ot98t1aR4eYiYTe%3DyavQygiQ%40mail.gmail.com
* Recognize some OR clauses as compatible with functional dependenciesTomas Vondra2020-03-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since commit 8f321bd16c functional dependencies can handle IN clauses, which however introduced a possible (and surprising) inconsistency, because IN clauses may be expressed as an OR clause, which are still considered incompatible. For example a IN (1, 2, 3) may be rewritten as (a = 1 OR a = 2 OR a = 3) The IN clause will work fine with functional dependencies, but the OR clause will force the estimation to fall back to plain per-column estimates, possibly introducing significant estimation errors. This commit recognizes OR clauses equivalent to an IN clause (when all arugments are compatible and reference the same attribute) as a special case, compatible with functional dependencies. This allows applying functional dependencies, just like for IN clauses. This does not eliminate the difference in estimating the clause itself, i.e. IN clause and OR clause still use different formulas. It would be possible to change that (for these special OR clauses), but that's not really about extended statistics - it was always like this. Moreover the errors are usually much smaller compared to ignoring dependencies. Author: Tomas Vondra Reviewed-by: Dean Rasheed Discussion: https://www.postgresql.org/message-id/flat/13902317.Eha0YfKkKy%40pierred-pdoc
* Fix wording of several extended stats commentsTomas Vondra2020-03-18
| | | | | Reported-by: Thomas Munro Discussion: https://www.postgresql.org/message-id/flat/20200113230008.g67iyk4cs3xbnjju@development
* Add missing errcode() in a few ereport calls.Amit Kapila2020-03-18
| | | | | | | | | | This will allow to specifying SQLSTATE error code for the errors in the missing places. Reported-by: Sawada Masahiko Author: Sawada Masahiko Backpatch-through: 9.5 Discussion: https://postgr.es/m/CA+fd4k6N8EjNvZpM8nme+y+05mz-SM8Z_BgkixzkA34R+ej0Kw@mail.gmail.com
* Fix typo in indexcmds.cMichael Paquier2020-03-18
| | | | | | Introduced by 61d7c7b. Backpatch-through: 12
* Assert that we don't acquire a heavyweight lock on another object afterAmit Kapila2020-03-18
| | | | | | | | | | | | | | | | | | | | relation extension lock. The only exception to the rule is that we can try to acquire the same relation extension lock more than once. This is allowed as we are not creating any new lock for this case. This restriction implies that the relation extension lock won't ever participate in the deadlock cycle because we can never wait for any other heavyweight lock after acquiring this lock. Such a restriction is okay for relation extension locks as unlike other heavyweight locks these are not held till the transaction end. These are taken for a short duration to extend a particular relation and then released. Author: Dilip Kumar, with few changes by Amit Kapila Reviewed-by: Amit Kapila, Kuntal Ghosh and Sawada Masahiko Discussion: https://postgr.es/m/CAD21AoCmT3cFQUN4aVvzy5chw7DuzXrJCbrjTU05B+Ss=Gn1LA@mail.gmail.com
* nbtree: Remove useless local variables.Peter Geoghegan2020-03-17
| | | | | | | | | | | | | | Copying block and offset numbers to local variables in _bt_insertonpg() made the code less readable. Remove the variables. There is already code that conditionally calls BufferGetBlockNumber() in the same block, so consistently do it that way instead. Calling BufferGetBlockNumber() is very cheap, but we might as well avoid it when it isn't truly necessary. It isn't truly necessary for _bt_insertonpg() to call BufferGetBlockNumber() in almost all cases. Spotted while working on a patch that refactors the fastpath rightmost leaf page cache optimization, which was added by commit 2b272734.
* Don't use EV_CLEAR for kqueue events.Thomas Munro2020-03-18
| | | | | | | | | | | For the semantics to match the epoll implementation, we need a socket to continue to appear readable/writable if you wait multiple times without doing I/O in between (in Linux terminology: level-triggered rather than edge-triggered). This distinction will be important for later commits. Similar to commit 3b790d256f8 for Windows. Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKGJAC4Oqao%3DqforhNey20J8CiG2R%3DoBPqvfR0vOJrFysGw%40mail.gmail.com
* Fix kqueue support under debugger on macOS.Thomas Munro2020-03-18
| | | | | | | | | | | | | | While running under a debugger, macOS's getppid() can return the debugger's PID. That could cause a backend to exit because it falsely believed that the postmaster had died, since commit 815c2f09. Continue to use getppid() as a fast postmaster check after adding the postmaster's PID to a kqueue, to close a PID-reuse race, but double check that it actually exited by trying to read the pipe. The new check isn't reached in the common case. Reported-by: Alexander Korotkov <a.korotkov@postgrespro.ru> Discussion: https://postgr.es/m/CA%2BhUKGKhAxJ8V8RVwCo6zJaeVrdOG1kFBHGZOOjf6DzW_omeMA%40mail.gmail.com
* Refactor our checks for valid function and aggregate signatures.Tom Lane2020-03-17
| | | | | | | | | | | | | | | | | | | | | | pg_proc.c and pg_aggregate.c had near-duplicate copies of the logic to decide whether a function or aggregate's signature is legal. This seems like a bad thing even without the problem that the upcoming "anycompatible" patch would roughly double the complexity of that logic. Hence, refactor so that the rules are localized in new subroutines supplied by parse_coerce.c. (One could quibble about just where to add that code, but putting it beside enforce_generic_type_consistency seems not totally unreasonable.) The fact that the rules are about to change would mandate some changes in the wording of the associated error messages in any case. I ended up spelling things out in a fairly literal fashion in the errdetail messages, eg "A result of type anyelement requires at least one input of type anyelement, anyarray, anynonarray, anyenum, or anyrange." Perhaps this is overkill, but once there's more than one subgroup of polymorphic types, people might get confused by more-abstract messages. Discussion: https://postgr.es/m/24137.1584139352@sss.pgh.pa.us
* Adjust handling of an ANYARRAY actual input for an ANYARRAY argument.Tom Lane2020-03-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Ordinarily it's impossible for an actual input of a function to have declared type ANYARRAY, since we'd resolve that to a concrete array type before doing argument type resolution for the function. But an exception arises for functions applied to certain columns of pg_statistic or pg_stats, since we abuse the "anyarray" pseudotype by using it to declare those columns. So parse_coerce.c has to deal with the case. Previously we allowed an ANYARRAY actual input to match an ANYARRAY polymorphic argument, but only if no other argument or result was declared ANYELEMENT. When that logic was written, those were the only two polymorphic types, and I fear nobody thought carefully about how it ought to extend to the other ones that came along later. But actually it was wrong even then, because if a function has two ANYARRAY arguments, it should be able to expect that they have identical element types, and we'd not be able to ensure that. The correct generalization is that we can match an ANYARRAY actual input to an ANYARRAY polymorphic argument only if no other argument or result is of any polymorphic type, so that no promises are being made about element type compatibility. check_generic_type_consistency can't test that condition, but it seems better anyway to accept such matches there and then throw an error if needed in enforce_generic_type_consistency. That way we can produce a specific error message rather than an unintuitive "function does not exist" complaint. (There's some risk perhaps of getting new ambiguous-function complaints, but I think that any set of functions for which that could happen would be ambiguous against ordinary array columns as well.) While we're at it, we can improve the message that's produced in cases that the code did already object to, as shown in the regression test changes. Also, remove a similar test that got cargo-culted in for ANYRANGE; there are no catalog columns of type ANYRANGE, and I hope we never create any, so that's not needed. (It was incomplete anyway.) While here, update some comments and rearrange the code a bit in preparation for upcoming additions of more polymorphic types. In practical situations I believe this is just exchanging one error message for another, hopefully better, one. So it doesn't seem needful to back-patch, even though the mistake is ancient. Discussion: https://postgr.es/m/21569.1584314271@sss.pgh.pa.us
* Remove logical_read_local_xlog_pageAlvaro Herrera2020-03-17
| | | | | | | | | | | | It devolved into a content-less wrapper over read_local_xlog_page, with nothing to add, plus it's easily confused with walsender's logical_read_xlog_page. There doesn't seem to be any reason for it to stay. src/include/replication/logicalfuncs.h becomes empty, so remove it too. The prototypes it initially had were absorbed by generated fmgrprotos.h. Discussion: https://postgr.es/m/20191115214102.GA15616@alvherre.pgsql
* Fix consistency issues with replication slot copyAlvaro Herrera2020-03-17
| | | | | | | | | | | | | | | | | | | Commit 9f06d79ef831's replication slot copying failed to properly reserve the WAL that the slot is expecting to see during DecodingContextFindStartpoint (to set the confirmed_flush LSN), so concurrent activity could remove that WAL and cause the copy process to error out. But it doesn't actually *need* that WAL anyway: instead of running decode to find confirmed_flush, it can be copied from the source slot. Fix this by rearranging things to avoid DecodingContextFindStartpoint() (leaving the target slot's confirmed_flush_lsn to invalid), and set that up afterwards by copying from the target slot's value. Also ensure the source slot's confirmed_flush_lsn is valid. Reported-by: Arseny Sher Author: Masahiko Sawada, Arseny Sher Discussion: https://postgr.es/m/871rr3ohbo.fsf@ars-thinkpad
* Remove bogus assertion about polymorphic SQL function result.Tom Lane2020-03-17
| | | | | | | | | | | | | | | | | | It is possible to reach check_sql_fn_retval() with an unresolved polymorphic rettype, resulting in an assertion failure as demonstrated by one of the added test cases. However, the code following that throws what seems an acceptable error message, so just remove the Assert and adjust commentary. While here, I thought it'd be a good idea to provide some parallel tests of SQL-function and PL/pgSQL-function polymorphism behavior. Some of these cases are perhaps duplicative of tests elsewhere, but we hadn't any organized coverage of the topic AFAICS. Although that assertion's been wrong all along, it won't have any effect in production builds, so I'm not bothering to back-patch. Discussion: https://postgr.es/m/21569.1584314271@sss.pgh.pa.us
* Fix comment in xlog.c.Fujii Masao2020-03-17
| | | | | | | | This commit fixes the comment about SharedHotStandbyActive variable. The comment was apparently copy-and-pasted. Author: Atsushi Torikoshi Discussion: https://postgr.es/m/CACZ0uYEjpqZB9wN2Rwc_RMvDybyYqdbkPuDr1NyxJg4f9yGfMw@mail.gmail.com
* Remove useless pfree()s at the ends of various ValuePerCall SRFs.Tom Lane2020-03-16
| | | | | | | | | | | | | | | | | | | | We don't need to manually clean up allocations in a SRF's multi_call_memory_ctx, because the SRF_RETURN_DONE infrastructure takes care of that (and also ensures that it will happen even if the function never gets a final call, which simple manual cleanup cannot do). Hence, the code removed by this patch is a waste of code and cycles. Worse, it gives the impression that cleaning up manually is a thing, which can lead to more serious errors such as those fixed in commits 085b6b667 and b4570d33a. So we should get rid of it. These are not quite actual bugs though, so I couldn't muster the enthusiasm to back-patch. Fix in HEAD only. Justin Pryzby Discussion: https://postgr.es/m/20200308173103.GC1357@telsasoft.com
* Avoid holding a directory FD open across assorted SRF calls.Tom Lane2020-03-16
| | | | | | | | | | | | | | | | | This extends the fixes made in commit 085b6b667 to other SRFs with the same bug, namely pg_logdir_ls(), pgrowlocks(), pg_timezone_names(), pg_ls_dir(), and pg_tablespace_databases(). Also adjust various comments and documentation to warn against expecting to clean up resources during a ValuePerCall SRF's final call. Back-patch to all supported branches, since these functions were all born broken. Justin Pryzby, with cosmetic tweaks by me Discussion: https://postgr.es/m/20200308173103.GC1357@telsasoft.com
* nbtree: Fix obsolete _bt_search() comment.Peter Geoghegan2020-03-16
| | | | Oversight in commit d2086b08b02.
* nbtree: Pass down MAXALIGN()'d itemsz for new item.Peter Geoghegan2020-03-16
| | | | | | | | | | | | | | Refactor nbtinsert.c so that the final itemsz of each new non-pivot tuple (the MAXALIGN()'d size) is determined once. Most of the functions used by leaf page inserts used the insertstate.itemsz value already. This commit makes everything use insertstate.itemsz as standard practice. The goal is to decouple tuple size from "effective" tuple size. Making this distinction isn't truly necessary right now, but that might change in the future. Also explain why we consistently apply MAXALIGN() to get an effective index tuple size. This was rather unclear, in part because it isn't actually strictly necessary right now.
* Introduce a maintenance_io_concurrency setting.Thomas Munro2020-03-16
| | | | | | | | | | | | | Introduce a GUC and a tablespace option to control I/O prefetching, much like effective_io_concurrency, but for work that is done on behalf of many client sessions. Use the new setting in heapam.c instead of the hard-coded formula effective_io_concurrency + 10 introduced by commit 558a9165e08. Go with a default value of 10 for now, because it's a round number pretty close to the value used for that existing case. Discussion: https://postgr.es/m/CA%2BhUKGJUw08dPs_3EUcdO6M90GnjofPYrWp4YSLaBkgYwS-AqA%40mail.gmail.com
* Simplify the effective_io_concurrency setting.Thomas Munro2020-03-16
| | | | | | | | | | | | | | | | | | | | | The effective_io_concurrency GUC and equivalent tablespace option were previously passed through a formula based on a theory about RAID spindles and probabilities, to arrive at the number of pages to prefetch in bitmap heap scans. Tomas Vondra, Andres Freund and others argued that it was anachronistic and hard to justify, and commit 558a9165e08 already started down the path of bypassing it in new code. We agreed to drop that logic and use the value directly. For the default setting of 1, there is no change in effect. Higher settings can be converted from the old meaning to the new with: select round(sum(OLD / n::float)) from generate_series(1, OLD) s(n); We might want to consider renaming the GUC before the next release given the change in meaning, but it's not clear that many users had set it very carefully anyway. That decision is deferred for now. Discussion: https://postgr.es/m/CA%2BhUKGJUw08dPs_3EUcdO6M90GnjofPYrWp4YSLaBkgYwS-AqA%40mail.gmail.com
* nbtree: Reorder nbtinsert.c prototypes.Peter Geoghegan2020-03-15
| | | | | Relocate _bt_newroot() prototype, so that the order that prototypes appear in matches the order that the functions are defined in.
* Add backend type to csvlog and optionally log_line_prefixPeter Eisentraut2020-03-15
| | | | | | | | | | | | | The backend type, which corresponds to what pg_stat_activity.backend_type shows, is added as a column to the csvlog and can optionally be added to log_line_prefix using the new %b placeholder. Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Reviewed-by: Kuntal Ghosh <kuntalghosh.2007@gmail.com> Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com> Reviewed-by: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://www.postgresql.org/message-id/flat/c65e5196-4f04-4ead-9353-6088c19615a3@2ndquadrant.com
* Rearrange pseudotypes.c to get rid of duplicative code.Tom Lane2020-03-14
| | | | | | | | | | | | | | | | | | | | Commit a5954de10 replaced a lot of manually-coded stub I/O routines with code generated by macros. That was a good idea but it didn't go far enough, because there were still manually-coded stub input routines for types that had live output routines. Refactor the macro so that we can generate just a stub input routine at need. Also create similar macros to generate stub binary I/O routines, since we have some of those now. The only stub functions that remain hand-coded are shell_in() and shell_out(), which need to be separate because they use different error messages. While here, rearrange the commentary to discuss each type not each function. This provides a better way to explain the *why* of which types need which support, rather than just duplicatively annotating the functions. Discussion: https://postgr.es/m/24137.1584139352@sss.pgh.pa.us
* Restructure polymorphic-type resolution in funcapi.c.Tom Lane2020-03-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | resolve_polymorphic_tupdesc() and resolve_polymorphic_argtypes() failed to cover the case of having to resolve anyarray given only an anyrange input. The bug was masked if anyelement was also used (as either input or output), which probably helps account for our not having noticed. While looking at this I noticed that resolve_generic_type() would produce the wrong answer if asked to make that same resolution. ISTM that resolve_generic_type() is confusingly defined and overly complex, so rather than fix it, let's just make funcapi.c do the specific lookups it requires for itself. With this change, resolve_generic_type() is not used anywhere, so remove it in HEAD. In the back branches, leave it alone (complete with bug) just in case any external code is using it. While we're here, make some other refactoring adjustments in funcapi.c with an eye to upcoming future expansion of the set of polymorphic types: * Simplify quick-exit tests by adding an overall have_polymorphic_result flag. This is about a wash now but will be a win when there are more flags. * Reduce duplication of code between resolve_polymorphic_tupdesc() and resolve_polymorphic_argtypes(). * Don't bother to validate correct matching of anynonarray or anyenum; the parser should have done that, and even if it didn't, just doing "return false" here would lead to a very confusing, off-point error message. (Really, "return false" in these two functions should only occur if the call_expr isn't supplied or we can't obtain data type info from it.) * For the same reason, throw an elog rather than "return false" if we fail to resolve a polymorphic type. The bug's been there since we added anyrange, so back-patch to all supported branches. Discussion: https://postgr.es/m/6093.1584202130@sss.pgh.pa.us
* Use multi-variate MCV lists to estimate ScalarArrayOpExprTomas Vondra2020-03-14
| | | | | | | | | | | | Commit 8f321bd16c added support for estimating ScalarArrayOpExpr clauses (IN/ANY) clauses using functional dependencies. There's no good reason not to support estimation of these clauses using multi-variate MCV lists too, so this commits implements that. That makes the behavior consistent and MCV lists can estimate all variants (ANY/ALL, inequalities, ...). Author: Tomas Vondra Review: Dean Rasheed Discussion: https://www.postgresql.org/message-id/flat/13902317.Eha0YfKkKy%40pierred-pdoc
* Use functional dependencies to estimate ScalarArrayOpExprTomas Vondra2020-03-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Until now functional dependencies supported only simple equality clauses and clauses that can be trivially translated to equalities. This commit allows estimation of some ScalarArrayOpExpr (IN/ANY) clauses. For IN clauses we can do this thanks to using operator with equality semantics, which means an IN clause WHERE c IN (1, 2, ..., N) can be translated to WHERE (c = 1 OR c = 2 OR ... OR c = N) IN clauses are now considered compatible with functional dependencies, and rely on the same assumption of consistency of queries with data (which is an assumption we already used for simple equality clauses). This applies also to ALL clauses with an equality operator, which can be considered equivalent to IN clause. ALL clauses are still considered incompatible, although there's some discussion about maybe relaxing this in the future. Author: Pierre Ducroquet Reviewed-by: Tomas Vondra, Dean Rasheed Discussion: https://www.postgresql.org/message-id/flat/13902317.Eha0YfKkKy%40pierred-pdoc
* Remove am_syslogger global variablePeter Eisentraut2020-03-13
| | | | | | | | | | | Use the new MyBackendType instead. More similar changes for other "am something" variables are possible. This one was just particularly simple. Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Reviewed-by: Kuntal Ghosh <kuntalghosh.2007@gmail.com> Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/c65e5196-4f04-4ead-9353-6088c19615a3@2ndquadrant.com
* Unify several ways to tracking backend typePeter Eisentraut2020-03-13
| | | | | | | | | | | | | | Add a new global variable MyBackendType that uses the same BackendType enum that was previously only used by the stats collector. That way several duplicate ways of checking what type a particular process is can be simplified. Since it's no longer just for stats, move to miscinit.c and rename existing functions to match the expanded purpose. Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Reviewed-by: Kuntal Ghosh <kuntalghosh.2007@gmail.com> Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/c65e5196-4f04-4ead-9353-6088c19615a3@2ndquadrant.com
* Preserve replica identity index across ALTER TABLE rewritePeter Eisentraut2020-03-13
| | | | | | | | | | | | If an index was explicitly set as replica identity index, this setting was lost when a table was rewritten by ALTER TABLE. Because this setting is part of pg_index but actually controlled by ALTER TABLE (not part of CREATE INDEX, say), we have to do some extra work to restore it. Based-on-patch-by: Quan Zongliang <quanzongliang@gmail.com> Reviewed-by: Euler Taveira <euler.taveira@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/c70fcab2-4866-0d9f-1d01-e75e189db342@gmail.com
* Avoid holding a directory FD open across pg_ls_dir_files() calls.Tom Lane2020-03-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This coding technique is undesirable because (a) it leaks the FD for the rest of the transaction if the SRF is not run to completion, and (b) allocated FDs are a scarce resource, but multiple interleaved uses of the relevant functions could eat many such FDs. In v11 and later, a query such as "SELECT pg_ls_waldir() LIMIT 1" yields a warning about the leaked FD, and the only reason there's no warning in earlier branches is that fd.c didn't whine about such leaks before commit 9cb7db3f0. Even disregarding the warning, it wouldn't be too hard to run a backend out of FDs with careless use of these SQL functions. Hence, rewrite the function so that it reads the directory within a single call, returning the results as a tuplestore rather than via value-per-call mode. There are half a dozen other built-in SRFs with similar problems, but let's fix this one to start with, just to see if the buildfarm finds anything wrong with the code. In passing, fix bogus error report for stat() failure: it was whining about the directory when it should be fingering the individual file. Doubtless a copy-and-paste error. Back-patch to v10 where this function was added. Justin Pryzby, with cosmetic tweaks and test cases by me Discussion: https://postgr.es/m/20200308173103.GC1357@telsasoft.com
* Refactor ps_status.c APIPeter Eisentraut2020-03-11
| | | | | | | | | | | | | | | | | | | | | | The init_ps_display() arguments were mostly lies by now, so to match typical usage, just use one argument and let the caller assemble it from multiple sources if necessary. The only user of the additional arguments is BackendInitialize(), which was already doing string assembly on the caller side anyway. Remove the second argument of set_ps_display() ("force") and just handle that in init_ps_display() internally. BackendInitialize() also used to set the initial status as "authentication", but that was very far from where authentication actually happened. So now it's set to "initializing" and then "authentication" just before the actual call to ClientAuthentication(). Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Reviewed-by: Kuntal Ghosh <kuntalghosh.2007@gmail.com> Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/c65e5196-4f04-4ead-9353-6088c19615a3@2ndquadrant.com
* Avoid duplicates in ALTER ... DEPENDS ON EXTENSIONAlvaro Herrera2020-03-11
| | | | | | | | | | | | | | | If the command is attempted for an extension that the object already depends on, silently do nothing. In particular, this means that if a database containing multiple such entries is dumped, the restore will silently do the right thing and record just the first one. (At least, in a world where pg_dump does dump such entries -- which it doesn't currently, but it will.) Backpatch to 9.6, where this kind of dependency was introduced. Reviewed-by: Ibrar Ahmed, Tom Lane (offlist) Discussion: https://postgr.es/m/20200217225333.GA30974@alvherre.pgsql
* Clean up order in miscinit.c a bitPeter Eisentraut2020-03-11
| | | | | | The code around InitPostmasterChild() from commit 31c453165b5 somehow ended up in the middle of a block of code related to "User ID state". Move it into its own block instead.
* Remove HAVE_WORKING_LINKPeter Eisentraut2020-03-11
| | | | | | | | | | | | | | Previously, hard links were not used on Windows and Cygwin, but they support them just fine in currently supported OS versions, so we can use them there as well. Since all supported platforms now support hard links, we can remove the alternative code paths. Rename durable_link_or_rename() to durable_rename_excl() to make the purpose more clear without referencing the implementation details. Discussion: https://www.postgresql.org/message-id/flat/72fff73f-dc9c-4ef4-83e8-d2e60c98df48%402ndquadrant.com
* nbtree: Move fastpath NULL descent stack assertion.Peter Geoghegan2020-03-10
| | | | | | | | | | | | | | | | Commit 074251db added an assertion that verified the fastpath/rightmost page insert optimization's assumption about free space: There should always be enough free space on the page to insert the new item without splitting the page. Otherwise, we end up using the "concurrent root page split" phony/fake stack path in _bt_insert_parent(). This does not lead to incorrect behavior, but it is likely to be far slower than simply using the regular _bt_search() path. The assertion catches serious performance bugs that would probably take a long time to detect any other way. It seems much more natural to make this assertion just before the point that we generate a fake/phony descent stack. Move the assert there. This also makes _bt_insertonpg() a bit more readable.
* Marginal comments and docs cleanup.Tom Lane2020-03-10
| | | | | | Fix up some imprecise comments and poor markup from ba79cb5dc. Also try to convert the documentation of log_min_duration_sample and friends into passable English.
* nbtree: Demote minus infinity "can't happen" error.Peter Geoghegan2020-03-10
| | | | | | | | | | | | | | | Only a very basic logic bug in a _bt_insertonpg() caller could lead to a violation of this invariant. Besides, any newitemoff used for an internal page is sanitized using other "can't happen" errors in _bt_getstackbuf() or its callers, before _bt_insertonpg() even gets called. Also, move the error/assertion from the insert-without-split path of _bt_insertonpg() to the top of the same function. There is no reason why this invariant only applies to insertions that happen to not result in a page split; cover every insertion. The assertion naturally belongs next to the existing generic assertions that document relatively high-level invariants for the item being inserted.
* Ensure that CREATE TABLE LIKE copies any NO INHERIT constraint property.Tom Lane2020-03-10
| | | | | | | | | | | | | | | | | Since the documentation about LIKE doesn't say that a copied constraint has properties different from the original, it seems that ignoring a NO INHERIT property doesn't meet the principle of least surprise. So make it copy that. (Note, however, that we still don't copy a NOT VALID property; CREATE TABLE offers no way to do that, plus it seems pointless.) Arguably this is a bug fix; but no back-patch, as it seems barely possible somebody is depending on the current behavior. Ildar Musin and Chris Travers; reviewed by Amit Langote and myself Discussion: https://postgr.es/m/CAONYFtMC6C+3AWCVp7Yd8H87Zn0GxG1_iQG6_bQKbaqYZY0=-g@mail.gmail.com
* Preserve integer and float values accurately in (de)serialize_deflist.Tom Lane2020-03-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, this code just smashed all types of DefElem values to strings, cavalierly reasoning that nobody would care. But in point of fact, most of the defGetFoo functions do distinguish among different input syntaxes; for instance defGetBoolean will accept 1 as an integer but not "1" as a string. This led to CREATE/ALTER TEXT SEARCH DICTIONARY accepting 0 and 1 as values for boolean dictionary properties, only to have the dictionary fail at runtime. We can upgrade this behavior by teaching serialize_deflist that it does not need to quote T_Integer or T_Float nodes' values on output, and then teaching deserialize_deflist to restore unquoted integer or float values as the appropriate node type. This should not break anything using pg_ts_dict.dictinitoption, since that field is just defined as being something valid to include in CREATE TEXT SEARCH DICTIONARY. deserialize_deflist is also used to parse the options arguments for the ts_headline family of functions, but so far as I can see this won't cause any problems there either: the only consumer of that output is prsd_headline which always uses defGetString. (Really that's a bad idea, but I won't risk changing it here.) This is surely a bug fix, but given the lack of field complaints I don't think it's necessary to back-patch. Discussion: https://postgr.es/m/CAMkU=1xRcs_BUPzR0+V3WndaCAv0E_m3h6aUEJ8NF-sY1nnHsw@mail.gmail.com
* Split out CreateCast into src/backend/catalog/pg_cast.cAlvaro Herrera2020-03-10
| | | | | | | | | | | | | This catalog-handling code was previously together with the rest of CastCreate() in src/backend/commands/functioncmds.c. A future patch will need a way to add casts internally, so this will be useful to have separate. Also, move the nearby get_cast_oid() function from functioncmds.c to lsyscache.c, which seems a more natural place for it. Author: Paul Jungwirth, minor edits by Álvaro Discussion: https://postgr.es/m/20200309210003.GA19992@alvherre.pgsql
* Remove utils/acl.h from catalog/objectaddress.hPeter Eisentraut2020-03-10
| | | | | | | | | | | | | | | | | | The need for this was removed by 8b9e9644dc6a9bd4b7a97950e6212f63880cf18b. A number of files now need to include utils/acl.h or parser/parse_node.h explicitly where they previously got it indirectly somehow. Since parser/parse_node.h already includes nodes/parsenodes.h, the latter is then removed where the former was added. Also, remove nodes/pg_list.h from objectaddress.h, since that's included via nodes/parsenodes.h. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/7601e258-26b2-8481-36d0-dc9dca6f28f1%402ndquadrant.com
* Support adding partitioned tables to publicationPeter Eisentraut2020-03-10
| | | | | | | | | | | | | | | When a partitioned table is added to a publication, changes of all of its partitions (current or future) are published via that publication. This change only affects which tables a publication considers as its members. The receiving side still sees the data coming from the individual leaf partitions. So existing restrictions that partition hierarchies can only be replicated one-to-one are not changed by this. Author: Amit Langote <amitlangote09@gmail.com> Reviewed-by: Rafia Sabih <rafia.pghackers@gmail.com> Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/CA+HiwqH=Y85vRK3mOdjEkqFK+E=ST=eQiHdpj43L=_eJMOOznQ@mail.gmail.com
* Prevent reindex of invalid indexes on TOAST tablesMichael Paquier2020-03-10
| | | | | | | | | | | | | | | | | | | Such indexes can only be duplicated leftovers of a previously failed REINDEX CONCURRENTLY command, and a valid equivalent is guaranteed to exist. As toast indexes can only be dropped if invalid, reindexing these would lead to useless duplicated indexes that can't be dropped anymore, except if the parent relation is dropped. Thanks to Justin Pryzby for reminding that this problem was reported long ago during the review of the original patch of REINDEX CONCURRENTLY, but the issue was never addressed. Reported-by: Sergei Kornilov, Justin Pryzby Author: Julien Rouhaud Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/36712441546604286%40sas1-890ba5c2334a.qloud-c.yandex.net Discussion: https://postgr.es/m/20200216190835.GA21832@telsasoft.com Backpatch-through: 12
* Tidy up XLogSource code in xlog.c.Fujii Masao2020-03-10
| | | | | | | | | | | This commit replaces 0 used as an initial value of XLogSource variable, with XLOG_FROM_ANY. Also this commit changes those variable so that XLogSource instead of int is used as the type for them. These changes are for code readability and debugger-friendliness. Author: Kyotaro Horiguchi Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/20200227.124830.2197604521555566121.horikyota.ntt@gmail.com
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-04 04:11:00 +0000