aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
...
* Disable anonymous record hash support except in special casesPeter Eisentraut2021-09-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 01e658fa74 added hash support for row types. This also added support for hashing anonymous record types, using the same approach that the type cache uses for comparison support for record types: It just reports that it works, but it might fail at run time if a component type doesn't actually support the operation. We get away with that for comparison because most types support that. But some types don't support hashing, so the current state can result in failures at run time where the planner chooses hashing over sorting, whereas that previously worked if only sorting was an option. We do, however, want the record hashing support for path tracking in recursive unions, and the SEARCH and CYCLE clauses built on that. In that case, hashing is the only plan option. So enable that, this commit implements the following approach: The type cache does not report that hashing is available for the record type. This undoes that part of 01e658fa74. Instead, callers that require hashing no matter what can override that result themselves. This patch only touches the callers to make the aforementioned recursive query cases work, namely the parse analysis of unions, as well as the hash_array() function. Reported-by: Sait Talha Nisanci <sait.nisanci@microsoft.com> Bug: #17158 Discussion: https://www.postgresql.org/message-id/flat/17158-8a2ba823982537a4%40postgresql.org
* Fix incorrect format placeholdersPeter Eisentraut2021-09-08
|
* Invalidate relcache for publications defined for all tables.Amit Kapila2021-09-08
| | | | | | | | | | | | | | | Updates/Deletes on a relation were allowed even without replica identity after we define the publication for all tables. This would later lead to an error on subscribers. The reason was that for such publications we were not invalidating the relcache and the publication information for relations was not getting rebuilt. Similarly, we were not invalidating the relcache after dropping of such publications which will prohibit Updates/Deletes without replica identity even without any publication. Author: Vignesh C and Hou Zhijie Reviewed-by: Hou Zhijie, Kyotaro Horiguchi, Amit Kapila Backpatch-through: 10, where it was introduced Discussion: https://postgr.es/m/CALDaNm0pF6zeWqCA8TCe2sDuwFAy8fCqba=nHampCKag-qLixg@mail.gmail.com
* Fix compilation warning in ipci.cMichael Paquier2021-09-08
| | | | | | A Size had better use %zu when printed. Oversight in bd17880, per buildfarm member lapwing.
* Introduce GUC shared_memory_sizeMichael Paquier2021-09-08
| | | | | | | | | | This runtime-computed GUC shows the size of the server's main shared memory area, taking into account the amount of shared memory allocated by extensions as this is calculated after processing shared_preload_libraries. Author: Nathan Bossart Discussion: https://postgr.es/m/F2772387-CE0F-46BF-B5F1-CC55516EB885@amazon.com
* Clean up some code using "(expr) ? true : false"Michael Paquier2021-09-08
| | | | | | | | | | All the code paths simplified here were already using a boolean or used an expression that led to zero or one, making the extra bits unnecessary. Author: Justin Pryzby Reviewed-by: Tom Lane, Michael Paquier, Peter Smith Discussion: https://postgr.es/m/20210428182936.GE27406@telsasoft.com
* Consistently use read-only instead of "read only"Magnus Hagander2021-09-07
| | | | | | | | This affects one message and some documentation that used the format "read only", unlike everything else that used read-only. Backpatch-through: 14 Discussion: https://postgr.es/m/CABUevExuxKwn0YM3+wdSeQSvK6CRrJ-hewocGVX3R4-xVX4eMw@mail.gmail.com
* Fix missing words in comment.Heikki Linnakangas2021-09-07
| | | | | | | Introduced by commit c3928b467a, backpatch to v14 like that one. Author: Amit Langote Discussion: https://www.postgresql.org/message-id/CA+HiwqFQgNLS6VGntMcuJV6erBFV425xA6wBVnY=41GK4zC0Bw@mail.gmail.com
* Log new catalog xmin candidate in LogicalIncreaseXminForSlot().Amit Kapila2021-09-07
| | | | | | | | | | | | Similar to LogicalIncreaseRestartDecodingForSlot() add a debug message to LogicalIncreaseXminForSlot() reporting a new catalog_xmin candidate. This just adds additional diagnostic information during logical decoding that can aid debugging. Author: Ashutosh Bapat Reviewed-by: Masahiko Sawada, Amit Kapila Discussion: https://postgr.es/m/CAExHW5usQWbiUz0hHOCu5twS1O9DvpcPojf6sor=8q--VUuMbA@mail.gmail.com
* Add PublicationTable and PublicationRelInfo structsAlvaro Herrera2021-09-06
| | | | | | | | | | | | | | | These encapsulate a relation when referred from replication DDL. Currently they don't do anything useful (they're just wrappers around RangeVar and Relation respectively) but in the future they'll be used to carry column lists. Extracted from a larger patch by Rahila Syed. Author: Rahila Syed <rahilasyed90@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://postgr.es/m/CAH2L28vddB_NFdRVpuyRBJEBWjz4BSyTB=_ektNRH8NJ1jf95g@mail.gmail.com
* Make timetz_zone() stable, and correct a bug for DYNTZ abbreviations.Tom Lane2021-09-06
| | | | | | | | | | | | | | | | | | | | | Historically, timetz_zone() has used time(NULL) as the reference point for deciding whether DST is active. That means its result can change intra-statement, requiring it to be marked VOLATILE (cf. 35979e6c3). But that definition is pretty inconsistent with the way we deal with timestamps elsewhere. Let's make it use the transaction start time ("now()") as the reference point instead. That lets it be marked STABLE, and also saves a kernel call per invocation. While at it, remove the function's use of pg_time_t and pg_localtime. Those are inconsistent with the other code in this area, which indeed created a bug: timetz_zone() delivered completely wrong answers if the zone was specified by a dynamic TZ abbreviation. (We need to do something about that in the back branches, but the fix will look different from this.) Aleksander Alekseev and Tom Lane Discussion: https://postgr.es/m/CAJ7c6TOMG8zSNEZtCn5SPe+cCk3Lfxb71ZaQwT2F4T7PJ_t=KA@mail.gmail.com
* Fix typo in comments.Fujii Masao2021-09-06
| | | | | Author: Hou Zhijie Discussion: https://postgr.es/m/OS0PR01MB5716E6A6535FDFDC5A1B004194CE9@OS0PR01MB5716.jpnprd01.prod.outlook.com
* Move the shared memory size calculation to its own functionMichael Paquier2021-09-06
| | | | | | | | | | This change refactors the shared memory size calculation in CreateSharedMemoryAndSemaphores() to its own function. This is intended for use in a future change related to the setup of huge pages and shared memory with some GUCs, while useful on its own for extensions. Author: Nathan Bossart Discussion: https://postgr.es/m/F2772387-CE0F-46BF-B5F1-CC55516EB885@amazon.com
* Further portability tweaks for float4/float8 hash functions.Tom Lane2021-09-04
| | | | | | | | | | Attempting to make hashfloat4() look as much as possible like hashfloat8(), I'd figured I could replace NaNs with get_float4_nan() before widening to float8. However, results from protosciurus and topminnow show that on some platforms that produces a different bit-pattern from get_float8_nan(), breaking the intent of ce773f230. Rearrange so that we use the result of get_float8_nan() for all NaN cases. As before, back-patch.
* Revert "Avoid creating archive status ".ready" files too early"Alvaro Herrera2021-09-04
| | | | | | | | | | This reverts commit 515e3d84a0b5 and equivalent commits in back branches. This solution to the problem has a number of problems, so we'll try again with a different approach. Per note from Andres Freund Discussion: https://postgr.es/m/20210831042949.52eqp5xwbxgrfank@alap3.anarazel.de
* Disallow creating an ICU collation if the DB encoding won't support it.Tom Lane2021-09-03
| | | | | | | | | | | | | | | | Previously this was allowed, but the collation effectively vanished into the ether because of the way lookup_collation() works: you could not use the collation, nor even drop it. Seems better to give an error up front than to leave the user wondering why it doesn't work. (Because this test is in DefineCollation not CreateCollation, it does not prevent pg_import_system_collations from creating ICU collations, regardless of the initially-chosen encoding.) Per bug #17170 from Andrew Bille. Back-patch to v10 where ICU support was added. Discussion: https://postgr.es/m/17170-95845cf3f0a9c36d@postgresql.org
* In count_usable_fds(), duplicate stderr not stdin.Tom Lane2021-09-02
| | | | | | | | | | | | | | | | | | | | We had a complaint that the postmaster fails to start if the invoking program closes stdin. That happens because count_usable_fds expects to be able to dup(0), and if it can't, we conclude there are no free FDs and go belly-up. So far as I can find, though, there is no other place in the server that touches stdin, and it's not unreasonable to expect that a daemon wouldn't use that file. As a simple improvement, let's dup FD 2 (stderr) instead. Unlike stdin, it *is* reasonable for us to expect that stderr be open; even if we are configured not to touch it, common libraries such as libc might try to write error messages there. Per gripe from Mario Emmenlauer. Given the lack of previous complaints, I'm not excited about pushing this into stable branches, but it seems OK to squeeze it into v14. Discussion: https://postgr.es/m/48bafc63-c30f-3962-2ded-f2e985d93e86@emmenlauer.de
* Fix float4/float8 hash functions to produce uniform results for NaNs.Tom Lane2021-09-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The IEEE 754 standard allows a wide variety of bit patterns for NaNs, of which at least two ("NaN" and "-NaN") are pretty easy to produce from SQL on most machines. This is problematic because our btree comparison functions deem all NaNs to be equal, but our float hash functions know nothing about NaNs and will happily produce varying hash codes for them. That causes unexpected results from queries that hash a column containing different NaN values. It could also produce unexpected lookup failures when using a hash index on a float column, i.e. "WHERE x = 'NaN'" will not find all the rows it should. To fix, special-case NaN in the float hash functions, not too much unlike the existing special case that forces zero and minus zero to hash the same. I arranged for the most vanilla sort of NaN (that coming from the C99 NAN constant) to still have the same hash code as before, to reduce the risk to existing hash indexes. I dithered about whether to back-patch this into stable branches, but ultimately decided to do so. It's a clear improvement for queries that hash internally. If there is anybody who has -NaN in a hash index, they'd be well advised to re-index after applying this patch ... but the misbehavior if they don't will not be much worse than the misbehavior they had before. Per bug #17172 from Ma Liangzhu. Discussion: https://postgr.es/m/17172-7505bea9e04e230f@postgresql.org
* Enhance pg_stat_reset_single_table_counters function.Fujii Masao2021-09-02
| | | | | | | | | | | This commit allows pg_stat_reset_single_table_counters() to reset statistics for a single relation shared across all databases in the cluster to zero. Bump catalog version. Author: B Sadhu Prasad Patro Reviewed-by: Mahendra Singh Thalor, Himanshu Upadhyaya, Dilip Kumar, Fujii Masao Discussion: https://postgr.es/m/CAFF0-CGy7EHeF=AqqkGMF85cySPQBgDcvNk73G2O0vL94O5U5A@mail.gmail.com
* Optimize fileset usage in apply worker.Amit Kapila2021-09-02
| | | | | | | | | | | | | | | | Use one fileset for the entire worker lifetime instead of using separate filesets for each streaming transaction. Now, the changes/subxacts files for every streaming transaction will be created under the same fileset and the files will be deleted after the transaction is completed. This patch extends the BufFileOpenFileSet and BufFileDeleteFileSet APIs to allow users to specify whether to give an error on missing files. Author: Dilip Kumar, based on suggestion by Thomas Munro Reviewed-by: Hou Zhijie, Masahiko Sawada, Amit Kapila Discussion: https://postgr.es/m/E1mCC6U-0004Ik-Fs@gemulon.postgresql.org
* Identify simple column references in extended statisticsTomas Vondra2021-09-01
| | | | | | | | | | | | | | | | | | | | | | | | | | Until now, when defining extended statistics, everything except a plain column reference was treated as complex expression. So for example "a" was a column reference, but "(a)" would be an expression. In most cases this does not matter much, but there were a couple strange consequences. For example CREATE STATISTICS s ON a FROM t; would fail, because extended stats require at least two columns. But CREATE STATISTICS s ON (a) FROM t; would succeed, because that requirement does not apply to expressions. Moreover, that statistics object is useless - the optimizer will always use the regular statistics collected for attribute "a". So do a bit more work to identify those expressions referencing a single column, and translate them to a simple column reference. Backpatch to 14, where support for extended statistics on expressions was introduced. Reported-by: Justin Pryzby Backpatch-through: 14 Discussion: https://postgr.es/m/20210816013255.GS10479%40telsasoft.com
* Fix incorrect format placeholdersPeter Eisentraut2021-09-01
|
* VACUUM VERBOSE: Don't report "pages removed".Peter Geoghegan2021-08-31
| | | | | | | | | | | | | | It doesn't make any sense to report this information, since VACUUM VERBOSE reports on heap relation truncation directly. This was an oversight in commit 7ab96cf6, which made VACUUM VERBOSE output a little more consistent with nearby autovacuum-specific log output. Adjust comments that describe how this is supposed to work in passing. Also bring truncation-related VACUUM VERBOSE output in line with the convention established for VACUUM VERBOSE output by commit f4f4a649. Author: Peter Geoghegan <pg@bowt.ie> Backpatch: 14-, where VACUUM VERBOSE's output changed.
* Refactor one conversion of SQLSTATE to string in elog.cMichael Paquier2021-09-01
| | | | | | | | | | | unpack_sql_state() has been introduced in d46bc44 to refactor the unpacking of a SQLSTATE into a string, but it forgot one code path when sending error reports to clients that could make use of it. This changes the code to also use unpack_sql_state() there, simplifying a bit the code. Author: Peter Smith Discussion: https://postgr.es/m/CAHut+PuYituuD1-VVZUNcmCQuc3ZzZMPoO57POgm8tnXOkwJAA@mail.gmail.com
* Remove obsolete nbtree relation extension comment.Peter Geoghegan2021-08-31
| | | | | | Commit 0d1fe9f7 improved the approach that vacuumlazy.c takes when it encounters an empty heap page. It no acquires the relation extension lock.
* vacuumlazy.c: Correct prune state comment.Peter Geoghegan2021-08-31
| | | | Oversight in commit 7ab96cf6b3.
* Don't print extra parens around expressions in extended statsTomas Vondra2021-09-01
| | | | | | | | | | | | | | The code printing expressions for extended statistics doubled the parens, producing results like ((a+1)), which is unnecessary and not consistent with how we print expressions elsewhere. Fixed by tweaking the code to produce just a single set of parens. Reported by Mark Dilger, fix by me. Backpatch to 14, where support for extended statistics on expressions was added. Reported-by: Mark Dilger Discussion: https://postgr.es/m/20210122040101.GF27167%40telsasoft.com
* Remove unneeded old_rel_pages VACUUM state field.Peter Geoghegan2021-08-31
| | | | | | | | | | The field hasn't been used since commit 3d351d91, which redefined pg_class.reltuples to be -1 before the first VACUUM or ANALYZE. Also rename a local variable of the same name ("old_rel_pages"). This is used by relation truncation to represent the original relation size at the start of the ongoing VACUUM operation. Rename it to orig_rel_pages, since that's a lot clearer. (This name matches similar nearby code.)
* Fix lookup error in extended stats ownership checkTomas Vondra2021-08-31
| | | | | | | | | | | | | | When an ownership check on extended statistics object failed, the code was calling aclcheck_error_type to report the failure, which is clearly wrong, resulting in cache lookup errors. Fix by calling aclcheck_error. This issue exists since the introduction of extended statistics, so backpatch all the way back to PostgreSQL 10. It went unnoticed because there were no tests triggering the error, so add one. Reported-by: Mark Dilger Backpatch-through: 10, where extended stats were introduced Discussion: https://postgr.es/m/1F238937-7CC2-4703-A1B1-6DC225B8978A%40enterprisedb.com
* Fix missed lock acquisition while inlining new-style SQL functions.Tom Lane2021-08-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | When starting to use a query parsetree loaded from the catalogs, we must begin by applying AcquireRewriteLocks(), to obtain the same relation locks that the parser would have gotten if the query were entered interactively, and to do some other cleanup such as dealing with later-dropped columns. New-style SQL functions are just as subject to this rule as other stored parsetrees; however, of the places dealing with such functions, only init_sql_fcache had gotten the memo. In particular, if we successfully inlined a new-style set-returning SQL function that contained any relation references, we'd either get an assertion failure or attempt to use those relation(s) sans locks. I also added AcquireRewriteLocks calls to fmgr_sql_validator and print_function_sqlbody. Desultory experiments didn't demonstrate any failures in those, but I suspect that I just didn't try hard enough. Certainly we don't expect nearby code paths to operate without locks. On the same logic of it-ought-to-have-the-same-effects-as-the-old-code, call pg_rewrite_query() in fmgr_sql_validator, too. It's possible that neither code path there needs to bother with rewriting, but doing the analysis to prove that is beyond my goals for today. Per bug #17161 from Alexander Lakhin. Discussion: https://postgr.es/m/17161-048a1cdff8422800@postgresql.org
* Report tuple address in data-corruption error messageAlvaro Herrera2021-08-30
| | | | | | | | | | | Most data-corruption reports mention the location of the problem, but this one failed to. Add it. Backpatch all the way back. In 12 and older, also assign the ERRCODE_DATA_CORRUPTED error code as was done in commit fd6ec93bf890 for 13 and later. Discussion: https://postgr.es/m/202108191637.oqyzrdtnheir@alvherre.pgsql
* Fix incorrect error code in StartupReplicationOrigin().Amit Kapila2021-08-30
| | | | | | | | | | ERRCODE_CONFIGURATION_LIMIT_EXCEEDED was used for checksum failure, use ERRCODE_DATA_CORRUPTED instead. Reported-by: Tatsuhito Kasahara Author: Tatsuhito Kasahara Backpatch-through: 9.6, where it was introduced Discussion: https://postgr.es/m/CAP0=ZVLHtYffs8SOWcFJWrBGoRzT9QQbk+_aP+E5AHLNXiOorA@mail.gmail.com
* Refactor sharedfileset.c to separate out fileset implementation.Amit Kapila2021-08-30
| | | | | | | | | | | | Move fileset related implementation out of sharedfileset.c to allow its usage by backends that don't want to share filesets among different processes. After this split, fileset infrastructure is used by both sharedfileset.c and worker.c for the named temporary files that survive across transactions. Author: Dilip Kumar, based on suggestion by Andres Freund Reviewed-by: Hou Zhijie, Masahiko Sawada, Amit Kapila Discussion: https://postgr.es/m/E1mCC6U-0004Ik-Fs@gemulon.postgresql.org
* Doc: add a little about LACON execution to src/backend/regex/README.Tom Lane2021-08-29
| | | | | | I wrote this while thinking about a possible optimization, but it's a useful description of the existing code regardless of whether the optimization ever happens. So push it separately.
* Keep stats up to date for partitioned tablesAlvaro Herrera2021-08-28
| | | | | | | | | | | | | | | | | | | | In the long-going saga for analyze on partitioned tables, one thing I missed while reverting 0827e8af70f4 is the maintenance of analyze count and last analyze time for partitioned tables. This is a mostly trivial change that enables users assess the need for invoking manual ANALYZE on partitioned tables. This patch, posted by Justin and modified a bit by me (Álvaro), can be mostly traced back to Hosoya-san, though any problems introduced with the scissors are mine. Backpatch to 14, in line with 6f8127b73901. Co-authored-by: Yuzuko Hosoya <yuzukohosoya@gmail.com> Co-authored-by: Justin Pryzby <pryzby@telsasoft.com> Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Reported-by: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://postgr.es/m/20210816222810.GE10479@telsasoft.com
* Fix data loss in wal_level=minimal crash recovery of CREATE TABLESPACE.Noah Misch2021-08-27
| | | | | | | | | | | | | | | | | | | | | | | | | If the system crashed between CREATE TABLESPACE and the next checkpoint, the result could be some files in the tablespace unexpectedly containing no rows. Affected files would be those for which the system did not write WAL; see the wal_skip_threshold documentation. Before v13, a different set of conditions governed the writing of WAL; see v12's <sect2 id="populate-pitr">. (The v12 conditions were broader in some ways and narrower in others.) Users may want to audit non-default tablespaces for unexpected short files. The bug could have truncated an index without affecting the associated table, and reindexing the index would fix that particular problem. This fixes the bug by making create_tablespace_directories() more like TablespaceCreateDbspace(). create_tablespace_directories() was recursively removing tablespace contents, reasoning that WAL redo would recreate everything removed that way. That assumption holds for other wal_level values. Under wal_level=minimal, the old approach could delete files for which no other copy existed. Back-patch to 9.6 (all supported versions). Reviewed by Robert Haas and Prabhat Sahu. Reported by Robert Haas. Discussion: https://postgr.es/m/CA+TgmoaLO9ncuwvr2nN-J4VEP5XyAcy=zKiHxQzBbFRxxGxm0w@mail.gmail.com
* Count SP-GiST index scans in pg_stat statistics.Tom Lane2021-08-27
| | | | | | | | | | | | | | | | | | | Somehow, spgist overlooked the need to call pgstat_count_index_scan(). Hence, pg_stat_all_indexes.idx_scan and equivalent columns never became nonzero for an SP-GiST index, although the related per-tuple counters worked fine. This fix works a bit differently from other index AMs, in that the counter increment occurs in spgrescan not spggettuple/spggetbitmap. It looks like this won't make the user-visible semantics noticeably different, so I won't go to the trouble of introducing an is-this- the-first-call flag just to make the counter bumps happen in the same places. Per bug #17163 from Christian Quest. Back-patch to all supported versions. Discussion: https://postgr.es/m/17163-b8c5cc88322a5e92@postgresql.org
* Use maintenance_io_concurrency for ANALYZE prefetchStephen Frost2021-08-27
| | | | | | | | | | | | | When prefetching pages for ANALYZE, we should be using maintenance_io_concurrenty (by calling get_tablespace_maintenance_io_concurrency(), not get_tablespace_io_concurrency()). ANALYZE prefetching was introduced in c6fc50c, so back-patch to 14. Backpatch-through: 14 Reported-By: Egor Rogov Discussion: https://postgr.es/m/9beada99-34ce-8c95-fadb-451768d08c64%40postgrespro.ru
* track_io_timing logging: Don't special case 0 ms.Peter Geoghegan2021-08-27
| | | | | | | | | | | | | | | Adjust track_io_timing related logging code added by commit 94d13d474d. Make it consistent with other nearby autovacuum and autoanalyze logging code by removing logic that suppressed zero millisecond outputs. log_autovacuum_min_duration log output now reliably shows "read:" and "write:" millisecond-based values in its report (when track_io_timing is enabled). Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Stephen Frost <sfrost@snowman.net> Discussion: https://postgr.es/m/CAH2-WznW0FNxSVQMSRazAMYNfZ6DR_gr5WE78hc6E1CBkkJpzw@mail.gmail.com Backpatch: 14-, where the track_io_timing logging was introduced.
* Reorder log_autovacuum_min_duration log output.Peter Geoghegan2021-08-27
| | | | | | | | | | This order seems more natural. It starts with details that are particular to heap and index data structures, and ends with system-level costs incurred during the autovacuum worker's VACUUM/ANALYZE operation. Author: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAH2-WzkzxK6ahA9xxsOftRtBX_R0swuHZsvo4QUbak1Bz7hb7Q@mail.gmail.com Backpatch: 14-, which enhanced the log output in various ways.
* vacuumlazy.c: Remove unnecessary parentheses.Peter Geoghegan2021-08-27
| | | | | | This was arguably a minor oversight in commit b4af70cb, which cleaned up the function signatures of functions that modify IndexBulkDeleteResult variables.
* Handle interaction of regexp's makesearch and MATCHALL more honestly.Tom Lane2021-08-27
| | | | | | | | | | | | | | | Second thoughts about commit 824bf7190: we apply makesearch() to an NFA after having determined whether it is a MATCHALL pattern. Prepending ".*" doesn't make it non-MATCHALL, but it does change the maximum possible match length, and makesearch() failed to update that. This has no ill effects given the stylized usage of search NFAs, but it seems like it's better to keep the data structure consistent. In particular, fixing this allows more honest handling of the MATCHALL check in matchuntil(): we can now assert that maxmatchall is infinity, instead of lamely assuming that it should act that way. In passing, improve the code in dump[c]nfa so that infinite maxmatchall is printed as "inf" not a magic number.
* Add logical change details to logical replication worker errcontext.Amit Kapila2021-08-27
| | | | | | | | | | | | | | | | | | | | | Previously, on the subscriber, we set the error context callback for the tuple data conversion failures. This commit replaces the existing error context callback with a comprehensive one so that it shows not only the details of data conversion failures but also the details of logical change being applied by the apply worker or table sync worker. The additional information displayed will be the command, transaction id, and timestamp. The error context is added to an error only when applying a change but not while doing other work like receiving data etc. This will help users in diagnosing the problems that occur during logical replication. It also can be used for future work that allows skipping a particular transaction on the subscriber. Author: Masahiko Sawada Reviewed-by: Hou Zhijie, Greg Nancarrow, Haiying Tang, Amit Kapila Tested-by: Haiying Tang Discussion: https://postgr.es/m/CAD21AoDeScrsHhLyEPYqN3sydg6PxAPVBboK=30xJfUVihNZDA@mail.gmail.com
* Fix handling of partitioned index in RelationGetNumberOfBlocksInFork()Peter Eisentraut2021-08-26
| | | | | | | | | | | | | Since a partitioned index doesn't have storage, getting the number of blocks from it will not give sensible results. Existing callers already check that they don't call it that way, so there doesn't appear to be a live problem. But for correctness, handle RELKIND_PARTITIONED_INDEX together with the other non-storage relkinds. Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/1d3a5fbe-f48b-8bea-80da-9a5c4244aef9@enterprisedb.com
* Remove redundant test.Tom Lane2021-08-25
| | | | | | | | | | The condition "context_start < context_end" is strictly weaker than "context_end - context_start >= 50", so we don't need both. Oversight in commit ffd3944ab, noted by tanghy.fnst. In passing, line-wrap a nearby test to make it more readable. Discussion: https://postgr.es/m/OS0PR01MB61137C4054774F44E3A9DC89FBC69@OS0PR01MB6113.jpnprd01.prod.outlook.com
* Fix broken snapshot handling in parallel workers.Robert Haas2021-08-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | Pengchengliu reported an assertion failure in a parallel woker while performing a parallel scan using an overflowed snapshot. The proximate cause is that TransactionXmin was set to an incorrect value. The underlying cause is incorrect snapshot handling in parallel.c. In particular, InitializeParallelDSM() was unconditionally calling GetTransactionSnapshot(), because I (rhaas) mistakenly thought that was always retrieving an existing snapshot whereas, at isolation levels less than REPEATABLE READ, it's actually taking a new one. So instead do this only at higher isolation levels where there actually is a single snapshot for the whole transaction. By itself, this is not a sufficient fix, because we still need to guarantee that TransactionXmin gets set properly in the workers. The easiest way to do that seems to be to install the leader's active snapshot as the transaction snapshot if the leader did not serialize a transaction snapshot. This doesn't affect the results of future GetTrasnactionSnapshot() calls since those have to take a new snapshot anyway; what we care about is the side effect of setting TransactionXmin. Report by Pengchengliu. Patch by Greg Nancarrow, except for some comment text which I supplied. Discussion: https://postgr.es/m/002f01d748ac$eaa781a0$bff684e0$@tju.edu.cn
* Fix toast rewrites in logical decoding.Amit Kapila2021-08-25
| | | | | | | | | | | | | | | | | | | Commit 325f2ec555 introduced pg_class.relwrite to skip operations on tables created as part of a heap rewrite during DDL. It links such transient heaps to the original relation OID via this new field in pg_class but forgot to do anything about toast tables. So, logical decoding was not able to skip operations on internally created toast tables. This leads to an error when we tried to decode the WAL for the next operation for which it appeared that there is a toast data where actually it didn't have any toast data. To fix this, we set pg_class.relwrite for internally created toast tables as well which allowed skipping operations on them during logical decoding. Author: Bertrand Drouvot Reviewed-by: David Zhang, Amit Kapila Backpatch-through: 11, where it was introduced Discussion: https://postgr.es/m/b5146fb1-ad9e-7d6e-f980-98ed68744a7c@amazon.com
* Avoid using ambiguous word "positive" in error message.Fujii Masao2021-08-25
| | | | | | | | | | | | | | There are two identical error messages about valid value of modulus for hash partition, in PostgreSQL source code. Commit 0e1275fb07 improved only one of them so that ambiguous word "positive" was avoided there, and forgot to improve the other. This commit improves the other. Which would reduce translator burden. Back-pach to v11 where the error message exists. Author: Kyotaro Horiguchi Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/20210819.170315.1413060634876301811.horikyota.ntt@gmail.com
* Improve error message about valid value for distance in phrase operator.Fujii Masao2021-08-25
| | | | | | | | | | | | | | The distance in phrase operator must be an integer value between zero and MAXENTRYPOS inclusive. But previously the error message about its valid value included the information about its upper limit but not lower limit (i.e., zero). This commit improves the error message so that it also includes the information about its lower limit. Back-patch to v9.6 where full-text phrase search was supported. Author: Kyotaro Horiguchi Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/20210819.170315.1413060634876301811.horikyota.ntt@gmail.com
* Fix regexp misbehavior with capturing parens inside "{0}".Tom Lane2021-08-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Regexps like "(.){0}...\1" drew an "invalid backreference number". That's not unreasonable on its face, since the capture group will never be matched if it's iterated zero times. However, other engines such as Perl's don't complain about this, nor do we throw an error for related cases such as "(.)|\1", even though that backref can never succeed either. Also, if the zero-iterations case happens at runtime rather than compile time --- say, "(x)*...\1" when there's no "x" to be found --- that's not an error, we just deem the backref to not match. Making this even less defensible, no error was thrown for nested cases such as "((.)){0}...\2"; and to add insult to injury, those cases could result in assertion failures instead. (It seems that nothing especially bad happened in non-assert builds, though.) Let's just fix it so that no error is thrown and instead the backref is deemed to never match, so that compile-time detection of no iterations behaves the same as run-time detection. Per report from Mark Dilger. This appears to be an aboriginal error in Spencer's library, so back-patch to all supported versions. Pre-v14, it turns out to also be necessary to back-patch one aspect of commits cb76fbd7e/00116dee5, namely to create capture-node subREs with the begin/end states of their subexpressions, not the current lp/rp of the outer parseqatom invocation. Otherwise delsub complains that we're trying to disconnect a state from itself. This is a bit scary but code examination shows that it's safe: in the pre-v14 code, if we want to wrap iteration around the subexpression, the first thing we do is overwrite the atom's begin/end fields with new states. So the bogus values didn't survive long enough to be used for anything, except if no iteration is required, in which case it doesn't matter. Discussion: https://postgr.es/m/A099E4A8-4377-4C64-A98C-3DEDDC075502@enterprisedb.com