From b461087789d1bfcce754d9019d4a2e321845ed45 Mon Sep 17 00:00:00 2001
From: Jeff Evans <jeff303@users.noreply.github.com>
Date: Thu, 4 Nov 2021 12:00:12 -0500
Subject: [PATCH] Change all active TEXT columns in MySQL app DB to LONGTEXT
(#18749)
* Change all active TEXT columns in MySQL app DB to LONGTEXT
Add new text.type Liquibase property to dynamically select the correct text type to use, per DB (which is LONGTEXT for MySQL), very similar to the existing blob.type one
Adding new migrations that update all existing app DB TEXT columns to LONGTEXT columns, only for mariadb/mysql
Update migrations linter to ensure no new text types get added, via a new predicate that searches for text types being added
Do the same logic for "blob" now (should be "${blob.type}"), update rule and test
Add test for migrations, that checks the new types, for all types in scope for this PR, to ensure that they have all been changed to their expected DB-specific type (either LONGTEXT for MySQL or TEXT/CLOB for others)
Updating a couple migrations to remove special casing that was originally done only for MySQL to simply make them universally "${text.type}", in order to unify behavior and reduce surprises going forward
Co-authored-by: Cam Saul <github@camsaul.com>
---
.../src/change_set/common.clj | 2 +-
.../src/lint_migrations_file.clj | 56 +
.../test/lint_migrations_file_test.clj | 17 +
.../serialization/cmd_test.clj | 2 +-
resources/migrations/000_migrations.yaml | 998 +++++++++++++++++-
test/metabase/db/schema_migrations_test.clj | 104 +-
.../db/schema_migrations_test/impl.clj | 25 +-
7 files changed, 1181 insertions(+), 23 deletions(-)
diff --git a/bin/lint-migrations-file/src/change_set/common.clj b/bin/lint-migrations-file/src/change_set/common.clj
index bf4d0797a3e..9ce9c8812e1 100644
--- a/bin/lint-migrations-file/src/change_set/common.clj
+++ b/bin/lint-migrations-file/src/change_set/common.clj
@@ -15,7 +15,7 @@
(s/def ::new-style-id
(s/and string?
- #(re-matches #"^v\d{2}\.\d{2}-\d{3}$" %)))
+ #(re-matches #"^v\d{2,}\.\d{2}-\d{3}$" %)))
(s/def ::id
(s/or
diff --git a/bin/lint-migrations-file/src/lint_migrations_file.clj b/bin/lint-migrations-file/src/lint_migrations_file.clj
index 48d82f7cf0e..b5f208a8671 100644
--- a/bin/lint-migrations-file/src/lint_migrations_file.clj
+++ b/bin/lint-migrations-file/src/lint_migrations_file.clj
@@ -4,6 +4,8 @@
[clojure.java.io :as io]
[clojure.pprint :as pprint]
[clojure.spec.alpha :as s]
+ [clojure.string :as str]
+ [clojure.walk :as walk]
[yaml.core :as yaml]))
(comment change-set.strict/keep-me
@@ -51,10 +53,64 @@
(let [ids (change-set-ids change-log)]
(= ids (sort-by identity compare-ids ids))))
+(defn- assert-no-types-in-change-set
+ "Walks over x (a changeset map) to ensure it doesn't add any columns of `target-types` (a set of strings).
+ `found-cols` is an atom of vector, in which any problematic changes to the `target-types` will be stored.
+
+ A partial application of this function will be passed to postwalk below.
+
+ TODO: add and conform to a spec instead?"
+ [target-types found-cols x]
+ {:pre [(set? target-types) (instance? clojure.lang.Atom found-cols)]}
+ (if
+ (map? x)
+ (cond
+ ;; a createTable or addColumn change; see if it adds a target-type col
+ (or (:createTable x) (:addColumn x))
+ (let [op (cond (:createTable x) :createTable (:addColumn x) :addColumn)
+ cols (filter (fn [col-def]
+ (contains? target-types
+ (str/lower-case (or (get-in col-def [:column :type]) ""))))
+ (get-in x [op :columns]))]
+ (doseq [col cols]
+ (swap! found-cols conj col))
+ x)
+
+ ;; a modifyDataType change; see if it changes a column to target-type
+ (:modifyDataType x)
+ (if (= target-types (str/lower-case (or (get-in x [:modifyDataType :newDataType]) "")))
+ (do (swap! found-cols conj x)
+ x)
+ x)
+
+ true ; some other kind of change; continue walking
+ x)
+ x))
+
+(defn no-bare-blob-or-text-types?
+ "Ensures that no \"text\" or \"blob\" type columns are added in changesets with id later than 320 (i.e. version
+ 0.42.0). From that point on, \"${text.type}\" should be used instead, so that MySQL can handle it correctly (by using
+ `LONGTEXT`). And similarly, from an earlier point, \"${blob.type\" should be used instead of \"blob\"."
+ [change-log]
+ (let [problem-cols (atom [])
+ walk-fn (partial assert-no-types-in-change-set #{"blob" "text"} problem-cols)]
+ (doseq [{{id :id} :changeSet, :as change-set} change-log
+ :when (and id
+ (string? id))]
+ [id change-set])
+ (doseq [{{id :id} :changeSet :as change-set} change-log
+ :when (and id
+ ;; only enforced in 42+ with new-style migration IDs.
+ (string? id)
+ (str/starts-with? id "v"))]
+ (walk/postwalk walk-fn change-set))
+ (empty? @problem-cols)))
+
;; TODO -- change sets must be distinct by ID.
(s/def ::databaseChangeLog
(s/and distinct-change-set-ids?
change-set-ids-in-order?
+ no-bare-blob-or-text-types?
(s/+ (s/alt :property (s/keys :req-un [::property])
:changeSet (s/keys :req-un [::changeSet])))))
diff --git a/bin/lint-migrations-file/test/lint_migrations_file_test.clj b/bin/lint-migrations-file/test/lint_migrations_file_test.clj
index e4769db5ba1..e966fd26557 100644
--- a/bin/lint-migrations-file/test/lint_migrations_file_test.clj
+++ b/bin/lint-migrations-file/test/lint_migrations_file_test.clj
@@ -182,3 +182,20 @@
clojure.lang.ExceptionInfo
#"new-style-id"
(validate-id "42.01-001"))))))
+
+(deftest prevent-text-types-test
+ (testing "should allow \"${text.type}\" columns from being added"
+ (is (= :ok
+ (validate
+ (mock-change-set
+ :id "v42.00-001"
+ :changes [(mock-add-column-changes :columns [(mock-column :type "${text.type")])]))))
+ (doseq [problem-type ["blob" "text"]]
+ (testing (format "should prevent \"%s\" columns from being added after ID 320" problem-type)
+ (is (thrown-with-msg?
+ clojure.lang.ExceptionInfo
+ #"(?s)^.*no-bare-blob-or-text-types\\?.*$"
+ (validate
+ (mock-change-set
+ :id "v42.00-001"
+ :changes [(mock-add-column-changes :columns [(mock-column :type problem-type)])]))))))))
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj
index 521ea504302..22b74a60ce8 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj
@@ -22,7 +22,7 @@
what we need."
[& body]
`(schema-migrations-test.impl/with-temp-empty-app-db [conn# :h2]
- (schema-migrations-test.impl/run-migrations-in-range! conn# [0 999999]) ; this should catch all migrations)
+ (schema-migrations-test.impl/run-migrations-in-range! conn# [0 "v99.00-000"]) ; this should catch all migrations)
;; since the actual group defs are not dynamic, we need with-redefs to change them here
(with-redefs [group/all-users (#'group/get-or-create-magic-group! group/all-users-group-name)
group/admin (#'group/get-or-create-magic-group! group/admin-group-name)
diff --git a/resources/migrations/000_migrations.yaml b/resources/migrations/000_migrations.yaml
index 6437aba5a75..6770d65d1e2 100644
--- a/resources/migrations/000_migrations.yaml
+++ b/resources/migrations/000_migrations.yaml
@@ -19,6 +19,15 @@ databaseChangeLog:
name: timestamp_type
value: timestamp(6)
dbms: mysql,mariadb
+ # In MySQL, use LONGTEXT instead of TEXT (#7006)
+ - property:
+ name: text.type
+ value: text
+ dbms: postgresql,h2
+ - property:
+ name: text.type
+ value: longtext
+ dbms: mysql,mariadb
- changeSet:
id: '1'
@@ -8679,6 +8688,7 @@ databaseChangeLog:
tableName: metabase_table
columnName: entity_name
+
- changeSet:
id: v42.00-001
author: camsaul
@@ -8789,19 +8799,985 @@ databaseChangeLog:
nullable: false
defaultValue: false
+ - changeSet: # this one is run unconditionally so unify the type to ${text.type} across ALL dbs
+ id: v42.00-004 # this differentiation was first done under changeSet 13 above
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of activity.details from text to ${text.type}
+ changes:
+ - modifyDataType:
+ tableName: activity
+ columnName: details
+ newDataType: ${text.type}
-# >>>>>>>>>> DO NOT ADD NEW MIGRATIONS BELOW THIS LINE! ADD THEM ABOVE <<<<<<<<<<
+ - changeSet:
+ id: v42.00-005
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of collection.description from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: collection
+ columnName: description
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
-########################################################################################################################
-#
-# ADVICE:
-#
-# 1) Run ./bin/lint-migrations-file.sh to run core.spec checks against any changes you make here. Liquibase is pretty
-# forgiving and won't complain if you accidentally mix up things like deleteCascade and onDelete: CASCADE. CI runs
-# this check but it's nicer to know now instead of waiting for CI.
-#
-# 2) Please post a message in the Metabase Slack #migrations channel to let others know you are creating a new
-# migration so someone else doesn't steal your ID number
+ - changeSet:
+ id: v42.00-006
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of collection.name from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: collection
+ columnName: name
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-007
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of computation_job.context from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: computation_job
+ columnName: context
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-008
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of computation_job_result.payload from text
+ to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: computation_job_result
+ columnName: payload
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-009
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of core_session.anti_csrf_token from text
+ to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: core_session
+ columnName: anti_csrf_token
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-010
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of core_user.login_attributes from text to
+ ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: core_user
+ columnName: login_attributes
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-011
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of group_table_access_policy.attribute_remappings
+ from text to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: group_table_access_policy
+ columnName: attribute_remappings
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-012
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of login_history.device_description from text
+ to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: login_history
+ columnName: device_description
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-013
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of login_history.ip_address from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: login_history
+ columnName: ip_address
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-014
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_database.caveats from text to
+ ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_database
+ columnName: caveats
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-015
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_database.description from text
+ to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_database
+ columnName: description
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-016
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_database.details from text to
+ ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_database
+ columnName: details
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-017
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_database.options from text to
+ ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_database
+ columnName: options
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-018
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_database.points_of_interest from
+ text to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_database
+ columnName: points_of_interest
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-019
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_field.caveats from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_field
+ columnName: caveats
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-020
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_field.database_type from text
+ to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_field
+ columnName: database_type
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-021
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_field.description from text to
+ ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_field
+ columnName: description
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-022
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_field.fingerprint from text to
+ ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_field
+ columnName: fingerprint
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-023
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_field.has_field_values from text
+ to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_field
+ columnName: has_field_values
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-024
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_field.points_of_interest from
+ text to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_field
+ columnName: points_of_interest
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-025
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_field.settings from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_field
+ columnName: settings
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-026
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_fieldvalues.human_readable_values
+ from text to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_fieldvalues
+ columnName: human_readable_values
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-027
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_fieldvalues.values from text to
+ ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_fieldvalues
+ columnName: values
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-028
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_table.caveats from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_table
+ columnName: caveats
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-029
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_table.description from text to
+ ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_table
+ columnName: description
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-030
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metabase_table.points_of_interest from
+ text to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metabase_table
+ columnName: points_of_interest
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-031
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metric.caveats from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metric
+ columnName: caveats
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-032
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metric.definition from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metric
+ columnName: definition
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-033
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metric.description from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metric
+ columnName: description
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-034
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metric.how_is_this_calculated from text
+ to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metric
+ columnName: how_is_this_calculated
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-035
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of metric.points_of_interest from text to
+ ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: metric
+ columnName: points_of_interest
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-036
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of moderation_review.text from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: moderation_review
+ columnName: text
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-037
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of native_query_snippet.content from text
+ to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: native_query_snippet
+ columnName: content
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-038
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of native_query_snippet.description from text
+ to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: native_query_snippet
+ columnName: description
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-039
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of pulse.parameters from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: pulse
+ columnName: parameters
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-040
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of pulse_channel.details from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: pulse_channel
+ columnName: details
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-041
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of query.query from text to ${text.type} on
+ mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: query
+ columnName: query
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-042
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of query_execution.error from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: query_execution
+ columnName: error
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-043
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of report_card.dataset_query from text to
+ ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: report_card
+ columnName: dataset_query
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-044
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of report_card.description from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: report_card
+ columnName: description
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-045
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of report_card.embedding_params from text
+ to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: report_card
+ columnName: embedding_params
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-046
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of report_card.result_metadata from text to
+ ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: report_card
+ columnName: result_metadata
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-047
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of report_card.visualization_settings from
+ text to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: report_card
+ columnName: visualization_settings
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-048
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of report_dashboard.caveats from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: report_dashboard
+ columnName: caveats
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-049
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of report_dashboard.description from text
+ to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: report_dashboard
+ columnName: description
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-050
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of report_dashboard.embedding_params from
+ text to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: report_dashboard
+ columnName: embedding_params
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-051
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of report_dashboard.parameters from text to
+ ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: report_dashboard
+ columnName: parameters
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-052
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of report_dashboard.points_of_interest from
+ text to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: report_dashboard
+ columnName: points_of_interest
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-053
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of report_dashboardcard.parameter_mappings
+ from text to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: report_dashboardcard
+ columnName: parameter_mappings
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-054
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of report_dashboardcard.visualization_settings
+ from text to ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: report_dashboardcard
+ columnName: visualization_settings
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-055
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of revision.message from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: revision
+ columnName: message
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet: # this one is run unconditionally so unify the type to ${text.type} across ALL dbs
+ id: v42.00-056 # this differentiation was first done under changeSet 10 above
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of revision.object from text to ${text.type}
+ changes:
+ - modifyDataType:
+ tableName: revision
+ columnName: object
+ newDataType: ${text.type}
+
+ - changeSet:
+ id: v42.00-057
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of segment.caveats from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: segment
+ columnName: caveats
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-058
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of segment.definition from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: segment
+ columnName: definition
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-059
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of segment.description from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: segment
+ columnName: description
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-060
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of segment.points_of_interest from text to
+ ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: segment
+ columnName: points_of_interest
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-061
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of setting.value from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: setting
+ columnName: value
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-062
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of task_history.task_details from text to
+ ${text.type} on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: task_history
+ columnName: task_details
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+ - changeSet:
+ id: v42.00-063
+ author: jeff303
+ comment: >-
+ Added 0.42.0 - modify type of view_log.metadata from text to ${text.type}
+ on mysql,mariadb
+ changes:
+ - modifyDataType:
+ tableName: view_log
+ columnName: metadata
+ newDataType: ${text.type}
+ preConditions:
+ - onFail: MARK_RAN
+ - dbms:
+ type: mysql,mariadb
+
+
+# >>>>>>>>>> DO NOT ADD NEW MIGRATIONS BELOW THIS LINE! ADD THEM ABOVE <<<<<<<<<<
+
+########################################################################################################################
+#
+# ADVICE:
+#
+# 1) Run ./bin/lint-migrations-file.sh to run core.spec checks against any changes you make here. Liquibase is pretty
+# forgiving and won't complain if you accidentally mix up things like deleteCascade and onDelete: CASCADE. CI runs
+# this check but it's nicer to know now instead of waiting for CI.
+#
+# 2) Please post a message in the Metabase Slack #migrations channel to let others know you are creating a new
+# migration so someone else doesn't steal your ID number
+#
+# 3) Migrations IDs should follow the format
+#
+# vMM.mm-NNN
+#
+# where
+#
+# M = major version
+# m = minor version
+# N = migration number relative to that major+minor version
+#
+# e.g. the first migration added to 0.42.0 should be numbered v42.00-000 and the second migration should be numbered
+# v42.00-001. The first migration for 0.42.1 should be numbered v42.01-000, and so forth.
+#
+# This numbering scheme was adopted beginning with version 0.42.0 so that we could go back and add migrations to patch
+# releases without the ID sequence getting wildly out of order. See PR #18821 for more information.
#
# 3) Migrations IDs should follow the format
#
diff --git a/test/metabase/db/schema_migrations_test.clj b/test/metabase/db/schema_migrations_test.clj
index 58d61eab105..f61daa1f1eb 100644
--- a/test/metabase/db/schema_migrations_test.clj
+++ b/test/metabase/db/schema_migrations_test.clj
@@ -8,14 +8,18 @@
5. verify that data looks like what we'd expect after running migration(s)
See `metabase.db.schema-migrations-test.impl` for the implementation of this functionality."
- (:require [clojure.test :refer :all]
+ (:require [clojure.java.jdbc :as jdbc]
+ [clojure.string :as str]
+ [clojure.test :refer :all]
[metabase.db.schema-migrations-test.impl :as impl]
+ [metabase.driver :as driver]
[metabase.models :refer [Database Field Table]]
[metabase.models.user :refer [User]]
[metabase.test.util :as tu]
[metabase.util :as u]
[toucan.db :as db])
- (:import java.util.UUID))
+ (:import java.sql.Connection
+ java.util.UUID))
(deftest database-position-test
(testing "Migration 165: add `database_position` to Field"
@@ -158,3 +162,99 @@
(map #(select-keys % [:base_type :effective_type :coercion_strategy
:semantic_type :name]))
(db/select Field :table_id table-id)))))))))
+
+(defn app-db-column-types
+ "Returns a map of all column names to their respective type names, for the given `table-name`, from the currently
+ bound app DB connection."
+ [^Connection conn table-name]
+ (with-open [rset (.getColumns (.getMetaData conn) nil nil table-name nil)]
+ (into {} (take-while some?)
+ (repeatedly
+ (fn []
+ (when (.next rset)
+ [(.getString rset "COLUMN_NAME") (.getString rset "TYPE_NAME")]))))))
+
+(deftest convert-text-to-longtext-migration-test
+ (testing "all columns that were TEXT type in MySQL were changed to"
+ (impl/test-migrations ["v42.00-004" "v42.00-063"] [migrate!]
+ (migrate!) ; just run migrations immediately, then check the new types
+ (let [all-text-cols [["activity" "details"]
+ ["collection" "description"]
+ ["collection" "name"]
+ ["computation_job" "context"]
+ ["computation_job_result" "payload"]
+ ["core_session" "anti_csrf_token"]
+ ["core_user" "login_attributes"]
+ ["group_table_access_policy" "attribute_remappings"]
+ ["login_history" "device_description"]
+ ["login_history" "ip_address"]
+ ["metabase_database" "caveats"]
+ ["metabase_database" "description"]
+ ["metabase_database" "details"]
+ ["metabase_database" "options"]
+ ["metabase_database" "points_of_interest"]
+ ["metabase_field" "caveats"]
+ ["metabase_field" "database_type"]
+ ["metabase_field" "description"]
+ ["metabase_field" "fingerprint"]
+ ["metabase_field" "has_field_values"]
+ ["metabase_field" "points_of_interest"]
+ ["metabase_field" "settings"]
+ ["metabase_fieldvalues" "human_readable_values"]
+ ["metabase_fieldvalues" "values"]
+ ["metabase_table" "caveats"]
+ ["metabase_table" "description"]
+ ["metabase_table" "points_of_interest"]
+ ["metric" "caveats"]
+ ["metric" "definition"]
+ ["metric" "description"]
+ ["metric" "how_is_this_calculated"]
+ ["metric" "points_of_interest"]
+ ["moderation_review" "text"]
+ ["native_query_snippet" "content"]
+ ["native_query_snippet" "description"]
+ ["pulse" "parameters"]
+ ["pulse_channel" "details"]
+ ["query" "query"]
+ ["query_execution" "error"]
+ ["report_card" "dataset_query"]
+ ["report_card" "description"]
+ ["report_card" "embedding_params"]
+ ["report_card" "result_metadata"]
+ ["report_card" "visualization_settings"]
+ ["report_dashboard" "caveats"]
+ ["report_dashboard" "description"]
+ ["report_dashboard" "embedding_params"]
+ ["report_dashboard" "parameters"]
+ ["report_dashboard" "points_of_interest"]
+ ["report_dashboardcard" "parameter_mappings"]
+ ["report_dashboardcard" "visualization_settings"]
+ ["revision" "message"]
+ ["revision" "object"]
+ ["segment" "caveats"]
+ ["segment" "definition"]
+ ["segment" "description"]
+ ["segment" "points_of_interest"]
+ ["setting" "value"]
+ ["task_history" "task_details"]
+ ["view_log" "metadata"]]]
+ (with-open [conn (jdbc/get-connection (db/connection))]
+ (doseq [[tbl-nm col-nms] (group-by first all-text-cols)]
+ (let [^String exp-type (case driver/*driver*
+ :mysql "longtext"
+ :h2 "CLOB"
+ "text")
+ name-fn (case driver/*driver*
+ :h2 str/upper-case
+ identity)
+ tbl-cols (app-db-column-types conn (name-fn tbl-nm))]
+ (doseq [col-nm (map last col-nms)]
+ (testing (format " %s in %s" exp-type driver/*driver*)
+ ;; just get the first/only scalar value from the results (which is a vec of maps)
+ (is (.equalsIgnoreCase exp-type (get tbl-cols (name-fn col-nm)))
+ (format "Using %s, type for %s.%s was supposed to be %s, but was %s"
+ driver/*driver*
+ tbl-nm
+ col-nm
+ exp-type
+ (get tbl-cols col-nm))))))))))))
diff --git a/test/metabase/db/schema_migrations_test/impl.clj b/test/metabase/db/schema_migrations_test/impl.clj
index 3afa93a4581..a6b53a4138a 100644
--- a/test/metabase/db/schema_migrations_test/impl.clj
+++ b/test/metabase/db/schema_migrations_test/impl.clj
@@ -97,18 +97,23 @@
(defn- migration-id-in-range?
"Whether `id` should be considered to be between `start-id` and `end-id`, inclusive. Handles both legacy plain-integer
and new-style `vMM.mm-NNN` style IDs."
- [start-id id end-id]
+ [start-id id end-id & [{:keys [inclusive-end?]
+ :or {inclusive-end? true}}]]
(let [start-id (migration-id->str start-id)
end-id (migration-id->str end-id)
id (migration-id->str id)]
- (= (sort [start-id id end-id])
- [start-id id end-id])))
+ (and (= (sort [start-id id end-id])
+ [start-id id end-id])
+ (or inclusive-end?
+ (not= id end-id)))))
(deftest migration-id-in-range?-test
(testing "legacy IDs"
(is (migration-id-in-range? 1 2 3))
+ (is (migration-id-in-range? 1 2 3 {:inclusive-end? false}))
(is (migration-id-in-range? 1 1 3))
(is (migration-id-in-range? 1 3 3))
+ (is (not (migration-id-in-range? 1 3 3 {:inclusive-end? false})))
(is (not (migration-id-in-range? 2 1 3)))
(is (not (migration-id-in-range? 2 4 3)))
(testing "strings"
@@ -116,20 +121,24 @@
(is (not (migration-id-in-range? 1 "13" 3)))))
(testing "new-style IDs"
(is (migration-id-in-range? "v42.00-001" "v42.00-002" "v42.00-003"))
+ (is (migration-id-in-range? "v42.00-001" "v42.00-002" "v42.00-002"))
+ (is (not (migration-id-in-range? "v42.00-001" "v42.00-002" "v42.00-002" {:inclusive-end? false})))
(is (not (migration-id-in-range? "v42.00-001" "v42.00-004" "v42.00-003")))
(is (not (migration-id-in-range? "v42.00-002" "v42.00-001" "v42.00-003"))))
(testing "mixed"
(is (migration-id-in-range? 1 3 "v42.00-001"))
(is (migration-id-in-range? 1 "v42.00-001" "v42.00-002"))
(is (not (migration-id-in-range? "v42.00-002" 1000 "v42.00-003")))
+ (is (not (migration-id-in-range? "v42.00-002" 1000 "v42.00-003" {:inclusive-end? false})))
(is (not (migration-id-in-range? 1 "v42.00-001" 1000)))
(is (not (migration-id-in-range? 1 "v42.00-001" 1000)))))
(defn run-migrations-in-range!
"Run Liquibase migrations from our migrations YAML file in the range of `start-id` -> `end-id` (inclusive) against a
DB with `jdbc-spec`."
- {:added "0.41.0"}
- [^java.sql.Connection conn [start-id end-id]]
+ {:added "0.41.0", :arglists '([conn [start-id end-id]]
+ [conn [start-id end-id] {:keys [inclusive-end?], :or {inclusive-end? true}}])}
+ [^java.sql.Connection conn [start-id end-id] & [range-options]]
(liquibase/with-liquibase [liquibase conn]
(let [change-log (.getDatabaseChangeLog liquibase)
;; create a new change log that only has the subset of migrations we want to run.
@@ -139,7 +148,7 @@
;; add the relevant migrations (change sets) to our subset change log
(doseq [^ChangeSet change-set (.getChangeSets change-log)
:let [id (.getId change-set)]
- :when (migration-id-in-range? start-id id end-id)]
+ :when (migration-id-in-range? start-id id end-id range-options)]
(.addChangeSet subset-change-log change-set))
;; now create a new instance of Liquibase that will run just the subset change log
(let [subset-liquibase (Liquibase. subset-change-log (.getResourceAccessor liquibase) (.getDatabase liquibase))]
@@ -159,7 +168,7 @@
(assert (zero? (count tables))
(str "'Empty' application DB is not actually empty. Found tables:\n"
(u/pprint-to-str tables))))))
- (run-migrations-in-range! conn [1 (dec start-id)])
+ (run-migrations-in-range! conn [1 start-id] {:inclusive-end? false})
(f #(run-migrations-in-range! conn [start-id end-id])))
(log/debug (u/format-color 'green "Done testing migrations for driver %s." driver)))
@@ -171,7 +180,7 @@
(let [[start-id end-id] (if (sequential? migration-range)
migration-range
[migration-range migration-range])]
- (testing (format "Migrations %d-%d" start-id end-id)
+ (testing (format "Migrations %s thru %s" start-id end-id)
(mt/test-drivers #{:h2 :mysql :postgres}
(test-migrations-for-driver driver/*driver* [start-id end-id] f)))))
--
GitLab