- 17 Oct, 2018 1 commit
-
-
Tom Lane authored
Looking at this code made my head hurt. Format the comments more like the way it's done elsewhere, break a few overly long lines. No actual code changes in this commit.
-
- 16 Oct, 2018 13 commits
-
-
Andres Freund authored
That's worth it, as fmgr_builtins is frequently accessed, and as fmgr_builtins is one of the biggest constant variables in a backend. On most 64bit systems this will change the size of the struct from 32byte to 24bytes. While that could make indexing into the array marginally more expensive, the higher cache hit ratio is worth more, especially because these days fmgr_builtins isn't searched with a binary search anymore (c.f. 212e6f34). Discussion: https://postgr.es/m/20181016201145.aa2dfeq54rhqzron@alap3.anarazel.de
-
Tom Lane authored
Rethink the solution applied in commit 5e221713 to get PL/Tcl to build on macOS Mojave. I feared that adding -isysroot globally might have undesirable consequences, and sure enough Jakob Egger reported one: it complicates building extensions with a different Xcode version than was used for the core server. (I find that a risky proposition in general, but apparently it works most of the time, so we shouldn't break it if we don't have to.) We'd already adopted the solution for PL/Perl of inserting the sysroot path directly into the -I switches used to find Perl's headers, and we can do the same thing for PL/Tcl by changing the -iwithsysroot switch that Apple's tclConfig.sh reports. This restricts the risks to PL/Perl and PL/Tcl themselves and directly-dependent extensions, which is a lot more pleasing in general than a global -isysroot switch. Along the way, tighten the test to see if we need to inject the sysroot path into $perl_includedir, as I'd speculated about upthread but not gotten round to doing. As before, back-patch to all supported versions. Discussion: https://postgr.es/m/20840.1537850987@sss.pgh.pa.us
-
Andres Freund authored
This allows the compiler / linker to mark affected pages as read-only. Doing so requires casting constness away, as CreateDestReceiver() returns both constant and non-constant dest receivers. That's fine though, as any modification of the statically allocated receivers would already have been a bug (and would now be caught on some platforms). Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de
-
Andres Freund authored
The new unconsitify(underlying_type, var) macro allows to cast constness away from a variable, but doesn't allow changing the underlying type. Enforcement of the latter currently only works for gcc like compilers. Please note IT IS NOT SAFE to cast constness away if the variable will ever be modified (it would be undefined behaviour). Doing so anyway can cause compiler misoptimizations or runtime crashes (modifying readonly memory). It is only safe to use when the the variable will not be modified, but API design or language restrictions prevent you from declaring that (e.g. because a function returns both const and non-const variables). This'll be used in an upcoming change, but seems like it's independent infrastructure. Author: Andres Freund Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de
-
Tom Lane authored
The previous code here simply threw away whatever it knew about cache entry ages whenever a counter overflow occurred. Since the counter is int width and will be bumped once per format function execution, overflows are not really so rare as to not be worth thinking about. Instead, let's deal with the situation by halving all the age values, essentially rescaling the age metric. In that way, we retain a pretty accurate (if not quite perfect) idea of which entries are oldest.
-
Tom Lane authored
We created a temp table, then switched to a new session, leaving the old session to clean up its temp objects in background. If that took long enough, the eventual attempt to drop the user that owns the temp table could fail, as exhibited today by sidewinder. Fix by dropping the temp table explicitly when we're done with it. It's been like this for quite some time, so back-patch to all supported branches. Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sidewinder&dt=2018-10-16%2014%3A45%3A00
-
Tom Lane authored
This eliminates circa 120KB of static data from Postgres' memory footprint. In some usage patterns that space will get allocated anyway, but in many processes it never will be allocated. We can improve matters further by allocating only as many cache entries as we actually use, rather than allocating the whole array on first use. However, to avoid wasting lots of space due to palloc's habit of rounding requests up to power-of-2 sizes, tweak the maximum cacheable format string length to make the struct sizes be powers of 2 or just less. The sizes I chose make the maximums a little bit less than they were before, but I doubt it matters much. While at it, rearrange struct FormatNode to avoid wasting quite so much padding space. This change actually halves the size of that struct on 64-bit machines. Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de
-
Andres Freund authored
This allows the compiler / linker to mark affected pages as read-only. There's a fair number of pre-requisite changes, to allow the const properly be propagated. Most of consts were already required for correctness anyway, just not represented on the type-level. Arguably we could be more aggressive in using consts in related code, but.. This requires using a few of the types underlying typedefs that removes pointers (e.g. const NameData *) as declaring the typedefed type constant doesn't have the same meaning (it makes the variable const, not what it points to). Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de
-
Tom Lane authored
Reporting only the stderr is unhelpful when the problem is that the server output we're getting doesn't match what was expected. So we should report the query output too; and just for good measure, let's print the query we used and the output we expected. Back-patch to 9.5 where poll_query_until was introduced. Discussion: https://postgr.es/m/17913.1539634756@sss.pgh.pa.us
-
Tom Lane authored
Commit b5febc1d added a contrib/btree_gist test case that has been observed to fail in the buildfarm as a result of background auto-analyze updating stats and changing the selected plan. Forestall that by forcibly analyzing in foreground, instead. The new plan choice is just as good for our purposes, since we really only care that an index-only plan does not get selected. Back-patch to 9.5, like the previous patch. Discussion: https://postgr.es/m/14643.1539629304@sss.pgh.pa.us
-
Tom Lane authored
localtime.c's "struct state" is a rather large object, ~23KB. We were statically allocating one for gmtsub() to use to represent the GMT timezone, even though that function is not at all heavily used and is never reached in most backends. Let's malloc it on-demand, instead. This does pose the question of how to handle a malloc failure, but there's already a well-defined error report convention here, ie set errno and return NULL. We have but one caller of pg_gmtime in HEAD, and two in back branches, neither of which were troubling to check for error. Make them do so. The possible errors are sufficiently unlikely (out-of-range timestamp, and now malloc failure) that I think elog() is adequate. Back-patch to all supported branches to keep our copies of the IANA timezone code in sync. This particular change is in a stanza that already differs from upstream, so it's a wash for maintenance purposes --- but only as long as we keep the branches the same. Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de
-
Andres Freund authored
This allows the compiler / linker to mark affected pages as read-only. There's other cases, but they're a bit more invasive, and should go through some review. These are easy. They were found with objdump -j .data -t src/backend/postgres|awk '{print $4, $5, $6}'|sort -r|less Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de
-
Andres Freund authored
There's several reasons for this change: 1) It reduces the total size of TupleTableSlot / reduces alignment padding, making the commonly accessed members fit into a single cacheline (but we currently do not force proper alignment, so that's not yet guaranteed to be helpful) 2) Combining the booleans into a flag allows to combine read/writes from memory. 3) With the upcoming slot abstraction changes, it allows to have core and extended flags, in a memory efficient way. Author: Ashutosh Bapat and Andres Freund Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
-
- 15 Oct, 2018 6 commits
-
-
Andres Freund authored
heaptuple.c was never a particular good fit for slot_getattr(), slot_getsomeattrs() and slot_getmissingattrs(), but in upcoming changes slots will be made more abstract (allowing slots that contain different types of tuples), making it clearly the wrong place. Note that slot_deform_tuple() remains in it's current place, as it clearly deals with a HeapTuple. getmissingattrs() also remains, but it's less clear that that's correct - but execTuples.c wouldn't be the right place. Author: Ashutosh Bapat. Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
-
Thomas Munro authored
Andres Freund complained about the 128KB of .bss occupied by LagTracker. It's only needed in the walsender process, so allocate it in heap memory there. Author: Thomas Munro Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya%40alap3.anarazel.de
-
Tom Lane authored
ProcessUtility can recurse, and indeed can be driven to infinite recursion, so it ought to have a check_stack_depth() call. This covers the reported bug (portal trying to execute itself) and a bunch of other cases that could perhaps arise somewhere. Per bug #15428 from Malthe Borch. Back-patch to all supported branches. Discussion: https://postgr.es/m/15428-b3c2915ec470b033@postgresql.org
-
Peter Eisentraut authored
When an error occurs during a benchmark run, exit with a nonzero exit code and write a message at the end. Previously, it would just print the error message when it happened but then proceed to print the run summary normally and exit with status 0. To still allow distinguishing setup from run-time errors, we use exit status 2 for the new state, whereas existing errors during pgbench initialization use exit status 1. Reviewed-by: Fabien COELHO <coelho@cri.ensmp.fr>
-
Peter Eisentraut authored
I used the preferred U.S. spelling, as we do in other cases.
-
Peter Eisentraut authored
With the PostgreSQL 11 release notes acknowledgments list, FOP reported WARNING: Glyph "?" (0x144, nacute) not available in font "Times-Roman". WARNING: Glyph "?" (0x15e, Scedilla) not available in font "Times-Roman". WARNING: Glyph "?" (0x15f, scedilla) not available in font "Times-Roman". WARNING: Glyph "?" (0x131, dotlessi) not available in font "Times-Roman". This is because we have some new contributors whose names use letters that we haven't used before, and apparently FOP can't handle them out of the box. For now, just fix this by "unaccenting" those names. In the future, maybe this can be fixed better with a different font configuration. There is also another warning WARNING: Glyph "?" (0x3c0, pi) not available in font "Times-Roman". but that existed in previous releases and is not touched here.
-
- 14 Oct, 2018 7 commits
-
-
Alexander Korotkov authored
Backpatch commits don't contain this error.
-
Alexander Korotkov authored
This commit documents rounding of "length" parameter and absence of support for unique indexes and NULLs searching. Backpatch to 9.6 where contrib/bloom was introduced. Discussion: https://postgr.es/m/CAF4Au4wPQQ7EHVSnzcLjsbY3oLSzVk6UemZLD1Sbmwysy3R61g%40mail.gmail.com Author: Oleg Bartunov with minor editorialization by me Backpatch-through: 9.6
-
Tom Lane authored
These test cases could be adversely affected by an upcoming change to allow pullup of FROM-less subqueries. Tweak them to ensure that they'll continue to test what they did before. Discussion: https://postgr.es/m/5395.1539275668@sss.pgh.pa.us
-
Tom Lane authored
This prevents failures in cases where we pull up a constant or var-free expression from a subquery and put it into a full join's qual. That can result in not recognizing the qual as containing a mergejoin-able or hashjoin-able condition. A PHV prevents the problem because it is still recognized as belonging to the side of the join the subquery is in. I'm not very sure about the net effect of this change on plan quality. In "typical" cases where the join keys are Vars, nothing changes. In an affected case, the PHV-wrapped expression is less likely to be seen as equal to PHV-less instances below the join, but more likely to be seen as equal to similar expressions above the join, so it may end up being a wash. In the one existing case where there's any visible change in a regression-test plan, it amounts to referencing a lower computation of a COALESCE result instead of recomputing it, which seems like a win. Given my uncertainty about that and the lack of field complaints, no back-patch, even though this is a very ancient problem. Discussion: https://postgr.es/m/32090.1539378124@sss.pgh.pa.us
-
Tom Lane authored
With the removal of the old abstime type, there are no longer any cases in this test where we need to use the weaker castcontext-ignoring form of binary coercibility check. (The other major source of such headaches, apparently-incompatible hash functions, is now hashvalidate()'s problem not this test script's problem.) Hence, just use binary_coercible() everywhere, and remove the comments explaining why we don't do so --- which were broken anyway by cda6a8d0. I left physically_coercible() in place but renamed it to better match what it's actually testing, and added some comments. Also, in test queries that have an assumption about the maximum number of function arguments they need to handle, add a clause to make them fail if someday there's a relevant function with more arguments. Otherwise we're likely not to notice that we need to extend the queries. Discussion: https://postgr.es/m/27637.1539388060@sss.pgh.pa.us
-
Michael Paquier authored
On a primary, sets of XLOG_RUNNING_XACTS records are generated on a periodic basis to allow recovery to build the initial state of transactions for a hot standby. The set of transaction IDs is created by scanning all the entries in ProcArray. However it happens that its logic never counted on the fact that two-phase transactions finishing to prepare can put ProcArray in a state where there are two entries with the same transaction ID, one for the initial transaction which gets cleared when prepare finishes, and a second, dummy, entry to track that the transaction is still running after prepare finishes. This way ensures a continuous presence of the transaction so as callers of for example TransactionIdIsInProgress() are always able to see it as alive. So, if a XLOG_RUNNING_XACTS takes a standby snapshot while a two-phase transaction finishes to prepare, the record can finish with duplicated XIDs, which is a state expected by design. If this record gets applied on a standby to initial its recovery state, then it would simply fail, so the odds of facing this failure are very low in practice. It would be tempting to change the generation of XLOG_RUNNING_XACTS so as duplicates are removed on the source, but this requires to hold on ProcArrayLock for longer and this would impact all workloads, particularly those using heavily two-phase transactions. XLOG_RUNNING_XACTS is also actually used only to initialize the standby state at recovery, so instead the solution is taken to discard duplicates when applying the initial snapshot. Diagnosed-by: Konstantin Knizhnik Author: Michael Paquier Discussion: https://postgr.es/m/0c96b653-4696-d4b4-6b5d-78143175d113@postgrespro.ru Backpatch-through: 9.3
-
Tom Lane authored
Justin Pryzby and myself. Discussion: https://postgr.es/m/20181006134249.GD871@telsasoft.com
-
- 13 Oct, 2018 3 commits
-
-
Tom Lane authored
Justin Pryzby, Jonathan S. Katz, and myself. Discussion: https://postgr.es/m/20181006134249.GD871@telsasoft.com
-
Tom Lane authored
Justin Pryzby, Jonathan S. Katz, and myself. Discussion: https://postgr.es/m/20181006134249.GD871@telsasoft.com
-
Tom Lane authored
Set the release date. Do a bunch of copy-editing and markup improvement, rearrange some stuff into what seemed a more sensible order, move some things that did not seem to be in the right section.
-
- 12 Oct, 2018 6 commits
-
-
Tom Lane authored
Removing the separate Windows expected-files in commit f1885386 turns out to have been too optimistic: on most (but not all!) of our Windows buildfarm members, the tests still print floats with three exponent digits, because they're invoking the native printf() not snprintf.c. But rather than put back the extra expected-files, let's hack the three tests in question so that they adjust float formatting the same way snprintf.c does. Discussion: https://postgr.es/m/18890.1539374107@sss.pgh.pa.us
-
Tom Lane authored
We can allow this macro to accept either abbreviated or non-abbreviated allocation parameters by making use of __VA_ARGS__. As noted by Andres Freund, it's unlikely that any compiler would have __builtin_constant_p but not __VA_ARGS__, so this gives up little or no error checking, and it avoids a minor but annoying API break for extensions. With this change, there is no reason for anybody to call AllocSetContextCreateExtended directly, so in HEAD I renamed it to AllocSetContextCreateInternal. It's probably too late for an ABI break like that in 11, though. Discussion: https://postgr.es/m/20181012170355.bhxi273skjt6sag4@alap3.anarazel.de
-
Tom Lane authored
I missed this in my prior commit because it doesn't matter in non-VPATH builds. Per buildfarm.
-
Alvaro Herrera authored
There was no code to handle foreign key constraints on partitioned tables in the case of ALTER TABLE DETACH; and if you happened to ATTACH a partition that already had an equivalent constraint, that one was ignored and a new constraint was created. Adding this to the fact that foreign key cloning reuses the constraint name on the partition instead of generating a new name (as it probably should, to cater to SQL standard rules about constraint naming within schemas), the result was a pretty poor user experience -- the most visible failure was that just detaching a partition and re-attaching it failed with an error such as ERROR: duplicate key value violates unique constraint "pg_constraint_conrelid_contypid_conname_index" DETAIL: Key (conrelid, contypid, conname)=(26702, 0, test_result_asset_id_fkey) already exists. because it would try to create an identically-named constraint in the partition. To make matters worse, if you tried to drop the constraint in the now-independent partition, that would fail because the constraint was still seen as dependent on the constraint in its former parent partitioned table: ERROR: cannot drop inherited constraint "test_result_asset_id_fkey" of relation "test_result_cbsystem_0001_0050_monthly_2018_09" This fix attacks the problem from two angles: first, when the partition is detached, the constraint is also marked as independent, so the drop now works. Second, when the partition is re-attached, we scan existing constraints searching for one matching the FK in the parent, and if one exists, we link that one to the parent constraint. So we don't end up with a duplicate -- and better yet, we don't need to scan the referenced table to verify that the constraint holds. To implement this I made a small change to previously planner-only struct ForeignKeyCacheInfo to contain the constraint OID; also relcache now maintains the list of FKs for partitioned tables too. Backpatch to 11. Reported-by: Michael Vitale (bug #15425) Discussion: https://postgr.es/m/15425-2dbc9d2aa999f816@postgresql.org
-
Tom Lane authored
Windows, alone among our supported platforms, likes to emit three-digit exponent fields even when two digits would do. Adjust such results to look like the way everyone else does it. Eliminate a bunch of variant expected-output files that were needed only because of this quirk. Discussion: https://postgr.es/m/2934.1539122454@sss.pgh.pa.us
-
Michael Paquier authored
All options available in the utility get coverage: - Tests with disabled page checksums. - Tests with enabled test checksums. - Emulation of corruption and broken checksums with a full scan and single relfilenode scan. This patch has been contributed mainly by Michael Banck and Magnus Hagander with things presented on various threads, and I have gathered all the contents into a single patch. Author: Michael Banck, Magnus Hagander, Michael Paquier Reviewed-by: Peter Eisentraut Discussion: https://postgr.es/m/20181005012645.GE1629@paquier.xyz
-
- 11 Oct, 2018 3 commits
-
-
Andres Freund authored
These types have been deprecated for a *long* time. Catversion bump, for obvious reasons. Author: Andres Freund Discussion: https://postgr.es/m/20181009192237.34wjp3nmw7oynmmr@alap3.anarazel.de https://postgr.es/m/20171213080506.cwjkpcz3bkk6yz2u@alap3.anarazel.de https://postgr.es/m/25615.1513115237@sss.pgh.pa.us
-
Andres Freund authored
The extension depended on old types which are about to be removed. As the code additionally was pretty crufty and didn't provide much in the way of functionality, removing the extension seems to be the best way forward. It's fairly trivial to write functionality in plpgsql that more than covers what timetravel did. Author: Andres Freund Discussion: https://postgr.es/m/20171213080506.cwjkpcz3bkk6yz2u@alap3.anarazel.de https://postgr.es/m/25615.1513115237@sss.pgh.pa.us
-
Andres Freund authored
nabstime.c is about to be removed, but timeofday() isn't related to the rest of the functionality therein, and some find it useful. Move to timestamp.c. Discussion: https://postgr.es/m/20181009192237.34wjp3nmw7oynmmr@alap3.anarazel.de https://postgr.es/m/20180928223240.kgwc4czzzekrpsid%40alap3.anarazel.de
-
- 10 Oct, 2018 1 commit
-
-
Andres Freund authored
Repeatedly rewriting a mapped catalog table with VACUUM FULL or CLUSTER could cause logical decoding to fail with: ERROR, "could not map filenode \"%s\" to relation OID" To trigger the problem the rewritten catalog had to have live tuples with toasted columns. The problem was triggered as during catalog table rewrites the heap_insert() check that prevents logical decoding information to be emitted for system catalogs, failed to treat the new heap's toast table as a system catalog (because the new heap is not recognized as a catalog table via RelationIsLogicallyLogged()). The relmapper, in contrast to the normal catalog contents, does not contain historical information. After a single rewrite of a mapped table the new relation is known to the relmapper, but if the table is rewritten twice before logical decoding occurs, the relfilenode cannot be mapped to a relation anymore. Which then leads us to error out. This only happens for toast tables, because the main table contents aren't re-inserted with heap_insert(). The fix is simple, add a new heap_insert() flag that prevents logical decoding information from being emitted, and accept during decoding that there might not be tuple data for toast tables. Unfortunately that does not fix pre-existing logical decoding errors. Doing so would require not throwing an error when a filenode cannot be mapped to a relation during decoding, and that seems too likely to hide bugs. If it's crucial to fix decoding for an existing slot, temporarily changing the ERROR in ReorderBufferCommit() to a WARNING appears to be the best fix. Author: Andres Freund Discussion: https://postgr.es/m/20180914021046.oi7dm4ra3ot2g2kt@alap3.anarazel.de Backpatch: 9.4-, where logical decoding was introduced
-