public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v27 1/5] Remove pg_collation.collversion.
13+ messages / 5 participants
[nested] [flat]

* [PATCH v27 1/5] Remove pg_collation.collversion.
@ 2019-05-28 18:15  Thomas Munro <[email protected]>
  0 siblings, 0 replies; 13+ 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>&lt;iteration count&gt;</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


--o56keubw4jpehgac
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename="v27-0002-Add-pg_depend.refobjversion.patch"



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: head fails to build on SLES 12 (wal_compression=zstd)
@ 2022-03-31 15:37  Justin Pryzby <[email protected]>
  0 siblings, 1 reply; 13+ messages in thread

From: Justin Pryzby @ 2022-03-31 15:37 UTC (permalink / raw)
  To: Devrim Gündüz <[email protected]>; +Cc: Tom Lane <[email protected]>; [email protected]; Michael Paquier <[email protected]>

On Thu, Mar 31, 2022 at 03:38:39PM +0100, Devrim Gündüz wrote:
> On Thu, 2022-03-31 at 10:26 -0400, Tom Lane wrote:
> > Devrim =?ISO-8859-1?Q?G=FCnd=FCz?= <[email protected]> writes:
> > > Latest snapshot tarball fails to build on SLES 12.5, which uses GCC
> > > 4.8-8. Build log is attached.
> > 
> > Hmm, what version of libzstd is present?
> 
> 1.3.3

That's due to commit e9537321a74a2b062c8f7a452314b4570913f780.

Possible responses look like:
 - Use 0 which also means "default" (need to verify that works across versions);
 - Or #ifndef ZSTD_CLEVEL_DEFAULT #define ZSTD_CLEVEL_DEFAULT 3;
 - Add a test for a minimum zstd version v1.3.7.  This may be a good idea for
   v15 in any case, since we're using a few different APIs (at least
   ZSTD_compress and ZSTD_compressStream2 and execve(zstd)).

I dug up this history:

commit b2632bcf6cf7b9b96e0ac99beea079df4d1eaec5
Merge: 170f948e 869e2718
Author: Yann Collet <[email protected]>
Date:   Tue Jun 12 12:09:01 2018 -0700

    Merge pull request #1174 from duc0/document_default_level
    
    Expose ZSTD_CLEVEL_DEFAULT and update documentation

commit e34c000e44444b9f8bd62e5af0a355ee186eb21f
Author: Duc Ngo <[email protected]>
Date:   Fri Jun 8 11:29:51 2018 -0700

    Expose ZSTD_CLEVEL_DEFAULT and update documentation

commit 6d4fef36de21908e333b2a1fde8ded0a7f086ae1
Author: Yann Collet <[email protected]>
Date:   Wed May 17 18:36:15 2017 -0700

    Added ZSTD_compress_generic()

    Used in fileio.c (zstd cli).
    Need to set macro ZSTD_NEWAPI to trigger it.

commit 236d94fa9a4ff8723922971274a119c6084d5dbc
Author: Yann Collet <[email protected]>
Date:   Wed May 18 12:06:33 2016 +0200

    reverted default compression level to 1





^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: head fails to build on SLES 12 (wal_compression=zstd)
@ 2022-03-31 15:44  Tom Lane <[email protected]>
  parent: Justin Pryzby <[email protected]>
  0 siblings, 2 replies; 13+ messages in thread

From: Tom Lane @ 2022-03-31 15:44 UTC (permalink / raw)
  To: Justin Pryzby <[email protected]>; +Cc: Robert Haas <[email protected]>; Devrim Gündüz <[email protected]>; [email protected]; Michael Paquier <[email protected]>

Justin Pryzby <[email protected]> writes:
> Possible responses look like:
>  - Use 0 which also means "default" (need to verify that works across versions);
>  - Or #ifndef ZSTD_CLEVEL_DEFAULT #define ZSTD_CLEVEL_DEFAULT 3;
>  - Add a test for a minimum zstd version v1.3.7.  This may be a good idea for
>    v15 in any case, since we're using a few different APIs (at least
>    ZSTD_compress and ZSTD_compressStream2 and execve(zstd)).

In view of 51c0d186d ("Allow parallel zstd compression"), I agree
that some clarity about the minimum supported version of zstd
seems essential.  I don't want to be dealing with threading bugs
in ancient zstd versions.  However, why do you suggest 1.3.7 in
particular?

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: head fails to build on SLES 12 (wal_compression=zstd)
@ 2022-03-31 16:26  Robert Haas <[email protected]>
  parent: Tom Lane <[email protected]>
  1 sibling, 2 replies; 13+ messages in thread

From: Robert Haas @ 2022-03-31 16:26 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Devrim Gündüz <[email protected]>; PostgreSQL Hackers <[email protected]>; Michael Paquier <[email protected]>

On Thu, Mar 31, 2022 at 11:44 AM Tom Lane <[email protected]> wrote:
> Justin Pryzby <[email protected]> writes:
> > Possible responses look like:
> >  - Use 0 which also means "default" (need to verify that works across versions);
> >  - Or #ifndef ZSTD_CLEVEL_DEFAULT #define ZSTD_CLEVEL_DEFAULT 3;
> >  - Add a test for a minimum zstd version v1.3.7.  This may be a good idea for
> >    v15 in any case, since we're using a few different APIs (at least
> >    ZSTD_compress and ZSTD_compressStream2 and execve(zstd)).
>
> In view of 51c0d186d ("Allow parallel zstd compression"), I agree
> that some clarity about the minimum supported version of zstd
> seems essential.  I don't want to be dealing with threading bugs
> in ancient zstd versions.  However, why do you suggest 1.3.7 in
> particular?

One thing to note is that apparently threading wasn't enabled in the
default build before 1.5.0, which was released in May 2021, but it did
exist as an option in the code for some period of time prior to that.
I don't know how long exactly. I don't want to jump to the conclusion
that other people's old versions are full of bugs, but if that should
happen to be true here, there's some chance that PostgreSQL users
won't be exposed to them just because threading wasn't enabled by
default until quite recently.

-- 
Robert Haas
EDB: http://www.enterprisedb.com





^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: head fails to build on SLES 12 (wal_compression=zstd)
@ 2022-03-31 16:34  Justin Pryzby <[email protected]>
  parent: Robert Haas <[email protected]>
  1 sibling, 0 replies; 13+ messages in thread

From: Justin Pryzby @ 2022-03-31 16:34 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Tom Lane <[email protected]>; Devrim Gündüz <[email protected]>; PostgreSQL Hackers <[email protected]>; Michael Paquier <[email protected]>

On Thu, Mar 31, 2022 at 12:26:46PM -0400, Robert Haas wrote:
> On Thu, Mar 31, 2022 at 11:44 AM Tom Lane <[email protected]> wrote:
> > Justin Pryzby <[email protected]> writes:
> > > Possible responses look like:
> > >  - Use 0 which also means "default" (need to verify that works across versions);
> > >  - Or #ifndef ZSTD_CLEVEL_DEFAULT #define ZSTD_CLEVEL_DEFAULT 3;
> > >  - Add a test for a minimum zstd version v1.3.7.  This may be a good idea for
> > >    v15 in any case, since we're using a few different APIs (at least
> > >    ZSTD_compress and ZSTD_compressStream2 and execve(zstd)).
> >
> > In view of 51c0d186d ("Allow parallel zstd compression"), I agree
> > that some clarity about the minimum supported version of zstd
> > seems essential.  I don't want to be dealing with threading bugs
> > in ancient zstd versions.  However, why do you suggest 1.3.7 in
> > particular?
> 
> One thing to note is that apparently threading wasn't enabled in the
> default build before 1.5.0, which was released in May 2021, but it did
> exist as an option in the code for some period of time prior to that.
> I don't know how long exactly. I don't want to jump to the conclusion
> that other people's old versions are full of bugs, but if that should
> happen to be true here, there's some chance that PostgreSQL users
> won't be exposed to them just because threading wasn't enabled by
> default until quite recently.

