On Fri, Feb 11, 2022 at 12:07:02PM +0100, Peter Eisentraut wrote: > On 10.02.22 12:08, Julien Rouhaud wrote: > > > + errhint("Rebuild all objects affected > > > by collation in the template database and run " > > > + "ALTER DATABASE %s > > > REFRESH COLLATION VERSION, " > > > + "or build PostgreSQL > > > with the right library version.", > > > + > > > quote_identifier(dbtemplate)))); > > > > After a second read I think the messages are slightly ambiguous. What do > > you > > think about specifying the problematic collation name and provider? > > > > For now we only support libc default collation so users will probably have > > to > > reindex almost everything on that database (not sure if the versioning is > > more > > fine grained on Windows), but we should probably still specify "affected by > > libc collation" in the errhint so they have a chance to avoid unnecessary > > reindex. > > I think accurate would be something like "objects using the default > collation", since objects using a specific collation are not meant, even if > they use the same provider.
Technically is the objects explicitly use the same collation as the default collation they should be impacted the same way, but agreed. > > > +/* > > > + * ALTER DATABASE name REFRESH COLLATION VERSION > > > + */ > > > +ObjectAddress > > > +AlterDatabaseRefreshColl(AlterDatabaseRefreshCollStmt *stmt) > > > > I'm wondering why you changed this function to return an ObjectAddress > > rather > > than an Oid? There's no event trigger support for ALTER DATABASE, and the > > rest > > of similar utility commands also returns Oid. > > Hmm, I was looking at RenameDatabase() and AlterDatabaseOwner(), which > return ObjectAddress. Apparently I managed to only check AlterDatabase and AlterDatabaseSet, which both return an Oid. Maybe we could also update those two to also return an ObjectAddress, for consistency?