From 25a1b2596686b9f8c2bb05b069a108c39394fba5 Mon Sep 17 00:00:00 2001
From: Cal Herries <39073188+calherries@users.noreply.github.com>
Date: Fri, 31 Mar 2023 13:36:54 +0800
Subject: [PATCH] Toggle JSON unfolding by field (#28742)
* Use type/JSON instead of type/SerializedJSON
* Tidy migration
* Update migration
* Fix rollback migration for h2
* whitespace
* Add test for migration
* Fix test
* Add rollback
* whitespace
* Test JSONB type as well as JSON
* Don't fingerprint JSON columns
* Remove comment, that might be wrong in the future
* Update test
* Add tests for fingerprinting
* Use base-type JSON for fingerprinting base query
* Add test for visibility-type=details-only
* undo .
* Change migration id
* Fix migration test
* Merge master
* Exclude mariadb from tests
* Make is-mariadb? public
* Migration for adding nfc_enabled
* Add nfc_enabled to field settings
* Update describe-nested-field-columns to only unfold fields that have not been disabled
* Remove spy
* Tidy
* Fix
* Clear nested fields immediately if folding is disabled
* Clear nested fields on nfc_enabled change
* Trim trailing whitespace
* Tidy
* Fix
* Add enable-json-unfolding-test
* Move to field api test
* nfc_enabled -> json_unfolding
* Tidy test
* Make json-unfolding in database details just the default for new settings
* Restore original visibility-type logic
* Fix
* Sync field json_unfolding according to db json_folding
* Rename to json-unfolding-default
* Add test for the case when the json-unfolding is false for the database
* Implement default json unfolding for first sync
* Update comment
* Update comment
* Restore PUT field
* Migration for populating json_unfolding for mysql and postgres
* Remove migration and use default on the frontend instead
* Update json-unfolding database setting copy
* Move Unfold JSON setting under semantic type setting
* Separate sentences with spaces
* Capitalize
* Restore handleChangeSemanticType
* Use base_type TYPE.JSON instead
* Change order of migrations
* Add h2 migration
* Add test for migration
* Update test description
* Remove validCheckSum
Co-authored-by: Ngoc Khuat <qn.khuat@gmail.com>
* Use ?? instead of ||
* Fix tests
* Remove outdated serdes stuff
* Remove unnecessary and
* Fix mysql migration
* Remove unused require
* Fix test
* Add false default value for json_unfolding
* Fix json-unfolding nil case
* Other suggestions
* whitespace
* Tidy describe-nested-field-columns
* Update comment
* Remove unused clear-nested-fields!
* Remove unused return value
* Fix H2 migration to use base_type not database_type
* Always set json-unfolding during sync
* Fix test
* Fix test
* Fix test
* Add comment explaining nested-field-column support for MySQL
* Fix tests
* Fix tests
* Fix test
* Fix test
* Default json_unfolding to false for new fields
* Fix merge
* Add json_unfolding to mock tables
* Don't capitalize prepositions
* Update setting description
* whitespace
* whitespace
* Fix fetch_metadata
* Fix fetch_metadata
* Fix tests
* Fix test
* Fix clj-kondo
* Remove postgres database-supports test
* Fix postgres test
* Fix postgres test
* Fix mysql migration
* Fix clj-kondo
* Don't test mariadb
* Fix mysql test
* Fix mysql json-unfolding nil case
* Add comments to test
* Add upterm step to mariadb test
* Move upterm step before tests
* Fix postgresql migration
* move upterm step to test-driver action
* Remove upterm step from drivers.yml
* Comment out everything else in test-driver
* Fix mariadb migration
* Remove upterm action
* Whitespace
* Only handle JSONObjects with JSON_VALUE, not JSONArray
* Fix fields.sync_metadata/update-field-metadata-if-needed!
* Add comment explaining `json-unfolding-default`
* Add comment for JSON_VALUE test
* Update json-unfolding-default-true-test to use fresh db
* Fix test descriptions
* Don't capitalize setting display name
* Fix update json_unfolding
* Fix unfolding json for only one JSON column, not all
* Add e2e test for JSON unfolding setting
* Fix test
* whitespace
* Fix test
* Fix json-unfolding for nested field columns
* Fix mysql and postgres tests for nested field columns
* Coerce nil json-unfolding to false for sync_metadata
* Add test for nil json-unfolding for sync_metadata
* Fix test
* Don't update json_unfolding from intial sync
* Add json_unfolding to update-field-test
* Fix tests
---------
Co-authored-by: Ngoc Khuat <qn.khuat@gmail.com>
---
.../admin/datamodel/field.cy.spec.js | 53 +++-
.../admin/datamodel/containers/FieldApp.jsx | 31 ++-
.../test/metabase/driver/snowflake_test.clj | 6 +-
.../test/metabase/driver/sqlite_test.clj | 11 +-
resources/migrations/000_migrations.yaml | 56 ++++-
src/metabase/api/field.clj | 35 ++-
src/metabase/db/liquibase/h2.clj | 28 ++-
src/metabase/driver/common.clj | 6 +-
src/metabase/driver/mysql.clj | 9 +-
src/metabase/driver/postgres.clj | 10 +-
.../driver/sql_jdbc/sync/describe_table.clj | 70 +++---
src/metabase/models/database.clj | 16 +-
src/metabase/sync/interface.clj | 1 +
.../sync_metadata/fields/fetch_metadata.clj | 4 +-
.../sync_metadata/fields/sync_instances.clj | 59 ++---
test/metabase/api/field_test.clj | 83 +++++++
test/metabase/cmd/dump_to_h2_test.clj | 2 +-
.../cmd/rotate_encryption_key_test.clj | 10 +-
test/metabase/db/schema_migrations_test.clj | 48 ++++
test/metabase/driver/mysql_test.clj | 29 +--
test/metabase/driver/postgres_test.clj | 226 +++++++++---------
.../sql_jdbc/sync/describe_table_test.clj | 32 +--
test/metabase/driver/sql_jdbc_test.clj | 18 +-
test/metabase/models/database_test.clj | 9 +
test/metabase/sync/sync_dynamic_test.clj | 9 +-
.../fields/fetch_metadata_test.clj | 12 +-
.../fields/sync_metadata_test.clj | 184 ++++++++------
test/metabase/sync_test.clj | 6 +
test/metabase/test/mock/toucanery.clj | 104 ++++----
test/metabase/test/mock/util.clj | 3 +-
30 files changed, 792 insertions(+), 378 deletions(-)
diff --git a/e2e/test/scenarios/admin/datamodel/field.cy.spec.js b/e2e/test/scenarios/admin/datamodel/field.cy.spec.js
index 8076868c03f..ddfed4dc3dd 100644
--- a/e2e/test/scenarios/admin/datamodel/field.cy.spec.js
+++ b/e2e/test/scenarios/admin/datamodel/field.cy.spec.js
@@ -3,9 +3,11 @@ import {
withDatabase,
visitAlias,
popover,
+ resetTestTable,
startNewQuestion,
+ resyncDatabase,
} from "e2e/support/helpers";
-import { SAMPLE_DB_ID } from "e2e/support/cypress_data";
+import { SAMPLE_DB_ID, WRITABLE_DB_ID } from "e2e/support/cypress_data";
import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
const { ORDERS, ORDERS_ID } = SAMPLE_DATABASE;
@@ -133,3 +135,52 @@ describe.skip("scenarios > admin > datamodel > field", () => {
});
});
});
+
+function getUnfoldJsonContent() {
+ return cy
+ .findByText("Unfold JSON")
+ .closest("section")
+ .findByTestId("select-button-content");
+}
+
+describe("Unfold JSON", () => {
+ beforeEach(() => {
+ resetTestTable({ type: "postgres", table: "many_data_types" });
+ restore(`postgres-writable`);
+ cy.signInAsAdmin();
+ resyncDatabase({ dbId: WRITABLE_DB_ID, tableName: "many_data_types" });
+ });
+
+ it("lets you enable/disable 'Unfold JSON' for JSON columns", () => {
+ cy.intercept("POST", `/api/database/${WRITABLE_DB_ID}/sync_schema`).as(
+ "sync_schema",
+ );
+ // Go to field settings
+ cy.visit(`/admin/datamodel/database/${WRITABLE_DB_ID}`);
+ cy.findByText(/Many Data Types/i).click();
+
+ // Check json is unfolded initially
+ cy.findByText(/json.a/i).should("be.visible");
+ cy.findByTestId("column-json").within(() => { cy.icon("gear").click(); });
+
+ getUnfoldJsonContent().findByText(/Yes/i).click();
+ popover().within(() => {
+ cy.findByText(/No/i).click();
+ });
+
+ // Check setting has persisted
+ cy.reload();
+ getUnfoldJsonContent().findByText(/No/i);
+
+ // Sync database
+ cy.visit(`/admin/databases/${WRITABLE_DB_ID}`);
+ cy.findByText(/Sync database schema now/i).click();
+ cy.wait("@sync_schema");
+ cy.findByText("Sync triggered!");
+
+ // Check json field is not unfolded
+ cy.visit(`/admin/datamodel/database/${WRITABLE_DB_ID}`);
+ cy.findByText(/Many Data Types/i).click();
+ cy.findByText(/json.a/i).should("not.exist");
+ });
+});
diff --git a/frontend/src/metabase/admin/datamodel/containers/FieldApp.jsx b/frontend/src/metabase/admin/datamodel/containers/FieldApp.jsx
index 7932a66fe40..464d0a9ef12 100644
--- a/frontend/src/metabase/admin/datamodel/containers/FieldApp.jsx
+++ b/frontend/src/metabase/admin/datamodel/containers/FieldApp.jsx
@@ -38,7 +38,8 @@ import { getGlobalSettingsForColumn } from "metabase/visualizations/lib/settings
import Databases from "metabase/entities/databases";
import Tables from "metabase/entities/tables";
import Fields from "metabase/entities/fields";
-import { isTypeFK, isCurrency } from "metabase-lib/types/utils/isa";
+import { TYPE } from "metabase-lib/types/constants";
+import { isTypeFK, isCurrency, isa } from "metabase-lib/types/utils/isa";
import { rescanFieldValues, discardFieldValues } from "../field";
import UpdateCachedFieldValues from "../components/UpdateCachedFieldValues";
import FieldRemapping from "../components/FieldRemapping";
@@ -225,6 +226,7 @@ class FieldApp extends React.Component {
fieldsError={fieldsError}
idfields={idfields}
table={table}
+ database={db}
metadata={metadata}
onUpdateFieldValues={this.onUpdateFieldValues}
onUpdateFieldProperties={this.onUpdateFieldProperties}
@@ -255,6 +257,7 @@ const FieldGeneralPane = ({
fieldsError,
idfields,
table,
+ database,
metadata,
onUpdateFieldValues,
onUpdateFieldProperties,
@@ -297,6 +300,31 @@ const FieldGeneralPane = ({
/>
</Section>
+ {isa(field.base_type, TYPE.JSON) &&
+ database.hasFeature("nested-field-columns") && (
+ <Section>
+ <SectionHeader
+ title={t`Unfold JSON`}
+ description={t`Unfold JSON into component fields, where each JSON key becomes a column. You can turn this off if performance is slow.`}
+ />
+ <Select
+ className="inline-block"
+ value={
+ field.json_unfolding ?? database.details["json-unfolding"] ?? true
+ }
+ onChange={({ target: { value } }) => {
+ return onUpdateFieldProperties({
+ json_unfolding: value,
+ });
+ }}
+ options={[
+ { name: t`Yes`, value: true },
+ { name: t`No`, value: false },
+ ]}
+ />
+ </Section>
+ )}
+
{!isTypeFK(field.semantic_type) && is_coerceable(field.base_type) && (
<Section>
<SectionHeader title={t`Cast to a specific data type`} />
@@ -326,6 +354,7 @@ const FieldGeneralPane = ({
/>
</Section>
)}
+
<Section>
<SectionHeader
title={t`Filtering on this field`}
diff --git a/modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj b/modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj
index a8debfbfba5..5ecc49c9151 100644
--- a/modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj
+++ b/modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj
@@ -130,13 +130,15 @@
:pk? true
:database-position 0
:database-is-auto-increment true
- :database-required false}
+ :database-required false
+ :json-unfolding false}
{:name "name"
:database-type "VARCHAR"
:base-type :type/Text
:database-position 1
:database-is-auto-increment false
- :database-required true}}}
+ :database-required true
+ :json-unfolding false}}}
(driver/describe-table :snowflake (assoc (mt/db) :name "ABC") (t2/select-one Table :id (mt/id :categories))))))))
(deftest describe-table-fks-test
diff --git a/modules/drivers/sqlite/test/metabase/driver/sqlite_test.clj b/modules/drivers/sqlite/test/metabase/driver/sqlite_test.clj
index ac42dcfcb95..b483dc91ee4 100644
--- a/modules/drivers/sqlite/test/metabase/driver/sqlite_test.clj
+++ b/modules/drivers/sqlite/test/metabase/driver/sqlite_test.clj
@@ -143,11 +143,12 @@
(testing "timestamp column should exist"
(is (= {:name "timestamp_table"
:schema nil
- :fields #{{:name "created_at"
- :database-type "TIMESTAMP"
- :base-type :type/DateTime
- :database-position 0
- :database-required false
+ :fields #{{:name "created_at"
+ :database-type "TIMESTAMP"
+ :base-type :type/DateTime
+ :database-position 0
+ :database-required false
+ :json-unfolding false
:database-is-auto-increment false}}}
(driver/describe-table driver db (t2/select-one Table :id (mt/id :timestamp_table)))))))))))))
diff --git a/resources/migrations/000_migrations.yaml b/resources/migrations/000_migrations.yaml
index accf625b03f..356cf5779f6 100644
--- a/resources/migrations/000_migrations.yaml
+++ b/resources/migrations/000_migrations.yaml
@@ -14312,7 +14312,6 @@ databaseChangeLog:
id: v47.00-001
author: calherries
comment: Added 0.47.0 -- set base-type to type/JSON for JSON database-types for postgres and mysql
- validCheckSum: ANY
changes:
- sql:
sql: >-
@@ -14362,6 +14361,61 @@ databaseChangeLog:
WHEN MATCHED THEN
UPDATE SET f.base_type = updates.base_type;
+ - changeSet:
+ id: v47.00-002
+ author: calherries
+ comment: Added 0.47.0 - Add json_unfolding column to metabase_field
+ changes:
+ - addColumn:
+ columns:
+ - column:
+ remarks: 'Enable/disable JSON unfolding for a field'
+ name: json_unfolding
+ type: boolean
+ defaultValueBoolean: false
+ constraints:
+ nullable: false
+ tableName: metabase_field
+
+ - changeSet:
+ id: v47.00-003
+ author: calherries
+ comment: Added 0.47.0 - Populate metabase_field.json_unfolding based on database details
+ changes:
+ - sql:
+ dbms: postgresql
+ sql: >-
+ UPDATE metabase_field f
+ SET json_unfolding = coalesce((d.details::jsonb->'json-unfolding') in ('true', 'null'), true)
+ FROM metabase_database d
+ JOIN metabase_table t ON t.db_id = d.id
+ WHERE f.table_id = t.id AND f.base_type = 'type/JSON'
+ - sql:
+ dbms: mysql,mariadb
+ sql: >-
+ UPDATE metabase_field f
+ JOIN metabase_table t ON f.table_id = t.id
+ JOIN metabase_database d ON t.db_id = d.id
+ SET json_unfolding = coalesce(JSON_UNQUOTE(JSON_EXTRACT(d.details, '$."json-unfolding"')) in ('true', 'null'), true)
+ WHERE f.base_type = 'type/JSON'
+ - sql:
+ dbms: h2
+ sql:
+ MERGE INTO metabase_field f
+ USING (
+ SELECT
+ coalesce(JSON_VALUE(d.details, 'json-unfolding'), true) as json_unfolding,
+ f.id
+ FROM metabase_field f
+ JOIN metabase_table t ON f.table_id = t.id
+ JOIN metabase_database d ON t.db_id = d.id
+ WHERE f.base_type = 'type/JSON'
+ ) as updates
+ ON f.id = updates.id
+ WHEN MATCHED THEN
+ UPDATE SET f.json_unfolding = updates.json_unfolding
+ rollback: # nothing to do, since json_unfolding is new in 47
+
- changeSet:
id: v47.00-004
author: qnkhuat
diff --git a/src/metabase/api/field.clj b/src/metabase/api/field.clj
index 916e6bbd636..7266bc32608 100644
--- a/src/metabase/api/field.clj
+++ b/src/metabase/api/field.clj
@@ -73,8 +73,7 @@
(defn- clear-dimension-on-fk-change! [{:keys [dimensions], :as _field}]
(doseq [{dimension-id :id, dimension-type :type} dimensions]
(when (and dimension-id (= :external dimension-type))
- (t2/delete! Dimension :id dimension-id)))
- true)
+ (t2/delete! Dimension :id dimension-id))))
(defn- removed-fk-semantic-type? [old-semantic-type new-semantic-type]
(and (not= old-semantic-type new-semantic-type)
@@ -97,14 +96,13 @@
(when (and old-dim-id
(= :internal old-dim-type)
(not (internal-remapping-allowed? base-type new-semantic-type)))
- (t2/delete! Dimension :id old-dim-id)))
- true)
+ (t2/delete! Dimension :id old-dim-id))))
#_{:clj-kondo/ignore [:deprecated-var]}
(api/defendpoint-schema PUT "/:id"
"Update `Field` with ID."
[id :as {{:keys [caveats description display_name fk_target_field_id points_of_interest semantic_type
- coercion_strategy visibility_type has_field_values settings nfc_path]
+ coercion_strategy visibility_type has_field_values settings nfc_path json_unfolding]
:as body} :body}]
{caveats (s/maybe su/NonBlankString)
description (s/maybe su/NonBlankString)
@@ -116,7 +114,8 @@
visibility_type (s/maybe FieldVisibilityType)
has_field_values (s/maybe (apply s/enum (map name field/has-field-values-options)))
settings (s/maybe su/Map)
- nfc_path (s/maybe [su/NonBlankString])}
+ nfc_path (s/maybe [su/NonBlankString])
+ json_unfolding (s/maybe s/Bool)}
(let [field (hydrate (api/write-check Field id) :dimensions)
new-semantic-type (keyword (get body :semantic_type (:semantic_type field)))
[effective-type coercion-strategy]
@@ -137,19 +136,17 @@
;; everything checks out, now update the field
(api/check-500
(t2/with-transaction [_conn]
- (and
- (if removed-fk?
- (clear-dimension-on-fk-change! field)
- true)
- (clear-dimension-on-type-change! field (:base_type field) new-semantic-type)
- (t2/update! Field id
- (u/select-keys-when (assoc body
- :fk_target_field_id (when-not removed-fk? fk-target-field-id)
- :effective_type effective-type
- :coercion_strategy coercion-strategy)
- :present #{:caveats :description :fk_target_field_id :points_of_interest :semantic_type :visibility_type
- :coercion_strategy :effective_type :has_field_values :nfc_path}
- :non-nil #{:display_name :settings})))))
+ (when removed-fk?
+ (clear-dimension-on-fk-change! field))
+ (clear-dimension-on-type-change! field (:base_type field) new-semantic-type)
+ (t2/update! Field id
+ (u/select-keys-when (assoc body
+ :fk_target_field_id (when-not removed-fk? fk-target-field-id)
+ :effective_type effective-type
+ :coercion_strategy coercion-strategy)
+ :present #{:caveats :description :fk_target_field_id :points_of_interest :semantic_type :visibility_type
+ :coercion_strategy :effective_type :has_field_values :nfc_path :json_unfolding}
+ :non-nil #{:display_name :settings}))))
;; return updated field. note the fingerprint on this might be out of date if the task below would replace them
;; but that shouldn't matter for the datamodel page
(u/prog1 (hydrate (t2/select-one Field :id id) :dimensions)
diff --git a/src/metabase/db/liquibase/h2.clj b/src/metabase/db/liquibase/h2.clj
index 65adac97e78..2f2dbfeb71a 100644
--- a/src/metabase/db/liquibase/h2.clj
+++ b/src/metabase/db/liquibase/h2.clj
@@ -11,6 +11,30 @@
(defn- upcase ^String [s]
(some-> s u/upper-case-en))
+(defn- create-udfs!
+ "Registers the JSON manipulation functions for an H2 database."
+ [^JdbcConnection conn]
+ ;; JSON_VALUE is tested in metabase.db.schema-migrations-test/migrate-field-json-unfolding-type-test
+ (.execute (.createStatement conn) "
+CREATE ALIAS IF NOT EXISTS JSON_VALUE AS $$
+import org.json.simple.JSONObject;
+import org.json.simple.JSONValue;
+@CODE
+String jsonStringValue(String s, String key) {
+ Object obj = JSONValue.parse(s);
+ JSONObject jsonObject = (JSONObject) obj;
+ if (jsonObject.containsKey(key)) {
+ if (jsonObject.get(key) != null) {
+ return jsonObject.get(key).toString();
+ } else {
+ return null;
+ }
+ } else {
+ return null;
+ }
+}
+$$;"))
+
(defn- h2-database* ^H2Database []
(proxy [H2Database] []
(quoteObject [object-name object-type]
@@ -49,7 +73,9 @@
(assert (.getPackage klass) (format "Failed to create package for proxy class %s." class-name)))))
(defn h2-database
- "A version of the Liquibase H2 implementation that always converts identifiers to uppercase and then quotes them."
+ "A version of the Liquibase H2 implementation that always converts identifiers to uppercase and then quotes them.
+ Also creates UDFs for JSON handling."
^H2Database [^JdbcConnection conn]
+ (create-udfs! conn)
(doto (h2-database*)
(.setConnection conn)))
diff --git a/src/metabase/driver/common.clj b/src/metabase/driver/common.clj
index 67f39d29669..974b675a792 100644
--- a/src/metabase/driver/common.clj
+++ b/src/metabase/driver/common.clj
@@ -174,12 +174,12 @@
(def json-unfolding
"Map representing the `json-unfolding` option in a DB connection form"
{:name "json-unfolding"
- :display-name (deferred-tru "Unfold JSON Columns")
+ :display-name (deferred-tru "Unfold JSON columns by default")
:type :boolean
:visible-if {"advanced-options" true}
:description (deferred-tru
- (str "We unfold JSON columns into component fields."
- "This is on by default but you can turn it off if performance is slow."))
+ (str "This enables unfolding JSON columns into component fields by default. "
+ "Disable unfolding if performance is slow. You can override this default in each field's Data Model settings."))
:default true})
(def refingerprint
diff --git a/src/metabase/driver/mysql.clj b/src/metabase/driver/mysql.clj
index a439f636ab7..ff4cfa4ea66 100644
--- a/src/metabase/driver/mysql.clj
+++ b/src/metabase/driver/mysql.clj
@@ -48,11 +48,10 @@
(defmethod driver/display-name :mysql [_] "MySQL")
-(defmethod driver/database-supports? [:mysql :nested-field-columns] [_ _ database]
- (let [json-setting (get-in database [:details :json-unfolding])]
- (if (nil? json-setting)
- true
- json-setting)))
+;; This is a bit of a lie since the JSON type was introduced for MySQL since 5.7.8.
+;; And MariaDB doesn't have the JSON type at all, though `JSON` was introduced as an alias for LONGTEXT in 10.2.7.
+;; But since JSON unfolding will only apply columns with JSON types, this won't cause any problems during sync.
+(defmethod driver/database-supports? [:mysql :nested-field-columns] [_driver _feat _db] true)
(defmethod driver/database-supports? [:mysql :persist-models] [_driver _feat _db] true)
diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj
index 7c017f286b4..9bc2cee683f 100644
--- a/src/metabase/driver/postgres.clj
+++ b/src/metabase/driver/postgres.clj
@@ -46,18 +46,16 @@
(driver/register! :postgres, :parent :sql-jdbc)
-(defmethod driver/database-supports? [:postgres :nested-field-columns] [_ _ database]
- (let [json-setting (get-in database [:details :json-unfolding])
- ;; If not set at all, default to true, actually
- setting-nil? (nil? json-setting)]
- (or json-setting setting-nil?)))
-
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | metabase.driver impls |
;;; +----------------------------------------------------------------------------------------------------------------+
(defmethod driver/display-name :postgres [_] "PostgreSQL")
+(defmethod driver/database-supports? [:postgres :nested-field-columns]
+ [_driver _feat _db]
+ true)
+
(defmethod driver/database-supports? [:postgres :datetime-diff]
[_driver _feat _db]
true)
diff --git a/src/metabase/driver/sql_jdbc/sync/describe_table.clj b/src/metabase/driver/sql_jdbc/sync/describe_table.clj
index 5d50e71e66c..ee8e8bf1381 100644
--- a/src/metabase/driver/sql_jdbc/sync/describe_table.clj
+++ b/src/metabase/driver/sql_jdbc/sync/describe_table.clj
@@ -13,10 +13,12 @@
[metabase.driver.sql-jdbc.sync.interface :as sql-jdbc.sync.interface]
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.mbql.schema :as mbql.s]
+ [metabase.models.database :as database]
[metabase.models.table :as table]
[metabase.util :as u]
[metabase.util.honeysql-extensions :as hx]
- [metabase.util.log :as log])
+ [metabase.util.log :as log]
+ [toucan2.core :as t2])
(:import
(java.sql Connection DatabaseMetaData ResultSet)))
@@ -154,20 +156,20 @@
"Returns a transducer for computing metatdata about the fields in `table`."
[driver table]
(map-indexed (fn [i {:keys [database-type], column-name :name, :as col}]
- (let [base-type (database-type->base-type-or-warn driver database-type)
- semantic-type (calculated-semantic-type driver column-name database-type)]
+ (let [base-type (database-type->base-type-or-warn driver database-type)
+ semantic-type (calculated-semantic-type driver column-name database-type)
+ db (table/database table)
+ json-unfolding (boolean (and (isa? base-type :type/JSON)
+ (driver/database-supports? driver :nested-field-columns db)
+ (database/json-unfolding-default db)))]
(merge
(u/select-non-nil-keys col [:name :database-type :field-comment :database-required :database-is-auto-increment])
{:base-type base-type
- :database-position i}
+ :database-position i
+ :json-unfolding json-unfolding}
(when semantic-type
{:semantic-type semantic-type})
- (when (and
- (isa? base-type :type/JSON)
- (driver/database-supports?
- driver
- :nested-field-columns
- (table/database table)))
+ (when json-unfolding
{:visibility-type :details-only}))))))
(defmulti describe-table-fields
@@ -385,6 +387,7 @@
:base-type curr-type
;; Postgres JSONB field, which gets most usage, doesn't maintain JSON object ordering...
:database-position 0
+ :json-unfolding false
:visibility-type :normal
:nfc-path field-path})))
field-hash (apply hash-set (filter some? valid-fields))]
@@ -402,20 +405,33 @@
json-fields (filter #(isa? (:base-type %) :type/JSON) table-fields)]
(if (nil? (seq json-fields))
#{}
- (binding [hx/*honey-sql-version* (sql.qp/honey-sql-version driver)]
- (let [json-field-names (mapv #(apply hx/identifier :field (into table-identifier-info [(:name %)])) json-fields)
- table-identifier (apply hx/identifier :table table-identifier-info)
- sql-args (sql.qp/format-honeysql driver {:select (mapv sql.qp/maybe-wrap-unaliased-expr json-field-names)
- :from [(sql.qp/maybe-wrap-unaliased-expr table-identifier)]
- :limit metadata-queries/nested-field-sample-limit})
- query (jdbc/reducible-query spec sql-args {:identifiers identity})
- field-types (transduce describe-json-xform describe-json-rf query)
- fields (field-types->fields field-types)]
- (if (> (count fields) max-nested-field-columns)
- (do
- (log/warn
- (format
- "More nested field columns detected than maximum. Limiting the number of nested field columns to %d."
- max-nested-field-columns))
- (set (take max-nested-field-columns fields)))
- fields)))))))
+ (let [existing-fields-by-name (m/index-by :name (t2/select 'Field :table_id (u/the-id table)))
+ unfold-json-fields (filter (fn [field]
+ ;; unfold json if the field has json_unfolding = true
+ ;; don't unfold json if the field has json_unfolding = false
+ ;; otherwise if the field doesn't exist
+ ;; use the database default
+ (let [existing-field (existing-fields-by-name (:name field))]
+ (or (:json_unfolding existing-field)
+ (and (nil? existing-field)
+ (database/json-unfolding-default (table/database table))))))
+ json-fields)]
+ (if (empty? unfold-json-fields)
+ #{}
+ (binding [hx/*honey-sql-version* (sql.qp/honey-sql-version driver)]
+ (let [json-field-names (mapv #(apply hx/identifier :field (into table-identifier-info [(:name %)])) unfold-json-fields)
+ table-identifier (apply hx/identifier :table table-identifier-info)
+ sql-args (sql.qp/format-honeysql driver {:select (mapv sql.qp/maybe-wrap-unaliased-expr json-field-names)
+ :from [(sql.qp/maybe-wrap-unaliased-expr table-identifier)]
+ :limit metadata-queries/nested-field-sample-limit})
+ query (jdbc/reducible-query spec sql-args {:identifiers identity})
+ field-types (transduce describe-json-xform describe-json-rf query)
+ fields (field-types->fields field-types)]
+ (if (> (count fields) max-nested-field-columns)
+ (do
+ (log/warn
+ (format
+ "More nested field columns detected than maximum. Limiting the number of nested field columns to %d."
+ max-nested-field-columns))
+ (set (take max-nested-field-columns fields)))
+ fields)))))))))
diff --git a/src/metabase/models/database.clj b/src/metabase/models/database.clj
index dc888e4124e..966d7fcac68 100644
--- a/src/metabase/models/database.clj
+++ b/src/metabase/models/database.clj
@@ -176,7 +176,13 @@
{:status-code 400
:existing-engine existing-engine
:new-engine new-engine}))
- (u/prog1 (handle-secrets-changes database)
+ (u/prog1 (-> database
+ (cond->
+ ;; If the engine doesn't support nested field columns, `json_unfolding` must be nil
+ (and (some? (:details database))
+ (not (driver/database-supports? (or new-engine existing-engine) :nested-field-columns database)))
+ (update :details dissoc :json_unfolding))
+ handle-secrets-changes)
;; TODO - this logic would make more sense in post-update if such a method existed
;; if the sync operation schedules have changed, we need to reschedule this DB
(when (or new-metadata-schedule new-fieldvalues-schedule)
@@ -343,3 +349,11 @@
(defmethod serdes/storage-path "Database" [{:keys [name]} _]
;; ["databases" "db_name" "db_name"] directory for the database with same-named file inside.
["databases" name name])
+
+(defn json-unfolding-default
+ "Returns true if JSON fields should be unfolded by default for this database, and false otherwise."
+ [database]
+ ;; If json-unfolding=nil in the database details, return true. This allows adding support for
+ ;; nested-field-columns for drivers in the future and have json-unfolding enabled by default, without
+ ;; needing a migration to add the `json-unfolding=true` key to the database details.
+ ((fnil identity true) (get-in database [:details :json-unfolding])))
diff --git a/src/metabase/sync/interface.clj b/src/metabase/sync/interface.clj
index 29e2751ce6e..9a95c52fa4c 100644
--- a/src/metabase/sync/interface.clj
+++ b/src/metabase/sync/interface.clj
@@ -33,6 +33,7 @@
(s/optional-key :field-comment) (s/maybe su/NonBlankString)
(s/optional-key :pk?) s/Bool
(s/optional-key :nested-fields) #{(s/recursive #'TableMetadataField)}
+ (s/optional-key :json-unfolding) s/Bool
(s/optional-key :nfc-path) [s/Any]
(s/optional-key :custom) {s/Any s/Any}
(s/optional-key :database-is-auto-increment) s/Bool
diff --git a/src/metabase/sync/sync_metadata/fields/fetch_metadata.clj b/src/metabase/sync/sync_metadata/fields/fetch_metadata.clj
index f1a769b6134..4672ecf446f 100644
--- a/src/metabase/sync/sync_metadata/fields/fetch_metadata.clj
+++ b/src/metabase/sync/sync_metadata/fields/fetch_metadata.clj
@@ -33,6 +33,7 @@
:semantic-type (:semantic_type field)
:pk? (isa? (:semantic_type field) :type/PK)
:field-comment (:description field)
+ :json-unfolding (:json_unfolding field)
:database-is-auto-increment (:database_is_auto_increment field)
:database-position (:database_position field)
:database-required (:database_required field)})
@@ -69,7 +70,8 @@
"Fetch active Fields from the Metabase application database for a given `table`."
[table :- i/TableInstance]
(t2/select [Field :name :database_type :base_type :effective_type :coercion_strategy :semantic_type
- :parent_id :id :description :database_position :nfc_path :database_is_auto_increment :database_required]
+ :parent_id :id :description :database_position :nfc_path :database_is_auto_increment :database_required
+ :json_unfolding]
:table_id (u/the-id table)
:active true
{:order-by table/field-order-rule}))
diff --git a/src/metabase/sync/sync_metadata/fields/sync_instances.clj b/src/metabase/sync/sync_metadata/fields/sync_instances.clj
index 85a31f7353e..2869360c9c6 100644
--- a/src/metabase/sync/sync_metadata/fields/sync_instances.clj
+++ b/src/metabase/sync/sync_metadata/fields/sync_instances.clj
@@ -42,36 +42,37 @@
(when (seq new-field-metadatas)
(t2/insert-returning-pks! Field
(for [{:keys [database-type database-is-auto-increment database-required base-type effective-type coercion-strategy
- field-comment database-position nfc-path visibility-type], field-name :name :as field} new-field-metadatas]
+ field-comment database-position nfc-path visibility-type json-unfolding], field-name :name :as field} new-field-metadatas]
(do
- (when (and effective-type
- base-type
- (not= effective-type base-type)
- (nil? coercion-strategy))
- (log/warn (u/format-color 'red
- (str
- "WARNING: Field `%s`: effective type `%s` provided but no coercion strategy provided."
- " Using base-type: `%s`")
- field-name
- effective-type
- base-type)))
- {:table_id (u/the-id table)
- :name field-name
- :display_name (humanization/name->human-readable-name field-name)
- :database_type (or database-type "NULL") ; placeholder for Fields w/ no type info (e.g. Mongo) & all NULL
- :base_type base-type
- ;; todo test this?
- :effective_type (if (and effective-type coercion-strategy) effective-type base-type)
- :coercion_strategy (when effective-type coercion-strategy)
- :semantic_type (common/semantic-type field)
- :parent_id parent-id
- :nfc_path nfc-path
- :description field-comment
- :position database-position
- :database_position database-position
- :database_is_auto_increment (or database-is-auto-increment false)
- :database_required (or database-required false)
- :visibility_type (or visibility-type :normal)})))))
+ (when (and effective-type
+ base-type
+ (not= effective-type base-type)
+ (nil? coercion-strategy))
+ (log/warn (u/format-color 'red
+ (str
+ "WARNING: Field `%s`: effective type `%s` provided but no coercion strategy provided."
+ " Using base-type: `%s`")
+ field-name
+ effective-type
+ base-type)))
+ {:table_id (u/the-id table)
+ :name field-name
+ :display_name (humanization/name->human-readable-name field-name)
+ :database_type (or database-type "NULL") ; placeholder for Fields w/ no type info (e.g. Mongo) & all NULL
+ :base_type base-type
+ ;; todo test this?
+ :effective_type (if (and effective-type coercion-strategy) effective-type base-type)
+ :coercion_strategy (when effective-type coercion-strategy)
+ :semantic_type (common/semantic-type field)
+ :parent_id parent-id
+ :nfc_path nfc-path
+ :description field-comment
+ :position database-position
+ :database_position database-position
+ :json_unfolding (or json-unfolding false)
+ :database_is_auto_increment (or database-is-auto-increment false)
+ :database_required (or database-required false)
+ :visibility_type (or visibility-type :normal)})))))
(s/defn ^:private create-or-reactivate-fields! :- (s/maybe [i/FieldInstance])
"Create (or reactivate) Metabase Field object(s) for any Fields in `new-field-metadatas`. Does *NOT* recursively
diff --git a/test/metabase/api/field_test.clj b/test/metabase/api/field_test.clj
index e9526f54c1d..3ecd251acea 100644
--- a/test/metabase/api/field_test.clj
+++ b/test/metabase/api/field_test.clj
@@ -4,6 +4,8 @@
[clojure.test :refer :all]
[medley.core :as m]
[metabase.api.field :as api.field]
+ [metabase.driver :as driver]
+ [metabase.driver.mysql-test :as mysql-test]
[metabase.driver.util :as driver.u]
[metabase.models :refer [Database Field FieldValues Table]]
[metabase.sync :as sync]
@@ -84,6 +86,7 @@
:description
:visibility_type
:semantic_type
+ :json_unfolding
:fk_target_field_id
:nfc_path]))
@@ -103,6 +106,7 @@
:description nil
:semantic_type nil
:visibility_type :normal
+ :json_unfolding false
:fk_target_field_id nil
:nfc_path nil}
original-val)))
@@ -111,6 +115,7 @@
:display_name "yay"
:description "foobar"
:semantic_type :type/Name
+ :json_unfolding true
:visibility_type :sensitive
:nfc_path ["bob" "dobbs"]})
(let [updated-val (simple-field-details (t2/select-one Field :id field-id))]
@@ -120,6 +125,7 @@
:description "foobar"
:semantic_type :type/Name
:visibility_type :sensitive
+ :json_unfolding true
:fk_target_field_id nil
:nfc_path ["bob" "dobbs"]}
updated-val)))
@@ -133,6 +139,7 @@
:description nil
:semantic_type nil
:visibility_type :sensitive
+ :json_unfolding true
:fk_target_field_id nil
:nfc_path nil}
(simple-field-details (t2/select-one Field :id field-id)))))))))
@@ -508,6 +515,7 @@
:visibility_type :normal
:semantic_type :type/FK
:fk_target_field_id true
+ :json_unfolding false
:nfc_path nil}
(mt/boolean-ids-and-timestamps (simple-field-details (t2/select-one Field :id field-id-2))))))
(mt/user-http-request :crowberto :put 200 (format "field/%d" field-id-2) {:semantic_type nil})
@@ -518,6 +526,7 @@
:visibility_type :normal
:semantic_type nil
:fk_target_field_id false
+ :json_unfolding false
:nfc_path nil}
(mt/boolean-ids-and-timestamps (simple-field-details (t2/select-one Field :id field-id-2)))))))))
@@ -536,6 +545,7 @@
:visibility_type :normal
:semantic_type :type/FK
:fk_target_field_id true
+ :json_unfolding false
:nfc_path nil}
(mt/boolean-ids-and-timestamps before-change))))
(mt/user-http-request :crowberto :put 200 (format "field/%d" field-id-3) {:fk_target_field_id field-id-2})
@@ -547,6 +557,7 @@
:visibility_type :normal
:semantic_type :type/FK
:fk_target_field_id true
+ :json_unfolding false
:nfc_path nil}
(mt/boolean-ids-and-timestamps after-change)))
(is (not= (:fk_target_field_id before-change)
@@ -564,6 +575,7 @@
:visibility_type :normal
:semantic_type nil
:fk_target_field_id false
+ :json_unfolding false
:nfc_path nil}
(mt/boolean-ids-and-timestamps (simple-field-details (t2/select-one Field :id field-id-2))))))
(mt/user-http-request :crowberto :put 200 (format "field/%d" field-id-2) {:semantic_type :type/FK
@@ -575,6 +587,7 @@
:visibility_type :normal
:semantic_type :type/FK
:fk_target_field_id true
+ :json_unfolding false
:nfc_path nil}
(mt/boolean-ids-and-timestamps (simple-field-details (t2/select-one Field :id field-id-2)))))))))
@@ -592,6 +605,7 @@
:visibility_type :normal
:semantic_type :type/FK
:fk_target_field_id true
+ :json_unfolding false
:nfc_path nil}
(mt/boolean-ids-and-timestamps (simple-field-details (t2/select-one Field :id field-id-2))))))
(mt/user-http-request :crowberto :put 200 (format "field/%d" field-id-2) {:description "foo"})
@@ -602,6 +616,7 @@
:visibility_type :normal
:semantic_type :type/FK
:fk_target_field_id true
+ :json_unfolding false
:nfc_path nil}
(mt/boolean-ids-and-timestamps (simple-field-details (t2/select-one Field :id field-id-2))))))))))
@@ -726,3 +741,71 @@
[2 "Small Marble Shoes"]
[3 "Synergistic Granite Chair"]]}
(mt/user-http-request :crowberto :get 200 (format "field/%d/values" (mt/id :orders :product_id)))))))))))
+
+(deftest json-unfolding-default-true-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :nested-field-columns)
+ (when-not (mysql-test/is-mariadb? (u/id (mt/db)))
+ (mt/dataset json
+ ;; Create a new database with the same details as the json dataset, with json unfolding enabled by default
+ (let [database (t2/select-one Database :id (mt/id))]
+ (mt/with-temp* [Database [database {:engine driver/*driver*, :details (assoc (:details database) :json-unfolding true)}]]
+ (mt/with-db database
+ ;; Sync the new database
+ (sync/sync-database! database)
+ (let [field (t2/select-one Field :id (mt/id :json :json_bit))
+ enable-json-unfolding! (fn [v]
+ (mt/user-http-request :crowberto :put 200 (format "field/%d" (mt/id :json :json_bit))
+ (assoc field :json_unfolding v)))
+ nested-fields (fn []
+ (->> (t2/select Field :table_id (mt/id :json) :active true :nfc_path [:not= nil])
+ (filter (fn [field] (= (first (:nfc_path field)) "json_bit")))))]
+ (testing "json-unfolding is enabled by default"
+ (is (true? (:json_unfolding field))))
+ (testing "nested fields are present since json unfolding is enabled by default"
+ (is (seq (nested-fields))))
+ (testing "nested fields are removed when json unfolding is disabled"
+ (enable-json-unfolding! false)
+ (sync/sync-database! database)
+ (is (empty? (nested-fields))))
+ (testing "json_unfolding is not overwritten by another DB sync"
+ (sync/sync-database! database)
+ (is (false? (:json_unfolding (t2/select-one Field (mt/id :json :json_bit)))))
+ (is (empty? (nested-fields))))
+ (testing "nested fields are added when json unfolding is enabled again"
+ (enable-json-unfolding! true)
+ (sync/sync-database! database)
+ (is (seq (nested-fields))))))))))))
+
+(deftest json-unfolding-default-false-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :nested-field-columns)
+ (when-not (mysql-test/is-mariadb? (u/id (mt/db)))
+ (mt/dataset json
+ (let [database (t2/select-one Database :id (mt/id))]
+ ;; Create a new database with the same details as the json dataset, with json unfolding disabled by default
+ (mt/with-temp* [Database [database {:engine driver/*driver*, :details (assoc (:details database) :json-unfolding false)}]]
+ (mt/with-db database
+ ;; Sync the new database
+ (sync/sync-database! database)
+ (let [field (t2/select-one Field :id (mt/id :json :json_bit))
+ enable-json-unfolding! (fn [v]
+ (mt/user-http-request :crowberto :put 200 (format "field/%d" (mt/id :json :json_bit))
+ (assoc field :json_unfolding v)))
+ nested-fields (fn []
+ (->> (t2/select Field :table_id (mt/id :json) :active true :nfc_path [:not= nil])
+ (filter (fn [field] (= (first (:nfc_path field)) "json_bit")))))]
+ (testing "json-unfolding is disabled by default"
+ (is (false? (:json_unfolding field))))
+ (testing "nested fields are not present since json unfolding is disabled by default"
+ (is (empty? (nested-fields))))
+ (testing "nested fields are added when json unfolding is enabled"
+ (enable-json-unfolding! true)
+ (sync/sync-database! database)
+ (is (seq (nested-fields))))
+ (testing "json_unfolding is not overwritten by another DB sync"
+ (sync/sync-database! database)
+ (is (true? (:json_unfolding (t2/select-one Field (mt/id :json :json_bit)))))
+ (is (seq (nested-fields))))
+ (testing "nested fields are removed when json unfolding is disabled again"
+ (enable-json-unfolding! false)
+ (sync/sync-database! database)
+ (is (empty? (nested-fields))))))))))))
diff --git a/test/metabase/cmd/dump_to_h2_test.clj b/test/metabase/cmd/dump_to_h2_test.clj
index 128e8d66d62..c0ef21b3de5 100644
--- a/test/metabase/cmd/dump_to_h2_test.clj
+++ b/test/metabase/cmd/dump_to_h2_test.clj
@@ -81,7 +81,7 @@
(load-from-h2/load-from-h2! h2-fixture-db-file)
(encryption-test/with-secret-key "89ulvIGoiYw6mNELuOoEZphQafnF/zYe+3vT+v70D1A="
(t2/insert! Setting {:key "my-site-admin", :value "baz"})
- (t2/update! Database 1 {:details "{\"db\":\"/tmp/test.db\"}"})
+ (t2/update! Database 1 {:details {:db "/tmp/test.db"}})
(dump-to-h2/dump-to-h2! h2-file-plaintext {:dump-plaintext? true})
(dump-to-h2/dump-to-h2! h2-file-enc {:dump-plaintext? false})
(dump-to-h2/dump-to-h2! h2-file-default-enc))
diff --git a/test/metabase/cmd/rotate_encryption_key_test.clj b/test/metabase/cmd/rotate_encryption_key_test.clj
index 7b52b8ff170..6ca754f531c 100644
--- a/test/metabase/cmd/rotate_encryption_key_test.clj
+++ b/test/metabase/cmd/rotate_encryption_key_test.clj
@@ -112,7 +112,7 @@
(reset! secret-id-unenc (u/the-id secret)))
(encryption-test/with-secret-key k1
(t2/insert! Setting {:key "k1crypted", :value "encrypted with k1"})
- (t2/update! Database 1 {:details "{\"db\":\"/tmp/test.db\"}"})
+ (t2/update! Database 1 {:details {:db "/tmp/test.db"}})
(let [secret (first (t2/insert-returning-instances! Secret {:name "My Secret (encrypted)"
:kind "password"
:value (.getBytes secret-val StandardCharsets/UTF_8)
@@ -153,11 +153,11 @@
(testing "full rollback when a database details looks encrypted with a different key than the current one"
(encryption-test/with-secret-key k3
- (let [db (first (t2/insert-returning-instances! Database {:name "k3", :engine :mysql, :details "{\"db\":\"/tmp/k3.db\"}"}))]
+ (let [db (first (t2/insert-returning-instances! Database {:name "k3", :engine :mysql, :details {:db "/tmp/k3.db"}}))]
(is (=? {:name "k3"}
db))))
(encryption-test/with-secret-key k2
- (let [db (first (t2/insert-returning-instances! Database {:name "k2", :engine :mysql, :details "{\"db\":\"/tmp/k2.db\"}"}))]
+ (let [db (first (t2/insert-returning-instances! Database {:name "k2", :engine :mysql, :details {:db "/tmp/k3.db"}}))]
(is (=? {:name "k2"}
db)))
(is (thrown-with-msg?
@@ -169,8 +169,8 @@
(is (= {:db "/tmp/k3.db"} (t2/select-one-fn :details Database :name "k3")))))
(testing "rotate-encryption-key! to nil decrypts the encrypted keys"
- (t2/update! Database 1 {:details "{\"db\":\"/tmp/test.db\"}"})
- (t2/update! Database {:name "k3"} {:details "{\"db\":\"/tmp/test.db\"}"})
+ (t2/update! Database 1 {:details {:db "/tmp/test.db"}})
+ (t2/update! Database {:name "k3"} {:details {:db "/tmp/test.db"}})
(encryption-test/with-secret-key k2 ; with the last key that we rotated to in the test
(rotate-encryption-key! nil))
(is (= "unencrypted value" (raw-value "nocrypt")))
diff --git a/test/metabase/db/schema_migrations_test.clj b/test/metabase/db/schema_migrations_test.clj
index 7b42749c079..9e99bf96085 100644
--- a/test/metabase/db/schema_migrations_test.clj
+++ b/test/metabase/db/schema_migrations_test.clj
@@ -1003,3 +1003,51 @@
pg-field-3-id :type/Text
mysql-field-1-id :type/SerializedJSON
mysql-field-2-id :type/Text)))))))
+
+(deftest migrate-field-json-unfolding-type-test
+ (testing "Migration v47.00-003: set base-type to type/JSON for JSON database-types for postgres and mysql"
+ (impl/test-migrations ["v47.00-003"] [migrate!]
+ (let [[enabled-db-id
+ enabled-by-default-db-1-id
+ enabled-by-default-db-2-id
+ disabled-db-id] (t2/insert-returning-pks! Database [{:name "enabled"
+ :engine "postgres"
+ :details {:json-unfolding true}}
+ {:name "enabled by default"
+ :engine "postgres"
+ :details {}}
+ {:name "enabled by default"
+ :engine "postgres"
+ :details {:json-unfolding nil}}
+ {:name "disabled"
+ :engine "postgres"
+ :details {:json-unfolding false}}])
+ ;; create a table for each database
+ [enabled-table-id
+ enabled-by-default-table-1-id
+ enabled-by-default-table-2-id
+ disabled-table-id] (t2/insert-returning-pks! Table (for [db-id [enabled-db-id
+ enabled-by-default-db-1-id
+ enabled-by-default-db-2-id
+ disabled-db-id]]
+ {:db_id db-id :name "Table" :active true}))
+ ;; create a JSON field for each table
+ [enabled-field-id
+ enabled-by-default-field-1-id
+ enabled-by-default-field-2-id
+ disabled-field-id] (t2/insert-returning-pks! Field (for [table-id [enabled-table-id
+ enabled-by-default-table-1-id
+ enabled-by-default-table-2-id
+ disabled-table-id]]
+ {:table_id table-id
+ :name "Field"
+ :active true
+ :base_type :type/JSON
+ :database_type "json"}))
+ _ (migrate!)
+ id->json-unfolding (t2/select-pk->fn :json_unfolding Field)]
+ (are [field-id expected] (= expected (get id->json-unfolding field-id))
+ enabled-field-id true ; {:json-unfolding true}
+ enabled-by-default-field-1-id true ; {}
+ enabled-by-default-field-2-id true ; {:json-unfolding nil}
+ disabled-field-id false))))) ; {:json-unfolding false}
diff --git a/test/metabase/driver/mysql_test.clj b/test/metabase/driver/mysql_test.clj
index 976b625cd68..156ab2372d1 100644
--- a/test/metabase/driver/mysql_test.clj
+++ b/test/metabase/driver/mysql_test.clj
@@ -438,60 +438,69 @@
:database-type "timestamp",
:base-type :type/DateTime,
:database-position 0,
+ :json-unfolding false,
:visibility-type :normal,
:nfc-path [:json_bit "1234123412314"]}
{:name "json_bit → boop",
:database-type "timestamp",
:base-type :type/DateTime,
:database-position 0,
+ :json-unfolding false,
:visibility-type :normal,
:nfc-path [:json_bit "boop"]}
{:name "json_bit → genres",
:database-type "text",
:base-type :type/Array,
:database-position 0,
+ :json-unfolding false,
:visibility-type :normal,
:nfc-path [:json_bit "genres"]}
{:name "json_bit → 1234",
:database-type "bigint",
:base-type :type/Integer,
:database-position 0,
+ :json-unfolding false,
:visibility-type :normal,
:nfc-path [:json_bit "1234"]}
{:name "json_bit → doop",
:database-type "text",
:base-type :type/Text,
:database-position 0,
+ :json-unfolding false,
:visibility-type :normal,
:nfc-path [:json_bit "doop"]}
{:name "json_bit → noop",
:database-type "timestamp",
:base-type :type/DateTime,
:database-position 0,
+ :json-unfolding false,
:visibility-type :normal,
:nfc-path [:json_bit "noop"]}
{:name "json_bit → zoop",
:database-type "timestamp",
:base-type :type/DateTime,
:database-position 0,
+ :json-unfolding false,
:visibility-type :normal,
:nfc-path [:json_bit "zoop"]}
{:name "json_bit → published",
:database-type "text",
:base-type :type/Text,
:database-position 0,
+ :json-unfolding false,
:visibility-type :normal,
:nfc-path [:json_bit "published"]}
{:name "json_bit → title",
:database-type "text",
:base-type :type/Text,
:database-position 0,
+ :json-unfolding false,
:visibility-type :normal,
:nfc-path [:json_bit "title"]}}
(sql-jdbc.sync/describe-nested-field-columns
:mysql
(mt/db)
- {:name "json"}))))))))
+ {:name "json" :id (mt/id "json")}))))))))
(deftest big-nested-field-column-test
(mt/test-driver :mysql
@@ -502,7 +511,7 @@
(count (sql-jdbc.sync/describe-nested-field-columns
:mysql
(mt/db)
- {:name "big_json"})))))))))
+ {:name "big_json" :id (mt/id "big_json")})))))))))
(deftest json-query-test
(let [boop-identifier (h2x/identifier :field "boop" "bleh -> meh")]
@@ -589,12 +598,14 @@
:database-type "boolean"
:base-type :type/Boolean
:database-position 0
+ :json-unfolding false
:visibility-type :normal
:nfc-path [:jsoncol "mybool"]}
{:name "jsoncol → myint"
:database-type "double precision"
:base-type :type/Number
:database-position 0
+ :json-unfolding false
:visibility-type :normal
:nfc-path [:jsoncol "myint"]}}
(sql-jdbc.sync/describe-nested-field-columns
@@ -602,20 +613,6 @@
(mt/db)
(t2/select-one Table :db_id (mt/id) :name "bigint-and-bool-table")))))))))
-(deftest can-shut-off-json-unwrapping
- (mt/test-driver :mysql
- ;; in here we fiddle with the mysql db details
- (let [db (t2/select-one Database :id (mt/id))]
- (try
- (t2/update! Database (mt/id) {:details (assoc (:details db) :json-unfolding true)})
- (is (= true (driver/database-supports? :mysql :nested-field-columns (mt/db))))
- (t2/update! Database (mt/id) {:details (assoc (:details db) :json-unfolding false)})
- (is (= false (driver/database-supports? :mysql :nested-field-columns (mt/db))))
- (t2/update! Database (mt/id) {:details (assoc (:details db) :json-unfolding nil)})
- (is (= true (driver/database-supports? :mysql :nested-field-columns (mt/db))))
- ;; un fiddle with the mysql db details.
- (finally (t2/update! Database (mt/id) {:details (:details db)}))))))
-
(deftest ddl-execute-with-timeout-test1
(mt/test-driver :mysql
(mt/dataset json
diff --git a/test/metabase/driver/postgres_test.clj b/test/metabase/driver/postgres_test.clj
index 637ebb287c7..8ab8143b475 100644
--- a/test/metabase/driver/postgres_test.clj
+++ b/test/metabase/driver/postgres_test.clj
@@ -27,6 +27,7 @@
[metabase.query-processor.store :as qp.store]
[metabase.sync :as sync]
[metabase.sync.sync-metadata :as sync-metadata]
+ [metabase.sync.sync-metadata.tables :as sync-tables]
[metabase.sync.util :as sync-util]
[metabase.test :as mt]
[metabase.util :as u]
@@ -347,14 +348,6 @@
;;; ----------------------------------------- Tests for exotic column types ------------------------------------------
-(deftest ^:parallel json-query-support-test
- (testing "JSON database support options behave as they're supposed to"
- (are [details expected] (= expected
- (driver/database-supports? :postgres :nested-field-columns {:details details}))
- {} true
- {:json-unfolding true} true
- {:json-unfolding false} false)))
-
(deftest ^:parallel json-query-test
(let [boop-identifier (h2x/identifier :field "boop" "bleh -> meh")]
(testing "Transforming MBQL query with JSON in it to postgres query works"
@@ -496,53 +489,61 @@
spec (sql-jdbc.conn/connection-details->spec :postgres details)]
(jdbc/with-db-connection [_conn (sql-jdbc.conn/connection-details->spec :postgres details)]
(jdbc/execute! spec [describe-json-table-sql]))
- (mt/with-temp Database [database {:engine :postgres, :details details}]
- (is (= [:type/JSON :type/SerializedJSON]
- (->> (sql-jdbc.sync/describe-table :postgres database {:name "describe_json_table"})
- (:fields)
- (:take 1)
- (first)
- ((juxt :base-type :semantic-type)))))
- (is (= '#{{:name "incoherent_json_val → b",
- :database-type "text",
- :base-type :type/Text,
- :database-position 0,
- :nfc-path [:incoherent_json_val "b"]
- :visibility-type :normal}
- {:name "coherent_json_val → a",
- :database-type "bigint",
- :base-type :type/Integer,
- :database-position 0,
- :nfc-path [:coherent_json_val "a"]
- :visibility-type :normal}
- {:name "coherent_json_val → b",
- :database-type "bigint",
- :base-type :type/Integer,
- :database-position 0,
- :nfc-path [:coherent_json_val "b"]
- :visibility-type :normal}
- {:name "coherent_json_val → c",
- :database-type "timestamp",
- :base-type :type/DateTime,
- :database-position 0,
- :visibility-type :normal,
- :nfc-path [:coherent_json_val "c"]}
- {:name "incoherent_json_val → c",
- :database-type "double precision",
- :base-type :type/Number,
- :database-position 0,
- :visibility-type :normal,
- :nfc-path [:incoherent_json_val "c"]}
- {:name "incoherent_json_val → d",
- :database-type "bigint",
- :base-type :type/Integer,
- :database-position 0,
- :visibility-type :normal,
- :nfc-path [:incoherent_json_val "d"]}}
- (sql-jdbc.sync/describe-nested-field-columns
- :postgres
- database
- {:name "describe_json_table"}))))))))
+ (mt/with-temp* [Database [database {:engine :postgres, :details details}]]
+ (mt/with-db database
+ (is (= [:type/JSON :type/SerializedJSON]
+ (->> (sql-jdbc.sync/describe-table :postgres database {:name "describe_json_table"})
+ (:fields)
+ (:take 1)
+ (first)
+ ((juxt :base-type :semantic-type)))))
+ (sync-tables/sync-tables-and-database! database)
+ (is (= '#{{:name "incoherent_json_val → b",
+ :database-type "text",
+ :base-type :type/Text,
+ :database-position 0,
+ :json-unfolding false
+ :nfc-path [:incoherent_json_val "b"]
+ :visibility-type :normal}
+ {:name "coherent_json_val → a",
+ :database-type "bigint",
+ :base-type :type/Integer,
+ :database-position 0,
+ :json-unfolding false
+ :nfc-path [:coherent_json_val "a"]
+ :visibility-type :normal}
+ {:name "coherent_json_val → b",
+ :database-type "bigint",
+ :base-type :type/Integer,
+ :database-position 0,
+ :json-unfolding false
+ :nfc-path [:coherent_json_val "b"]
+ :visibility-type :normal}
+ {:name "coherent_json_val → c",
+ :database-type "timestamp",
+ :base-type :type/DateTime,
+ :database-position 0,
+ :json-unfolding false
+ :visibility-type :normal,
+ :nfc-path [:coherent_json_val "c"]}
+ {:name "incoherent_json_val → c",
+ :database-type "double precision",
+ :base-type :type/Number,
+ :database-position 0,
+ :json-unfolding false
+ :visibility-type :normal,
+ :nfc-path [:incoherent_json_val "c"]}
+ {:name "incoherent_json_val → d",
+ :database-type "bigint",
+ :base-type :type/Integer,
+ :database-position 0,
+ :json-unfolding false
+ :visibility-type :normal,
+ :nfc-path [:incoherent_json_val "d"]}}
+ (sql-jdbc.sync/describe-nested-field-columns
+ :postgres
+ database
+ {:name "describe_json_table" :id (mt/id "describe_json_table")})))))))))
(deftest describe-nested-field-columns-identifier-test
(mt/test-driver :postgres
@@ -556,16 +557,19 @@
"CREATE TABLE bobdobbs.describe_json_table (trivial_json JSONB NOT NULL);"
"INSERT INTO bobdobbs.describe_json_table (trivial_json) VALUES ('{\"a\": 1}');")]))
(mt/with-temp Database [database {:engine :postgres, :details details}]
- (is (= #{{:name "trivial_json → a",
- :database-type "bigint",
- :base-type :type/Integer,
- :database-position 0,
- :visibility-type :normal,
- :nfc-path [:trivial_json "a"]}}
- (sql-jdbc.sync/describe-nested-field-columns
- :postgres
- database
- {:schema "bobdobbs" :name "describe_json_table"}))))))))
+ (mt/with-db database
+ (sync-tables/sync-tables-and-database! database)
+ (is (= #{{:name "trivial_json → a",
+ :database-type "bigint",
+ :base-type :type/Integer,
+ :database-position 0,
+ :json-unfolding false,
+ :visibility-type :normal,
+ :nfc-path [:trivial_json "a"]}}
+ (sql-jdbc.sync/describe-nested-field-columns
+ :postgres
+ database
+ {:schema "bobdobbs" :name "describe_json_table" :id (mt/id "describe_json_table")})))))))))
(deftest describe-funky-name-table-nested-field-columns-test
(mt/test-driver :postgres
@@ -579,16 +583,19 @@
"CREATE TABLE \"AAAH_#\".\"dESCribe_json_table_%\" (trivial_json JSONB NOT NULL);"
"INSERT INTO \"AAAH_#\".\"dESCribe_json_table_%\" (trivial_json) VALUES ('{\"a\": 1}');")]))
(mt/with-temp Database [database {:engine :postgres, :details details}]
- (is (= #{{:name "trivial_json → a",
- :database-type "bigint",
- :base-type :type/Integer,
- :database-position 0,
- :visibility-type :normal,
- :nfc-path [:trivial_json "a"]}}
- (sql-jdbc.sync/describe-nested-field-columns
- :postgres
- database
- {:schema "AAAH_#" :name "dESCribe_json_table_%"}))))))))
+ (mt/with-db database
+ (sync-tables/sync-tables-and-database! database)
+ (is (= #{{:name "trivial_json → a",
+ :database-type "bigint",
+ :base-type :type/Integer,
+ :database-position 0,
+ :json-unfolding false,
+ :visibility-type :normal,
+ :nfc-path [:trivial_json "a"]}}
+ (sql-jdbc.sync/describe-nested-field-columns
+ :postgres
+ database
+ {:schema "AAAH_#" :name "dESCribe_json_table_%" :id (mt/id "dESCribe_json_table_%")})))))))))
(deftest describe-big-nested-field-columns-test
(mt/test-driver :postgres
@@ -604,19 +611,21 @@
(jdbc/with-db-connection [_conn (sql-jdbc.conn/connection-details->spec :postgres details)]
(jdbc/execute! spec [sql]))
(mt/with-temp Database [database {:engine :postgres, :details details}]
- (is (= sql-jdbc.describe-table/max-nested-field-columns
- (count
- (sql-jdbc.sync/describe-nested-field-columns
- :postgres
- database
- {:name "big_json_table"}))))
- (is (str/includes?
- (get-in (mt/with-log-messages-for-level :warn
- (sql-jdbc.sync/describe-nested-field-columns
- :postgres
- database
- {:name "big_json_table"})) [0 2])
- "More nested field columns detected than maximum.")))))))
+ (mt/with-db database
+ (sync-tables/sync-tables-and-database! database)
+ (is (= sql-jdbc.describe-table/max-nested-field-columns
+ (count
+ (sql-jdbc.sync/describe-nested-field-columns
+ :postgres
+ database
+ {:name "big_json_table" :id (mt/id "big_json_table")}))))
+ (is (str/includes?
+ (get-in (mt/with-log-messages-for-level :warn
+ (sql-jdbc.sync/describe-nested-field-columns
+ :postgres
+ database
+ {:name "big_json_table" :id (mt/id "big_json_table")})) [0 2])
+ "More nested field columns detected than maximum."))))))))
(mt/defdataset with-uuid
[["users"
@@ -807,25 +816,28 @@
(testing "check that describe-table properly describes the database & base types of the enum fields"
(is (= {:name "birds"
- :fields #{{:name "name"
- :database-type "varchar"
- :base-type :type/Text
- :pk? true
- :database-position 0
- :database-required true
- :database-is-auto-increment false}
- {:name "status"
- :database-type "bird_status"
- :base-type :type/PostgresEnum
- :database-position 1
- :database-required true
- :database-is-auto-increment false}
- {:name "type"
- :database-type "bird type"
- :base-type :type/PostgresEnum
- :database-position 2
- :database-required true
- :database-is-auto-increment false}}}
+ :fields #{{:name "name"
+ :database-type "varchar"
+ :base-type :type/Text
+ :pk? true
+ :database-position 0
+ :database-required true
+ :database-is-auto-increment false
+ :json-unfolding false}
+ {:name "status"
+ :database-type "bird_status"
+ :base-type :type/PostgresEnum
+ :database-position 1
+ :database-required true
+ :database-is-auto-increment false
+ :json-unfolding false}
+ {:name "type"
+ :database-type "bird type"
+ :base-type :type/PostgresEnum
+ :database-position 2
+ :database-required true
+ :database-is-auto-increment false
+ :json-unfolding false}}}
(driver/describe-table :postgres db {:name "birds"}))))
(testing "check that when syncing the DB the enum types get recorded appropriately"
diff --git a/test/metabase/driver/sql_jdbc/sync/describe_table_test.clj b/test/metabase/driver/sql_jdbc/sync/describe_table_test.clj
index 9f18ced8c11..8de3e2e1586 100644
--- a/test/metabase/driver/sql_jdbc/sync/describe_table_test.clj
+++ b/test/metabase/driver/sql_jdbc/sync/describe_table_test.clj
@@ -27,12 +27,12 @@
(deftest describe-table-test
(is (= {:name "VENUES",
:fields
- #{{:name "ID", :database-type "BIGINT", :base-type :type/BigInteger, :database-position 0, :pk? true :database-required false :database-is-auto-increment true}
- {:name "NAME", :database-type "CHARACTER VARYING", :base-type :type/Text, :database-position 1 :database-required false :database-is-auto-increment false}
- {:name "CATEGORY_ID", :database-type "INTEGER", :base-type :type/Integer, :database-position 2 :database-required false :database-is-auto-increment false}
- {:name "LATITUDE", :database-type "DOUBLE PRECISION", :base-type :type/Float, :database-position 3 :database-required false :database-is-auto-increment false}
- {:name "LONGITUDE", :database-type "DOUBLE PRECISION", :base-type :type/Float, :database-position 4 :database-required false :database-is-auto-increment false}
- {:name "PRICE", :database-type "INTEGER", :base-type :type/Integer, :database-position 5 :database-required false :database-is-auto-increment false}}}
+ #{{:name "ID", :database-type "BIGINT", :base-type :type/BigInteger, :database-position 0, :pk? true :database-required false :database-is-auto-increment true :json-unfolding false}
+ {:name "NAME", :database-type "CHARACTER VARYING", :base-type :type/Text, :database-position 1 :database-required false :database-is-auto-increment false :json-unfolding false}
+ {:name "CATEGORY_ID", :database-type "INTEGER", :base-type :type/Integer, :database-position 2 :database-required false :database-is-auto-increment false :json-unfolding false}
+ {:name "LATITUDE", :database-type "DOUBLE PRECISION", :base-type :type/Float, :database-position 3 :database-required false :database-is-auto-increment false :json-unfolding false}
+ {:name "LONGITUDE", :database-type "DOUBLE PRECISION", :base-type :type/Float, :database-position 4 :database-required false :database-is-auto-increment false :json-unfolding false}
+ {:name "PRICE", :database-type "INTEGER", :base-type :type/Integer, :database-position 5 :database-required false :database-is-auto-increment false :json-unfolding false}}}
(sql-jdbc.describe-table/describe-table :h2 (mt/id) {:name "VENUES"}))))
(deftest describe-auto-increment-on-non-pk-field-test
@@ -54,19 +54,22 @@
:database-required false
:database-type "INTEGER"
:name "id"
- :pk? true}
+ :pk? true
+ :json-unfolding false}
{:base-type :type/Integer
:database-is-auto-increment true
:database-position 1
:database-required false
:database-type "INTEGER"
- :name "count"}
+ :name "count"
+ :json-unfolding false}
{:base-type :type/Integer
:database-is-auto-increment false
:database-position 2
:database-required true
:database-type "INTEGER"
- :name "rank"}}
+ :name "rank"
+ :json-unfolding false}}
:name "employee_counter"}
(sql-jdbc.describe-table/describe-table :h2 (mt/id) {:name "employee_counter"}))))))
@@ -174,12 +177,13 @@
(is (= types (#'sql-jdbc.describe-table/row->types row)))))
(testing "JSON row->types handles bigint that comes in and gets interpreted as Java bigint OK (#22732)"
(let [int-row {:zlob {"blob" (java.math.BigInteger. "123124124312134235234235345344324352")}}]
- (is (= #{{:name "zlob → blob",
- :database-type "decimal",
- :base-type :type/BigInteger,
+ (is (= #{{:name "zlob → blob",
+ :database-type "decimal",
+ :base-type :type/BigInteger,
:database-position 0,
- :visibility-type :normal,
- :nfc-path [:zlob "blob"]}}
+ :json-unfolding false
+ :visibility-type :normal,
+ :nfc-path [:zlob "blob"]}}
(-> int-row
(#'sql-jdbc.describe-table/row->types)
(#'sql-jdbc.describe-table/field-types->fields)))))))
diff --git a/test/metabase/driver/sql_jdbc_test.clj b/test/metabase/driver/sql_jdbc_test.clj
index 9518d8a98c2..d34ce4cba2f 100644
--- a/test/metabase/driver/sql_jdbc_test.clj
+++ b/test/metabase/driver/sql_jdbc_test.clj
@@ -27,37 +27,43 @@
:pk? true
:database-position 0
:database-required false
- :database-is-auto-increment true}
+ :database-is-auto-increment true
+ :json-unfolding false}
{:name "NAME"
:database-type "CHARACTER VARYING"
:base-type :type/Text
:database-position 1
:database-required false
- :database-is-auto-increment false}
+ :database-is-auto-increment false
+ :json-unfolding false}
{:name "CATEGORY_ID"
:database-type "INTEGER"
:base-type :type/Integer
:database-position 2
:database-required false
- :database-is-auto-increment false}
+ :database-is-auto-increment false
+ :json-unfolding false}
{:name "LATITUDE"
:database-type "DOUBLE PRECISION"
:base-type :type/Float
:database-position 3
:database-required false
- :database-is-auto-increment false}
+ :database-is-auto-increment false
+ :json-unfolding false}
{:name "LONGITUDE"
:database-type "DOUBLE PRECISION"
:base-type :type/Float
:database-position 4
:database-required false
- :database-is-auto-increment false}
+ :database-is-auto-increment false
+ :json-unfolding false}
{:name "PRICE"
:database-type "INTEGER"
:base-type :type/Integer
:database-position 5
:database-required false
- :database-is-auto-increment false}}}
+ :database-is-auto-increment false
+ :json-unfolding false}}}
(driver/describe-table :h2 (mt/db) (t2/select-one Table :id (mt/id :venues))))))
(deftest ^:parallel describe-table-fks-test
diff --git a/test/metabase/models/database_test.clj b/test/metabase/models/database_test.clj
index 7cf9975a23b..ef92f877c9e 100644
--- a/test/metabase/models/database_test.clj
+++ b/test/metabase/models/database_test.clj
@@ -313,3 +313,12 @@
(let [db (first (t2/insert-returning-instances! Database (dissoc (mt/with-temp-defaults Database) :details)))]
(is (partial= {:details {}}
db))))))
+
+(deftest ^:parallel json-unfolding-default-test
+ (testing "JSON Unfolding database support details behave as they're supposed to"
+ (are [details expected] (= expected
+ (database/json-unfolding-default {:details details}))
+ {} true
+ {:json-unfolding nil} true
+ {:json-unfolding true} true
+ {:json-unfolding false} false)))
diff --git a/test/metabase/sync/sync_dynamic_test.clj b/test/metabase/sync/sync_dynamic_test.clj
index a91e314623b..7e039018e2f 100644
--- a/test/metabase/sync/sync_dynamic_test.clj
+++ b/test/metabase/sync/sync_dynamic_test.clj
@@ -22,7 +22,14 @@
(for [field fields]
(u/select-non-nil-keys
field
- [:table_id :name :fk_target_field_id :parent_id :base_type :database_type :database_is_auto_increment]))))))))
+ [:table_id
+ :name
+ :fk_target_field_id
+ :parent_id
+ :base_type
+ :database_type
+ :database_is_auto_increment
+ :json-unfolding]))))))))
(defn- get-tables [database-or-id]
(->> (hydrate (t2/select Table, :db_id (u/the-id database-or-id), {:order-by [:id]}) :fields)
diff --git a/test/metabase/sync/sync_metadata/fields/fetch_metadata_test.clj b/test/metabase/sync/sync_metadata/fields/fetch_metadata_test.clj
index 899d89c572c..0018b66df55 100644
--- a/test/metabase/sync/sync_metadata/fields/fetch_metadata_test.clj
+++ b/test/metabase/sync/sync_metadata/fields/fetch_metadata_test.clj
@@ -23,7 +23,8 @@
:semantic-type :type/PK
:pk? true
:database-required false
- :database-is-auto-increment true}
+ :database-is-auto-increment true
+ :json-unfolding false}
{:name "buyer"
:database-type "OBJECT"
:base-type :type/Dictionary
@@ -31,12 +32,14 @@
:pk? false
:database-required false
:database-is-auto-increment false
+ :json-unfolding false
:nested-fields #{{:name "name"
:database-type "VARCHAR"
:base-type :type/Text
:effective-type :type/Text
:pk? false
:database-required false
+ :json-unfolding false
:database-is-auto-increment false}
{:name "cc"
:database-type "VARCHAR"
@@ -44,6 +47,7 @@
:effective-type :type/Text
:pk? false
:database-required false
+ :json-unfolding false
:database-is-auto-increment false}}}
{:name "ts"
:database-type "BIGINT"
@@ -52,6 +56,7 @@
:coercion-strategy :Coercion/UNIXMilliSeconds->DateTime
:pk? false
:database-is-auto-increment false
+ :json-unfolding false
:database-required false}
{:name "toucan"
:database-type "OBJECT"
@@ -60,12 +65,14 @@
:pk? false
:database-required false
:database-is-auto-increment false
+ :json-unfolding false
:nested-fields #{{:name "name"
:database-type "VARCHAR"
:base-type :type/Text
:effective-type :type/Text
:pk? false
:database-required false
+ :json-unfolding false
:database-is-auto-increment false}
{:name "details"
:database-type "OBJECT"
@@ -73,6 +80,7 @@
:effective-type :type/Dictionary
:pk? false
:database-required false
+ :json-unfolding false
:database-is-auto-increment false
:nested-fields #{{:name "weight"
:database-type "DECIMAL"
@@ -81,6 +89,7 @@
:semantic-type :type/Category
:pk? false
:database-required false
+ :json-unfolding false
:database-is-auto-increment false}
{:name "age"
:database-type "INT"
@@ -88,6 +97,7 @@
:effective-type :type/Integer
:pk? false
:database-required false
+ :json-unfolding false
:database-is-auto-increment false}}}}}}
(let [transactions-table-id (u/the-id (t2/select-one-pk Table :db_id (u/the-id db), :name "transactions"))
diff --git a/test/metabase/sync/sync_metadata/fields/sync_metadata_test.clj b/test/metabase/sync/sync_metadata/fields/sync_metadata_test.clj
index 605c26cc36f..ca3f673624d 100644
--- a/test/metabase/sync/sync_metadata/fields/sync_metadata_test.clj
+++ b/test/metabase/sync/sync_metadata/fields/sync_metadata_test.clj
@@ -22,54 +22,54 @@
(testing "test that if database-type changes we will update it in the DB"
(is (= [["Field" 1 {:database_type "Integer"}]]
(updates-that-will-be-performed
- {:name "My Field"
- :database-type "Integer"
- :base-type :type/Integer
- :database-position 0
- :database-required false
+ {:name "My Field"
+ :database-type "Integer"
+ :base-type :type/Integer
+ :database-position 0
+ :database-required false
:database-is-auto-increment false}
- {:name "My Field"
- :database-type "NULL"
- :base-type :type/Integer
- :id 1
- :database-position 0
- :database-required false
+ {:name "My Field"
+ :database-type "NULL"
+ :base-type :type/Integer
+ :id 1
+ :database-position 0
+ :database-required false
:database-is-auto-increment false})))))
(deftest database-required-changed-test
(testing "test that if database-required changes we will update it in the DB"
(is (= [["Field" 1 {:database_required false}]]
(updates-that-will-be-performed
- {:name "My Field"
- :database-type "Integer"
- :base-type :type/Integer
- :database-position 0
- :database-required false
+ {:name "My Field"
+ :database-type "Integer"
+ :base-type :type/Integer
+ :database-position 0
+ :database-required false
:database-is-auto-increment false}
- {:name "My Field"
- :database-type "Integer"
- :base-type :type/Integer
- :id 1
- :database-position 0
- :database-required true
+ {:name "My Field"
+ :database-type "Integer"
+ :base-type :type/Integer
+ :id 1
+ :database-position 0
+ :database-required true
:database-is-auto-increment false})))))
(deftest database-is-auto-increment-changed-test
(testing "test that if database-required changes we will update it in the DB"
(is (= [["Field" 1 {:database_is_auto_increment true}]]
(updates-that-will-be-performed
- {:name "My Field"
- :database-type "Integer"
- :base-type :type/Integer
- :database-position 0
- :database-required false
+ {:name "My Field"
+ :database-type "Integer"
+ :base-type :type/Integer
+ :database-position 0
+ :database-required false
:database-is-auto-increment true}
- {:name "My Field"
- :database-type "Integer"
- :base-type :type/Integer
- :id 1
- :database-position 0
- :database-required false
+ {:name "My Field"
+ :database-type "Integer"
+ :base-type :type/Integer
+ :id 1
+ :database-position 0
+ :database-required false
:database-is-auto-increment false})))
(is (= [["Field" 1 {:database_is_auto_increment false}]]
(updates-that-will-be-performed
@@ -79,31 +79,53 @@
:database-position 0
;; no :database-is-auto-increment key to test case where describe-table does not not return it
:database-required false}
- {:name "My Field"
- :database-type "Integer"
- :base-type :type/Integer
- :id 1
- :database-position 0
- :database-required false
+ {:name "My Field"
+ :database-type "Integer"
+ :base-type :type/Integer
+ :id 1
+ :database-position 0
+ :database-required false
:database-is-auto-increment true})))))
+(deftest json-unfolding-test
+ (testing "test that if json-unfolding changes the DB doesn't get updated"
+ (is (= []
+ (updates-that-will-be-performed
+ {:name "My Field"
+ :database-type "Integer"
+ :base-type :type/Integer
+ :database-position 0
+ :database-required false
+ :json-unfolding true
+ :database-is-auto-increment false}
+ {:name "My Field"
+ :database-type "Integer"
+ :base-type :type/Integer
+ :id 1
+ :database-position 0
+ :database-required false
+ :json-unfolding false
+ :database-is-auto-increment false})))))
+
(deftest no-op-test
(testing "no changes should be made (i.e., no calls to `update!`) if nothing changes"
(is (= []
(updates-that-will-be-performed
- {:name "My Field"
- :database-type "Integer"
- :base-type :type/Integer
- :database-position 0
- :database-required false
- :database-is-auto-increment true}
- {:name "My Field"
- :database-type "Integer"
- :base-type :type/Integer
- :id 1
- :database-position 0
- :database-required false
- :database-is-auto-increment true})))))
+ {:name "My Field"
+ :database-type "Integer"
+ :base-type :type/Integer
+ :database-position 0
+ :database-required false
+ :database-is-auto-increment true
+ :json-unfolding false}
+ {:name "My Field"
+ :database-type "Integer"
+ :base-type :type/Integer
+ :id 1
+ :database-position 0
+ :database-required false
+ :database-is-auto-increment true
+ :json-unfolding false})))))
(deftest nil-database-type-test
(testing (str "test that if `database-type` comes back as `nil` in the metadata from the sync process, we won't try "
@@ -111,37 +133,41 @@
"`TableMetadataField` schema.")
(is (= [["Field" 1 {:database_type "NULL"}]]
(updates-that-will-be-performed
- {:name "My Field"
- :database-type nil
- :base-type :type/Integer
- :database-position 0
- :database-required false
- :database-is-auto-increment false}
- {:name "My Field"
- :database-type "Integer"
- :base-type :type/Integer
- :database-position 0
- :id 1
- :database-required false
- :database-is-auto-increment false}))))
+ {:name "My Field"
+ :database-type nil
+ :base-type :type/Integer
+ :database-position 0
+ :database-required false
+ :database-is-auto-increment false
+ :json-unfolding false}
+ {:name "My Field"
+ :database-type "Integer"
+ :base-type :type/Integer
+ :database-position 0
+ :id 1
+ :database-required false
+ :database-is-auto-increment false
+ :json-unfolding false}))))
(testing (str "if `database-type` comes back as `nil` and was already saved in application DB as `NULL` no changes "
"should be made")
(is (= []
(updates-that-will-be-performed
- {:name "My Field"
- :database-type nil
- :base-type :type/Integer
- :database-position 0
- :database-required false
- :database-is-auto-increment false}
- {:name "My Field"
- :database-type "NULL"
- :base-type :type/Integer
- :id 1
- :database-position 0
- :database-required false
- :database-is-auto-increment false})))))
+ {:name "My Field"
+ :database-type nil
+ :base-type :type/Integer
+ :database-position 0
+ :database-required false
+ :database-is-auto-increment false
+ :json-unfolding false}
+ {:name "My Field"
+ :database-type "NULL"
+ :base-type :type/Integer
+ :id 1
+ :database-position 0
+ :database-required false
+ :database-is-auto-increment false
+ :json-unfolding false})))))
(deftest dont-overwrite-semantic-type-test
(testing "We should not override non-nil `semantic_type`s"
@@ -152,6 +178,7 @@
:base-type :type/Integer
:semantic-type nil
:database-position 0
+ :json-unfolding false
:database-required false
:database-is-auto-increment false}
{:name "My Field"
@@ -160,5 +187,6 @@
:semantic-type :type/Price
:id 1
:database-position 0
+ :json-unfolding false
:database-required false
:database-is-auto-increment false})))))
diff --git a/test/metabase/sync_test.clj b/test/metabase/sync_test.clj
index 488799f62ea..03441a8b148 100644
--- a/test/metabase/sync_test.clj
+++ b/test/metabase/sync_test.clj
@@ -32,17 +32,20 @@
:base-type :type/Integer
:semantic-type :type/PK
:database-is-auto-increment true
+ :json-unfolding false
:database-position 0}
{:name "title"
:database-type "VARCHAR"
:base-type :type/Text
:semantic-type :type/Title
:database-is-auto-increment false
+ :json-unfolding false
:database-position 1}
{:name "studio"
:database-type "VARCHAR"
:base-type :type/Text
:database-is-auto-increment false
+ :json-unfolding false
:database-position 2}}
:description nil}
"studio" {:name "studio"
@@ -52,11 +55,13 @@
:base-type :type/Text
:semantic-type :type/PK
:database-is-auto-increment false
+ :json-unfolding false
:database-position 0}
{:name "name"
:database-type "VARCHAR"
:base-type :type/Text
:database-is-auto-increment false
+ :json-unfolding false
:database-position 1}}
:description ""}})
@@ -120,6 +125,7 @@
:last_analyzed false
:parent_id false
:position 0
+ :json_unfolding false
:table_id true
:updated_at true}))
diff --git a/test/metabase/test/mock/toucanery.clj b/test/metabase/test/mock/toucanery.clj
index f45f5ab01ba..86163735409 100644
--- a/test/metabase/test/mock/toucanery.clj
+++ b/test/metabase/test/mock/toucanery.clj
@@ -9,60 +9,72 @@
(def toucanery-tables
{"transactions" {:name "transactions"
:schema nil
- :fields #{{:name "id"
- :pk? true
- :database-type "SERIAL"
- :base-type :type/Integer
+ :fields #{{:name "id"
+ :pk? true
+ :database-type "SERIAL"
+ :base-type :type/Integer
+ :json-unfolding false
:database-is-auto-increment true}
- {:name "ts"
- :database-type "BIGINT"
- :base-type :type/BigInteger
- :effective-type :type/DateTime
- :coercion-strategy :Coercion/UNIXMilliSeconds->DateTime
+ {:name "ts"
+ :database-type "BIGINT"
+ :base-type :type/BigInteger
+ :effective-type :type/DateTime
+ :coercion-strategy :Coercion/UNIXMilliSeconds->DateTime
+ :json-unfolding false
:database-is-auto-increment false}
- {:name "toucan"
- :database-type "OBJECT"
- :base-type :type/Dictionary
+ {:name "toucan"
+ :database-type "OBJECT"
+ :base-type :type/Dictionary
+ :json-unfolding false
:database-is-auto-increment false
- :nested-fields #{{:name "name"
- :database-type "VARCHAR"
- :base-type :type/Text
- :database-is-auto-increment false}
- {:name "details"
- :database-type "OBJECT"
- :base-type :type/Dictionary
- :database-is-auto-increment false
- :nested-fields #{{:name "age"
- :database-type "INT"
- :database-is-auto-increment false
- :base-type :type/Integer}
- {:name "weight"
- :database-type "DECIMAL"
- :database-is-auto-increment false
- :semantic-type :type/Category
- :base-type :type/Decimal}}}}}
- {:name "buyer"
- :database-type "OBJECT"
+ :nested-fields #{{:name "name"
+ :database-type "VARCHAR"
+ :base-type :type/Text
+ :json-unfolding false
+ :database-is-auto-increment false}
+ {:name "details"
+ :database-type "OBJECT"
+ :base-type :type/Dictionary
+ :json-unfolding false
+ :database-is-auto-increment false
+ :nested-fields #{{:name "age"
+ :database-type "INT"
+ :database-is-auto-increment false
+ :json-unfolding false
+ :base-type :type/Integer}
+ {:name "weight"
+ :database-type "DECIMAL"
+ :database-is-auto-increment false
+ :json-unfolding false
+ :semantic-type :type/Category
+ :base-type :type/Decimal}}}}}
+ {:name "buyer"
+ :database-type "OBJECT"
:database-is-auto-increment false
- :base-type :type/Dictionary
- :nested-fields #{{:name "name"
- :database-type "VARCHAR"
- :database-is-auto-increment false
- :base-type :type/Text}
- {:name "cc"
- :database-type "VARCHAR"
- :database-is-auto-increment false
- :base-type :type/Text}}}}}
+ :json-unfolding false
+ :base-type :type/Dictionary
+ :nested-fields #{{:name "name"
+ :database-type "VARCHAR"
+ :json-unfolding false
+ :base-type :type/Text
+ :database-is-auto-increment false}
+ {:name "cc"
+ :database-type "VARCHAR"
+ :json-unfolding false
+ :base-type :type/Text
+ :database-is-auto-increment false}}}}}
"employees" {:name "employees"
:schema nil
- :fields #{{:name "id"
- :database-type "SERIAL"
+ :fields #{{:name "id"
+ :database-type "SERIAL"
+ :json-unfolding false
:database-is-auto-increment true
- :base-type :type/Integer}
- {:name "name"
- :database-type "VARCHAR"
+ :base-type :type/Integer}
+ {:name "name"
+ :database-type "VARCHAR"
+ :json-unfolding false
:database-is-auto-increment false
- :base-type :type/Text}}}})
+ :base-type :type/Text}}}})
(driver/register! ::toucanery, :abstract? true)
diff --git a/test/metabase/test/mock/util.clj b/test/metabase/test/mock/util.clj
index 309635b0d54..2598e4faeef 100644
--- a/test/metabase/test/mock/util.clj
+++ b/test/metabase/test/mock/util.clj
@@ -33,7 +33,8 @@
:position 0
:visibility_type :normal
:preview_display true
- :created_at true})
+ :created_at true
+ :json_unfolding false})
(def pulse-channel-defaults
{:schedule_frame nil
--
GitLab