Right.  Importantly, it's a run-time failure condition if threading wasn't
enabled at compile time.  Postgres should still compile --with-zstd even if it
wasn't, and pg_basebackup should work, except if workers is specified (or maybe
if workers>0, but it's possible that allowing workers=0 wasn't true before some
version).  I'll write more later.

-- 
Justin





^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: head fails to build on SLES 12 (wal_compression=zstd)
@ 2022-03-31 16:37  Tom Lane <[email protected]>
  parent: Robert Haas <[email protected]>
  1 sibling, 1 reply; 13+ messages in thread

From: Tom Lane @ 2022-03-31 16:37 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Devrim Gündüz <[email protected]>; PostgreSQL Hackers <[email protected]>; Michael Paquier <[email protected]>

Robert Haas <[email protected]> writes:
> On Thu, Mar 31, 2022 at 11:44 AM Tom Lane <[email protected]> wrote:
>> In view of 51c0d186d ("Allow parallel zstd compression"), I agree
>> that some clarity about the minimum supported version of zstd
>> seems essential.  I don't want to be dealing with threading bugs
>> in ancient zstd versions.  However, why do you suggest 1.3.7 in
>> particular?

> One thing to note is that apparently threading wasn't enabled in the
> default build before 1.5.0, which was released in May 2021, but it did
> exist as an option in the code for some period of time prior to that.
> I don't know how long exactly. I don't want to jump to the conclusion
> that other people's old versions are full of bugs, but if that should
> happen to be true here, there's some chance that PostgreSQL users
> won't be exposed to them just because threading wasn't enabled by
> default until quite recently.

Hm.  After rereading 51c0d186d I see that we're not asking for
parallel compression unless the user tells us to, so I guess
our fallback answer for any complaints in that area can be
"if it hurts, don't do it".  Still, I like the idea of having
a well-defined minimum zstd version that we consider supported.
The evident fact that their APIs are still changing (or at
least have done so within the memory of LTS platforms) makes
that fairly pressing.  Question is what to set the minimum to.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: head fails to build on SLES 12 (wal_compression=zstd)
@ 2022-03-31 16:55  Robert Haas <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 0 replies; 13+ messages in thread

From: Robert Haas @ 2022-03-31 16:55 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Devrim Gündüz <[email protected]>; PostgreSQL Hackers <[email protected]>; Michael Paquier <[email protected]>

On Thu, Mar 31, 2022 at 12:37 PM Tom Lane <[email protected]> wrote:
> Hm.  After rereading 51c0d186d I see that we're not asking for
> parallel compression unless the user tells us to, so I guess
> our fallback answer for any complaints in that area can be
> "if it hurts, don't do it".

Right. We can also tell people that if they are running buggy versions
of libzstd or liblz4 or libz, they should upgrade to non-buggy
versions. Our ability to paper over bugs in compression libraries is
going to be extremely limited.

> Still, I like the idea of having
> a well-defined minimum zstd version that we consider supported.
> The evident fact that their APIs are still changing (or at
> least have done so within the memory of LTS platforms) makes
> that fairly pressing.  Question is what to set the minimum to.

I think we should aim, if we can, to be compatible with libzstd
versions that are still being shipped with still-supported releases of
mainstream Linux distributions. If that turns out to be too hard, we
can be less ambitious.

On the particular question of ZSTD_CLEVEL_DEFAULT, it does not seem
likely that the library would have only recently exposed a symbol that
is required for correct use of the API, so I bet there's a relatively
simple way to avoid needing that altogether (perhaps by writing "0"
instead).

-- 
Robert Haas
EDB: http://www.enterprisedb.com





