From ea4825a353dec79bef9338197ee312621cbcbc84 Mon Sep 17 00:00:00 2001
From: Howon Lee <hlee.howon@gmail.com>
Date: Tue, 7 Jun 2022 13:28:54 -0700
Subject: [PATCH] Do trivial identity for passthrough of symbols for nested
field column instead of default processing (#23136)
Should whack both #23026 and #23027.
Previous (default) behavior of the reducible query that gets the limited contents of the JSON to break out the nested field columns is to lowercase identifiers. This is root cause of #23026 and #23027.
But we want the proper case of those identifiers in order to be modifying the query correctly when we query the JSON. So we set the reducible query to just pass through the identifiers instead of default behavior.
---
src/metabase/driver/postgres.clj | 6 +++-
.../driver/sql_jdbc/sync/describe_table.clj | 7 ++--
test/metabase/driver/postgres_test.clj | 32 +++++++++++++++++++
3 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj
index 2875ed30b58..86381f035df 100644
--- a/src/metabase/driver/postgres.clj
+++ b/src/metabase/driver/postgres.clj
@@ -39,7 +39,11 @@
(driver/register! :postgres, :parent :sql-jdbc)
-(defmethod driver/database-supports? [:postgres :nested-field-columns] [_ _ database] (get-in database [:details :json-unfolding]))
+(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 |
diff --git a/src/metabase/driver/sql_jdbc/sync/describe_table.clj b/src/metabase/driver/sql_jdbc/sync/describe_table.clj
index 2656b2ec05c..94aacee1dae 100644
--- a/src/metabase/driver/sql_jdbc/sync/describe_table.clj
+++ b/src/metabase/driver/sql_jdbc/sync/describe_table.clj
@@ -347,11 +347,12 @@
(if (nil? (seq json-fields))
#{}
(let [json-field-names (mapv #(apply hx/identifier :field (into table-identifier-info [(:name %)])) json-fields)
- table-identifier (apply hx/identifier :field table-identifier-info)
+ table-identifier (apply hx/identifier :table table-identifier-info)
+ quote-type (case driver :postgres :ansi :mysql :mysql)
sql-args (hsql/format {:select json-field-names
:from [table-identifier]
- :limit nested-field-sample-limit} {:quoting :ansi})
- query (jdbc/reducible-query spec sql-args)
+ :limit nested-field-sample-limit} :quoting quote-type)
+ 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)
diff --git a/test/metabase/driver/postgres_test.clj b/test/metabase/driver/postgres_test.clj
index 1f20dc0fa62..fd46963ea4f 100644
--- a/test/metabase/driver/postgres_test.clj
+++ b/test/metabase/driver/postgres_test.clj
@@ -276,6 +276,15 @@
;;; ----------------------------------------- Tests for exotic column types ------------------------------------------
+(deftest json-query-support-test
+ (let [default-db {:details {}}
+ json-unfold-db {:details {:json-unfolding true}}
+ no-json-unfold-db {:details {:json-unfolding false}}]
+ (testing "JSON database support options behave as they're supposed to"
+ (is (= true (driver/database-supports? :postgres :nested-field-columns default-db)))
+ (is (= true (driver/database-supports? :postgres :nested-field-columns json-unfold-db)))
+ (is (= false (driver/database-supports? :postgres :nested-field-columns no-json-unfold-db))))))
+
(deftest ^:parallel json-query-test
(let [boop-identifier (hx/with-type-info (hx/identifier :field "boop" "bleh -> meh") {})]
(testing "Transforming MBQL query with JSON in it to postgres query works"
@@ -404,6 +413,29 @@
database
{:schema "bobdobbs" :name "describe_json_table"}))))))))
+(deftest describe-funky-name-table-nested-field-columns-test
+ (mt/test-driver :postgres
+ (testing "sync goes and still works with funky schema and table names, including caps and special chars (#23026, #23027)"
+ (drop-if-exists-and-create-db! "describe-json-funky-names-test")
+ (let [details (mt/dbdef->connection-details :postgres :db {:database-name "describe-json-funky-names-test"
+ :json-unfolding true})
+ 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 [(str "CREATE SCHEMA \"AAAH_#\";"
+ "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 "integer",
+ :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_%"}))))))))
+
(deftest describe-big-nested-field-columns-test
(mt/test-driver :postgres
(testing "blank out if huge. blank out instead of silently limiting"
--
GitLab