public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 6/8] Add default_toast_compression GUC 2+ messages / 2 participants [nested] [flat]
* [PATCH 6/8] Add default_toast_compression GUC @ 2021-03-10 08:35 Dilip Kumar <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Dilip Kumar @ 2021-03-10 08:35 UTC (permalink / raw) Justin Pryzby and Dilip Kumar --- src/backend/access/common/toast_compression.c | 46 +++++++++++++++++++ src/backend/access/common/tupdesc.c | 2 +- src/backend/bootstrap/bootstrap.c | 2 +- src/backend/commands/tablecmds.c | 8 ++-- src/backend/utils/misc/guc.c | 12 +++++ src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/access/toast_compression.h | 22 ++++++++- src/test/regress/expected/compression.out | 16 +++++++ src/test/regress/expected/compression_1.out | 19 ++++++++ src/test/regress/sql/compression.sql | 8 ++++ 10 files changed, 128 insertions(+), 8 deletions(-) diff --git a/src/backend/access/common/toast_compression.c b/src/backend/access/common/toast_compression.c index db4911ce43..27fe47b201 100644 --- a/src/backend/access/common/toast_compression.c +++ b/src/backend/access/common/toast_compression.c @@ -55,6 +55,9 @@ const CompressionRoutine toast_compression[] = } }; +/* Compile-time default */ +char *default_toast_compression = DEFAULT_TOAST_COMPRESSION; + /* * pglz_cmcompress - compression routine for pglz compression method * @@ -306,3 +309,46 @@ GetCompressionRoutines(char method) { return &toast_compression[CompressionMethodToId(method)]; } + +/* check_hook: validate new default_toast_compression */ +bool +check_default_toast_compression(char **newval, void **extra, GucSource source) +{ + if (**newval == '\0') + { + GUC_check_errdetail("%s cannot be empty.", + "default_toast_compression"); + return false; + } + + if (strlen(*newval) >= NAMEDATALEN) + { + GUC_check_errdetail("%s is too long (maximum %d characters).", + "default_toast_compression", NAMEDATALEN - 1); + return false; + } + + if (!CompressionMethodIsValid(CompressionNameToMethod(*newval))) + { + /* + * When source == PGC_S_TEST, don't throw a hard error for a + * nonexistent compression method, only a NOTICE. See comments in + * guc.h. + */ + if (source == PGC_S_TEST) + { + ereport(NOTICE, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("compression method \"%s\" does not exist", + *newval))); + } + else + { + GUC_check_errdetail("Compression method \"%s\" does not exist.", + *newval); + return false; + } + } + + return true; +} diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c index a66d1113db..362a371a6c 100644 --- a/src/backend/access/common/tupdesc.c +++ b/src/backend/access/common/tupdesc.c @@ -668,7 +668,7 @@ TupleDescInitEntry(TupleDesc desc, att->attcollation = typeForm->typcollation; if (IsStorageCompressible(typeForm->typstorage)) - att->attcompression = DefaultCompressionMethod; + att->attcompression = GetDefaultToastCompression(); else att->attcompression = InvalidCompressionMethod; diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index af0c0aa77e..9be0841d08 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -733,7 +733,7 @@ DefineAttr(char *name, char *type, int attnum, int nullness) attrtypes[attnum]->atttypmod = -1; attrtypes[attnum]->attislocal = true; if (IsStorageCompressible(attrtypes[attnum]->attstorage)) - attrtypes[attnum]->attcompression = DefaultCompressionMethod; + attrtypes[attnum]->attcompression = GetDefaultToastCompression(); else attrtypes[attnum]->attcompression = InvalidCompressionMethod; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index e20dd5f63c..a0d2af0cde 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -11971,7 +11971,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, if (!IsStorageCompressible(tform->typstorage)) attTup->attcompression = InvalidCompressionMethod; else if (!CompressionMethodIsValid(attTup->attcompression)) - attTup->attcompression = DefaultCompressionMethod; + attTup->attcompression = GetDefaultToastCompression(); } else attTup->attcompression = InvalidCompressionMethod; @@ -17847,9 +17847,9 @@ GetAttributeCompression(Form_pg_attribute att, char *compression) /* fallback to default compression if it's not specified */ if (compression == NULL) - return DefaultCompressionMethod; - - cmethod = CompressionNameToMethod(compression); + cmethod = GetDefaultToastCompression(); + else + cmethod = CompressionNameToMethod(compression); return cmethod; } diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 953837085f..e59ed5b214 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -33,6 +33,7 @@ #include "access/gin.h" #include "access/rmgr.h" #include "access/tableam.h" +#include "access/toast_compression.h" #include "access/transam.h" #include "access/twophase.h" #include "access/xact.h" @@ -3915,6 +3916,17 @@ static struct config_string ConfigureNamesString[] = check_default_table_access_method, NULL, NULL }, + { + {"default_toast_compression", PGC_USERSET, CLIENT_CONN_STATEMENT, + gettext_noop("Sets the default compression for new columns."), + NULL, + GUC_IS_NAME + }, + &default_toast_compression, + DEFAULT_TOAST_COMPRESSION, + check_default_toast_compression, NULL, NULL + }, + { {"default_tablespace", PGC_USERSET, CLIENT_CONN_STATEMENT, gettext_noop("Sets the default tablespace to create tables and indexes in."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index f46c2dd7a8..7d7a433dcc 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -659,6 +659,7 @@ #temp_tablespaces = '' # a list of tablespace names, '' uses # only default tablespace #default_table_access_method = 'heap' +#default_toast_compression = 'pglz' # 'pglz' or 'lz4' #check_function_bodies = on #default_transaction_isolation = 'read committed' #default_transaction_read_only = off diff --git a/src/include/access/toast_compression.h b/src/include/access/toast_compression.h index 38800cb97a..e9d9ce5634 100644 --- a/src/include/access/toast_compression.h +++ b/src/include/access/toast_compression.h @@ -15,6 +15,14 @@ #include "postgres.h" +#include "utils/guc.h" + +/* default compression method if not specified. */ +#define DEFAULT_TOAST_COMPRESSION "pglz" + +/* GUCs */ +extern char *default_toast_compression; + /* * Built-in compression methods. pg_attribute will store this in the * attcompression column. @@ -36,8 +44,6 @@ typedef enum CompressionId LZ4_COMPRESSION_ID = 1 } CompressionId; -/* use default compression method if it is not specified. */ -#define DefaultCompressionMethod PGLZ_COMPRESSION #define IsValidCompression(cm) ((cm) != InvalidCompressionMethod) #define IsStorageCompressible(storage) ((storage) != TYPSTORAGE_PLAIN && \ @@ -67,6 +73,8 @@ typedef struct CompressionRoutine extern char CompressionNameToMethod(char *compression); extern const CompressionRoutine *GetCompressionRoutines(char method); +extern bool check_default_toast_compression(char **newval, void **extra, + GucSource source); /* * CompressionMethodToId - Convert compression method to compression id. @@ -115,5 +123,15 @@ GetCompressionMethodName(char method) return GetCompressionRoutines(method)->cmname; } +/* + * GetDefaultToastCompression -- get the current toast compression + * + * This exists to hide the use of the default_toast_compression GUC variable. + */ +static inline char +GetDefaultToastCompression(void) +{ + return CompressionNameToMethod(default_toast_compression); +} #endif /* TOAST_COMPRESSION_H */ diff --git a/src/test/regress/expected/compression.out b/src/test/regress/expected/compression.out index 6981d580dc..9ddffc8b18 100644 --- a/src/test/regress/expected/compression.out +++ b/src/test/regress/expected/compression.out @@ -231,6 +231,22 @@ CREATE TABLE cminh(f1 TEXT COMPRESSION lz4) INHERITS(cmdata); NOTICE: merging column "f1" with inherited definition ERROR: column "f1" has a compression method conflict DETAIL: pglz versus lz4 +-- test default_toast_compression GUC +SET default_toast_compression = ''; +ERROR: invalid value for parameter "default_toast_compression": "" +DETAIL: default_toast_compression cannot be empty. +SET default_toast_compression = 'I do not exist compression'; +ERROR: invalid value for parameter "default_toast_compression": "I do not exist compression" +DETAIL: Compression method "I do not exist compression" does not exist. +SET default_toast_compression = 'lz4'; +DROP TABLE cmdata2; +CREATE TABLE cmdata2 (f1 text); +\d+ cmdata2 + Table "public.cmdata2" + Column | Type | Collation | Nullable | Default | Storage | Compression | Stats target | Description +--------+------+-----------+----------+---------+----------+-------------+--------------+------------- + f1 | text | | | | extended | lz4 | | + -- check data is ok SELECT length(f1) FROM cmdata; length diff --git a/src/test/regress/expected/compression_1.out b/src/test/regress/expected/compression_1.out index 893c18c7fa..03a7f46554 100644 --- a/src/test/regress/expected/compression_1.out +++ b/src/test/regress/expected/compression_1.out @@ -237,6 +237,25 @@ CREATE TABLE cminh(f1 TEXT COMPRESSION lz4) INHERITS(cmdata); NOTICE: merging column "f1" with inherited definition ERROR: column "f1" has a compression method conflict DETAIL: pglz versus lz4 +-- test default_toast_compression GUC +SET default_toast_compression = ''; +ERROR: invalid value for parameter "default_toast_compression": "" +DETAIL: default_toast_compression cannot be empty. +SET default_toast_compression = 'I do not exist compression'; +ERROR: invalid value for parameter "default_toast_compression": "I do not exist compression" +DETAIL: Compression method "I do not exist compression" does not exist. +SET default_toast_compression = 'lz4'; +ERROR: unsupported LZ4 compression method +DETAIL: This functionality requires the server to be built with lz4 support. +HINT: You need to rebuild PostgreSQL using --with-lz4. +DROP TABLE cmdata2; +CREATE TABLE cmdata2 (f1 text); +\d+ cmdata2 + Table "public.cmdata2" + Column | Type | Collation | Nullable | Default | Storage | Compression | Stats target | Description +--------+------+-----------+----------+---------+----------+-------------+--------------+------------- + f1 | text | | | | extended | pglz | | + -- check data is ok SELECT length(f1) FROM cmdata; length diff --git a/src/test/regress/sql/compression.sql b/src/test/regress/sql/compression.sql index b085c9fbd9..6c9b24f1f7 100644 --- a/src/test/regress/sql/compression.sql +++ b/src/test/regress/sql/compression.sql @@ -103,6 +103,14 @@ SELECT pg_column_compression(f1) FROM cmpart; CREATE TABLE cminh() INHERITS(cmdata, cmdata1); CREATE TABLE cminh(f1 TEXT COMPRESSION lz4) INHERITS(cmdata); +-- test default_toast_compression GUC +SET default_toast_compression = ''; +SET default_toast_compression = 'I do not exist compression'; +SET default_toast_compression = 'lz4'; +DROP TABLE cmdata2; +CREATE TABLE cmdata2 (f1 text); +\d+ cmdata2 + -- check data is ok SELECT length(f1) FROM cmdata; SELECT length(f1) FROM cmdata1; -- 2.17.0 --C94crkcyjafcjHxo Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0007-Alter-table-set-compression.patch" ^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: altering a column's collation leaves an invalid foreign key @ 2024-03-25 06:47 Paul Jungwirth <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Paul Jungwirth @ 2024-03-25 06:47 UTC (permalink / raw) To: PostgreSQL Hackers <[email protected]> On 3/23/24 10:04, Paul Jungwirth wrote: > Perhaps if the previous collation was nondeterministic we should force a re-check. Here is a patch implementing this. It was a bit more fuss than I expected, so maybe someone has a better way. We have had nondeterministic collations since v12, so perhaps it is something to back-patch? Yours, -- Paul ~{:-) [email protected] Attachments: [text/x-patch] v1-0001-Re-check-foreign-key-if-referenced-collation-was-.patch (13.4K, ../../[email protected]/2-v1-0001-Re-check-foreign-key-if-referenced-collation-was-.patch) download | inline diff: From a8078f85859400f4e4cffb57d8ec1b069e46bfb8 Mon Sep 17 00:00:00 2001 From: "Paul A. Jungwirth" <[email protected]> Date: Sun, 24 Mar 2024 23:37:48 -0700 Subject: [PATCH v1] Re-check foreign key if referenced collation was nondeterministic With deterministic collations, we break ties by looking at binary equality. But nondeterministic collations can allow non-identical values to be considered equal, e.g. 'a' and 'A' when case-insensitive. So some references that used to be valid may not be anymore. --- src/backend/commands/tablecmds.c | 96 ++++++++++++++++--- src/include/nodes/parsenodes.h | 2 + .../regress/expected/collate.icu.utf8.out | 9 ++ src/test/regress/sql/collate.icu.utf8.sql | 8 ++ 4 files changed, 103 insertions(+), 12 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 71740984f33..940b1845edb 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -194,6 +194,7 @@ typedef struct AlteredTableInfo /* Objects to rebuild after completing ALTER TYPE operations */ List *changedConstraintOids; /* OIDs of constraints to rebuild */ List *changedConstraintDefs; /* string definitions of same */ + List *changedCollationOids; /* collation of each attnum */ List *changedIndexOids; /* OIDs of indexes to rebuild */ List *changedIndexDefs; /* string definitions of same */ char *replicaIdentityIndex; /* index to reset as REPLICA IDENTITY */ @@ -572,6 +573,7 @@ static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, AlterTableCmd *cmd, LOCKMODE lockmode); static void RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, Relation rel, AttrNumber attnum, const char *colName); +static void RememberCollationForRebuilding(AttrNumber attnum, AlteredTableInfo *tab); static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab); static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab); static void RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab); @@ -579,12 +581,12 @@ static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode); static void ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, List **wqueue, LOCKMODE lockmode, - bool rewrite); + bool rewrite, List *collationOids); static void RebuildConstraintComment(AlteredTableInfo *tab, AlterTablePass pass, Oid objid, Relation rel, List *domname, const char *conname); static void TryReuseIndex(Oid oldId, IndexStmt *stmt); -static void TryReuseForeignKey(Oid oldId, Constraint *con); +static void TryReuseForeignKey(Oid oldId, Constraint *con, List *changedCollationOids); static ObjectAddress ATExecAlterColumnGenericOptions(Relation rel, const char *colName, List *options, LOCKMODE lockmode); static void change_owner_fix_column_acls(Oid relationOid, @@ -9826,6 +9828,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, bool old_check_ok; ObjectAddress address; ListCell *old_pfeqop_item = list_head(fkconstraint->old_conpfeqop); + ListCell *old_collation_item = list_head(fkconstraint->old_collations); /* * Grab ShareRowExclusiveLock on the pk table, so that someone doesn't @@ -10212,9 +10215,15 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, CoercionPathType new_pathtype; Oid old_castfunc; Oid new_castfunc; + Oid old_collation; Form_pg_attribute attr = TupleDescAttr(tab->oldDesc, fkattnum[i] - 1); + /* Get the collation for this key part. */ + old_collation = lfirst_oid(old_collation_item); + old_collation_item = lnext(fkconstraint->old_collations, + old_collation_item); + /* * Identify coercion pathways from each of the old and new FK-side * column types to the right (foreign) operand type of the pfeqop. @@ -10250,9 +10259,12 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, * turn conform to the domain. Consequently, we need not treat * domains specially here. * - * Since we require that all collations share the same notion of - * equality (which they do, because texteq reduces to bitwise - * equality), we don't compare collation here. + * All deterministic collations use bitwise equality to resolve + * ties, but if the previous collation was nondeterministic, + * we must re-check the foreign key, because some references + * that use to be "equal" may not be anymore. If we have + * InvalidOid here, either there was no collation or the + * attribute didn't change. * * We need not directly consider the PK type. It's necessarily * binary coercible to the opcintype of the unique index column, @@ -10261,6 +10273,8 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, */ old_check_ok = (new_pathtype == old_pathtype && new_castfunc == old_castfunc && + (old_collation == InvalidOid || + get_collation_isdeterministic(old_collation)) && (!IsPolymorphicType(pfeqop_right) || new_fktype == old_fktype)); } @@ -13985,6 +13999,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, relKind == RELKIND_PARTITIONED_INDEX) { Assert(foundObject.objectSubId == 0); + RememberCollationForRebuilding(attnum, tab); RememberIndexForRebuilding(foundObject.objectId, tab); } else if (relKind == RELKIND_SEQUENCE) @@ -14006,6 +14021,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, case OCLASS_CONSTRAINT: Assert(foundObject.objectSubId == 0); + RememberCollationForRebuilding(attnum, tab); RememberConstraintForRebuilding(foundObject.objectId, tab); break; @@ -14188,6 +14204,40 @@ RememberClusterOnForRebuilding(Oid indoid, AlteredTableInfo *tab) tab->clusterOnIndex = get_rel_name(indoid); } +/* + * Subroutine for ATExecAlterColumnType: remember what the collations were + * for each attribute of the table. This is because if one of them used to be + * nondeterministic, foreign keys that referenced that attribute must be + * re-checked. We make a list with one entry for each attribute in the table, + * so that FKs can easily look them up by attribute number. But only attributes + * that are changing get a non-zero value. + */ +static void +RememberCollationForRebuilding(AttrNumber attnum, AlteredTableInfo *tab) +{ + Oid typid; + int32 typmod; + Oid collid; + ListCell *lc; + + /* Fill in the list with InvalidOid if this is our first visit */ + if (tab->changedCollationOids == NIL) + { + int len = RelationGetNumberOfAttributes(tab->rel); + int i; + + for (i = 0; i < len; i++) + tab->changedCollationOids = lappend_oid(tab->changedCollationOids, + InvalidOid); + } + + get_atttypetypmodcoll(RelationGetRelid(tab->rel), attnum, + &typid, &typmod, &collid); + + lc = list_nth_cell(tab->changedCollationOids, attnum - 1); + lfirst_oid(lc) = collid; +} + /* * Subroutine for ATExecAlterColumnType: remember that a constraint needs * to be rebuilt (which we might already know). @@ -14393,7 +14443,8 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) ATPostAlterTypeParse(oldId, relid, confrelid, (char *) lfirst(def_item), - wqueue, lockmode, tab->rewrite); + wqueue, lockmode, tab->rewrite, + tab->changedCollationOids); } forboth(oid_item, tab->changedIndexOids, def_item, tab->changedIndexDefs) @@ -14404,7 +14455,8 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) relid = IndexGetRelation(oldId, false); ATPostAlterTypeParse(oldId, relid, InvalidOid, (char *) lfirst(def_item), - wqueue, lockmode, tab->rewrite); + wqueue, lockmode, tab->rewrite, + tab->changedCollationOids); ObjectAddressSet(obj, RelationRelationId, oldId); add_exact_object_address(&obj, objects); @@ -14420,7 +14472,8 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) relid = StatisticsGetRelation(oldId, false); ATPostAlterTypeParse(oldId, relid, InvalidOid, (char *) lfirst(def_item), - wqueue, lockmode, tab->rewrite); + wqueue, lockmode, tab->rewrite, + tab->changedCollationOids); ObjectAddressSet(obj, StatisticExtRelationId, oldId); add_exact_object_address(&obj, objects); @@ -14483,7 +14536,8 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) */ static void ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, - List **wqueue, LOCKMODE lockmode, bool rewrite) + List **wqueue, LOCKMODE lockmode, bool rewrite, + List *changedCollationOids) { List *raw_parsetree_list; List *querytree_list; @@ -14610,7 +14664,7 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, /* rewriting neither side of a FK */ if (con->contype == CONSTR_FOREIGN && !rewrite && tab->rewrite == 0) - TryReuseForeignKey(oldId, con); + TryReuseForeignKey(oldId, con, changedCollationOids); con->reset_default_tblspc = true; cmd->subtype = AT_ReAddConstraint; tab->subcmds[AT_PASS_OLD_CONSTR] = @@ -14768,20 +14822,22 @@ TryReuseIndex(Oid oldId, IndexStmt *stmt) * * Stash the old P-F equality operator into the Constraint node, for possible * use by ATAddForeignKeyConstraint() in determining whether revalidation of - * this constraint can be skipped. + * this constraint can be skipped. Also stash the collations. */ static void -TryReuseForeignKey(Oid oldId, Constraint *con) +TryReuseForeignKey(Oid oldId, Constraint *con, List *changedCollationOids) { HeapTuple tup; Datum adatum; ArrayType *arr; Oid *rawarr; + AttrNumber *attarr; int numkeys; int i; Assert(con->contype == CONSTR_FOREIGN); Assert(con->old_conpfeqop == NIL); /* already prepared this node */ + Assert(con->old_collations == NIL); /* already prepared this node */ tup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(oldId)); if (!HeapTupleIsValid(tup)) /* should not happen */ @@ -14802,6 +14858,22 @@ TryReuseForeignKey(Oid oldId, Constraint *con) for (i = 0; i < numkeys; i++) con->old_conpfeqop = lappend_oid(con->old_conpfeqop, rawarr[i]); + adatum = SysCacheGetAttrNotNull(CONSTROID, tup, + Anum_pg_constraint_conkey); + arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ + numkeys = ARR_DIMS(arr)[0]; + /* test follows the one in ri_FetchConstraintInfo() */ + if (ARR_NDIM(arr) != 1 || + ARR_HASNULL(arr) || + ARR_ELEMTYPE(arr) != INT2OID) + elog(ERROR, "conkey is not a 1-D smallint array"); + attarr = (AttrNumber *) ARR_DATA_PTR(arr); + + /* stash a List of the collation Oids in our Constraint node */ + for (i = 0; i < numkeys; i++) + con->old_collations = lappend_oid(con->old_collations, + list_nth_oid(changedCollationOids, attarr[i] - 1)); + ReleaseSysCache(tup); } diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index b89baef95d3..96c71c75115 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2691,6 +2691,8 @@ typedef struct Constraint char fk_del_action; /* ON DELETE action */ List *fk_del_set_cols; /* ON DELETE SET NULL/DEFAULT (col1, col2) */ List *old_conpfeqop; /* pg_constraint.conpfeqop of my former self */ + List *old_collations; /* collations of referenced columns, + before change */ Oid old_pktable_oid; /* pg_constraint.confrelid of my former * self */ diff --git a/src/test/regress/expected/collate.icu.utf8.out b/src/test/regress/expected/collate.icu.utf8.out index 4b8c8f143f3..4119776f0d8 100644 --- a/src/test/regress/expected/collate.icu.utf8.out +++ b/src/test/regress/expected/collate.icu.utf8.out @@ -1933,6 +1933,15 @@ SELECT * FROM test11fk; --- (0 rows) +-- Test altering the collation of the referenced column. +CREATE TABLE pktable (x text COLLATE case_insensitive PRIMARY KEY); +CREATE TABLE fktable (x text REFERENCES pktable); +INSERT INTO pktable VALUES ('a'); +INSERT INTO fktable VALUES ('A'); +-- should fail: +ALTER TABLE pktable ALTER COLUMN x TYPE text COLLATE "C"; +ERROR: insert or update on table "fktable" violates foreign key constraint "fktable_x_fkey" +DETAIL: Key (x)=(A) is not present in table "pktable". -- partitioning CREATE TABLE test20 (a int, b text COLLATE case_insensitive) PARTITION BY LIST (b); CREATE TABLE test20_1 PARTITION OF test20 FOR VALUES IN ('abc'); diff --git a/src/test/regress/sql/collate.icu.utf8.sql b/src/test/regress/sql/collate.icu.utf8.sql index 80f28a97d78..08a6964950b 100644 --- a/src/test/regress/sql/collate.icu.utf8.sql +++ b/src/test/regress/sql/collate.icu.utf8.sql @@ -721,6 +721,14 @@ DELETE FROM test11pk WHERE x = 'abc'; SELECT * FROM test11pk; SELECT * FROM test11fk; +-- Test altering the collation of the referenced column. +CREATE TABLE pktable (x text COLLATE case_insensitive PRIMARY KEY); +CREATE TABLE fktable (x text REFERENCES pktable); +INSERT INTO pktable VALUES ('a'); +INSERT INTO fktable VALUES ('A'); +-- should fail: +ALTER TABLE pktable ALTER COLUMN x TYPE text COLLATE "C"; + -- partitioning CREATE TABLE test20 (a int, b text COLLATE case_insensitive) PARTITION BY LIST (b); CREATE TABLE test20_1 PARTITION OF test20 FOR VALUES IN ('abc'); -- 2.42.0 ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2024-03-25 06:47 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-03-10 08:35 [PATCH 6/8] Add default_toast_compression GUC Dilip Kumar <[email protected]> 2024-03-25 06:47 Re: altering a column's collation leaves an invalid foreign key Paul Jungwirth <[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