Skip to content

Conversation

@labkey-martyp
Copy link
Contributor

Rationale

Initially this page allows dropping and recreating the EHR index on the clinical_observations dataset to resolve an issue introduced in 25.7 (and fixed in 25.11) with recreating indices by name after updating a domain. This creates actions to drop the index manually and recreate it manually after the domain update is complete.

Additionally we'll keep these actions and page as we prepare for migration from SQL Server to Postgres and go through the other EHR indices to see which are still needed. Or if new ones are needed.

Changes

  • Add new Postgres Migration admin page
  • Create actions for dropping and creating the EHR indices.
  • Only add clinical_observations index for now.

Comment on lines 1113 to 1117
String[] indexCols = new String[]{"participantid", "date", "include:taskid,lsid,category,observation,area,remark"};
String indexName = getIndexName(schema.getSqlDialect(), tableName, indexCols);

List<String> cols = Arrays.asList("participantid", "date");
String[] includedCols = new String[]{"taskid", "lsid", "category", "observation", "area", "remark"};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these column lists hard-coded twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the hard coded values in this file and the common index here.

{
List<String> messages = new ArrayList<>();

EHRIndexContext ctx = prepareEHRIndexContext(c, messages);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels over-engineered for a single index and under-engineered for supporting more indices in the future. Consider making prepareEHRIndexContext() return a List with a single entry for now, and iterate over the list when creating or dropping indices.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'm anticipating a lot of refactoring in this file as we look into these indices. I was planning on doing more refactoring at that point but went ahead and set this up more generically for more indices.

messages.add("Dropping index on column(s): " + StringUtils.join(cols, ", ") + " for dataset: " + tableName);
String sqlString = "DROP INDEX " + indexName + " ON " + realTable.getSelectName();
String sqlString;
if (realTable.getSqlDialect().isSqlServer())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we share index dropping code with ensureDatasetProperyDescriptor() instead of duplicating?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ensureDatasetProperyDescriptor() was previously using dropIndex in one place and not in another. Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants