- 01 Aug, 2019 7 commits
-
-
Tom Lane authored
This allows simplification of the plan tree in some common usage patterns: we can get rid of a join to the function RTE. In principle we could pull up any immutable expression, but restricting it to Consts avoids the risk that multiple evaluations of the expression might cost more than we can save. (Possibly this could be improved in future --- but we've more or less promised people that putting a function in FROM guarantees single evaluation, so we'd have to tread carefully.) To do this, we need to rearrange when eval_const_expressions() happens for expressions in function RTEs. I moved it to inline_set_returning_functions(), which already has to iterate over every function RTE, and in consequence renamed that function to preprocess_function_rtes(). A useful consequence is that inline_set_returning_function() no longer has to do this for itself, simplifying that code. In passing, break out pull_up_simple_subquery's code that knows where everything that needs pullup_replace_vars() processing is, so that the new pull_up_constant_function() routine can share it. We'd gotten away with one-and-a-half copies of that code so far, since pull_up_simple_values() could assume that a lot of cases didn't apply to it --- but I don't think pull_up_constant_function() can make any simplifying assumptions. Might as well make pull_up_simple_values() use it too. (Possibly this refactoring should go further: maybe we could share some of the code to fill in the pullup_replace_vars_context struct? For now, I left it that the callers fill that completely.) Note: the one existing test case that this patch changes has to be changed because inlining its function RTEs would destroy the point of the test, namely to check join order. Alexander Kuzmenkov and Aleksandr Parfenov, reviewed by Antonin Houska and Anastasia Lubennikova, and whacked around some more by me Discussion: https://postgr.es/m/402356c32eeb93d4fed01f66d6c7fe2d@postgrespro.ru
-
Peter Geoghegan authored
Oversight in commit 71dcd743.
-
Peter Geoghegan authored
Add sort support for inet, including support for abbreviated keys. Testing has shown that this reduces the time taken to sort medium to large inet/cidr inputs by ~50-60% in realistic cases. Author: Brandur Leach Reviewed-By: Peter Geoghegan, Edmund Horner Discussion: https://postgr.es/m/CABR_9B-PQ8o2MZNJ88wo6r-NxW2EFG70M96Wmcgf99G6HUQ3sw@mail.gmail.com
-
Tom Lane authored
Commit a1c1af2a added logic in the deadlock checker to handle lock grouping, but it was very poorly tested, as evidenced by the bug fixed in 3420851a. Add a test case that exercises that a bit better (and catches the bug --- if you revert 3420851a, this will hang). Since it's pretty hard to get parallel workers to take exclusive regular locks that their parents don't already have, this test operates by creating a deadlock among advisory locks taken in parallel workers. To make that happen, we must override the parallel-safety labeling of the advisory-lock functions, which we do by putting them in mislabeled, non-inlinable wrapper functions. We also have to remove the redundant PreventAdvisoryLocksInParallelMode checks in lockfuncs.c. That seems fine though; if some user accidentally does what this test is intentionally doing, not much harm will ensue. (If there are any remaining bugs that are reachable that way, they're probably reachable in other ways too.) Discussion: https://postgr.es/m/3243.1564437314@sss.pgh.pa.us
-
Tom Lane authored
There seems no good reason not to allow a parallel leader to execute these functions. (The workers still can't, though. Although the code would work, any such lock would go away at worker exit, which is not the documented behavior of advisory locks.) Discussion: https://postgr.es/m/11847.1564496844@sss.pgh.pa.us
-
Peter Eisentraut authored
In some cases we have elog(ERROR) while corruption is certain and we can give a clear error code ERRCODE_DATA_CORRUPTED or ERRCODE_INDEX_CORRUPTED. Author: Andrey Borodin <x4mmm@yandex-team.ru> Discussion: https://www.postgresql.org/message-id/flat/25F6C686-6442-4A6B-BAF8-A6F7B84B16DE@yandex-team.ru
-
Michael Paquier authored
When piling up loading of modules using check_password_hook_type, loading passwordcheck would remove any trace of a previously-loaded hook. Unloading the module would also cause previous hooks to be entirely gone. Reported-by: Rafael Castro Author: Michael Paquier Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/15932-78f48f9ef166778c@postgresql.org Backpatch-through: 9.4
-
- 31 Jul, 2019 5 commits
-
-
Tom Lane authored
Since pg_dump doesn't treat the member operators and functions of operator classes/families (that is, the pg_amop and pg_amproc entries, not the underlying operators/functions) as separate dumpable objects, it missed their dependency information. I think this was safe when the code was designed, because the default object sorting rule emits operators and functions before opclasses, and there were no dependency types that could mess that up. However, the introduction of range types in 9.2 broke it: now a type can have a dependency on an opclass, allowing dependency rules to push the opclass before the type and hence before custom operators. Lacking any information showing that it shouldn't do so, pg_dump emitted the objects in the wrong order. Fix by teaching getDependencies() to translate pg_depend entries for pg_amop/amproc rows to look like dependencies for their parent opfamily. I added a regression test for this in HEAD/v12, but not further back; life is too short to fight with 002_pg_dump.pl. Per bug #15934 from Tom Gottfried. Back-patch to all supported branches. Discussion: https://postgr.es/m/15934-58b8c8ab7a09ea15@postgresql.org
-
Peter Eisentraut authored
The tests collate.icu.utf8 and collate.linux.utf8 were previously only run when explicitly selected via EXTRA_TESTS. They require a UTF8 database, because the error messages in the expected files refer to that, and they use some non-ASCII characters in the tests. Since users can select any locale and encoding for the regression test run, it was not possible to include these tests automatically. To fix, use psql's \if facility to check various prerequisites such as platform and the server encoding and quit the tests at the very beginning if the configuration is not adequate. We then need to maintain alternative expected files for these tests, but they are very tiny and never need to change after this. These two tests are now run automatically as part of the regression tests. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/052295c2-a2e1-9a21-bd36-8fbff8686cf3%402ndquadrant.com
-
Andres Freund authored
These were introduced by pgindent due to fixe to broken indentation (c.f. 8255c7a5). Previously the mis-indentation of function prototypes was creatively used to reduce indentation in a few places. As that formatting only exists in master and REL_12_STABLE, it seems better to fix it in both, rather than having some odd indentation in v12 that somebody might copy for future patches or such. Author: Andres Freund Discussion: https://postgr.es/m/20190728013754.jwcbe5nfyt3533vx@alap3.anarazel.de Backpatch: 12-
-
Andres Freund authored
Author: Andres Freund
-
Michael Paquier authored
int_name has never been used for digest lookups since its introduction in e94dd6ab. Author: Daniel Gustafsson Discussion: https://postgr.es/m/386C26CB-628B-4A4C-8879-D8BF190F2C77@yesql.se
-
- 30 Jul, 2019 4 commits
-
-
Heikki Linnakangas authored
The rd_amcache allows an index AM to cache arbitrary information in a relcache entry. This commit moves the cleanup of rd_amcache so that it can also be used by table AMs. Nothing takes advantage of that yet, but I'm sure it'll come handy for anyone writing new table AMs. Backpatch to v12, where table AM interface was introduced. Reviewed-by: Julien Rouhaud
-
Heikki Linnakangas authored
This has been wrong ever since pg_rewind was added. The if-branch just above this, where we print the same error with an extra message supplied by XLogReadRecord() got this right, but the variable name was wrong in the else-branch. As a consequence, the error printed the WAL position as 0/0 if there was an error reading a WAL file. Backpatch to 9.5, where pg_rewind was added.
-
Tomas Vondra authored
When performing ANALYZE on inheritance trees, we collect two samples for each relation - one for the relation alone, and one for the inheritance subtree (relation and its child relations). And then we build statistics on each sample, so for each relation we get two sets of statistics. For regular (per-column) statistics this works fine, because the catalog includes a flag differentiating statistics built from those two samples. But we don't have such flag in the extended statistics catalogs, and we ended up updating the same row twice, triggering this error: ERROR: tuple already updated by self The simplest solution is to disable extended statistics on inheritance trees, which is what this commit is doing. In the future we may need to do something similar to per-column statistics, but that requires adding a flag to the catalog - and that's not backpatchable. Moreover, the current selectivity estimation code only works with individual relations, so building statistics on inheritance trees would be pointless anyway. Author: Tomas Vondra Backpatch-to: 10- Discussion: https://postgr.es/m/20190618231233.GA27470@telsasoft.com Reported-by: Justin Pryzby
-
Michael Paquier authored
When building a list of relations for a parallel processing of a schema or a database (or just a single-entry list for the non-parallel case with the database name), the list is allocated and built on-the-fly for each database processed, leaking after one database-level reindex is done. This accumulates leaks when processing all databases, and could become a visible issue with thousands of relations. This is fixed by introducing a new routine in simple_list.c to free all the elements in a simple list made of strings or OIDs. The header of the list may be using a variable declaration or an allocated pointer, so we don't have a routine to free this part to keep the interface simple. Per report from coverity for an issue introduced by 5ab892c3, and valgrind complains about the leak as well. The idea to introduce a new routine in simple_list.c is from Tom Lane. Author: Michael Paquier Reviewed-by: Tom Lane
-
- 29 Jul, 2019 4 commits
-
-
Tom Lane authored
A "break" statement erroneously left behind by commit a1c1af2a caused TopoSort to do the wrong thing if a lock's wait list contained multiple members of the same locking group. Because parallel workers don't normally need any locks not already taken by their leader, this is very hard --- maybe impossible --- to hit in production. Still, if it did happen, the queries involved in an otherwise-resolvable deadlock would block until canceled. In addition to removing the bogus "break", add an Assert showing that the conflicting uses of the beforeConstraints[] array (for both counts and flags) don't overlap, and add some commentary explaining why not; because it's not obvious without explanation, IMHO. Original report and patch from Rui Hai Jiang; additional assert and commentary by me. Back-patch to 9.6 where the bug came in. Discussion: https://postgr.es/m/CAEri+mLd3bpHLyW+a9pSe1y=aEkeuJpwBSwvo-+m4n7-ceRmXw@mail.gmail.com
-
Peter Eisentraut authored
It is not safe to simply report an fsync error and continue. We must exit the program instead. Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Sehrope Sarkuni <sehrope@jackdb.com> Discussion: https://www.postgresql.org/message-id/flat/9b49fe44-8f3e-eca9-5914-29e9e99030bf@2ndquadrant.com
-
Michael Paquier authored
This is numbered take 8, and addresses again a set of issues with code comments, variable names and unreferenced variables. Author: Alexander Lakhin Discussion: https://postgr.es/m/b137b5eb-9c95-9c2f-586e-38aba7d59788@gmail.com
-
Michael Paquier authored
When copying the definition of an index rebuilt concurrently for the new entry, the index information was taken directly from the old index using the relation cache. In this case, predicates and expressions have some post-processing to prepare things for the planner, which loses some information including the collations added in any of them. This inconsistency can cause issues when attempting for example a table rewrite, and makes the new indexes rebuilt concurrently inconsistent with the old entries. In order to fix the problem, fetch expressions and predicates directly from the catalog of the old entry, and fill in IndexInfo for the new index with that. This makes the process more consistent with DefineIndex(), and the code is refactored with the addition of a routine to create an IndexInfo node. Reported-by: Manuel Rigger Author: Michael Paquier Discussion: https://postgr.es/m/CA+u7OA5Hp0ra235F3czPom_FyAd-3+XwSJmX95r1+sRPOJc9VQ@mail.gmail.com Backpatch-through: 12
-
- 28 Jul, 2019 5 commits
-
-
Thomas Munro authored
Early previews of LLVM 9 reveal that our Min() macro causes compiler errors in LLVM headers reached by the #include directives in llvmjit_inline.cpp. Let's just undefine it. Per buildfarm animal seawasp. Back-patch to 11. Reviewed-by: Fabien Coelho, Tom Lane Discussion: https://postgr.es/m/20190606173216.GA6306%40alvherre.pgsql
-
Tom Lane authored
We had no actual end-to-end test of NOTIFY message delivery. In the core async.sql regression test, testing this is problematic because psql traditionally prints the PID of the sending backend, making the output unstable. We also have an isolation test script, but it likewise failed to prove that delivery worked, because isolationtester.c had no provisions for detecting/reporting NOTIFY messages. Hence, add such provisions to isolationtester.c, and extend async-notify.spec to include direct tests of basic NOTIFY functionality. I also added tests showing that NOTIFY de-duplicates messages normally, but not across subtransaction boundaries. (That's the historical behavior since we introduced subtransactions, though perhaps we ought to change it.) Patch by me, with suggestions/review by Andres Freund. Discussion: https://postgr.es/m/31304.1564246011@sss.pgh.pa.us
-
Michael Paquier authored
The table has not been updated for some commands introduced in recent releases, so refresh it. While on it, reorder entries alphabetically. Backpatch all the way down for all the commands which have gone missing. Reported-by: Jeremy Smith Discussion: https://postgr.es/m/15883-afff0ea3cc2dbbb6@postgresql.org Backpatch-through: 9.4
-
Michael Paquier authored
The frontend version of walkdir() is defined in file_utils.c, and not initdb.c. Author: Sehrope Sarkuni Discussion: https://postgr.es/m/CAH7T-artawnBt4=KODNCD8Mt2ZX4CCjJT8c=_=950xjutcRZ4Q@mail.gmail.com
-
Tom Lane authored
If a test sends a notice just before blocking, it's possible on slow machines for isolationtester to detect the blocked state before it's consumed the notice. (For this to happen, the notice would have to arrive after isolationtester has waited for data for 10ms, so on fast/lightly-loaded machines it's hard to reproduce the failure.) But, if we have seen the backend as blocked, it's certainly already sent any notices it's going to send. Therefore, one more round of PQconsumeInput and PQisBusy should be enough to collect and process any such notices. This appears to explain the instability noted in commit ebd49928, so undo the hack therein to not print notices from insert-conflict-specconflict. Patch by me, diagnosis by Andres Freund. Discussion: https://postgr.es/m/14616.1564251339@sss.pgh.pa.us
-
- 27 Jul, 2019 6 commits
-
-
Tom Lane authored
For its entire existence, isolationtester.c has forced client_min_messages to WARNING, but that seems like a very poor choice of test design. It should be up to individual test scripts to manage whether they emit notices and to ensure that the results are stable. (There were no NOTICE messages in the original set of isolation tests, so this was certainly dead code when committed, but perhaps it was needed at some earlier point.) It's possible that the original motivation was due to platform-dependent variations in the timing of stdout vs. stderr output. That should be moot since commits 73bcb76b/6eda3e9c, but just in case, adjust isotesterNoticeProcessor to print to stdout not stderr. (stderr seems like the wrong thing anyway: it should be for error printouts not expected test output.) Testing shows that the notices in insert-conflict-specconflict are indeed a bit timing-unstable on very slow machines, so hide them; maybe we can improve that later. Also, make the notices in plpgsql-toast a bit less verbose than the original code would've had them. Discussion: https://postgr.es/m/14616.1564251339@sss.pgh.pa.us
-
Michael Paquier authored
When doing a schema-level or a database-level operation, a list of relations to build is created which gets processed in parallel using multiple connections, based on the recent refactoring for parallel slots in src/bin/scripts/. System catalogs are processed first in a serialized fashion to prevent deadlocks, followed by the rest done in parallel. This new option is not compatible with --system as reindexing system catalogs in parallel can lead to deadlocks, and with --index as there is no conflict handling for indexes rebuilt in parallel depending in the same relation. Author: Julien Rouhaud Reviewed-by: Sergei Kornilov, Michael Paquier Discussion: https://postgr.es/m/CAOBaU_YrnH_Jqo46NhaJ7uRBiWWEcS40VNRQxgFbqYo9kApUsg@mail.gmail.com
-
Peter Eisentraut authored
Recently released xfsprogs 5.1.0 has reflink support enabled by default, so the note that it's not enabled by default can be removed.
-
Peter Eisentraut authored
Make the directory where the pg_upgrade binary resides the default for new bindir, as running the pg_upgrade binary from where the new cluster is installed is a very common scenario. Setting this as the defauly bindir for the new cluster will remove the need to provide it explicitly via -B in many cases. To support directories being missing from option parsing, extend the directory check with a missingOk mode where the path must be filled at a later point before being used. Also move the exec_path check to earlier in setup to make sure we know the new cluster bindir when we scan for required executables. This removes the exec_path from the OSInfo struct as it is not used anywhere. Author: Daniel Gustafsson <daniel@yesql.se> Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/9328.1552952117@sss.pgh.pa.us
-
Peter Eisentraut authored
Expand the validate_exec() calls to cover all the used binaries. Author: Daniel Gustafsson <daniel@yesql.se> Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/9328.1552952117@sss.pgh.pa.us
-
Peter Eisentraut authored
Author: Daniel Gustafsson <daniel@yesql.se>
-
- 26 Jul, 2019 8 commits
-
-
Alvaro Herrera authored
Per gripe from Ian Barwick Co-authored-by: Ian Barwick <ian@2ndquadrant.com> Discussion: https://postgr.es/m/CABvVfJWNnNKb8cHsTLhkTsvL1+G6BVcV+57+w1JZ61p8YGPdWQ@mail.gmail.com
-
Tom Lane authored
pg_timezone_names() tries to avoid showing the "Factory" zone in the view, mainly because that has traditionally had a very long "abbreviation" such as "Local time zone must be set--see zic manual page", so that showing it messes up psql's formatting of the whole view. Since tzdb version 2016g, IANA instead uses the abbreviation "-00", which is sane enough that there's no reason to discriminate against it. On the other hand, it emerges that FreeBSD and possibly other packagers are so wedded to backwards compatibility that they hack the IANA data to keep the old spelling --- and not just that old spelling, but even older spellings that IANA used back in the stone age. This caused the filter logic to fail to suppress "Factory" at all on such platforms, though the formatting problem is definitely real in that case. To solve both problems, get rid of the hard-wired assumption about exactly what Factory's abbreviation is, and instead reject abbreviations exceeding 31 characters. This will allow Factory to appear in the view if and only if it's using the modern abbreviation. In passing, simplify the code we add to zic.c to support "zic -P" to remove its now-obsolete hacks to not print the Factory zone's abbreviation. Unlike pg_timezone_names(), there's no reason for that code to support old/nonstandard timezone data. Since we generally prefer to keep timezone-related behavior the same in all branches, and since this is arguably a bug fix, back-patch to all supported branches. Discussion: https://postgr.es/m/3961.1564086915@sss.pgh.pa.us
-
Tom Lane authored
Some platforms create a file named "localtime" in the system timezone directory, making it a copy or link to the active time zone file. If Postgres is built with --with-system-tzdata, initdb will see that file as an exact match to localtime(3)'s behavior, and it may decide that "localtime" is the most preferred spelling of the active zone. That's a very bad choice though, because it's neither informative, nor portable, nor stable if someone changes the system timezone setting. Extend the preference logic added by commit e3846a00 so that we will prefer any other zone file that matches localtime's behavior over "localtime". On the same logic, also discriminate against "posixrules", which is another not-really-a-zone file that is often present in the timezone directory. (Since we install "posixrules" but not "localtime", this change can affect the behavior of Postgres with or without --with-system-tzdata.) Note that this change doesn't prevent anyone from choosing these pseudo-zones if they really want to (i.e., by setting TZ for initdb, or modifying the timezone GUC later on). It just prevents initdb from preferring these zone names when there are multiple matches to localtime's behavior. Since we generally prefer to keep timezone-related behavior the same in all branches, and since this is arguably a bug fix, back-patch to all supported branches. Discussion: https://postgr.es/m/CADT4RqCCnj6FKLisvT8tTPfTP4azPhhDFJqDF1JfBbOH5w4oyQ@mail.gmail.com Discussion: https://postgr.es/m/27991.1560984458@sss.pgh.pa.us
-
Tom Lane authored
Money values exceeding about 18 digits (depending on lc_monetary) could be inaccurately converted to numeric, due to select_div_scale() deciding it didn't need to compute any fractional digits. Force its hand by setting the dscale of one division input to equal the number of fractional digits we need. In passing, rearrange the logic to not do useless work in locales where money values are considered integral. Per bug #15925 from Slawomir Chodnicki. Back-patch to all supported branches. Discussion: https://postgr.es/m/15925-da9953e2674bb5c8@postgresql.org
-
Peter Eisentraut authored
Turn most mentions of libpq functions into links. At id attributes to most libpq functions, where not existing yet, so that they can be linked to. (In a handful of cases there were problems with the PDF processing toolchain, so those instances were not changed.) Author: Fabien COELHO <coelho@cri.ensmp.fr> Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/alpine.DEB.2.21.1905121032330.27203@lancre
-
Peter Eisentraut authored
When making an xref to a varlistentry, the stylesheets use the first <term> as the link text. In the cases fixed here, the <term> element contained extra whitespace that ended up being part of the link text, which looked strange in the output in some cases. This whitespace is significant, so remove it since we don't want it.
-
Peter Eisentraut authored
The ids for linking to libpq functions were previously all lower-case. Change to mixed-case, matching the actual function name, for easier readability in the source. The output isn't changed in a significant way, since the ids are converted to lower or upper case for file names and anchors.
-
- 25 Jul, 2019 1 commit
-
-
Thomas Munro authored
After starting slapd, wait until it can accept a connection before beginning the real test work. This avoids occasional test failures. Back-patch to 11, where the LDAP tests arrived. Author: Thomas Munro Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/20190719033013.GI1859%40paquier.xyz
-