aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* 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
* Remove function names from error messagesAlvaro Herrera2018-12-19
| | | | | They are not necessary, and having them there gives useless work for translators.
* Small improvements for allocation logic in ginHeapTupleFastCollect().Tom Lane2018-12-19
| | | | | | | | | | | | Avoid repetitive calls to repalloc() when the required size of the collector array grows more than 2x in one call. Also ensure that the array size is a power of 2 (since palloc will probably consume a power of 2 anyway) and doesn't start out very small (which'd likely just lead to extra repallocs). David Rowley, tweaked a bit by me Discussion: https://postgr.es/m/CAKJS1f8vn-iSBE8PKeVHrnhvyjRNYCxguPFFY08QLYmjWG9hPQ@mail.gmail.com
* Remove obsolete nbtree duplicate entries comment.Peter Geoghegan2018-12-18
| | | | | | | | Remove a comment from the Berkeley days claiming that nbtree must disambiguate duplicate keys within _bt_moveright(). There is no special care taken around duplicates within _bt_moveright(), at least since commit 9e85183bfc3 removed inscrutable _bt_moveright() code to handle pages full of duplicates.
* Correct obsolete nbtree recovery comments.Peter Geoghegan2018-12-18
| | | | | | | | | | | | | | | Commit 40dae7ec537, which made the handling of interrupted nbtree page splits more robust, removed an nbtree-specific end-of-recovery cleanup step. This meant that it was no longer possible to complete an interrupted page split during recovery. However, a reference to recovery as a reason for using a NULL stack while inserting into a parent page was missed. Remove the reference. Remove a similar obsolete reference to recovery that was introduced much more recently, as part of the btree fastpath optimization enhancement that made it into Postgres 11 (commit 2b272734, and follow-up commits). Backpatch: 11-, where the fastpath optimization was introduced.
* Make collation-aware system catalog columns use "C" collation.Tom Lane2018-12-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Up to now we allowed text columns in system catalogs to use collation "default", but that isn't really safe because it might mean something different in template0 than it means in a database cloned from template0. In particular, this could mean that cloned pg_statistic entries for such columns weren't entirely valid, possibly leading to bogus planner estimates, though (probably) not any outright failures. In the wake of commit 5e0928005, a better solution is available: if we label such columns with "C" collation, then their pg_statistic entries will also use that collation and hence will be valid independently of the database collation. This also provides a cleaner solution for indexes on such columns than the hack added by commit 0b28ea79c: the indexes will naturally inherit "C" collation and don't have to be forced to use text_pattern_ops. Also, with the planned improvement of type "name" to be collation-aware, this policy will apply cleanly to both text and name columns. Because of the pg_statistic angle, we should also apply this policy to the tables in information_schema. This patch does that by adjusting information_schema's textual domain types to specify "C" collation. That has the user-visible effect that order-sensitive comparisons to textual information_schema view columns will now use "C" collation by default. The SQL standard says that the collation of those view columns is implementation-defined, so I think this is legal per spec. At some point this might allow for translation of such comparisons into indexable conditions on the underlying "name" columns, although additional work will be needed before that can happen. Discussion: https://postgr.es/m/19346.1544895309@sss.pgh.pa.us
* Fix ancient thinko in mergejoin cost estimation.Tom Lane2018-12-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | "rescanratio" was computed as 1 + rescanned-tuples / total-inner-tuples, which is sensible if it's to be multiplied by total-inner-tuples or a cost value corresponding to scanning all the inner tuples. But in reality it was (mostly) multiplied by inner_rows or a related cost, numbers that take into account the possibility of stopping short of scanning the whole inner relation thanks to a limited key range in the outer relation. This'd still make sense if we could expect that stopping short would result in a proportional decrease in the number of tuples that have to be rescanned. It does not, however. The argument that establishes the validity of our estimate for that number is independent of whether we scan all of the inner relation or stop short, and experimentation also shows that stopping short doesn't reduce the number of rescanned tuples. So the correct calculation is 1 + rescanned-tuples / inner_rows, and we should be sure to multiply that by inner_rows or a corresponding cost value. Most of the time this doesn't make much difference, but if we have both a high rescan rate (due to lots of duplicate values) and an outer key range much smaller than the inner key range, then the error can be significant, leading to a large underestimate of the cost associated with rescanning. Per report from Vijaykumar Jain. This thinko appears to go all the way back to the introduction of the rescan estimation logic in commit 70fba7043, so back-patch to all supported branches. Discussion: https://postgr.es/m/CAE7uO5hMb_TZYJcZmLAgO6iD68AkEK6qCe7i=vZUkCpoKns+EQ@mail.gmail.com
* Include partitioned indexes to system view pg_indexesMichael Paquier2018-12-18
| | | | | | | | | pg_tables already includes partitioned tables, so for consistency pg_indexes should show partitioned indexes. Author: Suraj Kharage Reviewed-by: Amit Langote, Michael Paquier Discussion: https://postgr.es/m/CAF1DzPVrYo4XNTEnc=PqVp6aLJc7LFYpYR4rX=_5pV=wJ2KdZg@mail.gmail.com
* Fix tablespace handling for partitioned tablesAlvaro Herrera2018-12-17
| | | | | | | | | | | | | | | | | | When partitioned tables were introduced, we failed to realize that by copying the tablespace handling for other relation kinds with no physical storage we were causing the secondary effect that their partitions would not automatically inherit the tablespace setting. This is surprising and unhelpful, so change it to adopt the behavior introduced in pg11 (commit 33e6c34c3267) for partitioned indexes: the parent relation remembers the tablespace specification, which is then used for any new partitions that don't declare one. Because this commit changes behavior of the TABLESPACE clause for partitioned tables (it's no longer a no-op), it is not backpatched. Author: David Rowley, Álvaro Herrera Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CAKJS1f9SxVzqDrGD1teosFd6jBMM0UEaa14_8mRvcWE19Tu0hA@mail.gmail.com
* Remove extra semicolons.Amit Kapila2018-12-17
| | | | | | | | Reported-by: David Rowley Author: David Rowley Reviewed-by: Amit Kapila Backpatch-through: 10 Discussion: https://postgr.es/m/CAKJS1f8EneeYyzzvdjahVZ6gbAHFkHbSFB5m_C0Y6TUJs9Dgdg@mail.gmail.com
* Fix use-after-free bug when renaming constraintsMichael Paquier2018-12-17
| | | | | | | | | This is an oversight from recent commit b13fd344. While on it, tweak the previous test with a better name for the renamed primary key. Detected by buildfarm member prion which forces relation cache release with -DRELCACHE_FORCE_RELEASE. Back-patch down to 9.4 as the previous commit.
* Make constraint rename issue relcache invalidation on target relationMichael Paquier2018-12-17
| | | | | | | | | | | | | | | When a constraint gets renamed, it may have associated with it a target relation (for example domain constraints don't have one). Not invalidating the target relation cache when issuing the renaming can result in issues with subsequent commands that refer to the old constraint name using the relation cache, causing various failures. One pattern spotted was using CREATE TABLE LIKE after a constraint renaming. Reported-by: Stuart <sfbarbee@gmail.com> Author: Amit Langote Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/2047094.V130LYfLq4@station53.ousa.org
* Modernize our code for looking up descriptive strings for Unix signals.Tom Lane2018-12-16
| | | | | | | | | | | | | | | | | | | | | | At least as far back as the 2008 spec, POSIX has defined strsignal(3) for looking up descriptive strings for signal numbers. We hadn't gotten the word though, and were still using the crufty old sys_siglist array, which is in no standard even though most Unixen provide it. Aside from not being formally standards-compliant, this was just plain ugly because it involved #ifdef's at every place using the code. To eliminate the #ifdef's, create a portability function pg_strsignal, which wraps strsignal(3) if available and otherwise falls back to sys_siglist[] if available. The set of Unixen with neither API is probably empty these days, but on any platform with neither, you'll just get "unrecognized signal". All extant callers print the numeric signal number too, so no need to work harder than that. Along the way, upgrade pg_basebackup's child-error-exit reporting to match the rest of the system. Discussion: https://postgr.es/m/25758.1544983503@sss.pgh.pa.us
* Improve detection of child-process SIGPIPE failures.Tom Lane2018-12-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit ffa4cbd62 added logic to detect SIGPIPE failure of a COPY child process, but it only worked correctly if the SIGPIPE occurred in the immediate child process. Depending on the shell in use and the complexity of the shell command string, we might instead get back an exit code of 128 + SIGPIPE, representing a shell error exit reporting SIGPIPE in the child process. We could just hack up ClosePipeToProgram() to add the extra case, but it seems like this is a fairly general issue deserving a more general and better-documented solution. I chose to add a couple of functions in src/common/wait_error.c, which is a natural place to know about wait-result encodings, that will test for either a specific child-process signal type or any child-process signal failure. Then, adjust other places that were doing ad-hoc tests of this type to use the common functions. In RestoreArchivedFile, this fixes a race condition affecting whether the process will report an error or just silently proc_exit(1): before, that depended on whether the intermediate shell got SIGTERM'd itself or reported a child process failing on SIGTERM. Like the previous patch, back-patch to v10; we could go further but there seems no real need to. Per report from Erik Rijkers. Discussion: https://postgr.es/m/f3683f87ab1701bea5d86a7742b22432@xs4all.nl
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-04 19:58:28 +0000