public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v28 5/5] Doc: Add Collation Versions section.
2+ messages / 2 participants
[nested] [flat]

* [PATCH v28 5/5] Doc: Add Collation Versions section.
@ 2020-03-11 02:01  Thomas Munro <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Thomas Munro @ 2020-03-11 02:01 UTC (permalink / raw)

Supply a brief introduction to collation version concepts.

Author: Thomas Munro <[email protected]>
Reviewed-by: Julien Rouhaud <[email protected]>
Discussion: https://postgr.es/m/CAEepm%3D0uEQCpfq_%2BLYFBdArCe4Ot98t1aR4eYiYTe%3DyavQygiQ%40mail.gmail.com
---
 doc/src/sgml/charset.sgml | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/doc/src/sgml/charset.sgml b/doc/src/sgml/charset.sgml
index 4b4563c5b9..12a82ff18c 100644
--- a/doc/src/sgml/charset.sgml
+++ b/doc/src/sgml/charset.sgml
@@ -948,6 +948,41 @@ CREATE COLLATION ignore_accents (provider = icu, locale = 'und-u-ks-level1-kc-tr
     </tip>
    </sect3>
   </sect2>
+
+  <sect2 id="collation-versions">
+   <title>Collation Versions</title>
+
+   <para>
+    The ordering defined by a collation is not necessarily fixed over time.
+    If a collation changes for any reason, persistent data structures such as
+    b-trees that depend on a stable ordering of text might be corrupted.
+    <productname>PostgreSQL</productname> defends against this by recording
+    the current version of each referenced collation for any index that
+    depends on it in the
+    <link linkend="catalog-pg-depend"><structname>pg_depend</structname></link>
+    catalog, if the collation provider makes it available.  If the provider
+    later begins to report a different version, a warning will be issued
+    when the index is accessed, until either the <xref linkend="sql-reindex"/>
+    or the <xref linkend="sql-alterindex"/> command is used to update the
+    version.
+   </para>
+   <para>
+    Version information is available for collations from the
+    <literal>icu</literal> provider on all operating systems.  For the
+    <literal>libc</literal> provider, versions are currently only available
+    on systems using the GNU C library (most Linux systems).
+   </para>
+
+   <note>
+    <para>
+     When using the GNU C library for collations, the C library's version
+     is used as a proxy for the collation version.  Many Linux distributions
+     change collation definitions only when upgrading the C library, but this
+     approach is imperfect as maintainers are free to back-port newer
+     collation definitions to older C library releases.
+    </para>
+   </note>
+  </sect2>
  </sect1>
 
  <sect1 id="multibyte">
-- 
2.20.1


--gclmslubawp3yaq7--





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

* OID ordering dependency in pg_dump
@ 2023-02-13 21:26  Tom Lane <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Tom Lane @ 2023-02-13 21:26 UTC (permalink / raw)
  To: [email protected]

While starting to poke at the hashed-enum-partition-key problem
recently discussed [1], I realized that pg_dump's flagInhAttrs()
function has a logic issue: its loop changes state that will be
inspected in other iterations of the loop, and there's no guarantee
about the order in which related tables will be visited.  Typically
we'll see parent tables before children because parents tend to have
smaller OIDs, but there are plenty of ways in which that might not
be true.

As far as I can tell, the implications of this are just cosmetic:
we might dump DEFAULT or GENERATED expressions that we don't really
need to because they match properties of the parent.  Still, it's
buggy, and somebody might carelessly extend the logic in a way that
introduces more-serious bugs.  PFA a proposed patch.

			regards, tom lane

[1] https://www.postgresql.org/message-id/1376149.1675268279%40sss.pgh.pa.us



Attachments:

  [text/x-diff] v1-fix-flagInhAttrs-traversal-order-bugs.patch (4.0K, ../../[email protected]/2-v1-fix-flagInhAttrs-traversal-order-bugs.patch)
  download | inline diff:
diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index a43f2e5553..cbf00adbae 100644
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -471,6 +471,13 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables)
 				j,
 				k;
 
