Skip to content

Commit e956069

Browse files
committed
Rename tables and columns with names that are too long for PostgreSQL
1 parent 9291c28 commit e956069

9 files changed

Lines changed: 211 additions & 18 deletions

File tree

api/src/org/labkey/api/data/RecordFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public RecordFactory(Class<K> clazz)
5252
{
5353
Object[] params = Arrays.stream(_parameters).map(p -> {
5454
Object value = m.get(p.getName());
55-
return ConvertUtils.convert(value, p.getType());
55+
return value != null ? ConvertUtils.convert(value, p.getType()) : null;
5656
}).toArray();
5757

5858
try

api/src/org/labkey/api/data/SqlScriptExecutor.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ public void execute()
221221
{
222222
LOG.info("Executing {}", upgradeMethod.getDisplayName());
223223
_moduleContext.invokeUpgradeMethod(upgradeMethod);
224+
LOG.info("Finished executing {}", upgradeMethod.getDisplayName());
224225
}
225226
}
226227
catch (NoSuchMethodException e)
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package org.labkey.api.data.dialect;
2+
3+
import org.labkey.api.services.ServiceRegistry;
4+
5+
public interface PostgreSqlService
6+
{
7+
static PostgreSqlService get()
8+
{
9+
return ServiceRegistry.get().getService(PostgreSqlService.class);
10+
}
11+
12+
static void setInstance(PostgreSqlService impl)
13+
{
14+
ServiceRegistry.get().registerService(PostgreSqlService.class, impl);
15+
}
16+
17+
BasePostgreSqlDialect getDialect();
18+
}

core/src/org/labkey/core/CoreModule.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@
7777
import org.labkey.api.data.TestSchema;
7878
import org.labkey.api.data.WorkbookContainerType;
7979
import org.labkey.api.data.dialect.BasePostgreSqlDialect;
80+
import org.labkey.api.data.dialect.PostgreSqlService;
8081
import org.labkey.api.data.dialect.SqlDialect;
8182
import org.labkey.api.data.dialect.SqlDialectManager;
8283
import org.labkey.api.data.dialect.SqlDialectRegistry;
@@ -88,6 +89,7 @@
8889
import org.labkey.api.files.FileBrowserConfigWriter;
8990
import org.labkey.api.files.FileContentService;
9091
import org.labkey.api.markdown.MarkdownService;
92+
import org.labkey.api.mcp.McpService;
9193
import org.labkey.api.message.settings.MessageConfigService;
9294
import org.labkey.api.migration.DatabaseMigrationService;
9395
import org.labkey.api.module.FolderType;
@@ -98,7 +100,6 @@
98100
import org.labkey.api.module.SchemaUpdateType;
99101
import org.labkey.api.module.SpringModule;
100102
import org.labkey.api.module.Summary;
101-
import org.labkey.api.mcp.McpService;
102103
import org.labkey.api.notification.EmailMessage;
103104
import org.labkey.api.notification.EmailService;
104105
import org.labkey.api.notification.NotificationMenuView;
@@ -253,9 +254,9 @@
253254
import org.labkey.core.login.DbLoginAuthenticationProvider;
254255
import org.labkey.core.login.DbLoginManager;
255256
import org.labkey.core.login.LoginController;
257+
import org.labkey.core.mcp.McpServiceImpl;
256258
import org.labkey.core.metrics.SimpleMetricsServiceImpl;
257259
import org.labkey.core.metrics.WebSocketConnectionManager;
258-
import org.labkey.core.mcp.McpServiceImpl;
259260
import org.labkey.core.notification.EmailPreferenceConfigServiceImpl;
260261
import org.labkey.core.notification.EmailPreferenceContainerListener;
261262
import org.labkey.core.notification.EmailPreferenceUserListener;
@@ -560,11 +561,11 @@ public QuerySchema createSchema(DefaultSchema schema, Module module)
560561
ScriptEngineManagerImpl.registerEncryptionMigrationHandler();
561562

562563
McpService.get().register(new CoreMcp());
564+
PostgreSqlService.setInstance(PostgreSqlDialectFactory::getLatestSupportedDialect);
563565

564566
deleteTempFiles();
565567
}
566568

567-
568569
private void deleteTempFiles()
569570
{
570571
try

core/src/org/labkey/core/admin/sql/SqlScriptController.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1605,6 +1605,7 @@ public boolean handlePost(UpgradeCodeForm form, BindException errors) throws Exc
16051605
{
16061606
LOG.info("Executing {}.{}(ModuleContext moduleContext)", _method.getDeclaringClass().getSimpleName(), _method.getName());
16071607
_method.invoke(_code, _ctx);
1608+
LOG.info("Finished executing {}.{}(ModuleContext moduleContext)", _method.getDeclaringClass().getSimpleName(), _method.getName());
16081609
return true;
16091610
}
16101611

core/src/org/labkey/core/dialect/PostgreSqlDialectFactory.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.junit.Assert;
2525
import org.junit.Test;
2626
import org.labkey.api.data.dialect.AbstractDialectRetrievalTestCase;
27+
import org.labkey.api.data.dialect.BasePostgreSqlDialect;
2728
import org.labkey.api.data.dialect.DatabaseNotSupportedException;
2829
import org.labkey.api.data.dialect.JdbcHelperTest;
2930
import org.labkey.api.data.dialect.PostgreSqlServerType;
@@ -145,6 +146,11 @@ public static PostgreSql_13_Dialect getOldestSupportedDialect()
145146
return new PostgreSql_13_Dialect();
146147
}
147148

149+
public static BasePostgreSqlDialect getLatestSupportedDialect()
150+
{
151+
return new PostgreSql_18_Dialect();
152+
}
153+
148154
public static class DialectRetrievalTestCase extends AbstractDialectRetrievalTestCase
149155
{
150156
@Override

experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -178,23 +178,27 @@ public void afterSchema(DatabaseMigrationConfiguration configuration, DbSchema s
178178

179179
DbScope sourceScope = configuration.getSourceScope();
180180
DbScope targetScope = configuration.getTargetScope();
181-
DbSchema biologicsSourceSchema = sourceScope.getSchema("biologics", DbSchemaType.Migration);
182-
DbSchema biologicsTargetSchema = targetScope.getSchema("biologics", DbSchemaType.Module);
183181

184-
if (biologicsSourceSchema.existsInDatabase() && biologicsTargetSchema.existsInDatabase())
182+
if (sourceScope.getSchemaNames().contains("biologics") && targetScope.getSchemaNames().contains("biologics"))
185183
{
186-
TableInfo sourceTable = biologicsSourceSchema.getTable("SequenceIdentity");
187-
TableInfo targetTable = biologicsTargetSchema.getTable("SequenceIdentity");
184+
DbSchema biologicsSourceSchema = sourceScope.getSchema("biologics", DbSchemaType.Migration);
185+
DbSchema biologicsTargetSchema = targetScope.getSchema("biologics", DbSchemaType.Module);
188186

189-
DatabaseMigrationService.get().copySourceTableToTargetTable(configuration, sourceTable, targetTable, DbSchemaType.Module, true, null, new DefaultMigrationSchemaHandler(biologicsTargetSchema)
187+
if (biologicsSourceSchema.existsInDatabase() && biologicsTargetSchema.existsInDatabase())
190188
{
191-
@Override
192-
public FilterClause getTableFilterClause(TableInfo sourceTable, Set<GUID> containers)
189+
TableInfo sourceTable = biologicsSourceSchema.getTable("SequenceIdentity");
190+
TableInfo targetTable = biologicsTargetSchema.getTable("SequenceIdentity");
191+
192+
DatabaseMigrationService.get().copySourceTableToTargetTable(configuration, sourceTable, targetTable, DbSchemaType.Module, true, null, new DefaultMigrationSchemaHandler(biologicsTargetSchema)
193193
{
194-
// This is a global table, so no container clause. Just query and copy the sequence IDs referenced by data class rows we copied.
195-
return new InClause(FieldKey.fromParts("SequenceId"), SEQUENCE_IDS);
196-
}
197-
});
194+
@Override
195+
public FilterClause getTableFilterClause(TableInfo sourceTable, Set<GUID> containers)
196+
{
197+
// This is a global table, so no container clause. Just query and copy the sequence IDs referenced by data class rows we copied.
198+
return new InClause(FieldKey.fromParts("SequenceId"), SEQUENCE_IDS);
199+
}
200+
});
201+
}
198202
}
199203
}
200204

experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
*/
1616
package org.labkey.experiment;
1717

18+
import org.apache.commons.collections4.MultiValuedMap;
19+
import org.apache.commons.collections4.multimap.ArrayListValuedHashMap;
1820
import org.apache.commons.lang3.StringUtils;
1921
import org.apache.logging.log4j.Logger;
2022
import org.jetbrains.annotations.NotNull;
@@ -25,7 +27,10 @@
2527
import org.labkey.api.audit.SampleTimelineAuditEvent;
2628
import org.labkey.api.audit.TransactionAuditProvider;
2729
import org.labkey.api.collections.CaseInsensitiveHashSet;
30+
import org.labkey.api.collections.CsvSet;
31+
import org.labkey.api.collections.LabKeyCollectors;
2832
import org.labkey.api.data.ColumnInfo;
33+
import org.labkey.api.data.CompareType;
2934
import org.labkey.api.data.Container;
3035
import org.labkey.api.data.ContainerManager;
3136
import org.labkey.api.data.DbSchema;
@@ -39,11 +44,16 @@
3944
import org.labkey.api.data.SQLFragment;
4045
import org.labkey.api.data.SchemaTableInfo;
4146
import org.labkey.api.data.Selector;
47+
import org.labkey.api.data.SimpleFilter;
4248
import org.labkey.api.data.SqlExecutor;
4349
import org.labkey.api.data.SqlSelector;
50+
import org.labkey.api.data.Table;
4451
import org.labkey.api.data.TableInfo;
4552
import org.labkey.api.data.TableSelector;
4653
import org.labkey.api.data.UpgradeCode;
54+
import org.labkey.api.data.dialect.BasePostgreSqlDialect;
55+
import org.labkey.api.data.dialect.PostgreSqlService;
56+
import org.labkey.api.exp.OntologyManager;
4757
import org.labkey.api.exp.PropertyDescriptor;
4858
import org.labkey.api.exp.api.ExpSampleType;
4959
import org.labkey.api.exp.api.ExperimentService;
@@ -64,6 +74,8 @@
6474
import org.labkey.api.security.User;
6575
import org.labkey.api.security.roles.SiteAdminRole;
6676
import org.labkey.api.settings.AppProps;
77+
import org.labkey.api.util.PageFlowUtil;
78+
import org.labkey.api.util.StringUtilsLabKey;
6779
import org.labkey.api.util.logging.LogHelper;
6880
import org.labkey.experiment.api.DataClass;
6981
import org.labkey.experiment.api.DataClassDomainKind;
@@ -73,11 +85,14 @@
7385
import org.labkey.experiment.api.MaterialSource;
7486
import org.labkey.experiment.api.property.DomainImpl;
7587
import org.labkey.experiment.api.property.DomainPropertyImpl;
88+
import org.labkey.experiment.api.property.StorageNameGenerator;
7689
import org.labkey.experiment.api.property.StorageProvisionerImpl;
7790

91+
import java.nio.charset.StandardCharsets;
7892
import java.sql.Connection;
7993
import java.sql.SQLException;
8094
import java.util.ArrayList;
95+
import java.util.Collection;
8196
import java.util.Collections;
8297
import java.util.HashMap;
8398
import java.util.HashSet;
@@ -656,5 +671,147 @@ private static void fillRowId(ExpDataClassImpl dc, Domain domain, DbScope scope)
656671
LOG.info("DataClass '{}' ({}) populated 'rowId' column, count={}", dc.getName(), dc.getRowId(), count);
657672
}
658673

674+
record DomainRecord(Container container, int domainId, String name, String storageSchemaName, String storageTableName)
675+
{
676+
String fullName()
677+
{
678+
return storageSchemaName + "." + storageTableName;
679+
}
680+
}
681+
682+
record Property(int domainId, int propertyId, String domainName, String name, String storageSchemaName, String storageTableName, String storageColumnName)
683+
{
684+
String fullName()
685+
{
686+
// Have to bracket storage column name since it could have special characters
687+
return storageSchemaName + "." + storageTableName + "." + bracketIt(storageColumnName);
688+
}
689+
}
690+
691+
/**
692+
* Called from exp-????.sql, SQL Server only
693+
* Query all table & column storage names and rename the ones that are too long for PostgreSQL.
694+
* TODO: When this upgrade code is removed, get rid of the StorageProvisionerImpl.makeTableName() method it uses.
695+
*/
696+
@SuppressWarnings("unused")
697+
@DeferredUpgrade
698+
public static void shortenAllStorageNames(ModuleContext context)
699+
{
700+
if (context.isNewInstall())
701+
return;
702+
703+
// The PostgreSQL dialect knows which names are too long
704+
BasePostgreSqlDialect dialect = PostgreSqlService.get().getDialect();
705+
DbScope scope = DbScope.getLabKeyScope();
706+
SqlExecutor executor = new SqlExecutor(scope);
659707

708+
// Stream all the storage table names and rename the ones that are too long for PostgreSQL. The filtering must
709+
// be done in code by the dialect; SQL Server has BYTELENGTH(), but that function returns values that are not
710+
// consistent with our dialect check. Also, it looks like the function's behavior changed starting in SS 2019.
711+
TableInfo tinfoDomainDescriptor = OntologyManager.getTinfoDomainDescriptor();
712+
SimpleFilter filter = new SimpleFilter(FieldKey.fromString("StorageSchemaName"), null, CompareType.NONBLANK);
713+
filter.addCondition(FieldKey.fromString("StorageTableName"), null, CompareType.NONBLANK);
714+
try (DbScope.Transaction t = scope.beginTransaction())
715+
{
716+
new TableSelector(tinfoDomainDescriptor, new CsvSet("Container, DomainId, Name, StorageSchemaName, StorageTableName"), filter, null)
717+
.setJdbcCaching(false)
718+
.stream(DomainRecord.class)
719+
.filter(domain -> dialect.isIdentifierTooLong(domain.storageTableName()))
720+
.forEach(domain -> {
721+
String oldName = domain.fullName();
722+
String newName = StorageProvisionerImpl.get().makeTableName(dialect, domain.container(), domain.domainId(), domain.name());
723+
724+
executor.execute(new SQLFragment("EXEC sp_rename ?, ?").add(oldName).add(newName));
725+
Table.update(null, tinfoDomainDescriptor, PageFlowUtil.map("StorageTableName", newName), domain.domainId());
726+
727+
LOG.info(" Table \"{}\" renamed to \"{}\" ({} bytes)", oldName, newName, newName.getBytes(StandardCharsets.UTF_8).length);
728+
});
729+
730+
List<String> badTableNames = new TableSelector(tinfoDomainDescriptor, new CsvSet("StorageTableName"), filter, null)
731+
.setJdbcCaching(false)
732+
.stream(String.class)
733+
.filter(dialect::isIdentifierTooLong)
734+
.toList();
735+
736+
if (!badTableNames.isEmpty())
737+
LOG.error("Some storage table names are still too long!! {}", badTableNames);
738+
}
739+
740+
TableInfo tinfoPropertyDomain = OntologyManager.getTinfoPropertyDomain();
741+
TableInfo tinfoPropertyDescriptor = OntologyManager.getTinfoPropertyDescriptor();
742+
SQLFragment sql = new SQLFragment("SELECT dd.DomainId, dd.Name AS DomainName, px.PropertyId, StorageSchemaName, StorageTableName, StorageColumnName, px.Name FROM ")
743+
.append(tinfoDomainDescriptor, "dd")
744+
.append(" INNER JOIN ")
745+
.append(tinfoPropertyDomain, "pd")
746+
.append(" ON dd.DomainId = pd.DomainId INNER JOIN ")
747+
.append(tinfoPropertyDescriptor, "px")
748+
.append(" ON pd.PropertyId = px.PropertyId ")
749+
.append("WHERE StorageSchemaName IS NOT NULL AND StorageTableName IS NOT NULL AND StorageColumnName IS NOT NULL");
750+
751+
filter = new SimpleFilter(FieldKey.fromString("StorageSchemaName"), null, CompareType.NONBLANK);
752+
filter.addCondition(FieldKey.fromString("StorageTableName"), null, CompareType.NONBLANK);
753+
filter.addCondition(FieldKey.fromString("StorageColumnName"), null, CompareType.NONBLANK);
754+
MultiValuedMap<DomainRecord, Property> badDomainMap = new SqlSelector(scope, sql)
755+
.setJdbcCaching(false)
756+
.stream(Property.class)
757+
.filter(property -> dialect.isIdentifierTooLong(property.storageColumnName()))
758+
.collect(LabKeyCollectors.toMultiValuedMap(
759+
property -> new DomainRecord(null, property.domainId(), property.domainName(), property.storageSchemaName(), property.storageTableName()),
760+
property -> property,
761+
ArrayListValuedHashMap::new)
762+
);
763+
764+
LOG.info(" Found {} with storage column names that are too long for PostgreSQL:", StringUtilsLabKey.pluralize(badDomainMap.keySet().size(), "domain"));
765+
766+
try (DbScope.Transaction t = scope.beginTransaction())
767+
{
768+
// Now enumerate the bad domains and rename their bad storage columns
769+
badDomainMap.keySet()
770+
.forEach(domain -> {
771+
Collection<Property> badColumns = badDomainMap.get(domain);
772+
List<String> badColumnNames = badColumns.stream().map(Property::storageColumnName).toList();
773+
774+
// First, populate a new StorageNameGenerator with all the "good" names in this domain so we don't
775+
// accidentally try to re-use one of them
776+
StorageNameGenerator nameGenerator = new StorageNameGenerator(dialect);
777+
SQLFragment domainSql = new SQLFragment("SELECT StorageColumnName FROM ")
778+
.append(tinfoPropertyDomain, "pd")
779+
.append(" INNER JOIN ")
780+
.append(tinfoPropertyDescriptor, "px")
781+
.append(" ON pd.PropertyId = px.PropertyId ")
782+
.append("WHERE DomainId = ? AND StorageColumnName NOT ")
783+
.add(domain.domainId())
784+
.appendInClause(badColumnNames, scope.getSqlDialect());
785+
new SqlSelector(scope, domainSql).forEach(String.class, nameGenerator::claimName);
786+
787+
LOG.info(" Renaming {} in table \"{}\"", StringUtilsLabKey.pluralize(badColumns.size(), "column"), domain.fullName());
788+
789+
// Now use that StorageNameGenerator to create new names. Rename the column and update the PropertyDescriptor table.
790+
badColumns.forEach(property -> {
791+
String oldName = property.fullName();
792+
String newName = bracketIt(nameGenerator.generateColumnName(property.name())); // Could have special characters, so bracket it
793+
794+
executor.execute(new SQLFragment("EXEC sp_rename ?, ?, 'COLUMN'").add(oldName).add(newName));
795+
Table.update(null, tinfoPropertyDescriptor, PageFlowUtil.map("StorageColumnName", newName), property.propertyId());
796+
797+
LOG.info(" Column \"{}\" renamed to \"{}\" ({} bytes)", oldName, newName, newName.getBytes(StandardCharsets.UTF_8).length);
798+
});
799+
});
800+
801+
List<String> badColumnNames = new TableSelector(tinfoPropertyDescriptor, new CsvSet("StorageColumnName"), new SimpleFilter(FieldKey.fromString("StorageColumnName"), null, CompareType.NONBLANK), null)
802+
.setJdbcCaching(false)
803+
.stream(String.class)
804+
.filter(dialect::isIdentifierTooLong)
805+
.toList();
806+
807+
if (!badColumnNames.isEmpty())
808+
LOG.error("Some storage column names are still too long!! {}", badColumnNames);
809+
}
810+
}
811+
812+
// Bracket name and escape any internal ending brackets
813+
static String bracketIt(String name)
814+
{
815+
return "[" + name.replace("]", "]]") + "]";
816+
}
660817
}

experiment/src/org/labkey/experiment/api/property/StorageProvisionerImpl.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -615,8 +615,13 @@ public void changePropertyType(Domain domain, DomainProperty prop) throws Change
615615

616616
public String makeTableName(DomainKind<?> kind, Domain domain)
617617
{
618-
String rawTableName = String.format("c%sd%s_%s", domain.getContainer().getRowId(), domain.getTypeId(), domain.getName());
619-
SqlDialect dialect = kind.getScope().getSqlDialect();
618+
return makeTableName(kind.getScope().getSqlDialect(), domain.getContainer(), domain.getTypeId(), domain.getName());
619+
}
620+
621+
// Needed by ExperimentUpgradeCode.shortenAllStorageNames(). When that code is removed, combine this with above variant.
622+
public String makeTableName(SqlDialect dialect, Container c, int typeId, String domainName)
623+
{
624+
String rawTableName = String.format("c%sd%s_%s", c.getRowId(), typeId, domainName);
620625
return new StorageNameGenerator(dialect).generateTableName(rawTableName);
621626
}
622627

0 commit comments

Comments
 (0)