Skip to content

Fixes #37180: align entity_extension.extension with its table's collation on MySQL - #37184

Merged
manerow merged 4 commits into
mainfrom
fix/37180-entity-extension-collation
Nov 6, 2026
Merged

manerow merged 4 commits into
mainfrom
fix/37180-entity-extension-collation

Conversation

@manerow

@manerow manerow commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #37180

Why

Until #37167, the 2.0.3 MySQL migration switched entity_extension.extension to CHARACTER SET ascii COLLATE ascii_bin. PR #37181 stops that for upgrades that have not run 2.0.3 yet. Databases that already ran it (upgraded to 2.0.3 to 2.0.7 or 2.1.0-rc1) still have an ascii column that differs from every other text column in the table: it rejects non-ASCII keys and compares keys case-sensitively. Two states of the same column also mean every future change to it has to handle both.

What changes

A new MySQL data migration step, EntityExtensionCollationAlignment, puts the column back on its table's default character set and collation:

  • It only acts when the column differs from its table. Databases that never ran the old statement, or whose column was already converted ahead of the upgrade, do nothing.
  • The target is each table's own default, read from information_schema, not a fixed collation. OpenMetadata never set a collation, so tables carry the server default of the day they were created: utf8mb4_0900_ai_ci on MySQL 8, but older databases can have utf8mb4_general_ci. Forcing one collation would copy tables that are already consistent.
  • Collision check before copying. The table default usually ignores case, so two keys of one entity that differ only in case (for example values of custom properties Foo and foo) would become duplicates and the copy would fail with ERROR 1062, possibly hours in. The step looks for them first and, if any exist, stops with an error that names them and says what to do, leaving the database unchanged. The check reads only the small secondary index, which also holds id and extension: 1.7 s on a 1M-row, 4 GB table.
  • It logs before copying, with the table size, because MySQL copies the whole table and blocks writes to it until the copy ends.

It runs from the 2.0.8 Java migration, and again from 2.1.0 (with runOnce, like the other 2.0.8 steps there), so it also reaches databases already on 2.1 pre-release builds: a changed 2.1.0 migration class is pending again for them.

It runs from Java rather than schemaChanges.sql because it has to stop with a clear message, per ADR:2026-10-22-guarded-index-ddl-runs-from-a-java-data-migration. The 2.0.8 SQL file is not edited to point here, because changing the comment above an existing statement changes its recorded text and makes it run again on databases that already ran 2.0.8. Postgres is unchanged.

Decision record

ADR:2026-11-06-entity-extension-uses-its-tables-collation states the rule: on MySQL, entity_extension.extension uses its table's default collation, and no migration pins it to another one.

Tests

EntityExtensionCollationAlignmentTest runs the step on a real MySQL 8.0 container. An unchanged InnoDB table id means MySQL did not copy the table.

Case Result
ascii column, table on utf8mb4_0900_ai_ci Aligned, rows unchanged
Column already matching its table Untouched, no copy
ascii column, table on utf8mb4_general_ci Aligned to utf8mb4_general_ci
Foo and foo on the same entity Stops before copying, names them, column and table unchanged
Foo and foo on different entities Not a collision, aligned
Second run No change

Real upgrades with the migration runner, using the 2.1.0-rc1 release image with this step and PR #37181 added, on databases built by running the real migrations:

Database Upgrade with this change Table copied validate
Ran stock 2.0.3 (ascii_bin) Aligned to utf8mb4_0900_ai_ci; a second run changes nothing Once Passes
Ran stock 2.0.3, plus Foo and foo on one entity Stops before copying with the error below, column unchanged. After deleting one key, the rerun aligns it Only on the rerun
Already migrated by stock 2.1.0-rc1 (ascii_bin, 2.1.0 recorded) Aligned on the next upgrade, through the 2.1.0 step Once Passes
Ran 2.0.2 only (utf8mb4_0900_ai_ci, 256) Widened in place by PR #37181, nothing to align No Passes
Cannot align entity_extension.extension to utf8mb4_0900_ai_ci: some keys of the same entity differ only in letter case and would become duplicates. They usually come from two custom properties whose names differ only in case. Delete or rename one property of each pair, then run the migration again. Conflicting keys (entity id: keys, up to 20 shown): 11111111-2222-3333-4444-555555555555: table.customProperties.Foo, table.customProperties.foo