^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: head fails to build on SLES 12 (wal_compression=zstd)
@ 2022-03-31 22:42  Justin Pryzby <[email protected]>
  parent: Tom Lane <[email protected]>
  1 sibling, 1 reply; 13+ messages in thread

From: Justin Pryzby @ 2022-03-31 22:42 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; Devrim Gündüz <[email protected]>; [email protected]; Michael Paquier <[email protected]>

On Thu, Mar 31, 2022 at 11:44:40AM -0400, Tom Lane wrote:
> Justin Pryzby <[email protected]> writes:
> > Possible responses look like:
> >  - Use 0 which also means "default" (need to verify that works across versions);
> >  - Or #ifndef ZSTD_CLEVEL_DEFAULT #define ZSTD_CLEVEL_DEFAULT 3;
> >  - Add a test for a minimum zstd version v1.3.7.  This may be a good idea for
> >    v15 in any case, since we're using a few different APIs (at least
> >    ZSTD_compress and ZSTD_compressStream2 and execve(zstd)).
> 
> In view of 51c0d186d ("Allow parallel zstd compression"), I agree
> that some clarity about the minimum supported version of zstd
> seems essential.  I don't want to be dealing with threading bugs
> in ancient zstd versions.  However, why do you suggest 1.3.7 in
> particular?

That's where I found that ZSTD_CLEVEL_DEFAULT was added, in their git.

I've just installed a .deb for 1.3.8, and discovered that the APIs used by
basebackup were considered experimental/nonpublic/static-lib-only until 1.4.0
https://github.com/facebook/zstd/releases/tag/v1.4.0
ZSTD_CCtx_setParameter ZSTD_c_compressionLevel ZSTD_c_nbWorkers ZSTD_CCtx_reset ZSTD_reset_session_only ZSTD_compressStream2 ZSTD_e_continue ZSTD_e_end

FYI: it looks like parallel, thread workers were also a nonpublic,
"experimental" API until v1.3.7.  Actually, ZSTD_p_nbThreads was renamed to
ZSTD_p_nbWorkers and then (in 1.3.8) renamed to ZSTD_c_nbWorkers.

Versions less than 1.3.8 would have required compile-time conditionals for both
ZSTD_CLEVEL_DEFAULT and ZSTD_p_nbThreads/ZSTD_p_nbWorkers/ZSTD_c_nbWorkers (but
that is moot).

Negative compression levels were added in 1.3.4 (but I think the range of their
levels was originally -1..-7 and now expanded).  And long-distance matching
added in 1.3.2.

cirrus has installed:
linux (debian bullseye) 1.4.8
macos has 1.5.0
freebsd has 1.5.0

debian buster (oldstable) has v1.3.8, which is too old.
ubuntu focal LTS has 1.4.4 (bionic LTS has 1.3.3 which seems too old)
rhel7 has 1.5.2 in EPEL;

SLES 12.5 has zstd 1.3.3, so it won't be supported.  But postgres should fail
gracefully during ./configure rather than during build.

Note that check-world fails if wal_compression is enabled due to two
outstanding issues, so it's not currently possible to set that in CI or
buildfarm...
https://www.postgresql.org/message-id/c86ce84f-dd38-9951-102f-13a931210f52%40dunslane.net


^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: head fails to build on SLES 12 (wal_compression=zstd)
@ 2022-03-31 23:38  Tom Lane <[email protected]>
  parent: Justin Pryzby <[email protected]>
  0 siblings, 1 reply; 13+ messages in thread

From: Tom Lane @ 2022-03-31 23:38 UTC (permalink / raw)
  To: Justin Pryzby <[email protected]>; +Cc: Robert Haas <[email protected]>; Devrim Gündüz <[email protected]>; [email protected]; Michael Paquier <[email protected]>

Justin Pryzby <[email protected]> writes:
> On Thu, Mar 31, 2022 at 11:44:40AM -0400, Tom Lane wrote:
>> In view of 51c0d186d ("Allow parallel zstd compression"), I agree
>> that some clarity about the minimum supported version of zstd
>> seems essential.  I don't want to be dealing with threading bugs
>> in ancient zstd versions.  However, why do you suggest 1.3.7 in
>> particular?

> That's where I found that ZSTD_CLEVEL_DEFAULT was added, in their git.

> I've just installed a .deb for 1.3.8, and discovered that the APIs used by
> basebackup were considered experimental/nonpublic/static-lib-only until 1.4.0

Indeed.  I tried building against 1.3.6 (mainly because it was laying
around) and the error reported by Devrim is just the tip of the iceberg.
With "make -k", I see unknown-symbol failures on

ZSTD_CCtx_setParameter
ZSTD_c_compressionLevel
ZSTD_c_nbWorkers
ZSTD_CCtx_reset
ZSTD_reset_session_only
ZSTD_compressStream2
ZSTD_e_continue
ZSTD_e_end

I wonder whether Robert's ambition to be compatible with old versions
extends that far.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: head fails to build on SLES 12 (wal_compression=zstd)
@ 2022-04-01 00:39  Robert Haas <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 1 reply; 13+ messages in thread

From: Robert Haas @ 2022-04-01 00:39 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Devrim Gündüz <[email protected]>; PostgreSQL Hackers <[email protected]>; Michael Paquier <[email protected]>

On Thu, Mar 31, 2022 at 7:38 PM Tom Lane <[email protected]> wrote:
> Indeed.  I tried building against 1.3.6 (mainly because it was laying
> around) and the error reported by Devrim is just the tip of the iceberg.
> With "make -k", I see unknown-symbol failures on
>
> ZSTD_CCtx_setParameter
> ZSTD_c_compressionLevel
> ZSTD_c_nbWorkers
> ZSTD_CCtx_reset
> ZSTD_reset_session_only
> ZSTD_compressStream2
> ZSTD_e_continue
> ZSTD_e_end
>
> I wonder whether Robert's ambition to be compatible with old versions
> extends that far.

It definitely doesn't, and the fact that there's that much difference
in the APIs between 2018 and the present frankly makes my heart sink.

It looks like this stuff may be what libzstd calls the "advanced API"
which was considered unstable until 1.4.0 according to
https://github.com/facebook/zstd/releases -- so maybe we ought to
declare anything pre-1.4.0. to be unsupported. I really hope they're
serious about keeping the advanced API stable, though. I'm excited
about the potential of using libzstd, but I don't want to have to keep
adjusting our code to work with new library versions.

-- 
Robert Haas
EDB: http://www.enterprisedb.com






^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: head fails to build on SLES 12 (wal_compression=zstd)
@ 2022-04-01 01:10  Tom Lane <[email protected]>
  parent: Robert Haas <[email protected]>
  0 siblings, 2 replies; 13+ messages in thread

From: Tom Lane @ 2022-04-01 01:10 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Devrim Gündüz <[email protected]>; PostgreSQL Hackers <[email protected]>; Michael Paquier <[email protected]>

Robert Haas <[email protected]> writes:
> On Thu, Mar 31, 2022 at 7:38 PM Tom Lane <[email protected]> wrote:
>> Indeed.  I tried building against 1.3.6 (mainly because it was laying
>> around) and the error reported by Devrim is just the tip of the iceberg.
>> ...
>> I wonder whether Robert's ambition to be compatible with old versions
>> extends that far.

> It definitely doesn't, and the fact that there's that much difference
> in the APIs between 2018 and the present frankly makes my heart sink.

AFAICS it's just additions; this stuff is not evidence that they
broke anything.

> It looks like this stuff may be what libzstd calls the "advanced API"
> which was considered unstable until 1.4.0 according to
> https://github.com/facebook/zstd/releases -- so maybe we ought to
> declare anything pre-1.4.0. to be unsupported.

