fix(migration): v200 thread_entity guard, PostgreSQL jsonb cast, self-loop cycle detection

[yan-3005]

Context

This PR fixes six bugs introduced or exposed during the v2.0.0 migration, all of which caused CI failures or incorrect data on both MySQL and PostgreSQL. The bugs compound each other: the workflow seeding failures (#4 and #5) were only visible because the migration bugs (#1–3) caused the server to restart mid-migration and hit edge cases on re-run.

---

Bug 1: thread_entity crash on migration re-run (MySQL + PostgreSQL)

Root cause: OpenMetadata uses two independent tracking systems for migrations:

• Flyway tracks DDL SQL scripts in flyway_schema_history — each script is committed atomically and cannot be rolled back once applied.
• Java migration state is tracked in server_change_log inside a transaction — if a later migration step fails, the transaction rolls back and the Java state reverts to "v200 not yet run."

The v2.0.0 postDataMigrationSQLScript.sql renames thread_entity → thread_entity_legacy after the Java data migration runs. If any subsequent migration throws and rolls back the server_change_log transaction, Flyway has already committed the rename permanently. On the next server startup, Java re-executes migrateThreadTasksToTaskEntity, which tries to SELECT FROM thread_entity — a table that no longer exists — and crashes.

This also affects upgrades from certain versions where thread_entity was never present.

Fix: Guard migrateThreadTasksToTaskEntity with a tableExists(handle, "thread_entity") check at the top of the method and return early if the table is absent.

---

Bug 2: PostgreSQL jsonb type mismatch — all 1032 suggestion inserts fail

Root cause: The v2.0.0 PostgreSQL schema declares the following columns as jsonb NOT NULL:

• task_entity.json
• activity_stream.json
• activity_stream.domains

JDBI's .bind("fieldName", someString) always binds a Java String as SQL type varchar. PostgreSQL enforces strict type checking and does not implicitly cast varchar to jsonb, throwing:

ERROR: column "json" is of type jsonb but expression is of type character varying
  Hint: You will need to rewrite or cast the expression.

MySQL's json type is lenient and accepts string input without an explicit cast, so MySQL never surfaced this bug. PostgreSQL's strictness caused every single suggestion insert and every activity event insert to fail.

Fix: Pass ConnectionType through the call chain into insertTask and insertActivityEvent. When ConnectionType.POSTGRES, use an explicit ::jsonb cast in the SQL string (e.g., :json::jsonb). MySQL continues to use the plain :json binding.

---

Bug 3: MySQL migrateThreadTasksToTaskEntity silently disabled

Root cause: The MySQL migration had migrateThreadTasksToTaskEntity commented out as a temporary workaround after it caused CI failures:

// Causing issues with collate CI, needs to be fixed before enabling this migration @harshach
// migrateThreadTasksToTaskEntity(handle);

The actual root cause was the thread_entity crash described in Bug 1 — not the method itself. This meant MySQL deployments silently skipped migrating thread-based tasks to task_entity, resulting in missing task data for all MySQL installations.

Fix: Re-enable the method now that Bug 1's tableExists guard makes it safe to call. The commented-out code is removed.

---

Bug 4: BPMN self-loop edges falsely detected as cycles

Root cause: hasCycleDFS in WorkflowDefinitionRepository iterates over all outgoing edges of a node and recurses into each neighbor. It does not skip self-loop edges — edges where a node transitions to itself (e.g., AssignedStage → AssignedStage).

BPMN self-loops are a valid and intentional pattern for reassignment transitions: an incident can be reassigned to a different team while remaining in the AssignedStage state. The DFS treated this as a cycle and threw a validation exception when IncidentLifecycleWorkflow was seeded.

Fix: Skip the recursion when neighbor.equals(node) — a self-loop cannot form a multi-node cycle.

---

Bug 5: One failing seed entity aborts all subsequent seeds

Root cause: initSeedDataFromResources in EntityRepository iterates over workflow definitions alphabetically. Seed files are processed in alphabetical order, so IncidentLifecycleWorkflow is initialized before TableCertificationWorkflow. When Bug 4 caused IncidentLifecycleWorkflow initialization to throw an exception, the exception propagated out of the loop uncaught, aborting the entire seed operation. TableCertificationWorkflow and every other definition that came after were never seeded.

Fix: Wrap each entity initialization in a try-catch. Log the failure with context (entity type, FQN, error message and stack trace) and continue to the next entity. One bad workflow definition should not prevent all others from loading.

---

Bug 6: Migrated tasks missing assignees and createdBy in the API response

Root cause: TaskRepository.storeEntity() deliberately strips assignees, createdBy, about, reviewers, and watchers from the task JSON before persisting — these relationships are stored exclusively as rows in entity_relationship. Reads via getAssignees(), getTaskCreatedBy(), etc. all query entity_relationship, not the JSON column.

The v2.0.0 migration (migrateThreadTasksToTaskEntity, migrateSuggestionsToTaskEntity) inserted task rows via raw SQL INSERT, bypassing TaskRepository.storeRelationships(). No entity_relationship rows were ever created for migrated tasks. As a result:

• assignees always returned empty — "No Owners" in the UI
• createdBy always returned null
• about entity link was not registered

Fix: After each insertTask(...) call in both migration methods, call insertTaskLinkRelationships(...) which inserts the required entity_relationship rows:

JSON field	Relationship	Ordinal
assignees[]	ASSIGNED_TO	25
createdBy (user)	CREATED	1
about entity	MENTIONED_IN	5
reviewers[]	REVIEWS	16
watchers[]	FOLLOWS	11

Inserts are idempotent: MySQL uses INSERT IGNORE, PostgreSQL uses ON CONFLICT DO NOTHING.

---

Tests

Added unit tests in MigrationUtilTest covering:

• tableExists guard skips gracefully for migrateThreadTasksToTaskEntity (MySQL + PostgreSQL)
• tableExists guard skips gracefully for migrateSuggestionsToTaskEntity (MySQL + PostgreSQL)
• tableExists guard skips gracefully for migrateLegacyActivityThreadsToActivityStream (MySQL + PostgreSQL)
• migrateThreadTasksToTaskEntity uses ::jsonb cast for PostgreSQL
• migrateThreadTasksToTaskEntity does not use ::jsonb cast for MySQL
• migrateThreadTasksToTaskEntity inserts entity_relationship rows for assignees and about entity
• migrateSuggestionsToTaskEntity inserts entity_relationship rows for createdBy

All 15 tests pass.

Test plan

• MySQL CI: v200 migration completes without thread_entity does not exist crash
• PostgreSQL CI: v200 migration completes without column "json" is of type jsonb but expression is of type character varying
• All unit tests in MigrationUtilTest pass (15 total)
• IncidentLifecycleWorkflow seeds successfully on fresh install
• TableCertificationWorkflow seeds successfully on fresh install
• Migrated tasks return correct assignees and createdBy from the API
• mvn spotless:apply -pl openmetadata-service reports no formatting issues

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR hardens v200 migrations and seed/workflow initialization by making migrations resilient to schema drift (renamed tables), fixing PostgreSQL JSONB inserts, and relaxing workflow validation to allow intentional self-loop transitions.

Changes:

• Add v200 migration guards and PostgreSQL ::jsonb casts for task/activity inserts via ConnectionType.
• Re-enable MySQL thread-task migration with a thread_entity existence guard.
• Improve workflow seeding robustness by skipping self-loop edges in cycle detection and continuing seed initialization after per-entity failures.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v200/MigrationUtil.java	Adds ConnectionType-aware ::jsonb binding and a thread_entity existence guard; threads/suggestions/activity migrations now pass connection type into inserts.
openmetadata-service/src/main/java/org/openmetadata/service/migration/postgres/v200/Migration.java	Updates v200 Postgres migration to pass POSTGRES into updated MigrationUtil APIs.
openmetadata-service/src/main/java/org/openmetadata/service/migration/mysql/v200/Migration.java	Updates v200 MySQL migration to pass MYSQL and re-enables thread-task migration call.
openmetadata-service/src/test/java/org/openmetadata/service/migration/utils/v200/MigrationUtilTest.java	Adds tests for table-missing guards and attempts to cover Postgres/MySQL jsonb casting behavior.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/WorkflowDefinitionRepository.java	Skips self-loop edges during DFS cycle detection to allow valid reassignment transitions.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java	Makes seed initialization resilient by catching/logging per-entity failures instead of aborting all seeding.

[Copilot]

migrateThreadTasksUsesJsonbCastForPostgres doesn’t currently validate the ::jsonb insert behavior because the mocked thread list is empty, so insertTask(...) is never reached and Handle.createUpdate(...) is never called. Consider returning a non-empty threads result (one row with a minimal json payload) and then asserting that the SQL passed to createUpdate contains :json::jsonb for POSTGRES.

[yan-3005]

@gitar-bot Intentional design choice — each failure is already individually logged with full context (entity type, FQN, message, and full stack trace) via LOG.warn. Re-throwing only lastException would surface a single arbitrary failure while silently discarding the others, which is more misleading than the current behavior. If there's a systemic failure (e.g., DB connectivity loss), the N consecutive WARN lines in the logs make that obvious. The try-catch is specifically to prevent one bad workflow definition from blocking all subsequent ones from seeding — not to suppress errors.

[gitar-bot]

@gitar-bot Intentional design choice — each failure is already individually logged with full context (entity type, FQN, message, and full stack trace) via LOG.warn. Re-throwing only lastException would surface a single arbitrary failure while silently discarding the others, which is more misleading than the current behavior. If there's a systemic failure (e.g., DB connectivity loss), the N consecutive WARN lines in the logs make that obvious. The try-catch is specifically to prevent one bad workflow definition from blocking all subsequent ones from seeding — not to suppress errors.

That's a fair point — individual WARN logs with full context per entity do provide better diagnostic coverage than surfacing a single arbitrary lastException. The suggestion was meant to guard against the silent-all-fail scenario, but you're right that re-throwing one exception while discarding the rest would be misleading in the general case, and consecutive WARN lines are sufficient signal for systemic failures. Thanks for the clarification.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

[Copilot]

createdBy in thread JSON is optional (and may be present as null). Using threadJson.has("createdBy") ? threadJson.get("createdBy").asText() : "system" will treat a null value as the literal string "null", causing lookupUserId to fail and the CREATED relationship to be skipped. Prefer a null-safe read (e.g., threadJson.path("createdBy").asText("system") or an explicit isNull() check) so reruns backfill createdBy reliably.

Suggested change

	String createdByName =
	threadJson.has("createdBy") ? threadJson.get("createdBy").asText() : "system";
	String createdByName = threadJson.path("createdBy").asText("system");

[yan-3005]

Fixed — using threadJson.path("createdBy").asText("system") now.

[Copilot]

In the alreadyExists branch you backfill link relationships, but domain relationships (DOMAIN --HAS--> TASK) are still skipped. If a previous run inserted the task_entity row but didn’t complete insertTaskDomainRelationships (or domains were resolvable only after later steps), reruns will continue to leave tasks without domains and break domain-scoped task queries. Consider resolving domains and calling insertTaskDomainRelationships here too.

[yan-3005]

Not applicable here. TaskRepository didn't exist before v2.0.0 (task entity is new in this redesign), so there are no pre-existing domain relationships to backfill. Additionally, the original migration code already called insertTaskDomainRelationships for new tasks — only insertTaskLinkRelationships was missing. If old code ran, domain rows were already inserted.

[Copilot]

In the alreadyExists branch you backfill ASSIGNED_TO/REVIEWS/FOLLOWS/CREATED/MENTIONED_IN relationships, but you don’t backfill domain relationships. If a previous run inserted the task row but didn’t insert domains (or domains were resolvable only after later steps), reruns will still leave tasks without DOMAIN --HAS--> TASK rows. Consider re-running resolveDomainsForTaskAbout + insertTaskDomainRelationships in this branch as well.

Suggested change

	setAboutFromEntityLink(aboutJson, aboutLink, threadJson);
	setAboutFromEntityLink(aboutJson, aboutLink, threadJson);
	insertTaskDomainRelationships(
	handle,
	threadId,
	resolveDomainsForTaskAbout(handle, aboutJson, connectionType),
	connectionType);

[yan-3005]

Not applicable here. TaskRepository didn't exist before v2.0.0 (task entity is new in this redesign), so there are no pre-existing domain relationships to backfill. Additionally, the original migration code already called insertTaskDomainRelationships for new tasks — only insertTaskLinkRelationships was missing. If old code ran, domain rows were already inserted.

[gitar-bot]

Code Review 👍 Approved with suggestions 2 resolved / 3 findings

Migration adds thread_entity guards, PostgreSQL jsonb casting, and cycle detection, addressing faulty assertions on booleans and empty input tests. Consider refining the seed data initialization to explicitly handle or escalate failures rather than swallowing them.

💡 Edge Case: Seed data failures silently swallowed may hide systemic issues

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java:1231-1240

The new try/catch in initSeedDataFromResources (line 1231) logs a warning and continues when any entity fails to initialize. While this is the stated intent (one bad workflow shouldn't block others), a systemic failure (e.g., database connectivity loss) would cause every entity to fail silently, and the system would start with no seed data and only WARN-level logs. Consider adding a threshold: if all entities fail, re-throw the last exception to surface the systemic issue.

Suggested fix

int failCount = 0;
Exception lastException = null;
for (T entity : entities) {
  try {
    initializeEntity(entity);
  } catch (Exception e) {
    failCount++;
    lastException = e;
    LOG.warn("Failed to initialize ...", ...);
  }
}
if (failCount == entities.size() && !entities.isEmpty()) {
  throw new RuntimeException(
    "All seed entities failed to initialize", lastException);
}

✅ 2 resolved

✅ Quality: Jsonb cast tests pass vacuously with empty input data

📄 openmetadata-service/src/test/java/org/openmetadata/service/migration/utils/v200/MigrationUtilTest.java:114-128
The tests migrateThreadTasksUsesJsonbCastForPostgres (line 114) and migrateThreadTasksDoesNotUseJsonbCastForMysql (line 130) set up an empty list of threads via java.util.List.of(). Since no rows are returned, the migration loop body never executes and insertTask is never called. The verify(handle, never()).createUpdate(contains(...)) assertions pass trivially regardless of whether the jsonb cast logic is correct.

To meaningfully test the cast behavior, provide at least one valid thread row so insertTask is actually invoked, then assert on the SQL string passed to createUpdate.

✅ Bug: assertNotNull on boolean always passes — assertion is a no-op

📄 openmetadata-service/src/test/java/org/openmetadata/service/migration/utils/v200/MigrationUtilTest.java:268
At line 268, assertNotNull(entityRelationshipInserts >= 2) evaluates the comparison to a boolean primitive, which gets autoboxed to Boolean.TRUE or Boolean.FALSE. Neither is ever null, so this assertion always passes regardless of the actual count. This means the test will succeed even if zero entity_relationship INSERT statements are generated, completely defeating its purpose.

Compare with line 207 in migrateThreadTaskInsertsEntityRelationshipRowsForAssigneesAndAbout, which correctly uses assertEquals(true, entityRelationshipInserts >= 2).

🤖 Prompt for agents

Code Review: Migration adds thread_entity guards, PostgreSQL jsonb casting, and cycle detection, addressing faulty assertions on booleans and empty input tests. Consider refining the seed data initialization to explicitly handle or escalate failures rather than swallowing them.

1. 💡 Edge Case: Seed data failures silently swallowed may hide systemic issues
   Files: openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java:1231-1240

   The new try/catch in `initSeedDataFromResources` (line 1231) logs a warning and continues when any entity fails to initialize. While this is the stated intent (one bad workflow shouldn't block others), a systemic failure (e.g., database connectivity loss) would cause *every* entity to fail silently, and the system would start with no seed data and only WARN-level logs. Consider adding a threshold: if all entities fail, re-throw the last exception to surface the systemic issue.

   Suggested fix:
   int failCount = 0;
   Exception lastException = null;
   for (T entity : entities) {
     try {
       initializeEntity(entity);
     } catch (Exception e) {
       failCount++;
       lastException = e;
       LOG.warn("Failed to initialize ...", ...);
     }
   }
   if (failCount == entities.size() && !entities.isEmpty()) {
     throw new RuntimeException(
       "All seed entities failed to initialize", lastException);
   }

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[Copilot]

initSeedDataFromResources now swallows exceptions from initializeEntity and continues. This is a behavioral change that can impact startup/seed correctness, but there’s no unit test asserting that the loop continues (and that subsequent entities are still initialized) when one seed fails. Consider adding a focused test using a spy/stub repository to throw on the first initializeEntity call and verify later entities are still processed (and/or that the failure is logged with the entity FQN).

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ingestion'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud