- 30 Apr, 2019 7 commits
-
-
Tom Lane authored
The interaction of these parameters was a bit confused/confusing, and in fact v11 entirely misses the opportunity to apply partition constraints when a partition is accessed directly (rather than indirectly from its parent). In HEAD, establish the principle that enable_partition_pruning controls partition pruning and nothing else. When accessing a partition via its parent, we do partition pruning (if enabled by enable_partition_pruning) and then there is no need to consider partition constraints in the constraint_exclusion logic. When accessing a partition directly, its partition constraints are applied by the constraint_exclusion logic, only if constraint_exclusion = on. In v11, we can't have such a clean division of these GUCs' effects, partly because we don't want to break compatibility too much in a released branch, and partly because the clean coding requires inheritance_planner to have applied partition pruning to a partitioned target table, which it doesn't in v11. However, we can tweak things enough to cover the missed case, which seems like a good idea since it's potentially a performance regression from v10. This patch keeps v11's previous behavior in which enable_partition_pruning overrides constraint_exclusion for an inherited target table, though. In HEAD, also teach relation_excluded_by_constraints that it's okay to use inheritable constraints when trying to prune a traditional inheritance tree. This might not be thought worthy of effort given that that feature is semi-deprecated now, but we have enough infrastructure that it only takes a couple more lines of code to do it correctly. Amit Langote and Tom Lane Discussion: https://postgr.es/m/9813f079-f16b-61c8-9ab7-4363cab28d80@lab.ntt.co.jp Discussion: https://postgr.es/m/29069.1555970894@sss.pgh.pa.us
-
Bruce Momjian authored
-
Bruce Momjian authored
-
Alvaro Herrera authored
-
Alvaro Herrera authored
Mistake in ab0dfc96; progress reporting would have wrapped around for indexes created with more than 2^31 tuples. Reported-by: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-Wz=WbNxc5ob5NJ9yqo2RMJ0q4HXDS30GVCobeCvC9A1L9A@mail.gmail.com
-
Andres Freund authored
When reindexing individual indexes on pg_class it was possible to either trigger an assertion failure: TRAP: FailedAssertion("!(!ReindexIsProcessingIndex(((index)->rd_id))) That's because reindex_index() called SetReindexProcessing() - which enables an asserts ensuring no index insertions happen into the index - before calling RelationSetNewRelfilenode(). That not correct for indexes on pg_class, because RelationSetNewRelfilenode() updates the relevant pg_class row, which needs to update the indexes. The are two reasons this wasn't noticed earlier. Firstly the bug doesn't trigger when reindexing all of pg_class, as reindex_relation has code "hiding" all yet-to-be-reindexed indexes. Secondly, the bug only triggers when the the update to pg_class doesn't turn out to be a HOT update - otherwise there's no index insertion to trigger the bug. Most of the time there's enough space, making this bug hard to trigger. To fix, move RelationSetNewRelfilenode() to before the SetReindexProcessing() (and, together with some other code, to outside of the PG_TRY()). To make sure the error checking intended by SetReindexProcessing() is more robust, modify CatalogIndexInsert() to check ReindexIsProcessingIndex() even when the update is a HOT update. Also add a few regression tests for REINDEXing of system catalogs. The last two improvements would have prevented some of the issues fixed in 5c156060 from being introduced in the first place. Reported-By: Michael Paquier Diagnosed-By: Tom Lane and Andres Freund Author: Andres Freund Reviewed-By: Tom Lane Discussion: https://postgr.es/m/20190418011430.GA19133@paquier.xyz Backpatch: 9.4-, the bug is present in all branches
-
Andres Freund authored
Most of these stem from d25f5191 "tableam: relation creation, VACUUM FULL/CLUSTER, SET TABLESPACE.". 1) To pass data to the relation_set_new_filenode() RelationSetNewRelfilenode() was made to update RelationData.rd_rel directly. That's not OK however, as it makes the relcache entries temporarily inconsistent. Which among other scenarios is a problem if a REINDEX targets an index on pg_class - the CatalogTupleUpdate() in RelationSetNewRelfilenode(). Presumably that was introduced because other places in the code do so - while those aren't "good practice" they don't appear to be actively buggy (e.g. because system tables may not be targeted). I (Andres) should have caught this while reviewing and signficantly evolving the code in that commit, mea culpa. Fix that by instead passing in the new RelFileNode as separate argument to relation_set_new_filenode() and rely on the relcache to update the catalog entry. Also revert that the RelationMapUpdateMap() call was changed to immediate, and undo some other more unnecessary changes. 2) Document that the relation_set_new_filenode cannot rely on the whole relcache entry to be valid. It might be worthwhile to refactor the code to never have to rely on that, but given the way heap_create() is currently coded, that'd be a large change. 3) ATExecSetTableSpace() shouldn't do FlushRelationBuffers() itself. A table AM might not use shared buffers at all. Move to index_copy_data() and heapam_relation_copy_data(). 4) heapam_relation_set_new_filenode() previously sometimes accessed rel->rd_rel->relpersistence rather than the `persistence` argument. Code movement mistake. 5) Previously heapam_relation_set_new_filenode() re-opened the smgr relation to create the init for, if necesary. Instead have RelationCreateStorage() return the SMgrRelation and use it to create the init fork. 6) Add a note about the danger of modifying the relcache directly to ATExecSetTableSpace() - it's currently not a bug because there's a check ERRORing for catalog tables. Regression tests and assertion improvements that together trigger the bug described in 1) will be added in a later commit, as there is a related bug on all branches. Reported-By: Michael Paquier Diagnosed-By: Tom Lane and Andres Freund Author: Andres Freund Reviewed-By: Tom Lane Discussion: https://postgr.es/m/20190418011430.GA19133@paquier.xyz
-
- 29 Apr, 2019 5 commits
-
-
Peter Geoghegan authored
Remove a comment that refers to a coding practice that was fully removed by commit a8b8f4db, which introduced MarkBufferDirty(). It looks like the comment was even obsolete before then, since it concerns write-ordering dependencies with synchronous buffer writes.
-
Tom Lane authored
This is quite unsafe, even for the case of ereport(FATAL) where we won't return control to the interrupted code, and despite this code's use of a flag to restrict the areas where we'd try to do it. It's possible for example that we interrupt malloc or free while that's holding a lock that's meant to protect against cross-thread interference. Then, any attempt to do malloc or free within ereport() will result in a deadlock, preventing the walreceiver process from exiting in response to SIGTERM. We hypothesize that this explains some hard-to-reproduce failures seen in the buildfarm. Hence, get rid of the immediate-exit code in WalRcvShutdownHandler, as well as the logic associated with WalRcvImmediateInterruptOK. Instead, we need to take care that potentially-blocking operations in the walreceiver's data transmission logic (libpqwalreceiver.c) will respond reasonably promptly to the process's latch becoming set and then call ProcessWalRcvInterrupts. Much of the needed code for that was already present in libpqwalreceiver.c. I refactored things a bit so that all the uses of PQgetResult use latch-aware waiting, but didn't need to do much more. These changes should be enough to ensure that libpqwalreceiver.c will respond promptly to SIGTERM whenever it's waiting to receive data. In principle, it could block for a long time while waiting to send data too, and this patch does nothing to guard against that. I think that that hazard is mostly theoretical though: such blocking should occur only if we fill the kernel's data transmission buffers, and we don't generally send enough data to make that happen without waiting for input. If we find out that the hazard isn't just theoretical, we could fix it by using PQsetnonblocking, but that would require more ticklish changes than I care to make now. This is a bug fix, but it seems like too big a change to push into the back branches without much more testing than there's time for right now. Perhaps we'll back-patch once we have more confidence in the change. Patch by me; thanks to Thomas Munro for review. Discussion: https://postgr.es/m/20190416070119.GK2673@paquier.xyz
-
Michael Paquier authored
Author: Daniel Gustafsson Discussion: https://postgr.es/m/42kEeWei6VxLGh12QbR08hiI5Pm-c3XgbK7qj393PSttEhVbnnQoFXHKzXjPRZLUpndWAfHIuZuUqGZBzyXadmEUCSqm9xphWur_I8vESMA=@yesql.se
-
Alvaro Herrera authored
-
Peter Eisentraut authored
If a temporary table with an identity column and ON COMMIT DROP is created in a single-statement transaction (not useful, but allowed), it would leave the catalog corrupted. We need to add a CommandCounterIncrement() so that PreCommit_on_commit_actions() sees the created dependency between table and sequence and can clean it up. The analogous and more useful case of doing this in a transaction block already runs some CommandCounterIncrement() before it gets to the on-commit cleanup, so it wasn't a problem in practical use. Several locations for placing the new CommandCounterIncrement() call were discussed. This patch places it at the end of standard_ProcessUtility(). That would also help if other commands were to create catalog entries that some on-commit action would like to see. Bug: #15631 Reported-by: Serge Latyntsev <dnsl48@gmail.com> Author: Peter Eisentraut <peter.eisentraut@2ndquadrant.com> Reviewed-by: Michael Paquier <michael@paquier.xyz>
-
- 28 Apr, 2019 4 commits
-
-
Tom Lane authored
Run renumber_oids.pl to move high-numbered OIDs down, as per pre-beta tasks specified by RELEASE_CHANGES. (The only change is 8394 -> 3428.) Also run reformat_dat_file.pl while I'm here. While looking at the reformat diffs, I chanced to notice that type jsonpath had typsend and typreceive = '-', which surely is not the intention given that jsonpath_send and jsonpath_recv exist. Fix that. It's safe to assume that these functions have never been tested :-(. I didn't try, but somebody should.
-
Noah Misch authored
Emacs wrongly indented hundreds of subsequent lines.
-
Tom Lane authored
Be more consistent about use of XXXGetDatum macros in new jsonpath code. This is mostly to avoid having code that looks randomly different from everyplace else that's doing the exact same thing. In pg_regress.c, avoid an unreferenced-function warning from compilers that don't understand pg_attribute_unused(). Putting the function inside the same #ifdef as its only caller is more straightforward coding anyway. In be-secure-openssl.c, avoid use of pg_attribute_unused() on a label. That's pretty creative, but there's no good reason to suppose that it's portable, and there's absolutely no need to use goto's here in the first place. (This wasn't actually causing any buildfarm complaints, but it's new code in v12 so it has no portability track record.)
-
Michael Paquier authored
This fixes a couple of grammar mistakes, typos and inconsistencies in the documentation. Particularly, the configuration parsing allows only "kB" to mean kilobyte but there were references in the docs to "KB". Some instances of the latter are still in the code comments. Some parameter values were mentioned with "Minus-one", and using directly "-1" with proper markups is more helpful to the reader. Some of these have been pointed out by Justin, and some others are things I bumped into. Author: Justin Pryzby, Michael Paquier Reviewed-by: Tom Lane Discussion: https://postgr.es/m/20190330224333.GQ5815@telsasoft.com
-
- 27 Apr, 2019 7 commits
-
-
Tom Lane authored
foreign_grouping_ok() is willing to put fairly arbitrary expressions into the targetlist of a remote SELECT that's doing grouping or aggregation on the remote side, including expressions that have no foreign component to them at all. This is possibly a bit dubious from an efficiency standpoint; but it rises to the level of a crash-causing bug if the expression is just a Param or non-foreign Var. In that case, the expression will necessarily also appear in the fdw_exprs list of values we need to send to the remote server, and then setrefs.c's set_foreignscan_references will mistakenly replace the fdw_exprs entry with a Var referencing the targetlist result. The root cause of this problem is bad design in commit e7cb7ee1: it put logic into set_foreignscan_references that IMV is postgres_fdw-specific, and yet this bug shows that it isn't postgres_fdw-specific enough. The transformation being done on fdw_exprs assumes that fdw_exprs is to be evaluated with the fdw_scan_tlist as input, which is not how postgres_fdw uses it; yet it could be the right thing for some other FDW. (In the bigger picture, setrefs.c has no business assuming this for the other expression fields of a ForeignScan either.) The right fix therefore would be to expand the FDW API so that the FDW could inform setrefs.c how it intends to evaluate these various expressions. We can't change that in the back branches though, and we also can't just summarily change setrefs.c's behavior there, or we're likely to break external FDWs. As a stopgap, therefore, hack up postgres_fdw so that it won't attempt to send targetlist entries that look exactly like the fdw_exprs entries they'd produce. In most cases this actually produces a superior plan, IMO, with less data needing to be transmitted and returned; so we probably ought to think harder about whether we should ship tlist expressions at all when they don't contain any foreign Vars or Aggs. But that's an optimization not a bug fix so I left it for later. One case where this produces an inferior plan is where the expression in question is actually a GROUP BY expression: then the restriction prevents us from using remote grouping. It might be possible to work around that (since that would reduce to group-by-a-constant on the remote side); but it seems like a pretty unlikely corner case, so I'm not sure it's worth expending effort solely to improve that. In any case the right long-term answer is to fix the API as sketched above, and then revert this hack. Per bug #15781 from Sean Johnston. Back-patch to v10 where the problem was introduced. Discussion: https://postgr.es/m/15781-2601b1002bad087c@postgresql.org
-
Joe Conway authored
Recently added SVG image for storage page layout lacks a viewBox attribute which seems necessary to ensure propoer rendering. Add it. Author: Jonathan Katz Discussion: https://postgr.es/m/ba31e0e1-4c9b-b309-70e8-8e7ac14fc87e%40postgresql.org
-
Joe Conway authored
Recently added guidance on adding SVG images to the documentation sources lacks advice on making the images responsive when rendered in a variety of media types and viewports. Add some. Patch by Jonathan Katz with some editorialization by me. Author: Jonathan Katz Discussion: https://postgr.es/m/6358ae6f-7191-a02b-e7b5-68050636ae71@postgresql.org
-
Joe Conway authored
As pointed out by documentation comment, the URL for PL/R needs to be updated to the correct current repository. Back-patch to all supported branches.
-
Peter Eisentraut authored
-
Tom Lane authored
Missed an inttypes.h dependency in previous patch. Per buildfarm.
-
Michael Paquier authored
The documentation includes a section about index maintenance and reindexing, mentioning a set of steps based on CREATE INDEX CONCURRENTLY and ALTER TABLE (for constraint dependencies) to emulate REINDEX CONCURRENTLY. Now that REINDEX CONCURRENTLY is supported, let's just directly mention it instead. Reported-by: Peter Geoghegan Author: Michael Paquier Reviewed-by: Peter Eisentraut, Tom Lane Discussion: https://postgr.es/m/CAH2-WzmEL168t6w29aKrKXtpq9-apcmp0HC7K-fKt6ZgLXV6Dg@mail.gmail.com
-
- 26 Apr, 2019 7 commits
-
-
Tom Lane authored
This corrects a small bug in zic that caused it to output an incorrect year-2440 transition in the Africa/Casablanca zone. More interestingly, zic has grown a "-r" option that limits the range of zone transitions that it will put into the output files. That might be useful to people who don't like the weird GMT offsets that tzdb likes to use for very old dates. It appears that for dates before the cutoff time specified with -r, zic will use the zone's standard-time offset as of the cutoff time. So for example one might do make install ZIC_OPTIONS='-r @-1893456000' to cause all dates before 1910-01-01 to be treated as though 1910 standard time prevailed indefinitely far back. (Don't blame me for the unfriendly way of specifying the cutoff time --- it's seconds since or before the Unix epoch. You can use extract(epoch ...) to calculate it.) As usual, back-patch to all supported branches.
-
Tom Lane authored
DST law changes in Palestine and Metlakatla. Historical corrections for Israel. Etc/UCT is now a backward-compatibility link to Etc/UTC, instead of being a separate zone that generates the abbreviation "UCT", which nowadays is typically a typo. Postgres will still accept "UCT" as an input zone name, but it won't output it.
-
Peter Eisentraut authored
-
Tom Lane authored
Fix DefineIndex so that it doesn't attempt to pass down a to-be-reused index relfilenode to a child index creation, and fix TryReuseIndex to not think that reuse is sensible for a partitioned index. In v11, this fixes a problem where ALTER TABLE on a partitioned table could assign the same relfilenode to several different child indexes, causing very nasty catalog corruption --- in fact, attempting to DROP the partitioned table then leads not only to a database crash, but to inability to restart because the same crash will recur during WAL replay. Either of these two changes would be enough to prevent the failure, but since neither action could possibly be sane, let's put in both changes for future-proofing. In HEAD, no such bug manifests, but that's just an accidental consequence of having changed the pg_class representation of partitioned indexes to have relfilenode = 0. Both of these changes still seem like smart future-proofing. This is only a stop-gap because the code for ALTER TABLE on a partitioned table with a no-op type change still leaves a great deal to be desired. As the added regression tests show, it gets things wrong for comments on child indexes/constraints, and it is regenerating child indexes it doesn't have to. However, fixing those problems will take more work which may not get back-patched into v11. We need a fix for the corruption problem now. Per bug #15672 from Jianing Yang. Patch by me, regression test cases based on work by Amit Langote, who also did a lot of the investigative work. Discussion: https://postgr.es/m/15672-b9fa7db32698269f@postgresql.org
-
Alvaro Herrera authored
Commit f831d4ac changed pg_dump to emit (and pg_restore to understand) NULLs for unused members in ArchiveEntry structs, as a side effect of some code beautification. That broke pg_restore of dumps generated with older pg_dump, however, so it was reverted in 19455c9f. Since the archiver version number has been bumped in 3b925e90, we can put it back. Author: Dmitry Dolgov Discussion: https://postgr.es/m/CA+q6zcXx0XHqLsFJLaUU2j5BDiBAHig=YRoBC_YVq7VJGvzBEA@mail.gmail.com
-
Peter Eisentraut authored
The old section was ancient and didn't seem very helpful. Here, we add some concrete advice on particular mount options. Reviewed-by: Joe Conway <mail@joeconway.com> Discussion: https://www.postgresql.org/message-id/flat/e90f24bb-5423-6abb-58ec-501176eb4afc%402ndquadrant.com
-
Etsuro Fujita authored
Author: Laurenz Albe and Etsuro Fujita Reviewed-by: Laurenz Albe and Amit Langote Backpatch-through: 11 where support for that by FDWs was added Discussion: https://postgr.es/m/bf36a0288e8f31b4f2f40952e225bf892dc1ffc5.camel@cybertec.at
-
- 25 Apr, 2019 5 commits
-
-
Peter Geoghegan authored
Adopt a more defensive approach to accessing index tuples in contrib/amcheck: verify that each line pointer looks sane before accessing associated tuple using pointer arithmetic based on line pointer's offset. This avoids undefined behavior and assertion failures in cases where line pointers are corrupt. Issue spotted following a complaint about an assertion failure by Grigory Smolkin, which involved a test harness that deliberately corrupts indexes. This is arguably a bugfix, but no backpatch given the lack of field reports beyond Grigory's. Discussion: https://postgr.es/m/CAH2-WzmkurhCqnyLHxk0VkOZqd49+ZZsp1xAJOg7j2x7dmp_XQ@mail.gmail.com
-
Alvaro Herrera authored
When an existing index in a partition is attached to a new index on its parent, we forgot to set the "relispartition" flag correctly, which meant that it was not possible to find the index in various operations, such as adding a foreign key constraint that references that partitioned table. One of four places that was assigning the parent index was forgetting to do that, so fix by shifting responsibility of updating the flag to the routine that changes the parent. Author: Amit Langote, Álvaro Herrera Reported-by: Hubert "depesz" Lubaczewski Discussion: https://postgr.es/m/CA+HiwqHMsRtRYRWYTWavKJ8x14AFsv7bmAV46mYwnfD3vy8goQ@mail.gmail.com
-
Fujii Masao authored
-
Fujii Masao authored
Commit 3eb77eba renamed some functions, but forgot to update some comments referencing to those functions. This commit fixes those function names in the comments. Kyotaro Horiguchi
-
Alvaro Herrera authored
Commit ca410302 left a few loose ends. The most important one (broken pg_dump output) is already fixed by virtue of commit 3b23552a, but some things remained: * When ALTER TABLE rewrites tables, the indexes must remain in the tablespace they were originally in. This didn't work because index recreation during ALTER TABLE runs manufactured SQL (yuck), which runs afoul of default_tablespace in competition with the parent relation tablespace. To fix, reset default_tablespace to the empty string temporarily, and add the TABLESPACE clause as appropriate. * Setting a partitioned rel's tablespace to the database default is confusing; if it worked, it would direct the partitions to that tablespace regardless of default_tablespace. But in reality it does not work, and making it work is a larger project. Therefore, throw an error when this condition is detected, to alert the unwary. Add some docs and tests, too. Author: Álvaro Herrera Discussion: https://postgr.es/m/CAKJS1f_1c260nOt_vBJ067AZ3JXptXVRohDVMLEBmudX1YEx-A@mail.gmail.com
-
- 24 Apr, 2019 5 commits
-
-
Alvaro Herrera authored
Using PARTITION OF can result in column ordering being changed from the database being dumped, if the partition uses a column layout different from the parent's. It's not pg_dump's job to editorialize on table definitions, so this is not acceptable; back-patch all the way back to pg10, where partitioned tables where introduced. This change also ensures that partitions end up in the correct tablespace, if different from the parent's; this is an oversight in ca410302 (in pg12 only). Partitioned indexes (in pg11) don't have this problem, because they're already created as independent indexes and attached to their parents afterwards. This change also has the advantage that the partition is restorable from the dump (as a standalone table) even if its parent table isn't restored. Author: David Rowley Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/CAKJS1f_1c260nOt_vBJ067AZ3JXptXVRohDVMLEBmudX1YEx-A@mail.gmail.com Discussion: https://postgr.es/m/20190423185007.GA27954@alvherre.pgsql
-
Tom Lane authored
In sigusr1_handler, don't ignore PMSIGNAL_ADVANCE_STATE_MACHINE based on pmState. The restriction is unnecessary (PostmasterStateMachine should work in any state), not future-proof (since it makes too many assumptions about why the signal might be sent), and broken even today because a race condition can make it necessary to respond to the signal in PM_WAIT_READONLY state. The race condition seems unlikely, but if it did happen, a hot-standby postmaster could fail to shut down after receiving a smart-shutdown request. In MaybeStartWalReceiver, don't clear the WalReceiverRequested flag if the fork attempt fails. Leaving it set allows us to try again in future iterations of the postmaster idle loop. (The startup process would eventually send a fresh request signal, but this change may allow us to retry the fork sooner.) Remove an obsolete comment and unnecessary test in PostmasterStateMachine's handling of PM_SHUTDOWN_2 state. It's not possible to have a live walreceiver in that state, and AFAICT has not been possible since commit 5e85315e. This isn't a live bug, but the false comment is quite confusing to readers. In passing, rearrange sigusr1_handler's CheckPromoteSignal tests so that we don't uselessly perform stat() calls that we're going to ignore the results of. Add some comments clarifying the behavior of MaybeStartWalReceiver; I very nearly rearranged it in a way that'd reintroduce the race condition fixed in e5d494d7. Mea culpa for not commenting that properly at the time. Back-patch to all supported branches. The PMSIGNAL_ADVANCE_STATE_MACHINE change is the only one of even minor significance, but we might as well keep this code in sync across branches. Discussion: https://postgr.es/m/9001.1556046681@sss.pgh.pa.us
-
Alvaro Herrera authored
... for translatability purposes.
-
Etsuro Fujita authored
Commit 3d956d95 added support for update row movement in postgres_fdw. This patch fixes the following issues introduced by that commit: * When a remote partition chosen to insert routed rows into was also an UPDATE subplan target rel that would be updated later, the UPDATE that used a direct modification plan modified those routed rows incorrectly because those routed rows were visible to the later UPDATE command. The right fix for this would be to have some way in postgres_fdw in which the later UPDATE command ignores those routed rows, but it seems hard to do so with the current infrastructure. For now throw an error in that case. * When a remote partition chosen to insert routed rows into was also an UPDATE subplan target rel, fmstate created for the UPDATE that used a non-direct modification plan was mistakenly overridden by another fmstate created for inserting those routed rows into the partition. This caused 1) server crash when the partition would be updated later, and 2) resource leak when the partition had been already updated. To avoid that, adjust the treatment of the fmstate for the inserting. As for #1, since we would also have the incorrectness issue as mentioned above, error out in that case as well. Update the docs to mention that postgres_fdw currently does not handle the case where a remote partition chosen to insert a routed row into is also an UPDATE subplan target rel that will be updated later. Author: Amit Langote and Etsuro Fujita Reviewed-by: Amit Langote Backpatch-through: 11 where row movement in postgres_fdw was added Discussion: https://postgr.es/m/21e7eaa4-0d4d-20c2-a1f7-c7e96f4ce440@lab.ntt.co.jp
-
Andres Freund authored
This allows table AMs that don't need these horizons. This was already documented in the tableam relation_set_new_filenode callback, but an assert prevented if from actually working (the test AM code contained the change itself). Defang the asserts in the general code, and move the stronger ones into heap AM. Relatedly, after CLUSTER/VACUUM, we'd always assign a relfrozenxid / relminmxid. Change the table_relation_copy_for_cluster() interface to allow the AM to overwrite the horizons that get set on the pg_class entry. This'd also in the future allow AMs like heap to compute a relfrozenxid during rewrite that's the table's actual minimum rather than a pre-determined value. Arguably it'd have been better to move the whole computation / setting of those values into the callback, but it seems likely that for other reasons it'd be better to be able to use one value to vacuum/cluster multiple tables (e.g. a toast's horizon shouldn't be different than the table's). Reported-By: Heikki Linnakangas Author: Andres Freund Discussion: https://postgr.es/m/9a7fb9cc-2419-5db7-8840-ddc10c93f122@iki.fi
-