diff --git a/bin/lint-migrations-file/src/change_set/common.clj b/bin/lint-migrations-file/src/change_set/common.clj index bf4d0797a3e42f65d7a7ca084d8b756d89bfe832..9ce9c8812e1b5412eb073b8c0fb25a256bdf7d36 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 48d82f7cf0eac3b0a84b6dad69648a72acec2443..b5f208a8671c6f6871abad77fbe5b3145480a817 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 e4769db5ba14f1fe9017a4f3eeabbf3568fafcc2..e966fd26557b9d44e288086d1852f35e65d039f8 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 521ea5043020038fef0bcd403d94c3ddc99ccefc..22b74a60ce82c23872c2bbc7e8b4b8be931733b6 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 6437aba5a750de059c67c5132c31920df49f111a..6770d65d1e2d7df6c70b76e96003936cad97bb6a 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 58d61eab105ad793112a0d522403d63fd0cc2b8e..f61daa1f1ebe7dd071f51ea70265f04691f5ef34 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 3afa93a45815cf195a0bc7fd521a7348130b2522..a6b53a4138ac0ce55831277bca867694b7cd5ead 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)))))