aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* Fix unique INCLUDE indexes on partitioned tablesAlvaro Herrera2019-01-14
| | | | | | | | | | We were considering the INCLUDE columns as part of the key, allowing unicity-violating rows to be inserted in different partitions. Concurrent development conflict in eb7ed3f30634 and 8224de4f42cc. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20190109065109.GA4285@telsasoft.com
* Fix error message for logical replication targetsMichael Paquier2019-01-13
| | | | | | This fixes an oversight from 373bda6. Noted by Erik Rijkers.
* Change default of recovery_target_timeline to 'latest'Peter Eisentraut2019-01-13
| | | | | | | | | This is what one usually wants for recovery and almost always wants for a standby. Discussion: https://www.postgresql.org/message-id/flat/6dd2c23a-4162-8469-410f-bfe146e28c0c@2ndquadrant.com/ Reviewed-by: David Steele <david@pgmasters.net> Reviewed-by: Michael Paquier <michael@paquier.xyz>
* Improve error messages for incorrect types of logical replication targetsMichael Paquier2019-01-13
| | | | | | | | | | | If trying to use something else than a plain table as logical replication target, a rather-generic error message gets used to report the problem. This can be confusing when it comes to foreign tables and partitioned tables, so use more dedicated messages in these cases. Author: Amit Langote Reviewed-by: Peter Eisentraut, Magnus Hagander, Michael Paquier Discussion: https://postgr.es/m/41799bee-40eb-7bb5-80b1-325ce17518bc@lab.ntt.co.jp
* Remove heapam.h include made superfluous by b60c3975990.Andres Freund2019-01-12
| | | | | | Noticed this while working on another patch. Author: Andres Freund
* Free pre-modification HeapTuple in ALTER TABLE ... TYPE ...Andrew Dunstan2019-01-11
| | | | | | | | This was an oversight in commit 3b174b1a3. Per offline gripe from Alvaro Herrera Backpatch to release 11.
* Avoid sharing PARAM_EXEC slots between different levels of NestLoop.Tom Lane2019-01-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Up to now, createplan.c attempted to share PARAM_EXEC slots for NestLoopParams across different plan levels, if the same underlying Var was being fed down to different righthand-side subplan trees by different NestLoops. This was, I think, more of an artifact of using subselect.c's PlannerParamItem infrastructure than an explicit design goal, but anyway that was the end result. This works well enough as long as the plan tree is executing synchronously, but the feature whereby Gather can execute the parallelized subplan locally breaks it. An upper NestLoop node might execute for a row retrieved from a parallel worker, and assign a value for a PARAM_EXEC slot from that row, while the leader's copy of the parallelized subplan is suspended with a different active value of the row the Var comes from. When control eventually returns to the leader's subplan, it gets the wrong answers if the same PARAM_EXEC slot is being used within the subplan, as reported in bug #15577 from Bartosz Polnik. This is pretty reminiscent of the problem fixed in commit 46c508fbc, and the proper fix seems to be the same: don't try to share PARAM_EXEC slots across different levels of controlling NestLoop nodes. This requires decoupling NestLoopParam handling from PlannerParamItem handling, although the logic remains somewhat similar. To avoid bizarre division of labor between subselect.c and createplan.c, I decided to move all the param-slot-assignment logic for both cases out of those files and put it into a new file paramassign.c. Hopefully it's a bit better documented now, too. A regression test case for this might be nice, but we don't know a test case that triggers the problem with a suitably small amount of data. Back-patch to 9.6 where we added Gather nodes. It's conceivable that related problems exist in older branches; but without some evidence for that, I'll leave the older branches alone. Discussion: https://postgr.es/m/15577-ca61ab18904af852@postgresql.org
* Add value 'current' for recovery_target_timelinePeter Eisentraut2019-01-11
| | | | | | | | | | | | | This value represents the default behavior of using the current timeline. Previously, this was represented by an empty string. (Before the removal of recovery.conf, this setting could not be chosen explicitly but was used when recovery_target_timeline was not mentioned at all.) Discussion: https://www.postgresql.org/message-id/flat/6dd2c23a-4162-8469-410f-bfe146e28c0c@2ndquadrant.com/ Reviewed-by: David Steele <david@pgmasters.net> Reviewed-by: Michael Paquier <michael@paquier.xyz>
* Fix missing values when doing ALTER TABLE ALTER COLUMN TYPEAndrew Dunstan2019-01-10
| | | | | | | | | | | | | | This was an oversight in commit 16828d5c. If the table is going to be rewritten, we simply clear all the missing values from all the table's attributes, since there will no longer be any rows with the attributes missing. Otherwise, we repackage the missing value in an array constructed with the new type specifications. Backpatch to release 11. This fixes bug #15446, reported by Dmitry Molotkov Reviewed by Dean Rasheed
* Fix C++ compile failures in headers.Tom Lane2019-01-10
| | | | | | | | | | | | | Avoid using "typeid" as a parameter name in header files, since that is a C++ keyword. These cases were introduced recently, in 04fe805a1 and 586b98fdf. Since I'm an incurable neatnik, also rename these parameters in the underlying function definitions. That's not really necessary per project rules, but I don't like function declarations that don't quite agree with the underlying definitions. Per src/tools/pginclude/cpluspluscheck.
* Move inheritance expansion code into its own fileAlvaro Herrera2019-01-10
| | | | | | | | | | | | | | | | | This commit moves expand_inherited_tables and underlings from optimizer/prep/prepunionc.c to optimizer/utils/inherit.c. Also, all of the AppendRelInfo-based expression manipulation routines are moved to optimizer/utils/appendinfo.c. No functional code changes. One exception is the introduction of make_append_rel_info, but that's still just moving around code. Also, stop including <limits.h> in prepunion.c, which no longer needs it since 3fc6e2d7f5b6. I (Álvaro) noticed this because Amit was copying that to inherit.c, which likewise doesn't need it. Author: Amit Langote Discussion: https://postgr.es/m/3be67028-a00a-502c-199a-da00eec8fb6e@lab.ntt.co.jp
* Fix grammar mistakes in md.cMichael Paquier2019-01-10
| | | | | Author: Kirk Jamison Discussion: https://postgr.es/m/D09B13F772D2274BB348A310EE3027C640AC54@g01jpexmbkw24
* Reduce the size of the fmgr_builtin_oid_index[] array.Tom Lane2019-01-09
| | | | | | | | | | | | | | | | | This index array was originally defined to have 10000 entries (ranging up to FirstGenbkiObjectId), but we really only need entries up to the last existing builtin function OID, currently 6121. That saves close to 8K of never-accessed space in the server executable, at the small price of one more fetch in fmgr_isbuiltin(). We could reduce the array size still further by renumbering a few of the highest-numbered builtin functions; but there's a small risk of breaking clients that have chosen to hardwire those function OIDs, so it's not clear if it'd be worth the trouble. (We should, however, discourage future patches from choosing function OIDs above 6K as long as there's still lots of space below that.) Discussion: https://postgr.es/m/12359.1547063064@sss.pgh.pa.us
* Replace the data structure used for keyword lookup.Tom Lane2019-01-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, ScanKeywordLookup was passed an array of string pointers. This had some performance deficiencies: the strings themselves might be scattered all over the place depending on the compiler (and some quick checking shows that at least with gcc-on-Linux, they indeed weren't reliably close together). That led to very cache-unfriendly behavior as the binary search touched strings in many different pages. Also, depending on the platform, the string pointers might need to be adjusted at program start, so that they couldn't be simple constant data. And the ScanKeyword struct had been designed with an eye to 32-bit machines originally; on 64-bit it requires 16 bytes per keyword, making it even more cache-unfriendly. Redesign so that the keyword strings themselves are allocated consecutively (as part of one big char-string constant), thereby eliminating the touch-lots-of-unrelated-pages syndrome. And get rid of the ScanKeyword array in favor of three separate arrays: uint16 offsets into the keyword array, uint16 token codes, and uint8 keyword categories. That reduces the overhead per keyword to 5 bytes instead of 16 (even less in programs that only need one of the token codes and categories); moreover, the binary search only touches the offsets array, further reducing its cache footprint. This also lets us put the token codes somewhere else than the keyword strings are, which avoids some unpleasant build dependencies. While we're at it, wrap the data used by ScanKeywordLookup into a struct that can be treated as an opaque type by most callers. That doesn't change things much right now, but it will make it less painful to switch to a hash-based lookup method, as is being discussed in the mailing list thread. Most of the change here is associated with adding a generator script that can build the new data structure from the same list-of-PG_KEYWORD header representation we used before. The PG_KEYWORD lists that plpgsql and ecpg used to embed in their scanner .c files have to be moved into headers, and the Makefiles have to be taught to invoke the generator script. This work is also necessary if we're to consider hash-based lookup, since the generator script is what would be responsible for constructing a hash table. Aside from saving a few kilobytes in each program that includes the keyword table, this seems to speed up raw parsing (flex+bison) by a few percent. So it's worth doing even as it stands, though we think we can gain even more with a follow-on patch to switch to hash-based lookup. John Naylor, with further hacking by me Discussion: https://postgr.es/m/CAJVSVGXdFVU2sgym89XPL=Lv1zOS5=EHHQ8XWNzFL=mTXkKMLw@mail.gmail.com
* Don't create relfilenode for relations without storageAlvaro Herrera2019-01-04
| | | | | | | | | | | | Some relation kinds had relfilenode set to some non-zero value, but apparently the actual files did not really exist because creation was prevented elsewhere. Get rid of the phony pg_class.relfilenode values. Catversion bumped, but only because the sanity_test check will fail if run in a system initdb'd with the previous version. Reviewed-by: Kyotaro HORIGUCHI, Michael Paquier Discussion: https://postgr.es/m/20181206215552.fm2ypuxq6nhpwjuc@alvherre.pgsql
* Rename macro to RELKIND_HAS_STORAGEAlvaro Herrera2019-01-04
| | | | | | The original name was an unfortunate choice. Discussion: https://postgr.es/m/20181218.145600.172055615.horiguchi.kyotaro@lab.ntt.co.jp
* Move the built-in conversions into the initial catalog data.Tom Lane2019-01-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Instead of running a SQL script to create the standard conversion functions and pg_conversion entries, put those entries into the initial data in postgres.bki. This shaves a few percent off the runtime of initdb, and also allows accurate comments to be attached to the conversion functions; the previous script labeled them with machine-generated comments that were not quite right for multi-purpose conversion functions. Also, we can get rid of the duplicative Makefile and MSVC perl implementations of the generation code for that SQL script. A functional change is that these pg_proc and pg_conversion entries are now "pinned" by initdb. Leaving them unpinned was perhaps a good thing back while the conversions feature was under development, but there seems no valid reason for it now. Also, the conversion functions are now marked as immutable, where before they were volatile by virtue of lacking any explicit specification. That seems like it was just an oversight. To avoid using magic constants in pg_conversion.dat, extend genbki.pl to allow encoding names to be converted, much as it does for language, access method, etc names. John Naylor Discussion: https://postgr.es/m/CAJVSVGWtUqxpfAaxS88vEGvi+jKzWZb2EStu5io-UPc4p9rSJg@mail.gmail.com
* Use symbolic references for pg_language OIDs in the bootstrap data.Tom Lane2019-01-03
| | | | | | | | | | | | | | | | | | | This patch teaches genbki.pl to replace pg_language names by OIDs in much the same way as it already does for pg_am names etc, and converts pg_proc.dat to use such symbolic references in the prolang column. Aside from getting rid of a few more magic numbers in the initial catalog data, this means that Gen_fmgrtab.pl no longer needs to read pg_language.dat, since it doesn't have to know the OID of the "internal" language; now it's just looking for the string "internal". No need for a catversion bump, since the contents of postgres.bki don't actually change at all. John Naylor Discussion: https://postgr.es/m/CAJVSVGWtUqxpfAaxS88vEGvi+jKzWZb2EStu5io-UPc4p9rSJg@mail.gmail.com
* Improve ANALYZE's handling of concurrent-update scenarios.Tom Lane2019-01-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch changes the rule for whether or not a tuple seen by ANALYZE should be included in its sample. When we last touched this logic, in commit 51e1445f1, we weren't thinking very hard about tuples being UPDATEd by a long-running concurrent transaction. In such a case, we might see the pre-image as either LIVE or DELETE_IN_PROGRESS depending on timing; and we might see the post-image not at all, or as INSERT_IN_PROGRESS. Since the existing code will not sample either DELETE_IN_PROGRESS or INSERT_IN_PROGRESS tuples, this leads to concurrently-updated rows being omitted from the sample entirely. That's not very helpful, and it's especially the wrong thing if the concurrent transaction ends up rolling back. The right thing seems to be to sample DELETE_IN_PROGRESS rows just as if they were live. This makes the "sample it" and "count it" decisions the same, which seems good for consistency. It's clearly the right thing if the concurrent transaction ends up rolling back; in effect, we are sampling as though IN_PROGRESS transactions haven't happened yet. Also, this combination of choices ensures maximum robustness against the different combinations of whether and in which state we might see the pre- and post-images of an update. It's slightly annoying that we end up recording immediately-out-of-date stats in the case where the transaction does commit, but on the other hand the stats are fine for columns that didn't change in the update. And the alternative of sampling INSERT_IN_PROGRESS rows instead seems like a bad idea, because then the sampling would be inconsistent with the way rows are counted for the stats report. Per report from Mark Chambers; thanks to Jeff Janes for diagnosing what was happening. Back-patch to all supported versions. Discussion: https://postgr.es/m/CAFh58O_Myr6G3tcH3gcGrF-=OExB08PJdWZcSBcEcovaiPsrHA@mail.gmail.com
* Don't believe MinMaxExpr is leakproof without checking.Tom Lane2019-01-02
| | | | | | | | | | | | | | | | MinMaxExpr invokes the btree comparison function for its input datatype, so it's only leakproof if that function is. Many such functions are indeed leakproof, but others are not, and we should not just assume that they are. Hence, adjust contain_leaked_vars to verify the leakproofness of the referenced function explicitly. I didn't add a regression test because it would need to depend on some particular comparison function being leaky, and that's a moving target, per discussion. This has been wrong all along, so back-patch to supported branches. Discussion: https://postgr.es/m/31042.1546194242@sss.pgh.pa.us
* Ensure link commands list *.o files before LDFLAGS.Tom Lane2019-01-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's important for link commands to list *.o input files before -l switches for libraries, as library code may not get pulled into the link unless referenced by an earlier command-line entry. This is certainly necessary for static libraries (.a style). Apparently on some platforms it is also necessary for shared libraries, as reported by Donald Dong. We often put -l switches for within-tree libraries into LDFLAGS, meaning that link commands that list *.o files after LDFLAGS are hazardous. Most of our link commands got this right, but a few did not. In particular, places that relied on gmake's default implicit link rule failed, because that puts LDFLAGS first. Fix that by overriding the built-in rule with our own. The implicit link rules in src/makefiles/Makefile.* for single-.o-file shared libraries mostly got this wrong too, so fix them. I also changed the link rules for the backend and a couple of other places for consistency, even though they are not (currently) at risk because they aren't adding any -l switches to LDFLAGS. Arguably, the real problem here is that we're abusing LDFLAGS by putting -l switches in it and we should stop doing that. But changing that would be quite invasive, so I'm not eager to do so. Perhaps this is a candidate for back-patching, but so far it seems that problems can only be exhibited in test code we don't normally build, and at least some of the problems are new in HEAD anyway. So I'll refrain for now. Donald Dong and Tom Lane Discussion: https://postgr.es/m/CAKABAquXn-BF-vBeRZxhzvPyfMqgGuc74p8BmQZyCFDpyROBJQ@mail.gmail.com
* Update copyright for 2019Bruce Momjian2019-01-02
| | | | Backpatch-through: certain files through 9.4
* Remove configure switch --disable-strong-randomMichael Paquier2019-01-01
| | | | | | | | | | | | | | | | This removes a portion of infrastructure introduced by fe0a0b5 to allow compilation of Postgres in environments where no strong random source is available, meaning that there is no linking to OpenSSL and no /dev/urandom (Windows having its own CryptoAPI). No systems shipped this century lack /dev/urandom, and the buildfarm is actually not testing this switch at all, so just remove it. This simplifies particularly some backend code which included a fallback implementation using shared memory, and removes a set of alternate regression output files from pgcrypto. Author: Michael Paquier Reviewed-by: Tom Lane Discussion: https://postgr.es/m/20181230063219.GG608@paquier.xyz
* Improve comments and logs in do_pg_stop/start_backupMichael Paquier2019-01-01
| | | | | | | | | | | | | | | The function name pg_stop_backup() has been included for ages in some log messages when stopping the backup, which is confusing for base backups taken with the replication protocol because this function is never called. Some other comments and messages in this area are improved while on it. The new wording is based on input and suggestions from several people, all listed below. Author: Michael Paquier Reviewed-by: Peter Eisentraut, Álvaro Herrera, Tom Lane Discussion: https://postgr.es/m/20181221040510.GA12599@paquier.xyz
* Remove some useless codeAlvaro Herrera2018-12-31
| | | | | | | | | | | | | In commit 8b08f7d4820f I added member relationId to IndexStmt struct. I'm now not sure why; DefineIndex doesn't need it, since the relation OID is passed as a separate argument anyway. Remove it. Also remove a redundant assignment to the relationId argument (it wasn't redundant when added by commit e093dcdd285, but should have been removed in commit 5f173040e3), and use relationId instead of stmt->relation when locking the relation in the second phase of CREATE INDEX CONCURRENTLY, which is not only confusing but it means we resolve the name twice for no reason.
* Fix oversight in commit b5415e3c2187ab304390524f5ae66b4bd2c58279.Tom Lane2018-12-31
| | | | | | | | | | | | | While rearranging code in tidpath.c, I overlooked the fact that we ought to check restriction_is_securely_promotable when trying to use a join clause as a TID qual. Since tideq itself is leakproof, this wouldn't really allow any interesting leak AFAICT, but it still seems like we had better check it. For consistency with the corresponding logic in indxpath.c, also check rinfo->pseudoconstant. I'm not sure right now that it's possible for that to be set in a join clause, but if it were, a match couldn't be made anyway.
* Change "checkpoint starting" message to use "wal"Peter Eisentraut2018-12-30
| | | | | | | This catches up with the recent renaming of all user-facing mentions of "xlog" to "wal". Discussion: https://www.postgresql.org/message-id/flat/20181129084708.GA9562%40msg.credativ.de
* Add a hash opclass for type "tid".Tom Lane2018-12-30
| | | | | | | | | | | | | | | | | Up to now we've not worried much about joins where the join key is a relation's CTID column, reasoning that storing a table's CTIDs in some other table would be pretty useless. However, there are use-cases for this sort of query involving self-joins, so that argument doesn't really hold water. With larger relations, a merge or hash join is desirable. We had a btree opclass for type "tid", allowing merge joins on CTID, but no hash opclass so that hash joins weren't possible. Add the missing infrastructure. This also potentially enables hash aggregation on "tid", though the use-cases for that aren't too clear. Discussion: https://postgr.es/m/1853.1545453106@sss.pgh.pa.us
* Support parameterized TidPaths.Tom Lane2018-12-30
| | | | | | | | | | | | | | | | | | | Up to now we've not worried much about joins where the join key is a relation's CTID column, reasoning that storing a table's CTIDs in some other table would be pretty useless. However, there are use-cases for this sort of query involving self-joins, so that argument doesn't really hold water. This patch allows generating plans for joins on CTID that use a nestloop with inner TidScan, similar to what we might do with an index on the join column. This is the most efficient way to join when the outer side of the nestloop is expected to yield relatively few rows. This change requires upgrading tidpath.c and the generated TidPaths to work with RestrictInfos instead of bare qual clauses, but that's long-postponed technical debt anyway. Discussion: https://postgr.es/m/17443.1545435266@sss.pgh.pa.us
* Teach eval_const_expressions to constant-fold LEAST/GREATEST expressions.Tom Lane2018-12-30
| | | | | | | | | | | | | Doing this requires an assumption that the invoked btree comparison function is immutable. We could check that explicitly, but in other places such as contain_mutable_functions we just assume that it's true, so we may as well do likewise here. (If the comparison function's behavior isn't immutable, the sort order in indexes built with it would be unstable, so it seems certainly wrong for it not to be so.) Vik Fearing Discussion: https://postgr.es/m/c6e8504c-4c43-35fa-6c8f-3c0b80a912cc@2ndquadrant.com
* Use pg_strong_random() to select each server process's random seed.Tom Lane2018-12-29
| | | | | | | | | | | | | | | | | | | | | | | Previously we just set the seed based on process ID and start timestamp. Both those values are directly available within the session, and can be found out or guessed by other users too, making the session's series of random(3) values fairly predictable. Up to now, our backend-internal uses of random(3) haven't seemed security-critical, but commit 88bdbd3f7 added one that potentially is: when using log_statement_sample_rate, a user might be able to predict which of his SQL statements will get logged. To improve this situation, upgrade the per-process seed initialization method to use pg_strong_random() if available, greatly reducing the predictability of the initial seed value. This adds a few tens of microseconds to process start time, but since backend startup time is at least a couple of milliseconds, that seems an acceptable price. This means that pg_strong_random() needs to be able to run without reliance on any backend infrastructure, since it will be invoked before any of that is up. It was safe for that already, but adjust comments and #include commands to make it clearer. Discussion: https://postgr.es/m/3859.1545849900@sss.pgh.pa.us
* Use a separate random seed for SQL random()/setseed() functions.Tom Lane2018-12-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, the SQL random() function depended on libc's random(3), and setseed() invoked srandom(3). This results in interference between these functions and backend-internal uses of random(3). We'd never paid too much mind to that, but in the wake of commit 88bdbd3f7 which added log_statement_sample_rate, the interference arguably has a security consequence: if log_statement_sample_rate is active then an unprivileged user could probably control which if any of his SQL commands get logged, by issuing setseed() at the right times. That seems bad. To fix this reliably, we need random() and setseed() to use their own private random state variable. Standard random(3) isn't amenable to such usage, so let's switch to pg_erand48(). It's hard to say whether that's more or less "random" than any particular platform's version of random(3), but it does have a wider seed value and a longer period than are required by POSIX, so we can hope that this isn't a big downgrade. Also, we should now have uniform behavior of random() across platforms, which is worth something. While at it, upgrade the per-process seed initialization method to use pg_strong_random() if available, greatly reducing the predictability of the initial seed value. (I'll separately do something similar for the internal uses of random().) In addition to forestalling the possible security problem, this has a benefit in the other direction, which is that we can now document setseed() as guaranteeing a reproducible sequence of random() values. Previously, because of the possibility of internal calls of random(3), we could not promise any such thing. Discussion: https://postgr.es/m/3859.1545849900@sss.pgh.pa.us
* Fix thinko in previous commitAlvaro Herrera2018-12-28
|
* Rewrite ExecPartitionCheckEmitError for clarityAlvaro Herrera2018-12-28
| | | | | | The original was hard to follow and failed to comply with DRY principle. Discussion: https://postgr.es/m/20181206222221.g5witbsklvqthjll@alvherre.pgsql
* Remove obsolete IndexIs* macrosPeter Eisentraut2018-12-27
| | | | | | | | | Remove IndexIsValid(), IndexIsReady(), IndexIsLive() in favor of accessing the index structure directly. These macros haven't been used consistently, and the original reason of maintaining source compatibility with PostgreSQL 9.2 is gone. Discussion: https://www.postgresql.org/message-id/flat/d419147c-09d4-6196-5d9d-0234b230880a%402ndquadrant.com
* Remove entry tree root conflict checking from GIN predicate lockingAlexander Korotkov2018-12-27
| | | | | | | | | | | | | | According to README we acquire predicate locks on entry tree leafs and posting tree roots. However, when ginFindLeafPage() is going to lock leaf in exclusive mode, then it checks root for conflicts regardless whether it's a entry or posting tree. Assuming that we never place predicate lock on entry tree root (excluding corner case when root is leaf), this check is redundant. This commit removes this check. Now, root conflict checking is controlled by separate argument of ginFindLeafPage(). Discussion: https://postgr.es/m/CAPpHfdv7rrDyy%3DMgsaK-L9kk0AH7az0B-mdC3w3p0FSb9uoyEg%40mail.gmail.com Author: Alexander Korotkov Backpatch-through: 11
* Ignore inherited temp relations from other sessions when truncatingMichael Paquier2018-12-27
| | | | | | | | | | | | | | | | | | Inheritance trees can include temporary tables if the parent is permanent, which makes possible the presence of multiple temporary children from different sessions. Trying to issue a TRUNCATE on the parent in this scenario causes a failure, so similarly to any other queries just ignore such cases, which makes TRUNCATE work transparently. This makes truncation behave similarly to any other DML query working on the parent table with queries which need to be work on the children. A set of isolation tests is added to cover basic cases. Reported-by: Zhou Digoal Author: Amit Langote, Michael Paquier Discussion: https://postgr.es/m/15565-ce67a48d0244436a@postgresql.org Backpatch-through: 9.4
* Fix failure to check for open() or fsync() failures.Tom Lane2018-12-26
| | | | | | | | | | | While it seems OK to not be concerned about fsync() failure for a pre-existing signal file, it's not OK to not even check for open() failure. This at least causes complaints from static analyzers, and I think on some platforms passing -1 to fsync() or close() might trigger assertion-type failures. Also add (void) casts to make clear that we're ignoring fsync's result intentionally. Oversights in commit 2dedf4d9a, noted by Coverity.
* Prioritize history files when archivingMichael Paquier2018-12-24
| | | | | | | | | | | | | | | | | | | | | | | | At the end of recovery for the post-promotion process, a new history file is created followed by the last partial segment of the previous timeline. Based on the timing, the archiver would first try to archive the last partial segment and then the history file. This can delay the detection of a new timeline taken, particularly depending on the time it takes to transfer the last partial segment as it delays the moment the history file of the new timeline gets archived. This can cause promoted standbys to use the same timeline as one already taken depending on the circumstances if multiple instances look at archives at the same location. This commit changes the order of archiving so as history files are archived in priority over other file types, which reduces the likelihood of the same timeline being taken (still not reducing the window to zero), and it makes the archiver behave more consistently with the startup process doing its post-promotion business. Author: David Steele Reviewed-by: Michael Paquier, Kyotaro Horiguchi Discussion: https://postgr.es/m/929068cf-69e1-bba2-9dc0-e05986aed471@pgmasters.net Backpatch-through: 9.5
* Disable WAL-skipping optimization for COPY on views and foreign tablesMichael Paquier2018-12-23
| | | | | | | | | | | | | | | | | | | | COPY can skip writing WAL when loading data on a table which has been created in the same transaction as the one loading the data, however this cannot work on views or foreign table as this would result in trying to flush relation files which do not exist. So disable the optimization so as commands are able to work the same way with any configuration of wal_level. Tests are added to cover the different cases, which need to have wal_level set to minimal to allow the problem to show up, and that is not the default configuration. Reported-by: Luis M. Carril, Etsuro Fujita Author: Amit Langote, Michael Paquier Reviewed-by: Etsuro Fujita Discussion: https://postgr.es/m/15552-c64aa14c5c22f63c@postgresql.org Backpatch-through: 10, where support for COPY on views has been added, while v11 has added support for COPY on foreign tables.
* Add WRITE_*_ARRAY macrosPeter Eisentraut2018-12-22
| | | | | | | | | Add WRITE_ATTRNUMBER_ARRAY, WRITE_OID_ARRAY, WRITE_INT_ARRAY, WRITE_BOOL_ARRAY macros to outfuncs.c, mirroring the existing READ_*_ARRAY macros in readfuncs.c. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/8f2ebc67-e75f-9478-f5a5-bbbf090b1f8d%402ndquadrant.com
* Add some const decorationsPeter Eisentraut2018-12-22
| | | | These mainly help understanding the function signatures better.
* Check for conflicting queries during replay of gistvacuumpage()Alexander Korotkov2018-12-21
| | | | | | | | | | | | | | | | | | | | | | | 013ebc0a7b implements so-called GiST microvacuum. That is gistgettuple() marks index tuples as dead when kill_prior_tuple is set. Later, when new tuple insertion claims page space, those dead index tuples are physically deleted from page. When this deletion is replayed on standby, it might conflict with read-only queries. But 013ebc0a7b doesn't handle this. That may lead to disappearance of some tuples from read-only snapshots on standby. This commit implements resolving of conflicts between replay of GiST microvacuum and standby queries. On the master we implement new WAL record type XLOG_GIST_DELETE, which comprises necessary information. On stable releases we've to be tricky to keep WAL compatibility. Information required for conflict processing is just appended to data of XLOG_GIST_PAGE_UPDATE record. So, PostgreSQL version, which doesn't know about conflict processing, will just ignore that. Reported-by: Andres Freund Diagnosed-by: Andres Freund Discussion: https://postgr.es/m/20181212224524.scafnlyjindmrbe6%40alap3.anarazel.de Author: Alexander Korotkov Backpatch-through: 9.6
* Base information_schema.sql_identifier domain on name, not varchar.Tom Lane2018-12-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The SQL spec says that sql_identifier is a domain over varchar, but it also says that that domain is supposed to represent the set of valid identifiers for the implementation, in particular applying a length limit matching the implementation's identifier length limit. We were declaring sql_identifier as just "character varying", thus duplicating what the spec says about base type, but entirely failing at the rest of it. Instead, let's declare sql_identifier as a domain over type "name". (We can drop the COLLATE "C" added by commit 6b0faf723, since that's now implicit in "name".) With the recent improvements to name's comparison support, there's not a lot of functional difference between name and varchar. So although in principle this is a spec deviation, it's a pretty minor one. And correctly enforcing PG's name length limit is a good thing; on balance this seems closer to the intent of the spec than what we had. But that's all just language-lawyering. The *real* reason to do this is that it makes sql_identifier columns exposed by information_schema views be just direct representations of the underlying "name" catalog columns, eliminating a semantic mismatch that was disastrous for performance of typical queries on the information_schema. In combination with the recent change to allow dropping no-op CoerceToDomain nodes, this allows (for example) queries such as select ... from information_schema.tables where table_name = 'foo'; to produce an indexscan rather than a seqscan on pg_class. Discussion: https://postgr.es/m/CAFj8pRBUCX4LZ2rA2BbEkdD6NN59mgx+BLo1gO08Wod4RLtcTg@mail.gmail.com
* Avoid producing over-length specific_name outputs in information_schema.Tom Lane2018-12-20
| | | | | | | | | | | | | | | | | | | | | | | | information_schema output columns that are declared as being type sql_identifier are supposed to conform to the implementation's rules for valid identifiers, in particular the identifier length limit. Several places potentially violated this limit by concatenating a function's name and OID. (The OID is added to ensure name uniqueness within a schema, since the spec doesn't expect function name overloading.) Simply truncating the concatenation result to fit in "name" won't do, since losing part of the OID might wind up giving non-unique results. Instead, let's truncate the function name as necessary. The most practical way to do that is to do it in a C function; the information_schema.sql script doesn't have easy access to the value of NAMEDATALEN, nor does it have an easy way to truncate on the basis of resulting byte-length rather than number of characters. (There are still a couple of places that cast concatenation results to sql_identifier, but as far as I can see they are guaranteed not to produce over-length strings, at least with the normal value of NAMEDATALEN.) Discussion: https://postgr.es/m/23817.1545283477@sss.pgh.pa.us
* Fix lock level used for partition when detaching itAlvaro Herrera2018-12-20
| | | | | | | | | | | | | | For probably bogus reasons, we acquire only AccessShareLock on the partition when we try to detach it from its parent partitioned table. This can cause ugly things to happen if another transaction is doing any sort of DDL to the partition concurrently. Upgrade that lock to ShareUpdateExclusiveLock, which per discussion seems to be the minimum needed. Reported by Robert Haas. Discussion: https://postgr.es/m/CA+TgmoYruJQ+2qnFLtF1xQtr71pdwgfxy3Ziy-TxV28M6pEmyA@mail.gmail.com
* DETACH PARTITION: hold locks on indexes until end of transactionAlvaro Herrera2018-12-20
| | | | | | | | | | | | | | | | | | | | When a partition is detached from its parent, we acquire locks on all attached indexes to also detach them ... but we release those locks immediately. This is a violation of the policy of keeping locks on user objects to the end of the transaction. Bug introduced in 8b08f7d4820f. It's unclear that there are any ill effects possible, but it's clearly wrong nonetheless. It's likely that bad behavior *is* possible, but mostly because the relation that the index is for is only locked with AccessShareLock, which is an older bug that shall be fixed separately. While touching that line of code, close the index opened with index_open() using index_close() instead of relation_close(). No difference in practice, but let's be consistent. Unearthed by Robert Haas. Discussion: https://postgr.es/m/CA+TgmoYruJQ+2qnFLtF1xQtr71pdwgfxy3Ziy-TxV28M6pEmyA@mail.gmail.com
* Fix ADD IF NOT EXISTS used in conjunction with ALTER TABLE ONLYGreg Stark2018-12-19
| | | | | | The flag for IF NOT EXISTS was only being passed down in the normal recursing case. It's been this way since originally added in 9.6 in commit 2cd40adb85 so backpatch back to 9.6.
* Add text-vs-name cross-type operators, and unify name_ops with text_ops.Tom Lane2018-12-19
| | | | | | | | | | | | | | | | | | | Now that name comparison has effectively the same behavior as text comparison, we might as well merge the name_ops opfamily into text_ops, allowing cross-type comparisons to be processed without forcing a datatype coercion first. We need do little more than add cross-type operators to make the opfamily complete, and fix one or two places in the planner that assumed text_ops was a single-datatype opfamily. I chose to unify hash name_ops into hash text_ops as well, since the types have compatible hashing semantics. This allows marking the new cross-type equality operators as oprcanhash. (Note: this doesn't remove the name_ops opclasses, so there's no breakage of index definitions. Those opclasses are just reparented into the text_ops opfamily.) Discussion: https://postgr.es/m/15938.1544377821@sss.pgh.pa.us
* Make type "name" collation-aware.Tom Lane2018-12-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The "name" comparison operators now all support collations, making them functionally equivalent to "text" comparisons, except for the different physical representation of the datatype. They do, in fact, mostly share the varstr_cmp and varstr_sortsupport infrastructure, which has been slightly enlarged to handle the case. To avoid changes in the default behavior of the datatype, set name's typcollation to C_COLLATION_OID not DEFAULT_COLLATION_OID, so that by default comparisons to a name value will continue to use strcmp semantics. (This would have been the case for system catalog columns anyway, because of commit 6b0faf723, but doing this makes it true for user-created name columns as well. In particular, this avoids locale-dependent changes in our regression test results.) In consequence, tweak a couple of places that made assumptions about collatable base types always having typcollation DEFAULT_COLLATION_OID. I have not, however, attempted to relax the restriction that user- defined collatable types must have that. Hence, "name" doesn't behave quite like a user-defined type; it acts more like a domain with COLLATE "C". (Conceivably, if we ever get rid of the need for catalog name columns to be fixed-length, "name" could actually become such a domain over text. But that'd be a pretty massive undertaking, and I'm not volunteering.) Discussion: https://postgr.es/m/15938.1544377821@sss.pgh.pa.us
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-03 16:27:49 +0000