public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v28 1/5] Remove pg_collation.collversion.
6+ messages / 5 participants
[nested] [flat]
* [PATCH v28 1/5] Remove pg_collation.collversion.
@ 2019-05-28 18:15 Thomas Munro <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Munro @ 2019-05-28 18:15 UTC (permalink / raw)
While this allowed the system to detect changes in collation versions,
it didn't help us track what had to be done about it. A later patch
will add version tracking for individual database objects that depend
on collations.
Author: Thomas Munro <[email protected]>
Reviewed-by: Julien Rouhaud <[email protected]>
Reviewed-by: Peter Eisentraut <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://postgr.es/m/CAEepm%3D0uEQCpfq_%2BLYFBdArCe4Ot98t1aR4eYiYTe%3DyavQygiQ%40mail.gmail.com
---
doc/src/sgml/catalogs.sgml | 11 ---
doc/src/sgml/func.sgml | 6 +-
doc/src/sgml/ref/alter_collation.sgml | 63 -------------
doc/src/sgml/ref/create_collation.sgml | 21 -----
src/backend/catalog/pg_collation.c | 5 --
src/backend/commands/collationcmds.c | 88 -------------------
src/backend/nodes/copyfuncs.c | 13 ---
src/backend/nodes/equalfuncs.c | 11 ---
src/backend/parser/gram.y | 18 +---
src/backend/tcop/utility.c | 12 ---
src/backend/utils/adt/pg_locale.c | 37 --------
src/bin/pg_dump/pg_dump.c | 24 +----
src/include/catalog/pg_collation.dat | 7 +-
src/include/catalog/pg_collation.h | 5 --
src/include/catalog/toasting.h | 1 -
src/include/commands/collationcmds.h | 1 -
src/include/nodes/parsenodes.h | 11 ---
.../regress/expected/collate.icu.utf8.out | 3 -
.../regress/expected/collate.linux.utf8.out | 3 -
src/test/regress/sql/collate.icu.utf8.sql | 5 --
src/test/regress/sql/collate.linux.utf8.sql | 5 --
21 files changed, 7 insertions(+), 343 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 26fda20d19..a28ae89e13 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -2353,17 +2353,6 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
<symbol>LC_CTYPE</symbol> for this collation object
</para></entry>
</row>
-
- <row>
- <entry role="catalog_table_entry"><para role="column_definition">
- <structfield>collversion</structfield> <type>text</type>
- </para>
- <para>
- Provider-specific version of the collation. This is recorded when the
- collation is created and then checked when it is used, to detect
- changes in the collation definition that could lead to data corruption.
- </para></entry>
- </row>
</tbody>
</tgroup>
</table>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index f766c1bc67..8432e790f2 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -25438,11 +25438,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
</para>
<para>
Returns the actual version of the collation object as it is currently
- installed in the operating system. If this is different from the
- value in
- <structname>pg_collation</structname>.<structfield>collversion</structfield>,
- then objects depending on the collation might need to be rebuilt. See
- also <xref linkend="sql-altercollation"/>.
+ installed in the operating system.
</para></entry>
</row>
diff --git a/doc/src/sgml/ref/alter_collation.sgml b/doc/src/sgml/ref/alter_collation.sgml
index bee6f0dd3c..c985b0de56 100644
--- a/doc/src/sgml/ref/alter_collation.sgml
+++ b/doc/src/sgml/ref/alter_collation.sgml
@@ -21,8 +21,6 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
-ALTER COLLATION <replaceable>name</replaceable> REFRESH VERSION
-
ALTER COLLATION <replaceable>name</replaceable> RENAME TO <replaceable>new_name</replaceable>
ALTER COLLATION <replaceable>name</replaceable> OWNER TO { <replaceable>new_owner</replaceable> | CURRENT_USER | SESSION_USER }
ALTER COLLATION <replaceable>name</replaceable> SET SCHEMA <replaceable>new_schema</replaceable>
@@ -88,70 +86,9 @@ ALTER COLLATION <replaceable>name</replaceable> SET SCHEMA <replaceable>new_sche
</listitem>
</varlistentry>
- <varlistentry>
- <term><literal>REFRESH VERSION</literal></term>
- <listitem>
- <para>
- Update the collation's version.
- See <xref linkend="sql-altercollation-notes"/> below.
- </para>
- </listitem>
- </varlistentry>
</variablelist>
</refsect1>
- <refsect1 id="sql-altercollation-notes" xreflabel="Notes">
- <title>Notes</title>
-
- <para>
- When using collations provided by the ICU library, the ICU-specific version
- of the collator is recorded in the system catalog when the collation object
- is created. When the collation is used, the current version is
- checked against the recorded version, and a warning is issued when there is
- a mismatch, for example:
-<screen>
-WARNING: collation "xx-x-icu" has version mismatch
-DETAIL: The collation in the database was created using version 1.2.3.4, but the operating system provides version 2.3.4.5.
-HINT: Rebuild all objects affected by this collation and run ALTER COLLATION pg_catalog."xx-x-icu" REFRESH VERSION, or build PostgreSQL with the right library version.
-</screen>
- A change in collation definitions can lead to corrupt indexes and other
- problems because the database system relies on stored objects having a
- certain sort order. Generally, this should be avoided, but it can happen
- in legitimate circumstances, such as when
- using <command>pg_upgrade</command> to upgrade to server binaries linked
- with a newer version of ICU. When this happens, all objects depending on
- the collation should be rebuilt, for example,
- using <command>REINDEX</command>. When that is done, the collation version
- can be refreshed using the command <literal>ALTER COLLATION ... REFRESH
- VERSION</literal>. This will update the system catalog to record the
- current collator version and will make the warning go away. Note that this
- does not actually check whether all affected objects have been rebuilt
- correctly.
- </para>
- <para>
- When using collations provided by <literal>libc</literal> and
- <productname>PostgreSQL</productname> was built with the GNU C library, the
- C library's version is used as a collation version. Since collation
- definitions typically change only with GNU C library releases, this provides
- some defense against corruption, but it is not completely reliable.
- </para>
- <para>
- Currently, there is no version tracking for the database default collation.
- </para>
-
- <para>
- The following query can be used to identify all collations in the current
- database that need to be refreshed and the objects that depend on them:
-<programlisting><![CDATA[
-SELECT pg_describe_object(refclassid, refobjid, refobjsubid) AS "Collation",
- pg_describe_object(classid, objid, objsubid) AS "Object"
- FROM pg_depend d JOIN pg_collation c
- ON refclassid = 'pg_collation'::regclass AND refobjid = c.oid
- WHERE c.collversion <> pg_collation_actual_version(c.oid)
- ORDER BY 1, 2;
-]]></programlisting></para>
- </refsect1>
-
<refsect1>
<title>Examples</title>
diff --git a/doc/src/sgml/ref/create_collation.sgml b/doc/src/sgml/ref/create_collation.sgml
index 58f5f0cd63..b97842071f 100644
--- a/doc/src/sgml/ref/create_collation.sgml
+++ b/doc/src/sgml/ref/create_collation.sgml
@@ -27,7 +27,6 @@ CREATE COLLATION [ IF NOT EXISTS ] <replaceable>name</replaceable> (
[ LC_CTYPE = <replaceable>lc_ctype</replaceable>, ]
[ PROVIDER = <replaceable>provider</replaceable>, ]
[ DETERMINISTIC = <replaceable>boolean</replaceable>, ]
- [ VERSION = <replaceable>version</replaceable> ]
)
CREATE COLLATION [ IF NOT EXISTS ] <replaceable>name</replaceable> FROM <replaceable>existing_collation</replaceable>
</synopsis>
@@ -149,26 +148,6 @@ CREATE COLLATION [ IF NOT EXISTS ] <replaceable>name</replaceable> FROM <replace
</listitem>
</varlistentry>
- <varlistentry>
- <term><replaceable>version</replaceable></term>
-
- <listitem>
- <para>
- Specifies the version string to store with the collation. Normally,
- this should be omitted, which will cause the version to be computed
- from the actual version of the collation as provided by the operating
- system. This option is intended to be used
- by <command>pg_upgrade</command> for copying the version from an
- existing installation.
- </para>
-
- <para>
- See also <xref linkend="sql-altercollation"/> for how to handle
- collation version mismatches.
- </para>
- </listitem>
- </varlistentry>
-
<varlistentry>
<term><replaceable>existing_collation</replaceable></term>
diff --git a/src/backend/catalog/pg_collation.c b/src/backend/catalog/pg_collation.c
index 5fdf1acb7e..3c84378d02 100644
--- a/src/backend/catalog/pg_collation.c
+++ b/src/backend/catalog/pg_collation.c
@@ -49,7 +49,6 @@ CollationCreate(const char *collname, Oid collnamespace,
bool collisdeterministic,
int32 collencoding,
const char *collcollate, const char *collctype,
- const char *collversion,
bool if_not_exists,
bool quiet)
{
@@ -167,10 +166,6 @@ CollationCreate(const char *collname, Oid collnamespace,
values[Anum_pg_collation_collcollate - 1] = NameGetDatum(&name_collate);
namestrcpy(&name_ctype, collctype);
values[Anum_pg_collation_collctype - 1] = NameGetDatum(&name_ctype);
- if (collversion)
- values[Anum_pg_collation_collversion - 1] = CStringGetTextDatum(collversion);
- else
- nulls[Anum_pg_collation_collversion - 1] = true;
tup = heap_form_tuple(tupDesc, values, nulls);
diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c
index 9f6582c530..5ad8886e60 100644
--- a/src/backend/commands/collationcmds.c
+++ b/src/backend/commands/collationcmds.c
@@ -61,14 +61,12 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
DefElem *lcctypeEl = NULL;
DefElem *providerEl = NULL;
DefElem *deterministicEl = NULL;
- DefElem *versionEl = NULL;
char *collcollate = NULL;
char *collctype = NULL;
char *collproviderstr = NULL;
bool collisdeterministic = true;
int collencoding = 0;
char collprovider = 0;
- char *collversion = NULL;
Oid newoid;
ObjectAddress address;
@@ -96,8 +94,6 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
defelp = &providerEl;
else if (strcmp(defel->defname, "deterministic") == 0)
defelp = &deterministicEl;
- else if (strcmp(defel->defname, "version") == 0)
- defelp = &versionEl;
else
{
ereport(ERROR,
@@ -166,9 +162,6 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
if (deterministicEl)
collisdeterministic = defGetBoolean(deterministicEl);
- if (versionEl)
- collversion = defGetString(versionEl);
-
if (collproviderstr)
{
if (pg_strcasecmp(collproviderstr, "icu") == 0)
@@ -215,9 +208,6 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
}
}
- if (!collversion)
- collversion = get_collation_actual_version(collprovider, collcollate);
-
newoid = CollationCreate(collName,
collNamespace,
GetUserId(),
@@ -226,7 +216,6 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
collencoding,
collcollate,
collctype,
- collversion,
if_not_exists,
false); /* not quiet */
@@ -277,80 +266,6 @@ IsThereCollationInNamespace(const char *collname, Oid nspOid)
collname, get_namespace_name(nspOid))));
}
-/*
- * ALTER COLLATION
- */
-ObjectAddress
-AlterCollation(AlterCollationStmt *stmt)
-{
- Relation rel;
- Oid collOid;
- HeapTuple tup;
- Form_pg_collation collForm;
- Datum collversion;
- bool isnull;
- char *oldversion;
- char *newversion;
- ObjectAddress address;
-
- rel = table_open(CollationRelationId, RowExclusiveLock);
- collOid = get_collation_oid(stmt->collname, false);
-
- if (!pg_collation_ownercheck(collOid, GetUserId()))
- aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_COLLATION,
- NameListToString(stmt->collname));
-
- tup = SearchSysCacheCopy1(COLLOID, ObjectIdGetDatum(collOid));
- if (!HeapTupleIsValid(tup))
- elog(ERROR, "cache lookup failed for collation %u", collOid);
-
- collForm = (Form_pg_collation) GETSTRUCT(tup);
- collversion = SysCacheGetAttr(COLLOID, tup, Anum_pg_collation_collversion,
- &isnull);
- oldversion = isnull ? NULL : TextDatumGetCString(collversion);
-
- newversion = get_collation_actual_version(collForm->collprovider, NameStr(collForm->collcollate));
-
- /* cannot change from NULL to non-NULL or vice versa */
- if ((!oldversion && newversion) || (oldversion && !newversion))
- elog(ERROR, "invalid collation version change");
- else if (oldversion && newversion && strcmp(newversion, oldversion) != 0)
- {
- bool nulls[Natts_pg_collation];
- bool replaces[Natts_pg_collation];
- Datum values[Natts_pg_collation];
-
- ereport(NOTICE,
- (errmsg("changing version from %s to %s",
- oldversion, newversion)));
-
- memset(values, 0, sizeof(values));
- memset(nulls, false, sizeof(nulls));
- memset(replaces, false, sizeof(replaces));
-
- values[Anum_pg_collation_collversion - 1] = CStringGetTextDatum(newversion);
- replaces[Anum_pg_collation_collversion - 1] = true;
-
- tup = heap_modify_tuple(tup, RelationGetDescr(rel),
- values, nulls, replaces);
- }
- else
- ereport(NOTICE,
- (errmsg("version has not changed")));
-
- CatalogTupleUpdate(rel, &tup->t_self, tup);
-
- InvokeObjectPostAlterHook(CollationRelationId, collOid, 0);
-
- ObjectAddressSet(address, CollationRelationId, collOid);
-
- heap_freetuple(tup);
- table_close(rel, NoLock);
-
- return address;
-}
-
-
Datum
pg_collation_actual_version(PG_FUNCTION_ARGS)
{
@@ -608,7 +523,6 @@ pg_import_system_collations(PG_FUNCTION_ARGS)
collid = CollationCreate(localebuf, nspid, GetUserId(),
COLLPROVIDER_LIBC, true, enc,
localebuf, localebuf,
- get_collation_actual_version(COLLPROVIDER_LIBC, localebuf),
true, true);
if (OidIsValid(collid))
{
@@ -669,7 +583,6 @@ pg_import_system_collations(PG_FUNCTION_ARGS)
collid = CollationCreate(alias, nspid, GetUserId(),
COLLPROVIDER_LIBC, true, enc,
locale, locale,
- get_collation_actual_version(COLLPROVIDER_LIBC, locale),
true, true);
if (OidIsValid(collid))
{
@@ -731,7 +644,6 @@ pg_import_system_collations(PG_FUNCTION_ARGS)
nspid, GetUserId(),
COLLPROVIDER_ICU, true, -1,
collcollate, collcollate,
- get_collation_actual_version(COLLPROVIDER_ICU, collcollate),
true, true);
if (OidIsValid(collid))
{
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 89c409de66..2218f6e3db 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3226,16 +3226,6 @@ _copyAlterTableCmd(const AlterTableCmd *from)
return newnode;
}
-static AlterCollationStmt *
-_copyAlterCollationStmt(const AlterCollationStmt *from)
-{
- AlterCollationStmt *newnode = makeNode(AlterCollationStmt);
-
- COPY_NODE_FIELD(collname);
-
- return newnode;
-}
-
static AlterDomainStmt *
_copyAlterDomainStmt(const AlterDomainStmt *from)
{
@@ -5232,9 +5222,6 @@ copyObjectImpl(const void *from)
case T_AlterTableCmd:
retval = _copyAlterTableCmd(from);
break;
- case T_AlterCollationStmt:
- retval = _copyAlterCollationStmt(from);
- break;
case T_AlterDomainStmt:
retval = _copyAlterDomainStmt(from);
break;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index e3f33c40be..40eb879176 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1107,14 +1107,6 @@ _equalAlterTableCmd(const AlterTableCmd *a, const AlterTableCmd *b)
return true;
}
-static bool
-_equalAlterCollationStmt(const AlterCollationStmt *a, const AlterCollationStmt *b)
-{
- COMPARE_NODE_FIELD(collname);
-
- return true;
-}
-
static bool
_equalAlterDomainStmt(const AlterDomainStmt *a, const AlterDomainStmt *b)
{
@@ -3284,9 +3276,6 @@ equal(const void *a, const void *b)
case T_AlterTableCmd:
retval = _equalAlterTableCmd(a, b);
break;
- case T_AlterCollationStmt:
- retval = _equalAlterCollationStmt(a, b);
- break;
case T_AlterDomainStmt:
retval = _equalAlterDomainStmt(a, b);
break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index dbb47d4982..e432049f8e 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -254,7 +254,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
}
%type <node> stmt schema_stmt
- AlterEventTrigStmt AlterCollationStmt
+ AlterEventTrigStmt
AlterDatabaseStmt AlterDatabaseSetStmt AlterDomainStmt AlterEnumStmt
AlterFdwStmt AlterForeignServerStmt AlterGroupStmt
AlterObjectDependsStmt AlterObjectSchemaStmt AlterOwnerStmt
@@ -838,7 +838,6 @@ stmtmulti: stmtmulti ';' stmt
stmt :
AlterEventTrigStmt
- | AlterCollationStmt
| AlterDatabaseStmt
| AlterDatabaseSetStmt
| AlterDefaultPrivilegesStmt
@@ -10137,21 +10136,6 @@ drop_option:
}
;
-/*****************************************************************************
- *
- * ALTER COLLATION
- *
- *****************************************************************************/
-
-AlterCollationStmt: ALTER COLLATION any_name REFRESH VERSION_P
- {
- AlterCollationStmt *n = makeNode(AlterCollationStmt);
- n->collname = $3;
- $$ = (Node *)n;
- }
- ;
-
-
/*****************************************************************************
*
* ALTER SYSTEM
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 9b0c376c8c..fb8a964cc4 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1795,10 +1795,6 @@ ProcessUtilitySlow(ParseState *pstate,
address = AlterStatistics((AlterStatsStmt *) parsetree);
break;
- case T_AlterCollationStmt:
- address = AlterCollation((AlterCollationStmt *) parsetree);
- break;
-
default:
elog(ERROR, "unrecognized node type: %d",
(int) nodeTag(parsetree));
@@ -2946,10 +2942,6 @@ CreateCommandTag(Node *parsetree)
tag = CMDTAG_DROP_SUBSCRIPTION;
break;
- case T_AlterCollationStmt:
- tag = CMDTAG_ALTER_COLLATION;
- break;
-
case T_PrepareStmt:
tag = CMDTAG_PREPARE;
break;
@@ -3562,10 +3554,6 @@ GetCommandLogLevel(Node *parsetree)
lev = LOGSTMT_DDL;
break;
- case T_AlterCollationStmt:
- lev = LOGSTMT_DDL;
- break;
-
/* already-planned queries */
case T_PlannedStmt:
{
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 07299dbc09..514e0fa0af 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -1513,8 +1513,6 @@ pg_newlocale_from_collation(Oid collid)
const char *collctype pg_attribute_unused();
struct pg_locale_struct result;
pg_locale_t resultp;
- Datum collversion;
- bool isnull;
tp = SearchSysCache1(COLLOID, ObjectIdGetDatum(collid));
if (!HeapTupleIsValid(tp))
@@ -1616,41 +1614,6 @@ pg_newlocale_from_collation(Oid collid)
#endif /* not USE_ICU */
}
- collversion = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_collversion,
- &isnull);
- if (!isnull)
- {
- char *actual_versionstr;
- char *collversionstr;
-
- actual_versionstr = get_collation_actual_version(collform->collprovider, collcollate);
- if (!actual_versionstr)
- {
- /*
- * This could happen when specifying a version in CREATE
- * COLLATION for a libc locale, or manually creating a mess in
- * the catalogs.
- */
- ereport(ERROR,
- (errmsg("collation \"%s\" has no actual version, but a version was specified",
- NameStr(collform->collname))));
- }
- collversionstr = TextDatumGetCString(collversion);
-
- if (strcmp(actual_versionstr, collversionstr) != 0)
- ereport(WARNING,
- (errmsg("collation \"%s\" has version mismatch",
- NameStr(collform->collname)),
- errdetail("The collation in the database was created using version %s, "
- "but the operating system provides version %s.",
- collversionstr, actual_versionstr),
- errhint("Rebuild all objects affected by this collation and run "
- "ALTER COLLATION %s REFRESH VERSION, "
- "or build PostgreSQL with the right library version.",
- quote_qualified_identifier(get_namespace_name(collform->collnamespace),
- NameStr(collform->collname)))));
- }
-
ReleaseSysCache(tp);
/* We'll keep the pg_locale_t structures in TopMemoryContext */
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 9c8436dde6..af8d38488e 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -13548,12 +13548,10 @@ dumpCollation(Archive *fout, CollInfo *collinfo)
if (fout->remoteVersion >= 100000)
appendPQExpBufferStr(query,
- "collprovider, "
- "collversion, ");
+ "collprovider, ");
else
appendPQExpBufferStr(query,
- "'c' AS collprovider, "
- "NULL AS collversion, ");
+ "'c' AS collprovider, ");
if (fout->remoteVersion >= 120000)
appendPQExpBufferStr(query,
@@ -13614,24 +13612,6 @@ dumpCollation(Archive *fout, CollInfo *collinfo)
appendStringLiteralAH(q, collctype, fout);
}
- /*
- * For binary upgrade, carry over the collation version. For normal
- * dump/restore, omit the version, so that it is computed upon restore.
- */
- if (dopt->binary_upgrade)
- {
- int i_collversion;
-
- i_collversion = PQfnumber(res, "collversion");
- if (!PQgetisnull(res, 0, i_collversion))
- {
- appendPQExpBufferStr(q, ", version = ");
- appendStringLiteralAH(q,
- PQgetvalue(res, 0, i_collversion),
- fout);
- }
- }
-
appendPQExpBufferStr(q, ");\n");
if (dopt->binary_upgrade)
diff --git a/src/include/catalog/pg_collation.dat b/src/include/catalog/pg_collation.dat
index ba1b3e201b..45301ccdd7 100644
--- a/src/include/catalog/pg_collation.dat
+++ b/src/include/catalog/pg_collation.dat
@@ -15,17 +15,16 @@
{ oid => '100', oid_symbol => 'DEFAULT_COLLATION_OID',
descr => 'database\'s default collation',
collname => 'default', collnamespace => 'PGNSP', collowner => 'PGUID',
- collprovider => 'd', collencoding => '-1', collcollate => '', collctype => '',
- collversion => '_null_' },
+ collprovider => 'd', collencoding => '-1', collcollate => '', collctype => '' },
{ oid => '950', oid_symbol => 'C_COLLATION_OID',
descr => 'standard C collation',
collname => 'C', collnamespace => 'PGNSP', collowner => 'PGUID',
collprovider => 'c', collencoding => '-1', collcollate => 'C',
- collctype => 'C', collversion => '_null_' },
+ collctype => 'C' },
{ oid => '951', oid_symbol => 'POSIX_COLLATION_OID',
descr => 'standard POSIX collation',
collname => 'POSIX', collnamespace => 'PGNSP', collowner => 'PGUID',
collprovider => 'c', collencoding => '-1', collcollate => 'POSIX',
- collctype => 'POSIX', collversion => '_null_' },
+ collctype => 'POSIX' },
]
diff --git a/src/include/catalog/pg_collation.h b/src/include/catalog/pg_collation.h
index 27618b324d..e7e958b808 100644
--- a/src/include/catalog/pg_collation.h
+++ b/src/include/catalog/pg_collation.h
@@ -37,10 +37,6 @@ CATALOG(pg_collation,3456,CollationRelationId)
int32 collencoding; /* encoding for this collation; -1 = "all" */
NameData collcollate; /* LC_COLLATE setting */
NameData collctype; /* LC_CTYPE setting */
-#ifdef CATALOG_VARLEN /* variable-length fields start here */
- text collversion; /* provider-dependent version of collation
- * data */
-#endif
} FormData_pg_collation;
/* ----------------
@@ -65,7 +61,6 @@ extern Oid CollationCreate(const char *collname, Oid collnamespace,
bool collisdeterministic,
int32 collencoding,
const char *collcollate, const char *collctype,
- const char *collversion,
bool if_not_exists,
bool quiet);
diff --git a/src/include/catalog/toasting.h b/src/include/catalog/toasting.h
index 51491c4513..8f131893dc 100644
--- a/src/include/catalog/toasting.h
+++ b/src/include/catalog/toasting.h
@@ -51,7 +51,6 @@ extern void BootstrapToastTable(char *relName,
/* normal catalogs */
DECLARE_TOAST(pg_aggregate, 4159, 4160);
DECLARE_TOAST(pg_attrdef, 2830, 2831);
-DECLARE_TOAST(pg_collation, 4161, 4162);
DECLARE_TOAST(pg_constraint, 2832, 2833);
DECLARE_TOAST(pg_default_acl, 4143, 4144);
DECLARE_TOAST(pg_description, 2834, 2835);
diff --git a/src/include/commands/collationcmds.h b/src/include/commands/collationcmds.h
index 373b85374c..3e1c16ac7f 100644
--- a/src/include/commands/collationcmds.h
+++ b/src/include/commands/collationcmds.h
@@ -20,6 +20,5 @@
extern ObjectAddress DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_exists);
extern void IsThereCollationInNamespace(const char *collname, Oid nspOid);
-extern ObjectAddress AlterCollation(AlterCollationStmt *stmt);
#endif /* COLLATIONCMDS_H */
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 151bcdb7ef..a8da438d22 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1873,17 +1873,6 @@ typedef struct AlterTableCmd /* one subcommand of an ALTER TABLE */
} AlterTableCmd;
-/* ----------------------
- * Alter Collation
- * ----------------------
- */
-typedef struct AlterCollationStmt
-{
- NodeTag type;
- List *collname;
-} AlterCollationStmt;
-
-
/* ----------------------
* Alter Domain
*
diff --git a/src/test/regress/expected/collate.icu.utf8.out b/src/test/regress/expected/collate.icu.utf8.out
index 2b86ce9028..60d9263a2f 100644
--- a/src/test/regress/expected/collate.icu.utf8.out
+++ b/src/test/regress/expected/collate.icu.utf8.out
@@ -1082,9 +1082,6 @@ SELECT collname FROM pg_collation WHERE collname LIKE 'test%';
DROP SCHEMA test_schema;
DROP ROLE regress_test_role;
--- ALTER
-ALTER COLLATION "en-x-icu" REFRESH VERSION;
-NOTICE: version has not changed
-- dependencies
CREATE COLLATION test0 FROM "C";
CREATE TABLE collate_dep_test1 (a int, b text COLLATE test0);
diff --git a/src/test/regress/expected/collate.linux.utf8.out b/src/test/regress/expected/collate.linux.utf8.out
index f06ae543e4..580b00eea7 100644
--- a/src/test/regress/expected/collate.linux.utf8.out
+++ b/src/test/regress/expected/collate.linux.utf8.out
@@ -1093,9 +1093,6 @@ SELECT collname FROM pg_collation WHERE collname LIKE 'test%';
DROP SCHEMA test_schema;
DROP ROLE regress_test_role;
--- ALTER
-ALTER COLLATION "en_US" REFRESH VERSION;
-NOTICE: version has not changed
-- dependencies
CREATE COLLATION test0 FROM "C";
CREATE TABLE collate_dep_test1 (a int, b text COLLATE test0);
diff --git a/src/test/regress/sql/collate.icu.utf8.sql b/src/test/regress/sql/collate.icu.utf8.sql
index 67de7d9794..35acf91fbf 100644
--- a/src/test/regress/sql/collate.icu.utf8.sql
+++ b/src/test/regress/sql/collate.icu.utf8.sql
@@ -405,11 +405,6 @@ DROP SCHEMA test_schema;
DROP ROLE regress_test_role;
--- ALTER
-
-ALTER COLLATION "en-x-icu" REFRESH VERSION;
-
-
-- dependencies
CREATE COLLATION test0 FROM "C";
diff --git a/src/test/regress/sql/collate.linux.utf8.sql b/src/test/regress/sql/collate.linux.utf8.sql
index cbbd2203e4..c697c99488 100644
--- a/src/test/regress/sql/collate.linux.utf8.sql
+++ b/src/test/regress/sql/collate.linux.utf8.sql
@@ -406,11 +406,6 @@ DROP SCHEMA test_schema;
DROP ROLE regress_test_role;
--- ALTER
-
-ALTER COLLATION "en_US" REFRESH VERSION;
-
-
-- dependencies
CREATE COLLATION test0 FROM "C";
--
2.20.1
--gclmslubawp3yaq7
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
filename="v28-0002-Add-pg_depend.refobjversion.patch"
^ permalink raw reply [nested|flat] 6+ messages in thread
* Allow pg_archivecleanup to remove backup history files
@ 2023-04-25 07:38 torikoshia <[email protected]>
2023-04-25 08:29 ` Re: Allow pg_archivecleanup to remove backup history files Kyotaro Horiguchi <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: torikoshia @ 2023-04-25 07:38 UTC (permalink / raw)
To: pgsql-hackers
Hi,
Currently pg_archivecleanup doesn't remove backup history files even
when they're older than oldestkeptwalfile.
Of course the size of backup history files are smaller than WAL files
and they wouldn't consume much disk space, but a lot of backup history
files(e.g. daily backup for years) whose backups data have been already
removed are unnecessary and I would appreciate if pg_archivecleanup has
an option to remove them.
Attached a PoC patch, which added new option -b to remove files
including backup history files older than oldestkeptwalfile.
$ ls archivedir
000000010000000000000001 000000010000000000000003
000000010000000000000006
000000010000000000000008
000000010000000000000002 000000010000000000000004
000000010000000000000007
000000010000000000000009
000000010000000000000002.00000028.backup 000000010000000000000005
000000010000000000000007.00000028.backup
00000001000000000000000A.partial
$ pg_archivecleanup -b archivedir 000000010000000000000009
$ ls archivedir
000000010000000000000009 00000001000000000000000A.partial
Any thoughts?
--
Regards,
--
Atsushi Torikoshi
NTT DATA CORPORATION
Attachments:
[text/x-diff] v1-0001-Allow-pg_archivecleanup-to-remove-backuphistoryfile.patch (2.5K, ../../[email protected]/2-v1-0001-Allow-pg_archivecleanup-to-remove-backuphistoryfile.patch)
download | inline diff:
From ad87224ec5ba6ee13ccf934bf3e5adefb7e67212 Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <[email protected]>
Date: Mon, 24 Apr 2023 23:28:06 +0900
Subject: [PATCH v1] Allow pg_archivecleanup to remove backup history files
Add new option -b to remove files including backup history files older
than oldestkeptwalfile.
---
src/bin/pg_archivecleanup/pg_archivecleanup.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/bin/pg_archivecleanup/pg_archivecleanup.c b/src/bin/pg_archivecleanup/pg_archivecleanup.c
index 7726d05149..5bc90fbadd 100644
--- a/src/bin/pg_archivecleanup/pg_archivecleanup.c
+++ b/src/bin/pg_archivecleanup/pg_archivecleanup.c
@@ -23,6 +23,8 @@ const char *progname;
/* Options and defaults */
bool dryrun = false; /* are we performing a dry-run operation? */
+bool removeBackupHistoryFile = false; /* remove files including
+ * backup history files */
char *additional_ext = NULL; /* Extension to remove from filenames */
char *archiveLocation; /* where to find the archive? */
@@ -118,7 +120,8 @@ CleanupPriorWALFiles(void)
* file. Note that this means files are not removed in the order
* they were originally written, in case this worries you.
*/
- if ((IsXLogFileName(walfile) || IsPartialXLogFileName(walfile)) &&
+ if (((IsXLogFileName(walfile) || IsPartialXLogFileName(walfile)) ||
+ (removeBackupHistoryFile && IsBackupHistoryFileName(walfile))) &&
strcmp(walfile + 8, exclusiveCleanupFileName + 8) < 0)
{
char WALFilePath[MAXPGPATH * 2]; /* the file path
@@ -252,6 +255,7 @@ usage(void)
printf(_("Usage:\n"));
printf(_(" %s [OPTION]... ARCHIVELOCATION OLDESTKEPTWALFILE\n"), progname);
printf(_("\nOptions:\n"));
+ printf(_(" -b remove files including backup history files\n"));
printf(_(" -d generate debug output (verbose mode)\n"));
printf(_(" -n dry run, show the names of the files that would be removed\n"));
printf(_(" -V, --version output version information, then exit\n"));
@@ -294,10 +298,13 @@ main(int argc, char **argv)
}
}
- while ((c = getopt(argc, argv, "dnx:")) != -1)
+ while ((c = getopt(argc, argv, "bdnx:")) != -1)
{
switch (c)
{
+ case 'b': /* Remove backup history files too */
+ removeBackupHistoryFile = true;
+ break;
case 'd': /* Debug mode */
pg_logging_increase_verbosity();
break;
base-commit: c5e4ec293ea527394fc1d0006ab88047b3ce580f
--
2.25.1
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Allow pg_archivecleanup to remove backup history files
2023-04-25 07:38 Allow pg_archivecleanup to remove backup history files torikoshia <[email protected]>
@ 2023-04-25 08:29 ` Kyotaro Horiguchi <[email protected]>
2023-04-25 21:39 ` Re: Allow pg_archivecleanup to remove backup history files Michael Paquier <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Kyotaro Horiguchi @ 2023-04-25 08:29 UTC (permalink / raw)
To: [email protected]; +Cc: pgsql-hackers
At Tue, 25 Apr 2023 16:38:16 +0900, torikoshia <[email protected]> wrote in
> Hi,
>
> Currently pg_archivecleanup doesn't remove backup history files even
> when they're older than oldestkeptwalfile.
>
> Of course the size of backup history files are smaller than WAL files
> and they wouldn't consume much disk space, but a lot of backup history
> files(e.g. daily backup for years) whose backups data have been
> already removed are unnecessary and I would appreciate if
> pg_archivecleanup has an option to remove them.
>
> Attached a PoC patch, which added new option -b to remove files
> including backup history files older than oldestkeptwalfile.
>
> $ ls archivedir
> 000000010000000000000001 000000010000000000000003
> 000000010000000000000006
> 000000010000000000000008
> 000000010000000000000002 000000010000000000000004
> 000000010000000000000007
> 000000010000000000000009
> 000000010000000000000002.00000028.backup 000000010000000000000005
> 000000010000000000000007.00000028.backup
> 00000001000000000000000A.partial
>
> $ pg_archivecleanup -b archivedir 000000010000000000000009
>
> $ ls archivedir
> 000000010000000000000009 00000001000000000000000A.partial
>
> Any thoughts?
I thought that we have decided not to do that, but I coundn't find any
discussion about it in the ML archive. Anyway, I think it is great
that we have that option.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Allow pg_archivecleanup to remove backup history files
2023-04-25 07:38 Allow pg_archivecleanup to remove backup history files torikoshia <[email protected]>
2023-04-25 08:29 ` Re: Allow pg_archivecleanup to remove backup history files Kyotaro Horiguchi <[email protected]>
@ 2023-04-25 21:39 ` Michael Paquier <[email protected]>
2023-05-09 13:32 ` Re: Allow pg_archivecleanup to remove backup history files torikoshia <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Michael Paquier @ 2023-04-25 21:39 UTC (permalink / raw)
To: Kyotaro Horiguchi <[email protected]>; +Cc: [email protected]; pgsql-hackers
On Tue, Apr 25, 2023 at 05:29:48PM +0900, Kyotaro Horiguchi wrote:
> I thought that we have decided not to do that, but I coundn't find any
> discussion about it in the ML archive. Anyway, I think it is great
> that we have that option.
No objections from here to make that optional. It's been argued for a
couple of times that leaving the archive history files is good for
debugging, but I can also get why one would one to clean up all the
files older than a WAL retention policy. Even if these are just few
bytes, it can be noisy for the eye to see a large accumulation of
history files mixed with the WAL segments.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../ZEhIiaCa2rdc%[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Allow pg_archivecleanup to remove backup history files
2023-04-25 07:38 Allow pg_archivecleanup to remove backup history files torikoshia <[email protected]>
2023-04-25 08:29 ` Re: Allow pg_archivecleanup to remove backup history files Kyotaro Horiguchi <[email protected]>
2023-04-25 21:39 ` Re: Allow pg_archivecleanup to remove backup history files Michael Paquier <[email protected]>
@ 2023-05-09 13:32 ` torikoshia <[email protected]>
2023-05-10 08:52 ` Re: Allow pg_archivecleanup to remove backup history files Bharath Rupireddy <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: torikoshia @ 2023-05-09 13:32 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; Kyotaro Horiguchi <[email protected]>; +Cc: pgsql-hackers
Horiguchi-san, Michael-san
Thanks for your comments and information!
Attached a patch with documentation and regression tests.
On 2023-04-26 06:39, Michael Paquier wrote:
> On Tue, Apr 25, 2023 at 05:29:48PM +0900, Kyotaro Horiguchi wrote:
>> I thought that we have decided not to do that, but I coundn't find any
>> discussion about it in the ML archive. Anyway, I think it is great
>> that we have that option.
>
> No objections from here to make that optional. It's been argued for a
> couple of times that leaving the archive history files is good for
> debugging, but I can also get why one would one to clean up all the
> files older than a WAL retention policy. Even if these are just few
> bytes, it can be noisy for the eye to see a large accumulation of
> history files mixed with the WAL segments.
> --
> Michael
--
Regards,
--
Atsushi Torikoshi
NTT DATA CORPORATION
Attachments:
[text/x-diff] v2-0001-Allow-pg_archivecleanup-to-remove-backuphistoryfile.patch (5.9K, ../../[email protected]/2-v2-0001-Allow-pg_archivecleanup-to-remove-backuphistoryfile.patch)
download | inline diff:
From 7d44134e5de930ce04819285a5b7359e370708d4 Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <[email protected]>
Date: Tue, 9 May 2023 21:52:01 +0900
Subject: [PATCH v2] Allow pg_archivecleanup to remove backup history files
Add new option -b to remove files including backup history files older
than oldestkeptwalfile.
---
doc/src/sgml/ref/pgarchivecleanup.sgml | 11 +++++
src/bin/pg_archivecleanup/pg_archivecleanup.c | 11 ++++-
.../t/010_pg_archivecleanup.pl | 45 +++++++++++++++++--
3 files changed, 62 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/ref/pgarchivecleanup.sgml b/doc/src/sgml/ref/pgarchivecleanup.sgml
index 635e7c7685..6cb565156f 100644
--- a/doc/src/sgml/ref/pgarchivecleanup.sgml
+++ b/doc/src/sgml/ref/pgarchivecleanup.sgml
@@ -93,6 +93,17 @@ pg_archivecleanup: removing file "archive/00000001000000370000000E"
<variablelist>
+ <varlistentry>
+ <term><option>-b</option></term>
+ <listitem>
+ <para>
+ Remove files including backup history files, whose suffix is <filename>.backup</filename>.
+ Note that when <replaceable>oldestkeptwalfile</replaceable> is a backup history file,
+ specified file is kept and only preceding WAL files and backup history files are removed.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-d</option></term>
<listitem>
diff --git a/src/bin/pg_archivecleanup/pg_archivecleanup.c b/src/bin/pg_archivecleanup/pg_archivecleanup.c
index 7726d05149..ae6ae76f08 100644
--- a/src/bin/pg_archivecleanup/pg_archivecleanup.c
+++ b/src/bin/pg_archivecleanup/pg_archivecleanup.c
@@ -23,6 +23,8 @@ const char *progname;
/* Options and defaults */
bool dryrun = false; /* are we performing a dry-run operation? */
+bool removeBackupHistoryFile = false; /* remove files including
+ * backup history files */
char *additional_ext = NULL; /* Extension to remove from filenames */
char *archiveLocation; /* where to find the archive? */
@@ -118,7 +120,8 @@ CleanupPriorWALFiles(void)
* file. Note that this means files are not removed in the order
* they were originally written, in case this worries you.
*/
- if ((IsXLogFileName(walfile) || IsPartialXLogFileName(walfile)) &&
+ if (((IsXLogFileName(walfile) || IsPartialXLogFileName(walfile)) ||
+ (removeBackupHistoryFile && IsBackupHistoryFileName(walfile))) &&
strcmp(walfile + 8, exclusiveCleanupFileName + 8) < 0)
{
char WALFilePath[MAXPGPATH * 2]; /* the file path
@@ -252,6 +255,7 @@ usage(void)
printf(_("Usage:\n"));
printf(_(" %s [OPTION]... ARCHIVELOCATION OLDESTKEPTWALFILE\n"), progname);
printf(_("\nOptions:\n"));
+ printf(_(" -b remove files including backup history files\n"));
printf(_(" -d generate debug output (verbose mode)\n"));
printf(_(" -n dry run, show the names of the files that would be removed\n"));
printf(_(" -V, --version output version information, then exit\n"));
@@ -294,10 +298,13 @@ main(int argc, char **argv)
}
}
- while ((c = getopt(argc, argv, "dnx:")) != -1)
+ while ((c = getopt(argc, argv, "bdnx:")) != -1)
{
switch (c)
{
+ case 'b': /* Remove backup history files too */
+ removeBackupHistoryFile = true;
+ break;
case 'd': /* Debug mode */
pg_logging_increase_verbosity();
break;
diff --git a/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl b/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
index 76321d1284..afd2ff6209 100644
--- a/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
+++ b/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
@@ -16,9 +16,14 @@ my @walfiles = (
'00000001000000370000000C.gz', '00000001000000370000000D',
'00000001000000370000000E', '00000001000000370000000F.partial',);
+my @walfiles_with_backuphistoryfile = (
+ '000000020000003800000007.00000028.backup', '000000020000003800000008',
+ '000000020000003800000009',
+ '00000002000000380000000A', '00000002000000380000000B.007C9330.backup');
+
sub create_files
{
- foreach my $fn (@walfiles, 'unrelated_file')
+ foreach my $fn (@_, 'unrelated_file')
{
open my $file, '>', "$tempdir/$fn";
print $file 'CONTENT';
@@ -27,7 +32,7 @@ sub create_files
return;
}
-create_files();
+create_files(@walfiles);
command_fails_like(
['pg_archivecleanup'],
@@ -76,7 +81,7 @@ sub run_check
my ($suffix, $test_name) = @_;
- create_files();
+ create_files(@walfiles);
command_ok(
[
@@ -98,8 +103,42 @@ sub run_check
return;
}
+sub remove_backuphistoryfile_run_check
+{
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+ my ($suffix, $test_name) = @_;
+
+ create_files(@walfiles_with_backuphistoryfile);
+
+ command_ok(
+ [
+ 'pg_archivecleanup', '-b', $tempdir,
+ $walfiles_with_backuphistoryfile[2] . $suffix
+ ],
+ "$test_name: remove_backuphistoryfile_runs");
+
+ ok(!-f "$tempdir/$walfiles_with_backuphistoryfile[0]",
+ "$test_name: first .backup file was cleaned up");
+ ok(!-f "$tempdir/$walfiles_with_backuphistoryfile[1]",
+ "$test_name: second older WAL file was cleaned up");
+ ok(-f "$tempdir/$walfiles_with_backuphistoryfile[2]",
+ "$test_name: restartfile was not cleaned up");
+ ok(-f "$tempdir/$walfiles_with_backuphistoryfile[3]",
+ "$test_name: newer WAL file was not cleaned up");
+ ok(-f "$tempdir/$walfiles_with_backuphistoryfile[4]",
+ "$test_name: newer .backup file was not cleaned up");
+ ok(-f "$tempdir/unrelated_file",
+ "$test_name: unrelated file was not cleaned up");
+ return;
+}
+
run_check('', 'pg_archivecleanup');
run_check('.partial', 'pg_archivecleanup with .partial file');
run_check('.00000020.backup', 'pg_archivecleanup with .backup file');
+remove_backuphistoryfile_run_check('', 'pg_archivecleanup -b');
+remove_backuphistoryfile_run_check('.00000020.backup',
+ 'pg_archivecleanup -b with .backup file');
+
done_testing();
base-commit: c5b7f67fcc8c4a01c82660eb0996a3c697fac283
--
2.25.1
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Allow pg_archivecleanup to remove backup history files
2023-04-25 07:38 Allow pg_archivecleanup to remove backup history files torikoshia <[email protected]>
2023-04-25 08:29 ` Re: Allow pg_archivecleanup to remove backup history files Kyotaro Horiguchi <[email protected]>
2023-04-25 21:39 ` Re: Allow pg_archivecleanup to remove backup history files Michael Paquier <[email protected]>
2023-05-09 13:32 ` Re: Allow pg_archivecleanup to remove backup history files torikoshia <[email protected]>
@ 2023-05-10 08:52 ` Bharath Rupireddy <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Bharath Rupireddy @ 2023-05-10 08:52 UTC (permalink / raw)
To: torikoshia <[email protected]>; +Cc: Michael Paquier <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers
On Tue, May 9, 2023 at 7:03 PM torikoshia <[email protected]> wrote:
>
> Attached a patch with documentation and regression tests.
Thanks. I think pg_archivecleanup cleaning up history files makes it a
complete feature as there's no need to write custom code/scripts over
and above what pg_archivecleanup provides. It will help those who are
using pg_archivecleanup for cleaning up older WAL files, say from
their archive location.
Just curious to know the driving point behind this proposal - is
pg_archivecleanup deployed in production that was unable to clean up
the history files and there were many such history files left? It will
help us know how pg_archivecleanup is being used.
I'm wondering if making -x generic with '-x' '.backup', is simpler
than adding another option?
Comments on the patch:
1. Why just only the backup history files? Why not remove the timeline
history files too? Is it because there may not be as many tli switches
happening as backups?
2.+sub remove_backuphistoryfile_run_check
+{
Why to invent a new function when run_check() can be made generic with
few arguments passed? For instance, run_check() can receive
pg_archivecleanup command args, what to use for create_files(), in the
error condition if the pg_archivecleanup command args contain 'b',
then use a different message "$test_name: first older WAL file was
cleaned up" or "$test_name: first .backup file was cleaned up".
Otherwise, just modify the messages to be:
"$test_name: first older file %s was cleaned up", $files[0]);
"$test_name: second older file %s was cleaned up", $files[1]);
"$test_name: restart file %s was not cleaned up", $files[2]);
"$test_name: newer file %s was not cleaned up", $files[3]);
"$test_name: unrelated file %s was not cleaned up", $files[4]);
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2023-05-10 08:52 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-05-28 18:15 [PATCH v28 1/5] Remove pg_collation.collversion. Thomas Munro <[email protected]>
2023-04-25 07:38 Allow pg_archivecleanup to remove backup history files torikoshia <[email protected]>
2023-04-25 08:29 ` Re: Allow pg_archivecleanup to remove backup history files Kyotaro Horiguchi <[email protected]>
2023-04-25 21:39 ` Re: Allow pg_archivecleanup to remove backup history files Michael Paquier <[email protected]>
2023-05-09 13:32 ` Re: Allow pg_archivecleanup to remove backup history files torikoshia <[email protected]>
2023-05-10 08:52 ` Re: Allow pg_archivecleanup to remove backup history files Bharath Rupireddy <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox