Skip to content

Fix SQL Server migration issues#7418

Merged
labkey-adam merged 11 commits intodevelopfrom
fb_migrate_ss
Feb 18, 2026
Merged

Fix SQL Server migration issues#7418
labkey-adam merged 11 commits intodevelopfrom
fb_migrate_ss

Conversation

@labkey-adam
Copy link
Contributor

@labkey-adam labkey-adam commented Feb 14, 2026

Rationale

https://github.com/LabKey/kanban/issues/1577

Several inconsistencies in the migration code were uncovered when migrating TNPRC's SQL Server database to PostgreSQL:

  1. Linked schemas no longer resolve
  2. No issues.Issues rows are present
  3. Dataset, issue, etc. table names appear to be empty in the source DB
  4. New dataset tables are missing some columns.

Changes

  1. Hoist GuidMapperColumn into API. Use it to lowercase linked schema GUIDs that are stored in the ExternalSchema.DataSource column.
  2. Don't delete issues, comments, and related issues if no filter is in place
  3. Correct scope used in DbSchemaType.Migration
  4. Stop stashing the shared container in OntologyManager. A stale value left from pre-migration time caused domains with any shared property to lose all custom properties.
  5. Register migration handlers only if a migration is taking place
  6. Script reorderer now tracks the primary table associated with each constraint and uses that to order constraint statements (e.g., ALTER TABLE foo CHECK CONSTRAINT bar)

@labkey-adam labkey-adam self-assigned this Feb 14, 2026
Comment on lines 180 to 188
dd = ddArray.getFirst();

// if someone has explicitly inserted a descriptor with the same URI as an existing one ,
// and one of the two is in the shared project, use the project-level descriptor.
if (ddArray.size() > 1)
{
_log.debug("Multiple DomainDescriptors found for " + uriOrName);
if (dd.getProject().equals(ContainerManager.getSharedContainer()))
dd = ddArray.get(0);
dd = ddArray.getFirst();
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this wasn't your logic, but this doesn't right. dd = ddArray.getFirst(); then if it's the shared container dd = ddArray.getFirst(); again? Maybe the second one should be ddArray.get(1) or getLast()?

Copy link
Contributor Author

@labkey-adam labkey-adam Feb 17, 2026

Choose a reason for hiding this comment

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

The way I read the comment, we prefer a dd from the shared container. Seems like we should iterate over the list and return the first that's in shared else return the first. I'll tag Josh since it looks like he added this. @labkey-jeckels

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that this code isn't doing anything useful today, and looks flawed when I committed it.

I believe the comment is saying the opposite. If we have a match in /Shared and another match in /MyProject, use the one from /MyProject.

We have a unique constraint on exp.DomainDescriptor(domainuri, container). We're passing in two container values in the WHERE clause (which could be the same), so in the URI case, we should have at most two matches. However, @XingY added the uriOrName argument afterward, and we have no constraint on (name, container).

Spinning the list and looking for the first one that's not in /Shared, and then falling back on the first entry sounds like the right approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it... I was thinking "project" meant shared project not the other project. I can stick a fix in my other open platform PR. Unless you're dying to commit this fix.

@labkey-adam labkey-adam merged commit 6591e66 into develop Feb 18, 2026
10 checks passed
@labkey-adam labkey-adam deleted the fb_migrate_ss branch February 18, 2026 01:55
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

Comments