On 18.01.22 13:54, Peter Eisentraut wrote:
On 18.01.22 05:02, Julien Rouhaud wrote:
If this is applied, then in my estimation all these hunks will
completely
disappear from the global ICU patch. So this would be a significant
win.
Agreed, the patch will be quite smaller and also easier to review.
Are you
planning to apply it on its own or add it to the default ICU patchset?
My plan is to apply this patch in the next few days.
This patch has been committed.
Here is a second preparation patch: Change collate and ctype fields to
type text.
I think this is useful by itself because it allows longer locale names.
ICU locale names with several options appended can be longer than 63
bytes. This case is probably broken right now. Also, it saves space in
the typical case, since most locale names are not anywhere near 63 bytes.From 1c46bf3138ad42074971aa3130142236de7e63f7 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Fri, 21 Jan 2022 10:01:25 +0100
Subject: [PATCH] Change collate and ctype fields to type text
This changes the data type of the catalog fields datcollate, datctype,
collcollate, and collctype from name to text. There wasn't ever a
really good reason for them to be of type name; presumably this was
just carried over from when they were fixed-size fields in pg_control,
first into the corresponding pg_database fields, and then to
pg_collation. The values are not identifiers or object names, and we
don't ever look them up that way.
Changing to type text saves space in the typical case, since locale
names are typically less than 10 bytes long. But it is also possible
that an ICU locale name with several customization options appended
could be longer than 63 bytes, so this also enables that case, which
was previously probably broken.
Discussion:
https://www.postgresql.org/message-id/flat/5e756dd6-0e91-d778-96fd-b1bcb06c1...@2ndquadrant.com
---
doc/src/sgml/catalogs.sgml | 40 +++++++++++++--------------
src/backend/catalog/pg_collation.c | 10 ++-----
src/backend/commands/collationcmds.c | 41 +++++++++++++++++++---------
src/backend/commands/dbcommands.c | 21 ++++++++++----
src/backend/utils/adt/pg_locale.c | 29 +++++++++++++-------
src/backend/utils/init/postinit.c | 11 ++++++--
src/include/catalog/pg_collation.h | 4 +--
src/include/catalog/pg_database.h | 12 ++++----
src/include/utils/pg_locale.h | 2 ++
9 files changed, 104 insertions(+), 66 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 1e65c426b2..7d5b0b1656 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -2368,7 +2368,7 @@ <title><structname>pg_collation</structname>
Columns</title>
<row>
<entry role="catalog_table_entry"><para role="column_definition">
- <structfield>collcollate</structfield> <type>name</type>
+ <structfield>collcollate</structfield> <type>text</type>
</para>
<para>
<symbol>LC_COLLATE</symbol> for this collation object
@@ -2377,7 +2377,7 @@ <title><structname>pg_collation</structname>
Columns</title>
<row>
<entry role="catalog_table_entry"><para role="column_definition">
- <structfield>collctype</structfield> <type>name</type>
+ <structfield>collctype</structfield> <type>text</type>
</para>
<para>
<symbol>LC_CTYPE</symbol> for this collation object
@@ -2951,24 +2951,6 @@ <title><structname>pg_database</structname>
Columns</title>
</para></entry>
</row>
- <row>
- <entry role="catalog_table_entry"><para role="column_definition">
- <structfield>datcollate</structfield> <type>name</type>
- </para>
- <para>
- LC_COLLATE for this database
- </para></entry>
- </row>
-
- <row>
- <entry role="catalog_table_entry"><para role="column_definition">
- <structfield>datctype</structfield> <type>name</type>
- </para>
- <para>
- LC_CTYPE for this database
- </para></entry>
- </row>
-
<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>datistemplate</structfield> <type>bool</type>
@@ -3043,6 +3025,24 @@ <title><structname>pg_database</structname>
Columns</title>
</para></entry>
</row>
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>datcollate</structfield> <type>text</type>
+ </para>
+ <para>
+ LC_COLLATE for this database
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>datctype</structfield> <type>text</type>
+ </para>
+ <para>
+ LC_CTYPE for this database
+ </para></entry>
+ </row>
+
<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>datacl</structfield> <type>aclitem[]</type>
diff --git a/src/backend/catalog/pg_collation.c
b/src/backend/catalog/pg_collation.c
index 5be6600652..bfc02d3038 100644
--- a/src/backend/catalog/pg_collation.c
+++ b/src/backend/catalog/pg_collation.c
@@ -58,9 +58,7 @@ CollationCreate(const char *collname, Oid collnamespace,
HeapTuple tup;
Datum values[Natts_pg_collation];
bool nulls[Natts_pg_collation];
- NameData name_name,
- name_collate,
- name_ctype;
+ NameData name_name;
Oid oid;
ObjectAddress myself,
referenced;
@@ -163,10 +161,8 @@ CollationCreate(const char *collname, Oid collnamespace,
values[Anum_pg_collation_collprovider - 1] = CharGetDatum(collprovider);
values[Anum_pg_collation_collisdeterministic - 1] =
BoolGetDatum(collisdeterministic);
values[Anum_pg_collation_collencoding - 1] =
Int32GetDatum(collencoding);
- namestrcpy(&name_collate, collcollate);
- values[Anum_pg_collation_collcollate - 1] = NameGetDatum(&name_collate);
- namestrcpy(&name_ctype, collctype);
- values[Anum_pg_collation_collctype - 1] = NameGetDatum(&name_ctype);
+ values[Anum_pg_collation_collcollate - 1] =
CStringGetTextDatum(collcollate);
+ values[Anum_pg_collation_collctype - 1] =
CStringGetTextDatum(collctype);
if (collversion)
values[Anum_pg_collation_collversion - 1] =
CStringGetTextDatum(collversion);
else
diff --git a/src/backend/commands/collationcmds.c
b/src/backend/commands/collationcmds.c
index 56748551de..12fc2316f9 100644
--- a/src/backend/commands/collationcmds.c
+++ b/src/backend/commands/collationcmds.c
@@ -129,18 +129,30 @@ DefineCollation(ParseState *pstate, List *names, List
*parameters, bool if_not_e
{
Oid collid;
HeapTuple tp;
+ Datum datum;
+ bool isnull;
collid = get_collation_oid(defGetQualifiedName(fromEl), false);
tp = SearchSysCache1(COLLOID, ObjectIdGetDatum(collid));
if (!HeapTupleIsValid(tp))
elog(ERROR, "cache lookup failed for collation %u",
collid);
- collcollate = pstrdup(NameStr(((Form_pg_collation)
GETSTRUCT(tp))->collcollate));
- collctype = pstrdup(NameStr(((Form_pg_collation)
GETSTRUCT(tp))->collctype));
collprovider = ((Form_pg_collation)
GETSTRUCT(tp))->collprovider;
collisdeterministic = ((Form_pg_collation)
GETSTRUCT(tp))->collisdeterministic;
collencoding = ((Form_pg_collation)
GETSTRUCT(tp))->collencoding;
+ datum = SysCacheGetAttr(COLLOID, tp,
Anum_pg_collation_collcollate, &isnull);
+ if (!isnull)
+ collcollate = TextDatumGetCString(datum);
+ else
+ collcollate = NULL;
+
+ datum = SysCacheGetAttr(COLLOID, tp,
Anum_pg_collation_collctype, &isnull);
+ if (!isnull)
+ collctype = TextDatumGetCString(datum);
+ else
+ collctype = NULL;
+
ReleaseSysCache(tp);
/*
@@ -314,7 +326,7 @@ AlterCollation(AlterCollationStmt *stmt)
Oid collOid;
HeapTuple tup;
Form_pg_collation collForm;
- Datum collversion;
+ Datum datum;
bool isnull;
char *oldversion;
char *newversion;
@@ -332,11 +344,12 @@ AlterCollation(AlterCollationStmt *stmt)
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);
+ datum = SysCacheGetAttr(COLLOID, tup, Anum_pg_collation_collversion,
&isnull);
+ oldversion = isnull ? NULL : TextDatumGetCString(datum);
- newversion = get_collation_actual_version(collForm->collprovider,
NameStr(collForm->collcollate));
+ datum = SysCacheGetAttr(COLLOID, tup, Anum_pg_collation_collcollate,
&isnull);
+ Assert(!isnull);
+ newversion = get_collation_actual_version(collForm->collprovider,
TextDatumGetCString(datum));
/* cannot change from NULL to non-NULL or vice versa */
if ((!oldversion && newversion) || (oldversion && !newversion))
@@ -383,8 +396,9 @@ pg_collation_actual_version(PG_FUNCTION_ARGS)
{
Oid collid = PG_GETARG_OID(0);
HeapTuple tp;
- char *collcollate;
char collprovider;
+ Datum datum;
+ bool isnull;
char *version;
tp = SearchSysCache1(COLLOID, ObjectIdGetDatum(collid));
@@ -393,12 +407,13 @@ pg_collation_actual_version(PG_FUNCTION_ARGS)
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("collation with OID %u does not exist",
collid)));
- collcollate = pstrdup(NameStr(((Form_pg_collation)
GETSTRUCT(tp))->collcollate));
collprovider = ((Form_pg_collation) GETSTRUCT(tp))->collprovider;
- ReleaseSysCache(tp);
+ datum = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_collcollate,
&isnull);
+ Assert(!isnull);
+ version = get_collation_actual_version(collprovider,
TextDatumGetCString(datum));
- version = get_collation_actual_version(collprovider, collcollate);
+ ReleaseSysCache(tp);
if (version)
PG_RETURN_TEXT_P(cstring_to_text(version));
@@ -546,7 +561,7 @@ pg_import_system_collations(PG_FUNCTION_ARGS)
#ifdef READ_LOCALE_A_OUTPUT
{
FILE *locale_a_handle;
- char localebuf[NAMEDATALEN]; /* we assume ASCII so
this is fine */
+ char localebuf[LOCALE_NAME_BUFLEN];
int nvalid = 0;
Oid collid;
CollAliasData *aliases;
@@ -570,7 +585,7 @@ pg_import_system_collations(PG_FUNCTION_ARGS)
{
size_t len;
int enc;
- char alias[NAMEDATALEN];
+ char alias[LOCALE_NAME_BUFLEN];
len = strlen(localebuf);
diff --git a/src/backend/commands/dbcommands.c
b/src/backend/commands/dbcommands.c
index da8345561d..65b420aea6 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -523,10 +523,8 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
DirectFunctionCall1(namein, CStringGetDatum(dbname));
new_record[Anum_pg_database_datdba - 1] = ObjectIdGetDatum(datdba);
new_record[Anum_pg_database_encoding - 1] = Int32GetDatum(encoding);
- new_record[Anum_pg_database_datcollate - 1] =
- DirectFunctionCall1(namein, CStringGetDatum(dbcollate));
- new_record[Anum_pg_database_datctype - 1] =
- DirectFunctionCall1(namein, CStringGetDatum(dbctype));
+ new_record[Anum_pg_database_datcollate - 1] =
CStringGetTextDatum(dbcollate);
+ new_record[Anum_pg_database_datctype - 1] =
CStringGetTextDatum(dbctype);
new_record[Anum_pg_database_datistemplate - 1] =
BoolGetDatum(dbistemplate);
new_record[Anum_pg_database_datallowconn - 1] =
BoolGetDatum(dballowconnections);
new_record[Anum_pg_database_datconnlimit - 1] =
Int32GetDatum(dbconnlimit);
@@ -1820,6 +1818,9 @@ get_db_info(const char *name, LOCKMODE lockmode,
if (strcmp(name, NameStr(dbform->datname)) == 0)
{
+ Datum datum;
+ bool isnull;
+
/* oid of the database */
if (dbIdP)
*dbIdP = dbOid;
@@ -1846,9 +1847,17 @@ get_db_info(const char *name, LOCKMODE lockmode,
*dbTablespace = dbform->dattablespace;
/* default locale settings for this database */
if (dbCollate)
- *dbCollate =
pstrdup(NameStr(dbform->datcollate));
+ {
+ datum = SysCacheGetAttr(DATABASEOID,
tuple, Anum_pg_database_datcollate, &isnull);
+ Assert(!isnull);
+ *dbCollate = TextDatumGetCString(datum);
+ }
if (dbCtype)
- *dbCtype =
pstrdup(NameStr(dbform->datctype));
+ {
+ datum = SysCacheGetAttr(DATABASEOID,
tuple, Anum_pg_database_datctype, &isnull);
+ Assert(!isnull);
+ *dbCtype = TextDatumGetCString(datum);
+ }
ReleaseSysCache(tuple);
result = true;
break;
diff --git a/src/backend/utils/adt/pg_locale.c
b/src/backend/utils/adt/pg_locale.c
index 33cccc5c6c..aefa0818d0 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -179,7 +179,7 @@ pg_perm_setlocale(int category, const char *locale)
*/
if (category == LC_CTYPE)
{
- static char save_lc_ctype[NAMEDATALEN + 20];
+ static char save_lc_ctype[LOCALE_NAME_BUFLEN];
/* copy setlocale() return value before callee invokes it again
*/
strlcpy(save_lc_ctype, result, sizeof(save_lc_ctype));
@@ -1288,17 +1288,21 @@ lookup_collation_cache(Oid collation, bool set_flags)
{
/* Attempt to set the flags */
HeapTuple tp;
- Form_pg_collation collform;
+ Datum datum;
+ bool isnull;
const char *collcollate;
const char *collctype;
tp = SearchSysCache1(COLLOID, ObjectIdGetDatum(collation));
if (!HeapTupleIsValid(tp))
elog(ERROR, "cache lookup failed for collation %u",
collation);
- collform = (Form_pg_collation) GETSTRUCT(tp);
- collcollate = NameStr(collform->collcollate);
- collctype = NameStr(collform->collctype);
+ datum = SysCacheGetAttr(COLLOID, tp,
Anum_pg_collation_collcollate, &isnull);
+ Assert(!isnull);
+ collcollate = TextDatumGetCString(datum);
+ datum = SysCacheGetAttr(COLLOID, tp,
Anum_pg_collation_collctype, &isnull);
+ Assert(!isnull);
+ collctype = TextDatumGetCString(datum);
cache_entry->collate_is_c = ((strcmp(collcollate, "C") == 0) ||
(strcmp(collcollate, "POSIX") == 0));
@@ -1484,7 +1488,7 @@ pg_newlocale_from_collation(Oid collid)
const char *collctype pg_attribute_unused();
struct pg_locale_struct result;
pg_locale_t resultp;
- Datum collversion;
+ Datum datum;
bool isnull;
tp = SearchSysCache1(COLLOID, ObjectIdGetDatum(collid));
@@ -1492,8 +1496,12 @@ pg_newlocale_from_collation(Oid collid)
elog(ERROR, "cache lookup failed for collation %u",
collid);
collform = (Form_pg_collation) GETSTRUCT(tp);
- collcollate = NameStr(collform->collcollate);
- collctype = NameStr(collform->collctype);
+ datum = SysCacheGetAttr(COLLOID, tp,
Anum_pg_collation_collcollate, &isnull);
+ Assert(!isnull);
+ collcollate = TextDatumGetCString(datum);
+ datum = SysCacheGetAttr(COLLOID, tp,
Anum_pg_collation_collctype, &isnull);
+ Assert(!isnull);
+ collctype = TextDatumGetCString(datum);
/* We'll fill in the result struct locally before allocating
memory */
memset(&result, 0, sizeof(result));
@@ -1587,13 +1595,15 @@ pg_newlocale_from_collation(Oid collid)
#endif /* not USE_ICU */
}
- collversion = SysCacheGetAttr(COLLOID, tp,
Anum_pg_collation_collversion,
+ datum = SysCacheGetAttr(COLLOID, tp,
Anum_pg_collation_collversion,
&isnull);
if (!isnull)
{
char *actual_versionstr;
char *collversionstr;
+ collversionstr = TextDatumGetCString(datum);
+
actual_versionstr =
get_collation_actual_version(collform->collprovider, collcollate);
if (!actual_versionstr)
{
@@ -1606,7 +1616,6 @@ pg_newlocale_from_collation(Oid collid)
(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,
diff --git a/src/backend/utils/init/postinit.c
b/src/backend/utils/init/postinit.c
index 0236165f60..d046caabd7 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -53,6 +53,7 @@
#include "storage/sync.h"
#include "tcop/tcopprot.h"
#include "utils/acl.h"
+#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/guc.h"
#include "utils/memutils.h"
@@ -306,6 +307,8 @@ CheckMyDatabase(const char *name, bool am_superuser, bool
override_allow_connect
{
HeapTuple tup;
Form_pg_database dbform;
+ Datum datum;
+ bool isnull;
char *collate;
char *ctype;
@@ -389,8 +392,12 @@ CheckMyDatabase(const char *name, bool am_superuser, bool
override_allow_connect
PGC_BACKEND, PGC_S_DYNAMIC_DEFAULT);
/* assign locale variables */
- collate = NameStr(dbform->datcollate);
- ctype = NameStr(dbform->datctype);
+ datum = SysCacheGetAttr(DATABASEOID, tup, Anum_pg_database_datcollate,
&isnull);
+ Assert(!isnull);
+ collate = TextDatumGetCString(datum);
+ datum = SysCacheGetAttr(DATABASEOID, tup, Anum_pg_database_datctype,
&isnull);
+ Assert(!isnull);
+ ctype = TextDatumGetCString(datum);
if (pg_perm_setlocale(LC_COLLATE, collate) == NULL)
ereport(FATAL,
diff --git a/src/include/catalog/pg_collation.h
b/src/include/catalog/pg_collation.h
index bc746537c1..8763dd4080 100644
--- a/src/include/catalog/pg_collation.h
+++ b/src/include/catalog/pg_collation.h
@@ -39,9 +39,9 @@ CATALOG(pg_collation,3456,CollationRelationId)
char collprovider; /* see constants below */
bool collisdeterministic BKI_DEFAULT(t);
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 collcollate BKI_FORCE_NOT_NULL; /* LC_COLLATE
setting */
+ text collctype BKI_FORCE_NOT_NULL; /* LC_CTYPE
setting */
text collversion BKI_DEFAULT(_null_); /*
provider-dependent
* version of collation
* data */
diff --git a/src/include/catalog/pg_database.h
b/src/include/catalog/pg_database.h
index 1ff6d3e50c..90b43a4ecc 100644
--- a/src/include/catalog/pg_database.h
+++ b/src/include/catalog/pg_database.h
@@ -40,12 +40,6 @@ CATALOG(pg_database,1262,DatabaseRelationId)
BKI_SHARED_RELATION BKI_ROWTYPE_OID
/* character encoding */
int32 encoding;
- /* LC_COLLATE setting */
- NameData datcollate;
-
- /* LC_CTYPE setting */
- NameData datctype;
-
/* allowed as CREATE DATABASE template? */
bool datistemplate;
@@ -65,6 +59,12 @@ CATALOG(pg_database,1262,DatabaseRelationId)
BKI_SHARED_RELATION BKI_ROWTYPE_OID
Oid dattablespace BKI_LOOKUP(pg_tablespace);
#ifdef CATALOG_VARLEN /* variable-length fields start here */
+ /* LC_COLLATE setting */
+ text datcollate BKI_FORCE_NOT_NULL;
+
+ /* LC_CTYPE setting */
+ text datctype BKI_FORCE_NOT_NULL;
+
/* access permissions */
aclitem datacl[1];
#endif
diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h
index b4a2765983..30e423af0e 100644
--- a/src/include/utils/pg_locale.h
+++ b/src/include/utils/pg_locale.h
@@ -34,6 +34,8 @@
#endif
#endif
+/* use for libc locale names */
+#define LOCALE_NAME_BUFLEN 128
/* GUC settings */
extern char *locale_messages;
--
2.34.1