That seems like the appropriate answer to me.  I verified that we
build cleanly and pass check-world against 1.4.0, and later I'm
going to set up BF member longfin to use that.  So that will give
us an anchor that we support zstd that far back.  Had we written
this code earlier, maybe we'd have confined ourselves to 1.3.x
features ... but we didn't, and I don't see much value in doing
so now.

In short, I think we should push Justin's version-check patch,
and also fix the SGML docs to say that we require zstd >= 1.4.0.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: head fails to build on SLES 12 (wal_compression=zstd)
@ 2022-04-01 04:35  Michael Paquier <[email protected]>
  parent: Tom Lane <[email protected]>
  1 sibling, 0 replies; 13+ messages in thread

From: Michael Paquier @ 2022-04-01 04:35 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; Justin Pryzby <[email protected]>; Devrim Gündüz <[email protected]>; PostgreSQL Hackers <[email protected]>

On Thu, Mar 31, 2022 at 09:10:00PM -0400, Tom Lane wrote:
> In short, I think we should push Justin's version-check patch,
> and also fix the SGML docs to say that we require zstd >= 1.4.0.

1.4.0 was released in April 2019, just 3 years ago.  It does not sound
that bad to me to make this version number a requirement for 15~ if
one wants to use zstd.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: head fails to build on SLES 12 (wal_compression=zstd)
@ 2022-04-01 15:06  Tom Lane <[email protected]>
  parent: Tom Lane <[email protected]>
  1 sibling, 0 replies; 13+ messages in thread

From: Tom Lane @ 2022-04-01 15:06 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Devrim Gündüz <[email protected]>; PostgreSQL Hackers <[email protected]>; Michael Paquier <[email protected]>

I wrote:
> That seems like the appropriate answer to me.  I verified that we
> build cleanly and pass check-world against 1.4.0, and later I'm
> going to set up BF member longfin to use that.

Done ...

> In short, I think we should push Justin's version-check patch,
> and also fix the SGML docs to say that we require zstd >= 1.4.0.

... and done.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 13+ messages in thread


end of thread, other threads:[~2022-04-01 15:06 UTC | newest]

Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-05-28 18:15 [PATCH v27 1/5] Remove pg_collation.collversion. Thomas Munro <[email protected]>
2022-03-31 15:37 Re: head fails to build on SLES 12 (wal_compression=zstd) Justin Pryzby <[email protected]>
2022-03-31 15:44 ` Re: head fails to build on SLES 12 (wal_compression=zstd) Tom Lane <[email protected]>
2022-03-31 16:26   ` Re: head fails to build on SLES 12 (wal_compression=zstd) Robert Haas <[email protected]>
2022-03-31 16:34     ` Re: head fails to build on SLES 12 (wal_compression=zstd) Justin Pryzby <[email protected]>
2022-03-31 16:37     ` Re: head fails to build on SLES 12 (wal_compression=zstd) Tom Lane <[email protected]>
2022-03-31 16:55       ` Re: head fails to build on SLES 12 (wal_compression=zstd) Robert Haas <[email protected]>
2022-03-31 22:42   ` Re: head fails to build on SLES 12 (wal_compression=zstd) Justin Pryzby <[email protected]>
2022-03-31 23:38     ` Re: head fails to build on SLES 12 (wal_compression=zstd) Tom Lane <[email protected]>
2022-04-01 00:39       ` Re: head fails to build on SLES 12 (wal_compression=zstd) Robert Haas <[email protected]>
2022-04-01 01:10         ` Re: head fails to build on SLES 12 (wal_compression=zstd) Tom Lane <[email protected]>
2022-04-01 04:35           ` Re: head fails to build on SLES 12 (wal_compression=zstd) Michael Paquier <[email protected]>
2022-04-01 15:06           ` Re: head fails to build on SLES 12 (wal_compression=zstd) Tom Lane <[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