+	/*
+	 * We scan the tables in OID order, since that's how tblinfo[] is sorted.
+	 * Hence we will typically visit parents before their children --- but
+	 * that is *not* guaranteed.  Thus this loop must be careful that it does
+	 * not alter table properties in a way that would change decisions made at
+	 * their child tables during other iterations.
+	 */
 	for (i = 0; i < numTables; i++)
 	{
 		TableInfo  *tbinfo = &(tblinfo[i]);
@@ -519,15 +526,18 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables)
 										parent->numatts);
 				if (inhAttrInd >= 0)
 				{
+					AttrDefInfo *parentDef = parent->attrdefs[inhAttrInd];
+
 					foundNotNull |= parent->notnull[inhAttrInd];
-					foundDefault |= (parent->attrdefs[inhAttrInd] != NULL &&
+					foundDefault |= (parentDef != NULL &&
+									 strcmp(parentDef->adef_expr, "NULL") != 0 &&
 									 !parent->attgenerated[inhAttrInd]);
 					if (parent->attgenerated[inhAttrInd])
 					{
 						/* these pointer nullness checks are just paranoia */
-						if (parent->attrdefs[inhAttrInd] != NULL &&
+						if (parentDef != NULL &&
 							tbinfo->attrdefs[j] != NULL &&
-							strcmp(parent->attrdefs[inhAttrInd]->adef_expr,
+							strcmp(parentDef->adef_expr,
 								   tbinfo->attrdefs[j]->adef_expr) == 0)
 							foundSameGenerated = true;
 						else
@@ -539,7 +549,14 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables)
 			/* Remember if we found inherited NOT NULL */
 			tbinfo->inhNotNull[j] = foundNotNull;
 
-			/* Manufacture a DEFAULT NULL clause if necessary */
+			/*
+			 * Manufacture a DEFAULT NULL clause if necessary.  This breaks
+			 * the advice given above to avoid changing state that might get
+			 * inspected in other loop iterations.  We prevent trouble by
+			 * having the foundDefault test above check whether adef_expr is
+			 * "NULL", so that it will reach the same conclusion before or
+			 * after this is done.
+			 */
 			if (foundDefault && tbinfo->attrdefs[j] == NULL)
 			{
 				AttrDefInfo *attrDef;
@@ -575,10 +592,10 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables)
 				tbinfo->attrdefs[j] = attrDef;
 			}
 
-			/* Remove generation expression from child if possible */
+			/* No need to dump generation expression if it's inheritable */
 			if (foundSameGenerated && !foundDiffGenerated &&
 				!tbinfo->ispartition && !dopt->binary_upgrade)
-				tbinfo->attrdefs[j] = NULL;
+				tbinfo->attrdefs[j]->dobj.dump = DUMP_COMPONENT_NONE;
 		}
 	}
 }
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 527c7651ab..8cd8f10d5e 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -8531,9 +8531,9 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 				 * Column generation expressions cannot be dumped separately,
 				 * because there is no syntax for it.  By setting separate to
 				 * false here we prevent the "default" from being processed as
-				 * its own dumpable object, and flagInhAttrs() will remove it
-				 * from the table if possible (that is, if it can be inherited
-				 * from a parent).
+				 * its own dumpable object.  Later, flagInhAttrs() will mark
+				 * it as not to be dumped at all, if possible (that is, if it
+				 * can be inherited from a parent).
 				 */
 				attrdefs[j].separate = false;
 			}
@@ -15351,9 +15351,11 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
 					bool		print_notnull;
 
 					/*
-					 * Default value --- suppress if to be printed separately.
+					 * Default value --- suppress if to be printed separately
+					 * or not at all.
 					 */
 					print_default = (tbinfo->attrdefs[j] != NULL &&
+									 tbinfo->attrdefs[j]->dobj.dump &&
 									 !tbinfo->attrdefs[j]->separate);
 
 					/*


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


end of thread, other threads:[~2023-02-13 21:26 UTC | newest]

Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-03-11 02:01 [PATCH v28 5/5] Doc: Add Collation Versions section. Thomas Munro <[email protected]>
2023-02-13 21:26 OID ordering dependency in pg_dump 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