- 31 Jul, 2019 4 commits
-
-
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 9 commits
-
-
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
-
Andres Freund authored
Backpatch: 12-, like the previous commit
-
Andres Freund authored
Since 15d8f831 we assert that - and since 7ef04e4d, 4da597ed rely on - the slot type for an expression's ecxt_{outer,inner,scan}tuple not changing, unless explicitly flagged as such. That allows to either skip deforming (for a virtual tuple slot) or optimize the code for JIT accelerated deforming appropriately (for other known slot types). This assumption was sometimes violated for grouping sets, when nodeAgg.c internally uses tuplesorts, and the child node doesn't return a TTSOpsMinimalTuple type slot. Detect that case, and flag that the outer slot might not be "fixed". It's probably worthwhile to optimize this further in the future, and more granularly determine whether the slot is fixed. As we already instantiate per-phase transition and equal expressions, we could cheaply set the slot type appropriately for each phase. But that's a separate change from this bugfix. This commit does include a very minor optimization by avoiding to create a slot for handling tuplesorts, if no such sorts are performed. Previously we created that slot unnecessarily in the common case of computing all grouping sets via hashing. The code looked too confusing without that, as the conditions for needing a sort slot and flagging that the slot type isn't fixed, are the same. Reported-By: Ashutosh Sharma Author: Andres Freund Discussion: https://postgr.es/m/CAE9k0PmNaMD2oHTEAhRyxnxpaDaYkuBYkLa1dpOpn=RS0iS2AQ@mail.gmail.com Backpatch: 12-, where the bug was introduced in 15d8f831
-
Tom Lane authored
Per buildfarm.
-
Tom Lane authored
libpq failed to ignore Windows-style newlines in connection service files. This normally wasn't a problem on Windows itself, because fgets() would convert \r\n to just \n. But if libpq were running inside a program that changes the default fopen mode to binary, it would see the \r's and think they were data. In any case, it's project policy to ignore \r in text files unconditionally, because people sometimes try to use files with DOS-style newlines on Unix machines, where the C library won't hide that from us. Hence, adjust parseServiceFile() to ignore \r as well as \n at the end of the line. In HEAD, go a little further and make it ignore all trailing whitespace, to match what it's always done with leading whitespace. In HEAD, also run around and fix up everyplace where we have newline-chomping code to make all those places look consistent and uniformly drop \r. It is not clear whether any of those changes are fixing live bugs. Most of the non-cosmetic changes are in places that are reading popen output, and the jury is still out as to whether popen on Windows can return \r\n. (The Windows-specific code in pipe_read_line seems to think so, but our lack of support for this elsewhere suggests maybe it's not a problem in practice.) Hence, I desisted from applying those changes to back branches, except in run_ssl_passphrase_command() which is new enough and little-tested enough that we'd probably not have heard about any problems there. Tom Lane and Michael Paquier, per bug #15827 from Jorge Gustavo Rocha. Back-patch the parseServiceFile() change to all supported branches, and the run_ssl_passphrase_command() change to v11 where that was added. Discussion: https://postgr.es/m/15827-e6ba53a3a7ed543c@postgresql.org
-
Andrew Dunstan authored
Add a line to the project file setting the target SDK. Otherwise, in for example VS2017, if the default but optional 8.1 SDK is not installed the build will fail. Patch from Peifeng Qiu, slightly edited by me. Discussion: https://postgr.es/m/CABmtVJhw1boP_bd4=b3Qv5YnqEdL696NtHFi2ruiyQ6mFHkeQQ@mail.gmail.com Backpatch to all live branches.
-
Tom Lane authored
As of Fedora 30, it seems that the system-provided macros for setting up user privileges in SELinux policies don't grant the ability to read /etc/passwd, as they formerly did. This restriction breaks psql (which tries to use getpwuid() to obtain the user name it's running under) and thereby the contrib/sepgsql regression test. Add explicit specifications that we need the right to read /etc/passwd. Mike Palmiotto, per a report from me. Back-patch to all supported branches. Discussion: https://postgr.es/m/23856.1563381159@sss.pgh.pa.us
-
Peter Eisentraut authored
-
Andres Freund authored
After 277cb789 ON CONFLICT ... SET ... RETURNING failed with ERROR: virtual tuple table slot does not have system attributes when taking the update path, as the slot used to insert into the table (and then process RETURNING) was defined to be a virtual slot in that commit. Virtual slots don't support system columns except for tableoid and ctid, as the other system columns are AM dependent. Fix that by using a slot of the table's type. Add tests for system column accesses in ON CONFLICT ... RETURNING. Reported-By: Roby, bisected to the relevant commit by Jeff Janes Author: Andres Freund Discussion: https://postgr.es/m/73436355-6432-49B1-92ED-1FE4F7E7E100@finefun.com.au Backpatch: 12-, where the bug was introduced in 277cb789
-