From 2376361839091b0dcdcc0b77f938b809b5f21646 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Thu, 25 Feb 2021 14:32:18 -0800 Subject: [PATCH 1/5] VACUUM VERBOSE: Count "newly deleted" index pages. Teach VACUUM VERBOSE to report on pages deleted by the _current_ VACUUM operation -- these are newly deleted pages. VACUUM VERBOSE continues to report on the total number of deleted pages in the entire index (no change there). The former is a subset of the latter. The distinction between each category of deleted index page only arises with index AMs where page deletion is supported and is decoupled from page recycling for performance reasons. This is follow-up work to commit e5d8a999, which made nbtree store 64-bit XIDs (not 32-bit XIDs) in pages at the point at which they're deleted. Note that the btm_last_cleanup_num_delpages metapage field added by that commit usually gets set to pages_newly_deleted. The exceptions (the scenarios in which they're not equal) all seem to be tricky cases for the implementation (of page deletion and recycling) in general. Author: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-WznpdHvujGUwYZ8sihX%3Dd5u-tRYhi-F4wnV2uN2zHpMUXw%40mail.gmail.com --- src/backend/access/gin/ginvacuum.c | 1 + src/backend/access/gist/gistvacuum.c | 19 ++++++++-- src/backend/access/heap/vacuumlazy.c | 4 +- src/backend/access/nbtree/nbtpage.c | 54 +++++++++++++++------------ src/backend/access/nbtree/nbtree.c | 33 ++++++---------- src/backend/access/spgist/spgvacuum.c | 1 + src/include/access/genam.h | 10 +++-- src/include/access/nbtree.h | 16 +++++++- 8 files changed, 85 insertions(+), 53 deletions(-) diff --git a/src/backend/access/gin/ginvacuum.c b/src/backend/access/gin/ginvacuum.c index a0453b36cde5b..a276eb020b5dd 100644 --- a/src/backend/access/gin/ginvacuum.c +++ b/src/backend/access/gin/ginvacuum.c @@ -231,6 +231,7 @@ ginDeletePage(GinVacuumState *gvs, BlockNumber deleteBlkno, BlockNumber leftBlkn END_CRIT_SECTION(); + gvs->result->pages_newly_deleted++; gvs->result->pages_deleted++; } diff --git a/src/backend/access/gist/gistvacuum.c b/src/backend/access/gist/gistvacuum.c index ddecb8ab18ef4..0663193531a73 100644 --- a/src/backend/access/gist/gistvacuum.c +++ b/src/backend/access/gist/gistvacuum.c @@ -133,9 +133,21 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats, MemoryContext oldctx; /* - * Reset counts that will be incremented during the scan; needed in case - * of multiple scans during a single VACUUM command. + * Reset fields that track information about the entire index now. This + * avoids double-counting in the case where a single VACUUM command + * requires multiple scans of the index. + * + * Avoid resetting the tuples_removed and pages_newly_deleted fields here, + * since they track information about the VACUUM command, and so must last + * across each call to gistvacuumscan(). + * + * (Note that pages_free is treated as state about the whole index, not + * the current VACUUM. This is appropriate because RecordFreeIndexPage() + * calls are idempotent, and get repeated for the same deleted pages in + * some scenarios. The point for us is to track the number of recyclable + * pages in the index at the end of the VACUUM command.) */ + stats->num_pages = 0; stats->estimated_count = false; stats->num_index_tuples = 0; stats->pages_deleted = 0; @@ -281,8 +293,8 @@ gistvacuumpage(GistVacState *vstate, BlockNumber blkno, BlockNumber orig_blkno) { /* Okay to recycle this page */ RecordFreeIndexPage(rel, blkno); - vstate->stats->pages_free++; vstate->stats->pages_deleted++; + vstate->stats->pages_free++; } else if (GistPageIsDeleted(page)) { @@ -636,6 +648,7 @@ gistdeletepage(IndexVacuumInfo *info, IndexBulkDeleteResult *stats, /* mark the page as deleted */ MarkBufferDirty(leafBuffer); GistPageSetDeleted(leafPage, txid); + stats->pages_newly_deleted++; stats->pages_deleted++; /* remove the downlink from the parent */ diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 0bb78162f546d..d8f847b0e6673 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -2521,9 +2521,11 @@ lazy_cleanup_index(Relation indrel, (*stats)->num_index_tuples, (*stats)->num_pages), errdetail("%.0f index row versions were removed.\n" - "%u index pages have been deleted, %u are currently reusable.\n" + "%u index pages were newly deleted.\n" + "%u index pages are currently deleted, of which %u are currently reusable.\n" "%s.", (*stats)->tuples_removed, + (*stats)->pages_newly_deleted, (*stats)->pages_deleted, (*stats)->pages_free, pg_rusage_show(&ru0)))); } diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c index a43805a7b09e1..629a23628ef6c 100644 --- a/src/backend/access/nbtree/nbtpage.c +++ b/src/backend/access/nbtree/nbtpage.c @@ -50,7 +50,7 @@ static bool _bt_mark_page_halfdead(Relation rel, Buffer leafbuf, static bool _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, BlockNumber scanblkno, bool *rightsib_empty, - uint32 *ndeleted); + BTVacState *vstate); static bool _bt_lock_subtree_parent(Relation rel, BlockNumber child, BTStack stack, Buffer *subtreeparent, @@ -1760,20 +1760,22 @@ _bt_rightsib_halfdeadflag(Relation rel, BlockNumber leafrightsib) * should never pass a buffer containing an existing deleted page here. The * lock and pin on caller's buffer will be dropped before we return. * - * Returns the number of pages successfully deleted (zero if page cannot - * be deleted now; could be more than one if parent or right sibling pages - * were deleted too). Note that this does not include pages that we delete - * that the btvacuumscan scan has yet to reach; they'll get counted later - * instead. + * Maintains bulk delete stats for caller, which are taken from vstate. We + * need to cooperate closely with caller here so that whole VACUUM operation + * reliably avoids any double counting of subsidiary-to-leafbuf pages that we + * delete in passing. If such pages happen to be from a block number that is + * ahead of the current scanblkno position, then caller is expected to count + * them directly later on. It's simpler for us to understand caller's + * requirements than it would be for caller to understand when or how a + * deleted page became deleted after the fact. * * NOTE: this leaks memory. Rather than trying to clean up everything * carefully, it's better to run it in a temp context that can be reset * frequently. */ -uint32 -_bt_pagedel(Relation rel, Buffer leafbuf) +void +_bt_pagedel(Relation rel, Buffer leafbuf, BTVacState *vstate) { - uint32 ndeleted = 0; BlockNumber rightsib; bool rightsib_empty; Page page; @@ -1781,7 +1783,8 @@ _bt_pagedel(Relation rel, Buffer leafbuf) /* * Save original leafbuf block number from caller. Only deleted blocks - * that are <= scanblkno get counted in ndeleted return value. + * that are <= scanblkno are added to bulk delete stat's pages_deleted + * count. */ BlockNumber scanblkno = BufferGetBlockNumber(leafbuf); @@ -1843,7 +1846,7 @@ _bt_pagedel(Relation rel, Buffer leafbuf) RelationGetRelationName(rel)))); _bt_relbuf(rel, leafbuf); - return ndeleted; + return; } /* @@ -1873,7 +1876,7 @@ _bt_pagedel(Relation rel, Buffer leafbuf) Assert(!P_ISHALFDEAD(opaque)); _bt_relbuf(rel, leafbuf); - return ndeleted; + return; } /* @@ -1922,8 +1925,7 @@ _bt_pagedel(Relation rel, Buffer leafbuf) if (_bt_leftsib_splitflag(rel, leftsib, leafblkno)) { ReleaseBuffer(leafbuf); - Assert(ndeleted == 0); - return ndeleted; + return; } /* we need an insertion scan key for the search, so build one */ @@ -1964,7 +1966,7 @@ _bt_pagedel(Relation rel, Buffer leafbuf) if (!_bt_mark_page_halfdead(rel, leafbuf, stack)) { _bt_relbuf(rel, leafbuf); - return ndeleted; + return; } } @@ -1979,7 +1981,7 @@ _bt_pagedel(Relation rel, Buffer leafbuf) { /* Check for interrupts in _bt_unlink_halfdead_page */ if (!_bt_unlink_halfdead_page(rel, leafbuf, scanblkno, - &rightsib_empty, &ndeleted)) + &rightsib_empty, vstate)) { /* * _bt_unlink_halfdead_page should never fail, since we @@ -1990,7 +1992,7 @@ _bt_pagedel(Relation rel, Buffer leafbuf) * lock and pin on leafbuf for us. */ Assert(false); - return ndeleted; + return; } } @@ -2026,8 +2028,6 @@ _bt_pagedel(Relation rel, Buffer leafbuf) leafbuf = _bt_getbuf(rel, rightsib, BT_WRITE); } - - return ndeleted; } /* @@ -2262,9 +2262,10 @@ _bt_mark_page_halfdead(Relation rel, Buffer leafbuf, BTStack stack) */ static bool _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, BlockNumber scanblkno, - bool *rightsib_empty, uint32 *ndeleted) + bool *rightsib_empty, BTVacState *vstate) { BlockNumber leafblkno = BufferGetBlockNumber(leafbuf); + IndexBulkDeleteResult *stats = vstate->stats; BlockNumber leafleftsib; BlockNumber leafrightsib; BlockNumber target; @@ -2674,12 +2675,17 @@ _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, BlockNumber scanblkno, _bt_relbuf(rel, buf); /* - * If btvacuumscan won't revisit this page in a future btvacuumpage call - * and count it as deleted then, we count it as deleted by current - * btvacuumpage call + * Maintain pages_newly_deleted, which is simply the number of pages + * deleted by the ongoing VACUUM operation. + * + * Maintain pages_deleted in a way that takes into account how + * btvacuumpage() will count deleted pages that have yet to become + * scanblkno -- only count page when it's not going to get that treatment + * later on. */ + stats->pages_newly_deleted++; if (target <= scanblkno) - (*ndeleted)++; + stats->pages_deleted++; return true; } diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c index 3b2e0aa5cb794..504f5bef17a43 100644 --- a/src/backend/access/nbtree/nbtree.c +++ b/src/backend/access/nbtree/nbtree.c @@ -38,17 +38,6 @@ #include "utils/memutils.h" -/* Working state needed by btvacuumpage */ -typedef struct -{ - IndexVacuumInfo *info; - IndexBulkDeleteResult *stats; - IndexBulkDeleteCallback callback; - void *callback_state; - BTCycleId cycleid; - MemoryContext pagedelcontext; -} BTVacState; - /* * BTPARALLEL_NOT_INITIALIZED indicates that the scan has not started. * @@ -1016,9 +1005,9 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats, * avoids double-counting in the case where a single VACUUM command * requires multiple scans of the index. * - * Avoid resetting the tuples_removed field here, since it tracks - * information about the VACUUM command, and so must last across each call - * to btvacuumscan(). + * Avoid resetting the tuples_removed and pages_newly_deleted fields here, + * since they track information about the VACUUM command, and so must last + * across each call to btvacuumscan(). * * (Note that pages_free is treated as state about the whole index, not * the current VACUUM. This is appropriate because RecordFreeIndexPage() @@ -1237,11 +1226,13 @@ btvacuumpage(BTVacState *vstate, BlockNumber scanblkno) } else if (P_ISHALFDEAD(opaque)) { + /* Half-dead leaf page (from interrupted VACUUM) -- finish deleting */ + attempt_pagedel = true; + /* - * Half-dead leaf page. Try to delete now. Might update - * pages_deleted below. + * _bt_pagedel() will increment both pages_newly_deleted and + * pages_deleted stats in all cases (barring corruption) */ - attempt_pagedel = true; } else if (P_ISLEAF(opaque)) { @@ -1451,12 +1442,12 @@ btvacuumpage(BTVacState *vstate, BlockNumber scanblkno) oldcontext = MemoryContextSwitchTo(vstate->pagedelcontext); /* - * We trust the _bt_pagedel return value because it does not include - * any page that a future call here from btvacuumscan is expected to - * count. There will be no double-counting. + * _bt_pagedel maintains the bulk delete stats on our behalf; + * pages_newly_deleted and pages_deleted are likely to be incremented + * during call */ Assert(blkno == scanblkno); - stats->pages_deleted += _bt_pagedel(rel, buf); + _bt_pagedel(rel, buf, vstate); MemoryContextSwitchTo(oldcontext); /* pagedel released buffer, so we shouldn't */ diff --git a/src/backend/access/spgist/spgvacuum.c b/src/backend/access/spgist/spgvacuum.c index 0d02a02222e9e..a9ffca5183bd2 100644 --- a/src/backend/access/spgist/spgvacuum.c +++ b/src/backend/access/spgist/spgvacuum.c @@ -891,6 +891,7 @@ spgvacuumscan(spgBulkDeleteState *bds) /* Report final stats */ bds->stats->num_pages = num_pages; + bds->stats->pages_newly_deleted = bds->stats->pages_deleted; bds->stats->pages_free = bds->stats->pages_deleted; } diff --git a/src/include/access/genam.h b/src/include/access/genam.h index ffa1a4c80dbe1..4515401869fb4 100644 --- a/src/include/access/genam.h +++ b/src/include/access/genam.h @@ -63,8 +63,11 @@ typedef struct IndexVacuumInfo * of which this is just the first field; this provides a way for ambulkdelete * to communicate additional private data to amvacuumcleanup. * - * Note: pages_deleted and pages_free refer to free space within the index - * file. Some index AMs may compute num_index_tuples by reference to + * Note: pages_newly_deleted is the number of pages in the index that were + * deleted by the current vacuum operation. pages_deleted and pages_free + * refer to free space within the index file. + * + * Note: Some index AMs may compute num_index_tuples by reference to * num_heap_tuples, in which case they should copy the estimated_count field * from IndexVacuumInfo. */ @@ -74,7 +77,8 @@ typedef struct IndexBulkDeleteResult bool estimated_count; /* num_index_tuples is an estimate */ double num_index_tuples; /* tuples remaining */ double tuples_removed; /* # removed during vacuum operation */ - BlockNumber pages_deleted; /* # unused pages in index */ + BlockNumber pages_newly_deleted; /* # pages marked deleted by us */ + BlockNumber pages_deleted; /* # pages marked deleted (could be by us) */ BlockNumber pages_free; /* # pages available for reuse */ } IndexBulkDeleteResult; diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h index 9ac90d7439836..b56b7b7868eb4 100644 --- a/src/include/access/nbtree.h +++ b/src/include/access/nbtree.h @@ -312,6 +312,20 @@ BTPageIsRecyclable(Page page) return false; } +/* + * BTVacState is private nbtree.c state used during VACUUM. It is exported + * for use by page deletion related code in nbtpage.c. + */ +typedef struct BTVacState +{ + IndexVacuumInfo *info; + IndexBulkDeleteResult *stats; + IndexBulkDeleteCallback callback; + void *callback_state; + BTCycleId cycleid; + MemoryContext pagedelcontext; +} BTVacState; + /* * Lehman and Yao's algorithm requires a ``high key'' on every non-rightmost * page. The high key is not a tuple that is used to visit the heap. It is @@ -1181,7 +1195,7 @@ extern void _bt_delitems_vacuum(Relation rel, Buffer buf, extern void _bt_delitems_delete_check(Relation rel, Buffer buf, Relation heapRel, TM_IndexDeleteOp *delstate); -extern uint32 _bt_pagedel(Relation rel, Buffer leafbuf); +extern void _bt_pagedel(Relation rel, Buffer leafbuf, BTVacState *vstate); /* * prototypes for functions in nbtsearch.c From 80ca8464fe02296c8efefd53746e6d6a3f456d1e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 25 Feb 2021 20:47:32 -0500 Subject: [PATCH 2/5] Fix list-manipulation bug in WITH RECURSIVE processing. makeDependencyGraphWalker and checkWellFormedRecursionWalker thought they could hold onto a pointer to a list's first cons cell while the list was modified by recursive calls. That was okay when the cons cell was actually separately palloc'd ... but since commit 1cff1b95a, it's quite unsafe, leading to core dumps or incorrect complaints of faulty WITH nesting. In the field this'd require at least a seven-deep WITH nest to cause an issue, but enabling DEBUG_LIST_MEMORY_USAGE allows the bug to be seen with lesser nesting depths. Per bug #16801 from Alexander Lakhin. Back-patch to v13. Michael Paquier and Tom Lane Discussion: https://postgr.es/m/16801-393c7922143eaa4d@postgresql.org --- src/backend/parser/parse_cte.c | 12 +++--- src/test/regress/expected/with.out | 59 ++++++++++++++++++++++++++++++ src/test/regress/sql/with.sql | 44 ++++++++++++++++++++++ 3 files changed, 109 insertions(+), 6 deletions(-) diff --git a/src/backend/parser/parse_cte.c b/src/backend/parser/parse_cte.c index f4f7041ead09d..f46d63d45131a 100644 --- a/src/backend/parser/parse_cte.c +++ b/src/backend/parser/parse_cte.c @@ -730,15 +730,15 @@ makeDependencyGraphWalker(Node *node, CteState *cstate) * In the non-RECURSIVE case, query names are visible to the * WITH items after them and to the main query. */ - ListCell *cell1; - cstate->innerwiths = lcons(NIL, cstate->innerwiths); - cell1 = list_head(cstate->innerwiths); foreach(lc, stmt->withClause->ctes) { CommonTableExpr *cte = (CommonTableExpr *) lfirst(lc); + ListCell *cell1; (void) makeDependencyGraphWalker(cte->ctequery, cstate); + /* note that recursion could mutate innerwiths list */ + cell1 = list_head(cstate->innerwiths); lfirst(cell1) = lappend((List *) lfirst(cell1), cte); } (void) raw_expression_tree_walker(node, @@ -1006,15 +1006,15 @@ checkWellFormedRecursionWalker(Node *node, CteState *cstate) * In the non-RECURSIVE case, query names are visible to the * WITH items after them and to the main query. */ - ListCell *cell1; - cstate->innerwiths = lcons(NIL, cstate->innerwiths); - cell1 = list_head(cstate->innerwiths); foreach(lc, stmt->withClause->ctes) { CommonTableExpr *cte = (CommonTableExpr *) lfirst(lc); + ListCell *cell1; (void) checkWellFormedRecursionWalker(cte->ctequery, cstate); + /* note that recursion could mutate innerwiths list */ + cell1 = list_head(cstate->innerwiths); lfirst(cell1) = lappend((List *) lfirst(cell1), cte); } checkWellFormedSelectStmt(stmt, cstate); diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out index c519a61c4fe79..a7a652822c914 100644 --- a/src/test/regress/expected/with.out +++ b/src/test/regress/expected/with.out @@ -176,6 +176,65 @@ ERROR: operator does not exist: text + integer LINE 4: SELECT n+1 FROM t WHERE n < 10 ^ HINT: No operator matches the given name and argument types. You might need to add explicit type casts. +-- Deeply nested WITH caused a list-munging problem in v13 +-- Detection of cross-references and self-references +WITH RECURSIVE w1(c1) AS + (WITH w2(c2) AS + (WITH w3(c3) AS + (WITH w4(c4) AS + (WITH w5(c5) AS + (WITH RECURSIVE w6(c6) AS + (WITH w6(c6) AS + (WITH w8(c8) AS + (SELECT 1) + SELECT * FROM w8) + SELECT * FROM w6) + SELECT * FROM w6) + SELECT * FROM w5) + SELECT * FROM w4) + SELECT * FROM w3) + SELECT * FROM w2) +SELECT * FROM w1; + c1 +---- + 1 +(1 row) + +-- Detection of invalid self-references +WITH RECURSIVE outermost(x) AS ( + SELECT 1 + UNION (WITH innermost1 AS ( + SELECT 2 + UNION (WITH innermost2 AS ( + SELECT 3 + UNION (WITH innermost3 AS ( + SELECT 4 + UNION (WITH innermost4 AS ( + SELECT 5 + UNION (WITH innermost5 AS ( + SELECT 6 + UNION (WITH innermost6 AS + (SELECT 7) + SELECT * FROM innermost6)) + SELECT * FROM innermost5)) + SELECT * FROM innermost4)) + SELECT * FROM innermost3)) + SELECT * FROM innermost2)) + SELECT * FROM outermost + UNION SELECT * FROM innermost1) + ) + SELECT * FROM outermost ORDER BY 1; + x +--- + 1 + 2 + 3 + 4 + 5 + 6 + 7 +(7 rows) + -- -- Some examples with a tree -- diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sql index f4ba0d8e39942..85a671c6300a2 100644 --- a/src/test/regress/sql/with.sql +++ b/src/test/regress/sql/with.sql @@ -95,6 +95,50 @@ UNION ALL ) SELECT n, pg_typeof(n) FROM t; +-- Deeply nested WITH caused a list-munging problem in v13 +-- Detection of cross-references and self-references +WITH RECURSIVE w1(c1) AS + (WITH w2(c2) AS + (WITH w3(c3) AS + (WITH w4(c4) AS + (WITH w5(c5) AS + (WITH RECURSIVE w6(c6) AS + (WITH w6(c6) AS + (WITH w8(c8) AS + (SELECT 1) + SELECT * FROM w8) + SELECT * FROM w6) + SELECT * FROM w6) + SELECT * FROM w5) + SELECT * FROM w4) + SELECT * FROM w3) + SELECT * FROM w2) +SELECT * FROM w1; +-- Detection of invalid self-references +WITH RECURSIVE outermost(x) AS ( + SELECT 1 + UNION (WITH innermost1 AS ( + SELECT 2 + UNION (WITH innermost2 AS ( + SELECT 3 + UNION (WITH innermost3 AS ( + SELECT 4 + UNION (WITH innermost4 AS ( + SELECT 5 + UNION (WITH innermost5 AS ( + SELECT 6 + UNION (WITH innermost6 AS + (SELECT 7) + SELECT * FROM innermost6)) + SELECT * FROM innermost5)) + SELECT * FROM innermost4)) + SELECT * FROM innermost3)) + SELECT * FROM innermost2)) + SELECT * FROM outermost + UNION SELECT * FROM innermost1) + ) + SELECT * FROM outermost ORDER BY 1; + -- -- Some examples with a tree -- From 8556267b2b1b8e1c26037c4c25cf390ee5afb5d9 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Fri, 26 Feb 2021 15:29:27 +1300 Subject: [PATCH 3/5] Revert "pg_collation_actual_version() -> pg_collation_current_version()." This reverts commit 9cf184cc0599b6e65e7e5ecd9d91cd42e278bcd8. Name change less well received than anticipated. Discussion: https://postgr.es/m/afcfb97e-88a1-a540-db95-6c573b93bc2b%40eisentraut.org --- doc/src/sgml/func.sgml | 8 ++++---- src/backend/commands/collationcmds.c | 2 +- src/backend/utils/adt/pg_locale.c | 14 +++++++------- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_proc.dat | 4 ++-- src/test/regress/expected/collate.icu.utf8.out | 6 +++--- src/test/regress/sql/collate.icu.utf8.sql | 6 +++--- 7 files changed, 21 insertions(+), 21 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index c5048a199886c..08f08322ca51b 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -26238,14 +26238,14 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); - pg_collation_current_version + pg_collation_actual_version - pg_collation_current_version ( oid ) + pg_collation_actual_version ( oid ) text - Returns the version of the collation object as reported by the ICU - library or operating system. null is returned + Returns the actual version of the collation object as it is currently + installed in the operating system. null is returned on operating systems where PostgreSQL doesn't have support for versions. diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c index 4b76a6051d234..a7ee452e192eb 100644 --- a/src/backend/commands/collationcmds.c +++ b/src/backend/commands/collationcmds.c @@ -268,7 +268,7 @@ IsThereCollationInNamespace(const char *collname, Oid nspOid) } Datum -pg_collation_current_version(PG_FUNCTION_ARGS) +pg_collation_actual_version(PG_FUNCTION_ARGS) { Oid collid = PG_GETARG_OID(0); char *version; diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c index df1f36132d38a..aa4874163f88e 100644 --- a/src/backend/utils/adt/pg_locale.c +++ b/src/backend/utils/adt/pg_locale.c @@ -127,8 +127,8 @@ static char *IsoLocaleName(const char *); /* MSVC specific */ static void icu_set_collation_attributes(UCollator *collator, const char *loc); #endif -static char *get_collation_current_version(char collprovider, - const char *collcollate); +static char *get_collation_actual_version(char collprovider, + const char *collcollate); /* * pg_perm_setlocale @@ -1610,7 +1610,7 @@ pg_newlocale_from_collation(Oid collid) * the operating system/library. */ static char * -get_collation_current_version(char collprovider, const char *collcollate) +get_collation_actual_version(char collprovider, const char *collcollate) { char *collversion = NULL; @@ -1717,8 +1717,8 @@ get_collation_version_for_oid(Oid oid, bool missing_ok) if (!HeapTupleIsValid(tp)) elog(ERROR, "cache lookup failed for database %u", MyDatabaseId); dbform = (Form_pg_database) GETSTRUCT(tp); - version = get_collation_current_version(COLLPROVIDER_LIBC, - NameStr(dbform->datcollate)); + version = get_collation_actual_version(COLLPROVIDER_LIBC, + NameStr(dbform->datcollate)); } else { @@ -1732,8 +1732,8 @@ get_collation_version_for_oid(Oid oid, bool missing_ok) elog(ERROR, "cache lookup failed for collation %u", oid); } collform = (Form_pg_collation) GETSTRUCT(tp); - version = get_collation_current_version(collform->collprovider, - NameStr(collform->collcollate)); + version = get_collation_actual_version(collform->collprovider, + NameStr(collform->collcollate)); } ReleaseSysCache(tp); diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index e94f97437045a..4cc94de224a85 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202102221 +#define CATALOG_VERSION_NO 202102191 #endif diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 16044125ba465..1487710d59077 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -11321,9 +11321,9 @@ { oid => '3448', descr => 'get actual version of collation from operating system', - proname => 'pg_collation_current_version', procost => '100', + proname => 'pg_collation_actual_version', procost => '100', provolatile => 'v', prorettype => 'text', proargtypes => 'oid', - prosrc => 'pg_collation_current_version' }, + prosrc => 'pg_collation_actual_version' }, # system management/monitoring related functions { oid => '3353', descr => 'list files in the log directory', diff --git a/src/test/regress/expected/collate.icu.utf8.out b/src/test/regress/expected/collate.icu.utf8.out index 43fbff1de2017..de70cb121274d 100644 --- a/src/test/regress/expected/collate.icu.utf8.out +++ b/src/test/regress/expected/collate.icu.utf8.out @@ -2018,7 +2018,7 @@ SELECT objid::regclass::text collate "C", refobjid::regcollation::text collate " CASE WHEN refobjid = 'default'::regcollation THEN 'XXX' -- depends on libc version support WHEN refobjversion IS NULL THEN 'version not tracked' -WHEN refobjversion = pg_collation_current_version(refobjid) THEN 'up to date' +WHEN refobjversion = pg_collation_actual_version(refobjid) THEN 'up to date' ELSE 'out of date' END AS version FROM pg_depend d @@ -2156,14 +2156,14 @@ RESET client_min_messages; -- leave a collation for pg_upgrade test CREATE COLLATION coll_icu_upgrade FROM "und-x-icu"; -- Test user-visible function for inspecting versions -SELECT pg_collation_current_version('"en-x-icu"'::regcollation) is not null; +SELECT pg_collation_actual_version('"en-x-icu"'::regcollation) is not null; ?column? ---------- t (1 row) -- Invalid OIDs are silently ignored -SELECT pg_collation_current_version(0) is null; +SELECT pg_collation_actual_version(0) is null; ?column? ---------- t diff --git a/src/test/regress/sql/collate.icu.utf8.sql b/src/test/regress/sql/collate.icu.utf8.sql index 8b341dbb2429a..dd5d208854747 100644 --- a/src/test/regress/sql/collate.icu.utf8.sql +++ b/src/test/regress/sql/collate.icu.utf8.sql @@ -820,7 +820,7 @@ SELECT objid::regclass::text collate "C", refobjid::regcollation::text collate " CASE WHEN refobjid = 'default'::regcollation THEN 'XXX' -- depends on libc version support WHEN refobjversion IS NULL THEN 'version not tracked' -WHEN refobjversion = pg_collation_current_version(refobjid) THEN 'up to date' +WHEN refobjversion = pg_collation_actual_version(refobjid) THEN 'up to date' ELSE 'out of date' END AS version FROM pg_depend d @@ -885,6 +885,6 @@ RESET client_min_messages; CREATE COLLATION coll_icu_upgrade FROM "und-x-icu"; -- Test user-visible function for inspecting versions -SELECT pg_collation_current_version('"en-x-icu"'::regcollation) is not null; +SELECT pg_collation_actual_version('"en-x-icu"'::regcollation) is not null; -- Invalid OIDs are silently ignored -SELECT pg_collation_current_version(0) is null; +SELECT pg_collation_actual_version(0) is null; From 329784e11849ff691f0157f3b27c50f652bce76a Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 26 Feb 2021 14:39:03 +0900 Subject: [PATCH 4/5] doc: Improve {archive,restore}_command for compressed logs The commands mentioned in the docs with gzip and gunzip did not prefix the archives with ".gz" and used inconsistent paths for the archives, which can be confusing. Reported-by: Philipp Gramzow Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/161397938841.15451.13129264141285167267@wrigleys.postgresql.org --- doc/src/sgml/backup.sgml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml index 3c8aaed0b6203..21094c6a9d056 100644 --- a/doc/src/sgml/backup.sgml +++ b/doc/src/sgml/backup.sgml @@ -1520,11 +1520,11 @@ tar -rf /var/lib/pgsql/backup.tar /var/lib/pgsql/archive/ If archive storage size is a concern, you can use gzip to compress the archive files: -archive_command = 'gzip < %p > /var/lib/pgsql/archive/%f' +archive_command = 'gzip < %p > /mnt/server/archivedir/%f.gz' You will then need to use gunzip during recovery: -restore_command = 'gunzip < /mnt/server/archivedir/%f > %p' +restore_command = 'gunzip < /mnt/server/archivedir/%f.gz > %p' From b3a9e9897ec702d56602b26a8cdc0950f23b29dc Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Fri, 26 Feb 2021 09:11:15 +0100 Subject: [PATCH 5/5] Extend a test case a little This will possibly help a subsequent patch by making sure the notice messages are distinct so that it's clear that they come out in the right order. Author: Fabien COELHO Discussion: https://www.postgresql.org/message-id/alpine.DEB.2.21.1904240654120.3407%40lancre --- src/test/regress/expected/copydml.out | 16 ++++++++-------- src/test/regress/sql/copydml.sql | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/test/regress/expected/copydml.out b/src/test/regress/expected/copydml.out index 1b533962c6b9c..b5a225628f46e 100644 --- a/src/test/regress/expected/copydml.out +++ b/src/test/regress/expected/copydml.out @@ -84,10 +84,10 @@ drop rule qqq on copydml_test; create function qqq_trig() returns trigger as $$ begin if tg_op in ('INSERT', 'UPDATE') then - raise notice '% %', tg_op, new.id; + raise notice '% % %', tg_when, tg_op, new.id; return new; else - raise notice '% %', tg_op, old.id; + raise notice '% % %', tg_when, tg_op, old.id; return old; end if; end @@ -97,16 +97,16 @@ create trigger qqqbef before insert or update or delete on copydml_test create trigger qqqaf after insert or update or delete on copydml_test for each row execute procedure qqq_trig(); copy (insert into copydml_test (t) values ('f') returning id) to stdout; -NOTICE: INSERT 8 +NOTICE: BEFORE INSERT 8 8 -NOTICE: INSERT 8 +NOTICE: AFTER INSERT 8 copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout; -NOTICE: UPDATE 8 +NOTICE: BEFORE UPDATE 8 8 -NOTICE: UPDATE 8 +NOTICE: AFTER UPDATE 8 copy (delete from copydml_test where t = 'g' returning id) to stdout; -NOTICE: DELETE 8 +NOTICE: BEFORE DELETE 8 8 -NOTICE: DELETE 8 +NOTICE: AFTER DELETE 8 drop table copydml_test; drop function qqq_trig(); diff --git a/src/test/regress/sql/copydml.sql b/src/test/regress/sql/copydml.sql index 9a29f9c9acc7e..4578342253b62 100644 --- a/src/test/regress/sql/copydml.sql +++ b/src/test/regress/sql/copydml.sql @@ -70,10 +70,10 @@ drop rule qqq on copydml_test; create function qqq_trig() returns trigger as $$ begin if tg_op in ('INSERT', 'UPDATE') then - raise notice '% %', tg_op, new.id; + raise notice '% % %', tg_when, tg_op, new.id; return new; else - raise notice '% %', tg_op, old.id; + raise notice '% % %', tg_when, tg_op, old.id; return old; end if; end