aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Small code simplificationsPeter Eisentraut2020-12-03
| | | | | strVal() can be used in a couple of places instead of coding the same thing by hand.
* Improve estimation of OR clauses using extended statistics.Dean Rasheed2020-12-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Formerly we only applied extended statistics to an OR clause as part of the clauselist_selectivity() code path for an OR clause appearing in an implicitly-ANDed list of clauses. This meant that it could only use extended statistics if all sub-clauses of the OR clause were covered by a single extended statistics object. Instead, teach clause_selectivity() how to apply extended statistics to an OR clause by handling its ORed list of sub-clauses in a similar manner to an implicitly-ANDed list of sub-clauses, but with different combination rules. This allows one or more extended statistics objects to be used to estimate all or part of the list of sub-clauses. Any remaining sub-clauses are then treated as if they are independent. Additionally, to avoid double-application of extended statistics, this introduces "extended" versions of clause_selectivity() and clauselist_selectivity(), which include an option to ignore extended statistics. This replaces the old clauselist_selectivity_simple() function which failed to completely ignore extended statistics when called from the extended statistics code. A known limitation of the current infrastructure is that an AND clause under an OR clause is not treated as compatible with extended statistics (because we don't build RestrictInfos for such sub-AND clauses). Thus, for example, "(a=1 AND b=1) OR (a=2 AND b=2)" will currently be treated as two independent AND clauses (each of which may be estimated using extended statistics), but extended statistics will not currently be used to account for any possible overlap between those clauses. Improving that is left as a task for the future. Original patch by Tomas Vondra, with additional improvements by me. Discussion: https://postgr.es/m/20200113230008.g67iyk4cs3xbnjju@development
* Refactor CLUSTER and REINDEX grammar to use DefElem for option listsMichael Paquier2020-12-03
| | | | | | | | | | | | | | This changes CLUSTER and REINDEX so as a parenthesized grammar becomes possible for options, while unifying the grammar parsing rules for option lists with the existing ones. This is a follow-up of the work done in 873ea9e for VACUUM, ANALYZE and EXPLAIN. This benefits REINDEX for a potential backend-side filtering for collatable-sensitive indexes and TABLESPACE, while CLUSTER would benefit from the latter. Author: Alexey Kondratov, Justin Pryzby Discussion: https://postgr.es/m/8a8f5f73-00d3-55f8-7583-1375ca8f6a91@postgrespro.ru
* Add GSS information to connection authorized log messageStephen Frost2020-12-02
| | | | | | | | | | | | | | | GSS information (if used) such as if the connection was authorized using GSS or if it was encrypted using GSS, and perhaps most importantly, what the GSS principal used for the authentication was, is extremely useful but wasn't being included in the connection authorized log message. Therefore, add to the connection authorized log message that information, in a similar manner to how we log SSL information when SSL is used for a connection. Author: Vignesh C Reviewed-by: Bharath Rupireddy Discussion: https://www.postgresql.org/message-id/CALDaNm2N1385_Ltoo%3DS7VGT-ESu_bRQa-sC1wg6ikrM2L2Z49w%40mail.gmail.com
* Track total number of WAL records, FPIs and bytes generated in the cluster.Fujii Masao2020-12-02
| | | | | | | | | | | | | | | Commit 6b466bf5f2 allowed pg_stat_statements to track the number of WAL records, full page images and bytes that each statement generated. Similarly this commit allows us to track the cluster-wide WAL statistics counters. New columns wal_records, wal_fpi and wal_bytes are added into the pg_stat_wal view, and reports the total number of WAL records, full page images and bytes generated in the , respectively. Author: Masahiro Ikeda Reviewed-by: Amit Kapila, Movead Li, Kyotaro Horiguchi, Fujii Masao Discussion: https://postgr.es/m/35ef960128b90bfae3b3fdf60a3a860f@oss.nttdata.com
* Fix compilation warnings in cryptohash_openssl.cMichael Paquier2020-12-02
| | | | | | | These showed up with -O2. Oversight in 87ae969. Author: Fujii Masao Discussion: https://postgr.es/m/cee3df00-566a-400c-1252-67c3701f918a@oss.nttdata.com
* Allow restore_command parameter to be changed with reload.Fujii Masao2020-12-02
| | | | | | | | | | | | | | | | | | | | | | This commit changes restore_command from PGC_POSTMASTER to PGC_SIGHUP. As the side effect of this commit, restore_command can be reset to empty during archive recovery. In this setting, archive recovery tries to replay only WAL files available in pg_wal directory. This is the same behavior as when the command that always fails is specified in restore_command. Note that restore_command still must be specified (not empty) when starting archive recovery, even after applying this commit. This is necessary as the safeguard to prevent users from forgetting to specify restore_command and starting archive recovery. Thanks to Peter Eisentraut, Michael Paquier, Andres Freund, Robert Haas and Anastasia Lubennikova for discussion. Author: Sergei Kornilov Reviewed-by: Kyotaro Horiguchi, Fujii Masao Discussion: https://postgr.es/m/2317771549527294@sas2-985f744271ca.qloud-c.yandex.net
* Move SHA2 routines to a new generic API layer for crypto hashesMichael Paquier2020-12-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Two new routines to allocate a hash context and to free it are created, as these become necessary for the goal behind this refactoring: switch the all cryptohash implementations for OpenSSL to use EVP (for FIPS and also because upstream does not recommend the use of low-level cryptohash functions for 20 years). Note that OpenSSL hides the internals of cryptohash contexts since 1.1.0, so it is necessary to leave the allocation to OpenSSL itself, explaining the need for those two new routines. This part is going to require more work to properly track hash contexts with resource owners, but this not introduced here. Still, this refactoring makes the move possible. This reduces the number of routines for all SHA2 implementations from twelve (SHA{224,256,386,512} with init, update and final calls) to five (create, free, init, update and final calls) by incorporating the hash type directly into the hash context data. The new cryptohash routines are moved to a new file, called cryptohash.c for the fallback implementations, with SHA2 specifics becoming a part internal to src/common/. OpenSSL specifics are part of cryptohash_openssl.c. This infrastructure is usable for more hash types, like MD5 or HMAC. Any code paths using the internal SHA2 routines are adapted to report correctly errors, which are most of the changes of this commit. The zones mostly impacted are checksum manifests, libpq and SCRAM. Note that e21cbb4 was a first attempt to switch SHA2 to EVP, but it lacked the refactoring needed for libpq, as done here. This patch has been tested on Linux and Windows, with and without OpenSSL, and down to 1.0.1, the oldest version supported on HEAD. Author: Michael Paquier Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/20200924025314.GE7405@paquier.xyz
* pg_checksums: data_checksum_version is unsigned so use %u not %dBruce Momjian2020-12-01
| | | | | | | While the previous behavior didn't generate a warning, we might as well use an accurate *printf specification. Backpatch-through: 12
* Ensure that expandTableLikeClause() re-examines the same table.Tom Lane2020-12-01
| | | | | | | | | | | | | | | | | | | | | | As it stood, expandTableLikeClause() re-did the same relation_openrv call that transformTableLikeClause() had done. However there are scenarios where this would not find the same table as expected. We hold lock on the LIKE source table, so it can't be renamed or dropped, but another table could appear before it in the search path. This explains the odd behavior reported in bug #16758 when cloning a table as a temp table of the same name. This case worked as expected before commit 502898192 introduced the need to open the source table twice, so we should fix it. To make really sure we get the same table, let's re-open it by OID not name. That requires adding an OID field to struct TableLikeClause, which is a little nervous-making from an ABI standpoint, but as long as it's at the end I don't think there's any serious risk. Per bug #16758 from Marc Boeren. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/16758-840e84a6cfab276d@postgresql.org
* Avoid memcpy() with a NULL source pointer and count == 0Alvaro Herrera2020-12-01
| | | | | | | | | | | | | | | | When memcpy() is called on a pointer, the compiler is entitled to assume that the pointer is not null, which can lead to optimizing nearby code in potentially undesirable ways. We still want such optimizations (gcc's -fdelete-null-pointer-checks) in cases where they're valid. Related: commit 13bba02271dc. Backpatch to pg11, where this particular instance appeared. Reported-by: Ranier Vilela <ranier.vf@gmail.com> Reported-by: Zhihong Yu <zyu@yugabyte.com> Discussion: https://postgr.es/m/CAEudQApUndmQkr5fLrCKXQ7+ib44i7S+Kk93pyVThS85PnG3bQ@mail.gmail.com Discussion: https://postgr.es/m/CALNJ-vSdhwSM5f4tnNn1cdLHvXMVe_S+V3nR5GwNrmCPNB2VtQ@mail.gmail.com
* Use truncate(2) where appropriate.Thomas Munro2020-12-01
| | | | | | | When truncating files by name, use truncate(2). Windows hasn't got it, so keep our previous coding based on ftruncate(2) as a fallback. Discussion: https://postgr.es/m/16663-fe97ccf9932fc800%40postgresql.org
* Free disk space for dropped relations on commit.Thomas Munro2020-12-01
| | | | | | | | | | | | | | | | | | | | | When committing a transaction that dropped a relation, we previously truncated only the first segment file to free up disk space (the one that won't be unlinked until the next checkpoint). Truncate higher numbered segments too, even though we unlink them on commit. This frees the disk space immediately, even if other backends have open file descriptors and might take a long time to get around to handling shared invalidation events and closing them. Also extend the same behavior to the first segment, in recovery. Back-patch to all supported releases. Bug: #16663 Reported-by: Denis Patron <denis.patron@previnet.it> Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com> Reviewed-by: Neil Chen <carpenter.nail.cz@gmail.com> Reviewed-by: David Zhang <david.zhang@highgo.ca> Discussion: https://postgr.es/m/16663-fe97ccf9932fc800%40postgresql.org
* Fix missing outfuncs.c support for IncrementalSortPath.Tom Lane2020-11-30
| | | | | | | | | | | | | | For debugging purposes, Path nodes are supposed to have outfuncs support, but this was overlooked in the original incremental sort patch. While at it, clean up a couple other minor oversights, as well as bizarre choice of return type for create_incremental_sort_path(). (All the existing callers just cast it to "Path *" immediately, so they don't care, but some future caller might care.) outfuncs.c fix by Zhijie Hou, the rest by me Discussion: https://postgr.es/m/324c4d81d8134117972a5b1f6cdf9560@G08CNEXMBPEKD05.g08.fujitsu.local
* Prevent parallel index build in a standalone backend.Tom Lane2020-11-30
| | | | | | | | | | | | | | | | | This can't work if there's no postmaster, and indeed the code got an assertion failure trying. There should be a check on IsUnderPostmaster gating the use of parallelism, as the planner has for ordinary parallel queries. Commit 40d964ec9 got this right, so follow its model of checking IsUnderPostmaster at the same place where we check for max_parallel_maintenance_workers == 0. In general, new code implementing parallel utility operations should do the same. Report and patch by Yulin Pei, cosmetically adjusted by me. Back-patch to v11 where this code came in. Discussion: https://postgr.es/m/HK0PR01MB22747D839F77142D7E76A45DF4F50@HK0PR01MB2274.apcprd01.prod.exchangelabs.com
* Fix miscomputation of direct_lateral_relids for join relations.Tom Lane2020-11-30
| | | | | | | | | | | | | | If a PlaceHolderVar is to be evaluated at a join relation, but its value is only needed there and not at higher levels, we neglected to update the joinrel's direct_lateral_relids to include the PHV's source rel. This causes problems because join_is_legal() then won't allow joining the joinrel to the PHV's source rel at all, leading to "failed to build any N-way joins" planner failures. Per report from Andreas Seltenreich. Back-patch to 9.5 where the problem originated. Discussion: https://postgr.es/m/87blfgqa4t.fsf@aurora.ydns.eu
* Refactor parsing rules for option lists of EXPLAIN, VACUUM and ANALYZEMichael Paquier2020-11-30
| | | | | | | | | | | Those three commands have been using the same grammar rules to handle a a list of parenthesized options. This refactors the code so as they use the same parsing rules, shaving some code. A future commit will make use of those option parsing rules for more utility commands, like REINDEX and CLUSTER. Author: Alexey Kondratov, Justin Pryzby Discussion: https://postgr.es/m/8a8f5f73-00d3-55f8-7583-1375ca8f6a91@postgrespro.ru
* Remove leftover comments, left behind by removal of WITH OIDS.Heikki Linnakangas2020-11-30
| | | | | Author: Amit Langote Discussion: https://www.postgresql.org/message-id/CA%2BHiwqGaRoF3XrhPW-Y7P%2BG7bKo84Z_h%3DkQHvMh-80%3Dav3wmOw%40mail.gmail.com
* Fix typo in comment.Fujii Masao2020-11-30
| | | | | Author: Haiying Tang <tanghy.fnst@cn.fujitsu.com> Discussion: https://postgr.es/m/48a0928ac94b497d9c40acf1de394c15@G08CNEXMBPEKD05.g08.fujitsu.local
* Improve log message about termination of background workers.Fujii Masao2020-11-30
| | | | | | | | | | | | | | | | Previously the shutdown of a background worker that uses die() as SIGTERM signal handler produced the log message "terminating connection due to administrator command". This log message was confusing because a background worker is not a connection. This commit improves that log message to "terminating background worker XXX due to administrator command" (XXX is replaced with the name of the background worker). This is the same log message as another SIGTERM signal handler bgworker_die() for a background worker reports. Author: Bharath Rupireddy Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/3f292fbb-f155-9a01-7cb2-7ccc9007ab3f@oss.nttdata.com
* Fix recently-introduced breakage in psql's \connect command.Tom Lane2020-11-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Through my misreading of what the existing code actually did, commits 85c54287a et al. broke psql's behavior for the case where "\c connstring" provides a password in the connstring. We should use that password in such a case, but as of 85c54287a we ignored it (and instead, prompted for a password). Commit 94929f1cf fixed that in HEAD, but since I thought it was cleaning up a longstanding misbehavior and not one I'd just created, I didn't back-patch it. Hence, back-patch the portions of 94929f1cf having to do with password management. In addition to fixing the introduced bug, this means that "\c -reuse-previous=on connstring" will allow re-use of an existing connection's password if the connstring doesn't change user/host/port. That didn't happen before, but it seems like a bug fix, and anyway I'm loath to have significant differences in this code across versions. Also fix an error with the same root cause about whether or not to override a connstring's setting of client_encoding. As of 85c54287a we always did so; restore the previous behavior of overriding only when stdin/stdout are a terminal and there's no environment setting of PGCLIENTENCODING. (I find that definition a bit surprising, but right now doesn't seem like the time to revisit it.) Per bug #16746 from Krzysztof Gradek. As with the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/16746-44b30e2edf4335d4@postgresql.org
* Retry initial slurp_file("current_logfiles"), in test 004_logrotate.pl.Noah Misch2020-11-28
| | | | | | Buildfarm member topminnow failed when the test script attempted this before the syslogger would have created the file. Back-patch to v12, which introduced the test.
* Clean up after tests in src/test/locale/.Tom Lane2020-11-28
| | | | Oversight in 257836a75, which added these tests.
* Fix a recently-introduced race condition in LISTEN/NOTIFY handling.Tom Lane2020-11-28
| | | | | | | | | | | | | | | | | | | | | | | | | | Commit 566372b3d fixed some race conditions involving concurrent SimpleLruTruncate calls, but it introduced new ones in async.c. A newly-listening backend could attempt to read Notify SLRU pages that were in process of being truncated, possibly causing an error. Also, the QUEUE_TAIL pointer could become set to a value that's not equal to the queue position of any backend. While that's fairly harmless in v13 and up (thanks to commit 51004c717), in older branches it resulted in near-permanent disabling of the queue truncation logic, so that continued use of NOTIFY led to queue-fill warnings and eventual inability to send any more notifies. (A server restart is enough to make that go away, but it's still pretty unpleasant.) The core of the problem is confusion about whether QUEUE_TAIL represents the "logical" tail of the queue (i.e., the oldest still-interesting data) or the "physical" tail (the oldest data we've not yet truncated away). To fix, split that into two variables. QUEUE_TAIL regains its definition as the logical tail, and we introduce a new variable to track the oldest un-truncated page. Per report from Mikael Gustavsson. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/1b8561412e8a4f038d7a491c8b922788@smhi.se
* Fix CLUSTER progress reporting of number of blocks scanned.Fujii Masao2020-11-27
| | | | | | | | | | | | | | | | | | | | | | | | | Previously pg_stat_progress_cluster view reported the current block number in heap scan as the number of heap blocks scanned (i.e., heap_blks_scanned). This reported number could be incorrect when synchronize_seqscans is enabled, because it allowed the heap scan to start at block in middle. This could result in wraparounds in the heap_blks_scanned column when the heap scan wrapped around. This commit fixes the bug by calculating the number of blocks from the block that the heap scan starts at to the current block in scan, and reporting that number in the heap_blks_scanned column. Also, in pg_stat_progress_cluster view, previously heap_blks_scanned could not reach heap_blks_total at the end of heap scan phase if the last pages scanned were empty. This commit fixes the bug by manually updating heap_blks_scanned to the same value as heap_blks_total when the heap scan phase finishes. Back-patch to v12 where pg_stat_progress_cluster view was introduced. Reported-by: Matthias van de Meent Author: Matthias van de Meent Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CAEze2WjCBWSGkVfYag001Rc4+-nNLDpWM7QbyD6yPvuhKs-gYQ@mail.gmail.com
* Use standard SIGTERM signal handler die() in test_shm_mq worker.Fujii Masao2020-11-27
| | | | | | | | | | | | | | | | | | | | | Previously test_shm_mq worker used the stripped-down version of die() as the SIGTERM signal handler. This commit makes it use die(), instead, to simplify the code. In terms of the code, the difference between die() and the stripped-down version previously used is whether the signal handler directly may call ProcessInterrupts() or not. But this difference doesn't exist in a background worker because, in bgworker, DoingCommandRead flag will never be true and die() will never call ProcessInterrupts() directly. Therefore test_shm_mq worker can safely use die(), like other bgworker proceses (e.g., logical replication apply launcher or autoprewarm worker) currently do. Thanks to Craig Ringer for the report and investigation of the issue. Author: Bharath Rupireddy Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CAGRY4nxsAe_1k_9g5b47orA0S011iBoHsXHFMH7cg7HV0O1bwQ@mail.gmail.com
* Use standard SIGHUP and SIGTERM signal handlers in worker_spi.Fujii Masao2020-11-27
| | | | | | | | | | | | | | | | | | | | | | | | Previously worker_spi used its custom signal handlers for SIGHUP and SIGTERM. This commit makes worker_spi use the standard signal handlers, to simplify the code. Note that die() is used as the standard SIGTERM signal handler in worker_spi instead of SignalHandlerForShutdownRequest() or bgworker_die(). Previously the exit handling was only able to exit from within the main loop, and not from within the backend code it calls. This is why die() needs to be used here, so worker_spi can respond to SIGTERM signal while it's executing a query. Maybe we can say that it's a bug that worker_spi could not respond to SIGTERM during query execution. But since worker_spi is a just example of the background worker code, we don't do the back-patch. Thanks to Craig Ringer for the report and investigation of the issue. Author: Bharath Rupireddy Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CALj2ACXDEZhAFOTDcqO9cFSRvrFLyYOnPKrcA1UG4uZn9hUAVg@mail.gmail.com Discussion: https://postgr.es/m/CAGRY4nxsAe_1k_9g5b47orA0S011iBoHsXHFMH7cg7HV0O1bwQ@mail.gmail.com
* Fix replication of in-progress transactions in tablesync worker.Amit Kapila2020-11-27
| | | | | | | | | | | | | | | | | Tablesync worker runs under a single transaction but in streaming mode, we were committing the transaction on stream_stop, stream_abort, and stream_commit. We need to avoid committing the transaction in a streaming mode in tablesync worker. In passing move the call to process_syncing_tables in apply_handle_stream_commit after clean up of stream files. This will allow clean up of files to happen before the exit of tablesync worker which would otherwise be handled by one of the proc exit routines. Author: Dilip Kumar Reviewed-by: Amit Kapila and Peter Smith Tested-by: Peter Smith Discussion: https://postgr.es/m/CAHut+Pt4PyKQCwqzQ=EFF=bpKKJD7XKt_S23F6L20ayQNxg77A@mail.gmail.com
* Restore lock level to update statusFlagsAlvaro Herrera2020-11-26
| | | | | | | | | | | | | | Reverts 27838981be9d (some comments are kept). Per discussion, it does not seem safe to relax the lock level used for this; in order for it to be safe, there would have to be memory barriers between the point we set the flag and the point we set the trasaction Xid, which perhaps would not be so bad; but there would also have to be barriers at the readers' side, which from a performance perspective might be bad. Now maybe this analysis is wrong and it *is* safe for some reason, but proof of that is not trivial. Discussion: https://postgr.es/m/20201118190928.vnztes7c2sldu43a@alap3.anarazel.de
* pg_stat_statements: Track number of times pgss entries were deallocated.Fujii Masao2020-11-26
| | | | | | | | | | | | | | | | | | | | | | If more distinct statements than pg_stat_statements.max are observed, pg_stat_statements entries about the least-executed statements are deallocated. This commit enables us to track the total number of times those entries were deallocated. That number can be viewed in the pg_stat_statements_info view that this commit adds. It's useful when tuning pg_stat_statements.max parameter. If it's high, i.e., the entries are deallocated very frequently, which might cause the performance regression and we can increase pg_stat_statements.max to avoid those frequent deallocations. The pg_stat_statements_info view is intended to display the statistics of pg_stat_statements module itself. Currently it has only one column "dealloc" indicating the number of times entries were deallocated. But an upcoming patch will add other columns (for example, the time at which pg_stat_statements statistics were last reset) into the view. Author: Katsuragi Yuta, Yuki Seino Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/0d9f1107772cf5c3f954e985464c7298@oss.nttdata.com
* Use Enums for logical replication message types at more places.Amit Kapila2020-11-26
| | | | | | | | | Commit 644f0d7cc9 added logical replication message type enums to use instead of character literals but some char substitutions were overlooked. Author: Peter Smith Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CAHut+PsTG=Vrv8hgrvOnAvCNR21jhqMdPk2n0a1uJPoW0p+UfQ@mail.gmail.com
* Avoid spurious waits in concurrent indexingAlvaro Herrera2020-11-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | In the various waiting phases of CREATE INDEX CONCURRENTLY (CIC) and REINDEX CONCURRENTLY (RC), we wait for other processes to release their snapshots; this is necessary in general for correctness. However, processes doing CIC in other tables cannot possibly affect CIC or RC done in "this" table, so we don't need to wait for those. This commit adds a flag in MyProc->statusFlags to indicate that the current process is doing CIC, so that other processes doing CIC or RC can ignore it when waiting. Note that this logic is only valid if the index does not access other tables. For simplicity we avoid setting the flag if the index has a column that's an expression, or has a WHERE predicate. (It is possible to have expressional or partial indexes that do not access other tables, but figuring that out would require more work.) This flag can potentially also be used by processes doing REINDEX CONCURRENTLY to be skipped; and by VACUUM to ignore processes in CIC or RC for the purposes of computing an Xmin. That's left for future commits. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Author: Dimitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/20200810233815.GA18970@alvherre.pgsql
* In psql's \d commands, don't truncate attribute default values.Tom Lane2020-11-25
| | | | | | | | | | | | | | | | | | | | | | | | | Historically, psql has truncated the text of a column's default expression at 128 characters. This is unlike any other behavior in describe.c, and it's become particularly confusing now that the limit is only applied to the expression proper and not to the "generated always as (...) stored" text that may get wrapped around it. Excavation in our git history suggests that the original motivation for this limit was not really to limit the display width (as I'd long supposed), but to make it safe to use a fixed-width output buffer to store the result. That implementation restriction is long gone of course, but the limit remained. Let's just get rid of it. While here, rearrange the logic about when to free the output string so that it's not so dependent on unstated assumptions about the possible values of attidentity and attgenerated. Per bug #16743 from David Turon. Back-patch to v12 where GENERATED came in. (Arguably we could take it back further, but I'm hesitant to change the behavior of long-stable branches for this.) Discussion: https://postgr.es/m/16743-7b1bacc4af76e7ad@postgresql.org
* Avoid spamming the client with multiple ParameterStatus messages.Tom Lane2020-11-25
| | | | | | | | | | | | | | | | | | Up to now, we sent a ParameterStatus message to the client immediately upon any change in the active value of any GUC_REPORT variable. This was only barely okay when the feature was designed; now that we have things like function SET clauses, there are very plausible use-cases where a GUC_REPORT variable might change many times within a query --- and even end up back at its original value, perhaps. Fortunately most of our GUC_REPORT variables are unlikely to be changed often; but there are proposals in play to enlarge that set, or even make it user-configurable. Hence, let's fix things to not generate more than one ParameterStatus message per variable per query, and to not send any message at all unless the end-of-query value is different from what we last reported. Discussion: https://postgr.es/m/5708.1601145259@sss.pgh.pa.us
* Make error hint from bind() failure more accuratePeter Eisentraut2020-11-25
| | | | | | | | | | | | | | | | | The hint "Is another postmaster already running ..." should only be printed for errors that are really about something else already using the address. In other cases it is misleading. So only show that hint if errno == EADDRINUSE. Also, since Unix-domain sockets in the file-system namespace never report EADDRINUSE for an existing file (they would just overwrite it), the part of the hint saying "If not, remove socket file \"%s\" and retry." can never happen, so remove it. Unix-domain sockets in the abstract namespace can report EADDRINUSE, but in that case there is no file to remove, so the hint doesn't work there either. Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/6dee8574-b0ad-fc49-9c8c-2edc796f0033@2ndquadrant.com
* Add support for abstract Unix-domain socketsPeter Eisentraut2020-11-25
| | | | | | | | | | This is a variant of the normal Unix-domain sockets that don't use the file system but a separate "abstract" namespace. At the user interface, such sockets are represented by names starting with "@". Supported on Linux and Windows right now. Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/6dee8574-b0ad-fc49-9c8c-2edc796f0033@2ndquadrant.com
* Fix WaitLatch(NULL) on Windows.Thomas Munro2020-11-25
| | | | | | | | | | Further to commit 733fa9aa, on Windows when a latch is triggered but we aren't currently waiting for it, we need to locate the latch's HANDLE rather than calling ResetEvent(NULL). Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reported-by: Ranier Vilela <ranier.vf@gmail.com> Discussion: https://postgr.es/m/CAEudQArTPi1YBc%2Bn1fo0Asy3QBFhVjp_QgyKG-8yksVn%2ByRTiw%40mail.gmail.com
* Remove obsolete comment atop ri_PlanCheck.Amit Kapila2020-11-25
| | | | | | | | | Commit 5b7ba75f7f removed the unused parameter but forgot to update the nearby comments. Author: Li Japin Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/0E2F62A2-B2F1-4052-83AE-F0BEC8A75789@hotmail.com
* Stop gap fix for __attribute__((cold)) compiler bug in MinGW 8.1David Rowley2020-11-25
| | | | | | | | | | | | | The buildfarm animal walleye, running MinGW 8.1 has been having problems ever since 697e1d02f and 913ec71d6 went in. This appears to be a bug in assembler which was fixed in a later version. For now, in order to get that animal running green again, let's just define pg_attribute_cold and pg_attribute_hot to be empty macros on that compiler. Hopefully, we can get the support of the owner of the animal to upgrade to a less buggy compiler and revert this at a later date. Discussion: https://postgr.es/m/286560.1606233316@sss.pgh.pa.us
* Remove catalog function currtid()Michael Paquier2020-11-25
| | | | | | | | | | | | | | | | | | | | | | | | | currtid() and currtid2() are an undocumented set of functions whose sole known user is the Postgres ODBC driver, able to retrieve the latest TID version for a tuple given by the caller of those functions. As used by Postgres ODBC, currtid() is a shortcut able to retrieve the last TID loaded into a backend by passing an OID of 0 (magic value) after a tuple insertion. This is removed in this commit, as it became obsolete after the driver began using "RETURNING ctid" with inserts, a clause supported since Postgres 8.2 (using RETURNING is better for performance anyway as it reduces the number of round-trips to the backend). currtid2() is still used by the driver, so this remains around for now. Note that this function is kept in its original shape for backward compatibility reasons. Per discussion with many people, including Andres Freund, Peter Eisentraut, Álvaro Herrera, Hiroshi Inoue, Tom Lane and myself. Bump catalog version. Discussion: https://postgr.es/m/20200603021448.GB89559@paquier.xyz
* Properly check index mark/restore in ExecSupportsMarkRestore.Andrew Gierth2020-11-24
| | | | | | | | | | | | | | | | Previously this code assumed that all IndexScan nodes supported mark/restore, which is not true since it depends on optional index AM support functions. This could lead to errors about missing support functions in rare edge cases of mergejoins with no sort keys, where an unordered non-btree index scan was placed on the inner path without a protecting Materialize node. (Normally, the fact that merge join requires ordered input would avoid this error.) Backpatch all the way since this bug is ancient. Per report from Eugen Konkov on irc. Discussion: https://postgr.es/m/87o8jn50be.fsf@news-spur.riddles.org.uk
* Tidy up definitions of pg_attribute_hot and pg_attribute_coldDavid Rowley2020-11-25
| | | | | | | | | | | | | | 1fa22a43a was a quick fix for portability problem I introduced in 697e1d02f. 1fa22a43a adds a few more cases to the preprocessor logic than I'd have liked. Andres Freund and Dagfinn Ilmari Mannsåker suggested a better way to do this. In passing, also adjust the only current usage of these macros so that the macro comes before the function's return type in the declaration of the function. This now matches what the definition of the function does. Discussion: https://postgr.es/m/20200625163553.lt6wocbjhklp5pl4@alap3.anarazel.de Discussion: https://postgr.es/m/87pn43bmok.fsf@wibble.ilmari.org
* Put "inline" marker on declarations of inline functions.Tom Lane2020-11-24
| | | | | | I'm having a hard time telling whether the letter of the C standard requires this, but we do have a couple of buildfarm members that throw warnings when this is not done. Oversight in c532d15dd.
* Move per-agg and per-trans duplicate finding to the planner.Heikki Linnakangas2020-11-24
| | | | | | | | | | This has the advantage that the cost estimates for aggregates can count the number of calls to transition and final functions correctly. Bump catalog version, because views can contain Aggrefs. Reviewed-by: Andres Freund Discussion: https://www.postgresql.org/message-id/b2e3536b-1dbc-8303-c97e-89cb0b4a9a48%40iki.fi
* Use macros instead of hardcoded offsets for LWLock initializationMichael Paquier2020-11-24
| | | | | | | | | This makes the code slightly easier to follow, as the initialization relies on an offset that overlapped with an equivalent set of macros defined, which are used in other places already. Author: Japin Li Discussion: https://postgr.es/m/MEYP282MB1669FB410006758402F2C3A2B6E00@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
* Centralize logic for skipping useless ereport/elog calls.Tom Lane2020-11-23
| | | | | | | | | | | | | | | | | | | | | While ereport() and elog() themselves are quite cheap when the error message level is too low to be printed, some places need to do substantial work before they can call those macros at all. To allow optimizing away such setup work when nothing is to be printed, make elog.c export a new function message_level_is_interesting(elevel) that reports whether ereport/elog will do anything. Make use of that in various places that had ad-hoc direct tests of log_min_messages etc. Also teach ProcSleep to use it to avoid some work. (There may well be other places that could usefully use this; I didn't search hard.) Within elog.c, refactor a little bit to avoid having duplicate copies of the policy-setting logic. When that code was written, we weren't relying on the availability of inline functions; so it had some duplications in the name of efficiency, which I got rid of. Alvaro Herrera and Tom Lane Discussion: https://postgr.es/m/129515.1606166429@sss.pgh.pa.us
* Fix unportable usage of __has_attributeDavid Rowley2020-11-24
| | | | | | | | | | | | | | This should fix the breakages caused by 697e1d02f, which seems to break the build for GCC version < 5. It seems, according to the GCC manual that __has_attribute is a "special operator" and must be tested without any other conditions in the preprocessor test. Per recommendation from the GCC manual via Greg Nancarrow Reported-by: Greg Nancarrow Discussion: https://postgr.es/m/CAJcOf-euSu8fhC10v476o9dqnjqKysVs1_vRms-_fvajpZ3kFw@mail.gmail.com
* Improve compiler code layout in elog/ereport ERROR callsDavid Rowley2020-11-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Here we use a bit of preprocessor trickery to coax supporting compilers into laying out their generated code so that the code that's in the same branch as elog(ERROR)/ereport(ERROR) calls is moved away from the hot path. Effectively, this reduces the size of the hot code meaning that it can sit on fewer cache lines. Performance improvements of between 10-15% have been seen on highly CPU bound workloads using pgbench's TPC-b benchmark. What's achieved here is very similar to putting the error condition inside an unlikely() macro. For example; if (unlikely(x < 0)) elog(ERROR, "invalid x value"); now there's no need to make use of unlikely() here as the common macro used by elog and ereport will now see that elevel is >= ERROR and make use of a pg_attribute_cold marked version of errstart(). When elevel < ERROR or if it cannot be determined to be constant, the original behavior is maintained. Author: David Rowley Reviewed-by: Andres Freund, Peter Eisentraut Discussion: https://postgr.es/m/CAApHDvrVpasrEzLL2er7p9iwZFZ%3DJj6WisePcFeunwfrV0js_A%40mail.gmail.com
* Define pg_attribute_cold and pg_attribute_hot macrosDavid Rowley2020-11-24
| | | | | | | | | | | | For compilers supporting __has_attribute and __has_attribute (hot/cold). __has_attribute is supported on gcc >= 5, clang >= 2.9 and icc >= 17. A followup commit will implement some usages of these macros. Author: David Rowley Reviewed-by: Andres Freund, Peter Eisentraut Discussion: https://postgr.es/m/CAApHDvrVpasrEzLL2er7p9iwZFZ%3DJj6WisePcFeunwfrV0js_A%40mail.gmail.com
* Remove unnecessary #include.Tom Lane2020-11-23
| | | | | | Justin Pryzby Discussion: https://postgr.es/m/20201123205505.GJ24052@telsasoft.com