Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unable to migrate to 49 due to v49.00-059 migration #40547

Merged
merged 2 commits into from
Mar 25, 2024

Conversation

qnkhuat
Copy link
Contributor

@qnkhuat qnkhuat commented Mar 25, 2024

Fixes #40546

In #37160 we added a migration to convert all datetime columns to timestamp with time zone by using a fixed list of what to convert. But it turns out not all instances have the same list, not sure why, could be that liquibase interprets the timestamp columns differently between version/dialect.

This PR removed the fixed list and used a query to get a list of all datetime columns.

Note: I don't think we need to update the checksum because custom migration has the class name, not the implementation. Tested locally.

timestamp with time zone by using a fixed list of what to convert. but
it turns out some customer instances has a different list. So this PR
removed the fixed list and use a query to get a list of all datetime
columns.
@qnkhuat qnkhuat requested a review from camsaul as a code owner March 25, 2024 07:12
@metabase-bot metabase-bot bot added the .Team/BackendComponents also known as BEC label Mar 25, 2024
@qnkhuat qnkhuat added the backport Automatically create PR on current release branch on merge label Mar 25, 2024
@qnkhuat qnkhuat added this to the 0.49.2 milestone Mar 25, 2024
Copy link

replay-io bot commented Mar 25, 2024

Status Complete ↗︎
Commit 10a0066
Results
⚠️ 7 Flaky
2374 Passed

@qnkhuat qnkhuat requested a review from a team March 25, 2024 07:25
(map #(update-vals % (comp keyword lower-case-en)))
(remove (fn [{:keys [table_name]}]
(or (#{:databasechangelog :databasechangeloglock} table_name)
(str/starts-with? (name table_name) "v_"))))
Copy link
Contributor

Choose a reason for hiding this comment

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

ehm, what's v_?

Copy link
Contributor Author

@qnkhuat qnkhuat Mar 25, 2024

Choose a reason for hiding this comment

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

those are views

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'll put a comment there

@qnkhuat qnkhuat enabled auto-merge (squash) March 25, 2024 08:39
@paoliniluis
Copy link
Contributor

Most probably because of different MySQL vendors

@qnkhuat qnkhuat merged commit 8b728be into master Mar 25, 2024
107 checks passed
@qnkhuat qnkhuat deleted the unify-timestamp-using-dynamic-list branch March 25, 2024 10:49
qnkhuat added a commit that referenced this pull request Mar 25, 2024
* In #37160 we added a migration to convert all datetime columns to
timestamp with time zone by using a fixed list of what to convert. but
it turns out some customer instances has a different list. So this PR
removed the fixed list and use a query to get a list of all datetime
columns.
qnkhuat added a commit that referenced this pull request Mar 25, 2024
)

* In #37160 we added a migration to convert all datetime columns to
timestamp with time zone by using a fixed list of what to convert. but
it turns out some customer instances has a different list. So this PR
removed the fixed list and use a query to get a list of all datetime
columns.

Co-authored-by: Ngoc Khuat <qn.khuat@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Automatically create PR on current release branch on merge .Team/BackendComponents also known as BEC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v0.49.x version of metabase fails to initialize when using mysql 8.0 as database
3 participants