@manerow
manerow requested a review from a team as a code owner November 5, 2026 15:41
@manerow
manerow requested a review from harshach November 5, 2026 15:41
@manerow manerow added safe to test Add this label to run secure Github workflows on PRs backend labels Nov 5, 2026
@manerow manerow self-assigned this Nov 5, 2026
# Conflicts:
#	openmetadata-service/src/main/java/org/openmetadata/service/migration/mysql/v208/Migration.java
#	openmetadata-service/src/main/java/org/openmetadata/service/migration/mysql/v210/Migration.java
@gitar-bot

gitar-bot Bot commented Nov 6, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 closed / 1 findings

🔴 High risk · Aligning MySQL entity_extension collation rewrites the table and changes key comparisons, potentially blocking writes or halting upgrades on case collisions.

Adds a MySQL data migration step, EntityExtensionCollationAlignment, that moves entity_extension.extension from ascii_bin back to its table's default collation, with a pre-copy check for case-only key collisions. The open decision-record finding was resolved, and no open issues remain.

✅ 1 closed
✅ Quality: No decision record for the entity_extension.extension collation rule

📄 openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v208/EntityExtensionCollationAlignment.java:21-35 📄 openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v208/EntityExtensionCollationAlignment.java:64-66
This PR decides what character set and collation entity_extension.extension uses on MySQL, and nothing in docs/decisions/ records that rule. No record mentions collation or ascii_bin. Keys that differ only in case now collide, so two custom properties named Foo and foo can't both exist on one entity. A later migration could undo this rule as easily as 2.0.3 did. The description gives the rule in prose ("The target is each table's own default read from information_schema, not a fixed collation"), and since the repo squash-merges, that prose never reaches git log.

Please add a record stating: "On MySQL, entity_extension.extension uses its table's default character set and collation. Migrations never pin it to a fixed collation such as ascii_bin. A migration that changes the column's collation first refuses to run if keys of one entity would become equal." Cite it from EntityExtensionCollationAlignment as ADR:<date>-<slug>.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Not enabled

Set up

For more verification details, comment gitar display:verbose.

Options

Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

@manerow
manerow merged commit 1cbce0c into main Nov 6, 2026
69 checks passed
@manerow
manerow deleted the fix/37180-entity-extension-collation branch November 6, 2026 13:45
mohityadav766 added a commit that referenced this pull request Nov 6, 2026
#37186 added EntityExtensionCollationAlignment to the 2.0.8 MySQL data migration without the matching change to WorkflowRedeployEntryPointTest, so the test's mocked Handle reached the step's query. This is the test change main made in #37184.
mohityadav766 added a commit that referenced this pull request Nov 6, 2026
…37162)

* fix(sql): bind names in the prefix-rename statements

The FQN prefix renames, the tag-usage prefix rewrites and the thread-link rename bind every value; only the table name stays a template attribute. A guard test fails on any DAO statement that is a bare template attribute. Tag lists count usage with the bound per-tag query. ListFilter.escapeForMySqlRegexReplacement stays, deprecated, for extensions.

* fix(auth): resolve our own tokens to the account they name

A token verified against OpenMetadata's own signing key, on a request or a WebSocket connection, resolves to the user name it was minted for instead of the configured principal claims, whose email local part can belong to another account. SecurityUtil.getUserName uses the principal name whole.

* fix(sessions): logout retires the request's token; password changes end sign-ins

Logout denylists the verified bearer token of the request, not a token named in the body. Denylist entries are keyed by a digest of the token's signed part, hold only who signed out and when, and are capped. A new password, from a reset link, the user, an admin or the reset-password command, ends the user's refresh tokens, live sessions and MCP grants; personal access tokens and bot tokens stay.

* fix(sessions): replace a refresh token in one update

A refresh replaces its refresh token with the new one in a single UPDATE keyed by the old token, for basic and LDAP sign-ins. Only the request whose update changed the row gets new tokens; a token deleted while the request ran is not renewed.

* fix(sso): a SAML login completes in the browser that started it

The login keeps its AuthnRequest id, and the ACS accepts only a response whose signed assertion answers it. The ACS no longer signs anyone in: it attaches the account to the pending login and redirects to /api/v1/saml/complete on the login's own origin with a one-time code. Completing the login needs both the browser's session cookie and that code, read from the session store.

* fix(input): bound parsing of names, patches and descriptions

The email pattern matches in linear time and accepts the same addresses. The message parser's entity-link pattern cannot backtrack across unclosed links. JSON Patch bodies are read with Jackson limits and an operation cap, and a patch on the document root is refused. Description placeholders are restored in one pass, in text positions only.

* fix(users): keep account restore, deletion and role changes with admins

A PUT naming the caller's own account refuses a deleted self with 404 and treats an unknown self as a sign-up, allowed only when self sign-up is. Deleting your own account needs an admin. A role list that differs from the stored one, in either direction, is an admin change, and a non-admin PUT keeps the stored admin and bot flags and, when omitted, the stored roles. A non-admin CSV row fails for a stored admin or bot.

* fix(policies): reviewers, disabled and owner-team policies, RBAC writes

Rules of a disabled policy no longer apply. An owning team's policies give their allow rules to its members only. A reviewer is matched by user id. Startup no longer rewrites seeded policies. Impersonated requests cannot write policies or roles, and BotNonAdminImpersonationPolicy denies bot targets, added to stored copies by the 2.0.8 migration. A soft-deleted team or role keeps the policy conditions that name it; only a hard delete rewrites them.

* fix(patch-authz): authorize every location a patch touches

A JSON Patch is authorized at every location it touches, the from location of a move or copy included. Narrow edit operations apply only at real fields of the entity; tag changes are compared with their location, and an operation beside a tag change keeps that change's tag operation. The user PATCH guards and the domain check read the same locations. A patch's copies are bounded in size, and a date field refuses an object or array.

* fix(users): decide team membership and self-delete on the stored result; refuse impersonated team grants

A user's team changes are decided on the patched result, whatever shape the PATCH operations take, and joining or leaving a closed team needs an admin; a non-admin PUT that omits teams keeps the stored ones. The by-name self-delete check resolves the user and compares ids. Team writes that change default roles or policies, and team CSV imports, refuse impersonated requests.

* fix(ui): offer Join and Leave team only where the user may change the membership

The team page shows Join and Leave only on a joinable team, or to an admin, as the server allows joining and leaving a closed team only for admins.

* fix(create-put): authorize creates and PUTs against the stored entity

- On create, hasDomain() needs a domain of the stored container and every domain the request
  names.
- A root parent reference without an id is loaded by name and a missing one fails the request;
  TagMapper resolves the classification by name.
- A tag's parent must belong to the tag's classification.
- PUT contexts see soft-deleted entities, as the write does.
- A system tag's fully qualified name cannot change through its parent or classification either.

ADR:2026-11-03-a-create-is-authorized-against-its-stored-container

* fix(delete): a hard delete removes only rows of the deleted entity

- The FQN-prefix cleanup of field relationships runs only for taggable types and for types, whose custom property definitions live under their name; tag labels are removed by FQN prefix only for taggable types.
- Custom property definitions are removed only when their type is deleted.
- The tag-FQN label delete without a source is removed; tags and glossary terms keep removing their own source-bound labels.

ADR:2026-11-03-a-hard-delete-removes-only-rows-of-the-deleted-entity

* fix(reads): read handlers authorize like the entity reads they mirror

- Tasks read by id, and their versions, also need ViewBasic on the entity they are about.
- Activity by entity link matches the link's entity type as well as its FQN.
- Team hierarchy, task counts, domain hierarchy and the type custom-property reads need ViewBasic on their type; the team hierarchy still serves joinable teams to someone signing up. The role list uses the standard listing.
- The global profiler setting is always authorized on tables.

ADR:2026-11-03-read-handlers-authorize-like-the-entity-reads-they-mirror

* test(mcp): give create_entity's root parent a repository in the tool test

CreateResourceContext now loads the root parent a create names (a glossary term's glossary) and no longer swallows a failed load, so the test's static Entity stub has to answer for that parent's repository.

* test(sessions): fit the session ITs to 2.0's authenticator and tokens

2.0's BasicAuthenticator reads the authorizer configuration at init, so PasswordChangeIT gives it one. 2.0 tokens carry no jti, so the logout test mints its two tokens with different expiries; two tokens minted in the same second would otherwise be the same token.

* docs(decisions): name only the patch guards 2.0 has

The 2.0 record no longer mentions the connection and sourceConfig copy guard, context-file sharing or conversations, which 2.0 does not have.

* test(migrations): mock the collation step in the v208 entry-point test

#37186 added EntityExtensionCollationAlignment to the 2.0.8 MySQL data migration without the matching change to WorkflowRedeployEntryPointTest, so the test's mocked Handle reached the step's query. This is the test change main made in #37184.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MySQL: realign entity_extension.extension on databases already switched to ascii by 2.0.3

2 participants