From 5d3335957841abda6f8050a115dc5c775667c4fa Mon Sep 17 00:00:00 2001
From: Ngoc Khuat <qn.khuat@gmail.com>
Date: Wed, 13 Mar 2024 06:04:30 +0700
Subject: [PATCH] Postgres: Fix sync does not recognize schema with backslashes
(#38946)
---
src/metabase/driver.clj | 2 +-
src/metabase/driver/postgres.clj | 10 ++++++++--
test/metabase/driver/postgres_test.clj | 15 ++++++++++++++-
.../sql_jdbc/sync/describe_database_test.clj | 19 +++++++++++++++++++
4 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index 747cf969c48..b868f3439da 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -332,7 +332,7 @@
driver has difference escaping rules for table or schema names when used from metadata.
For example, oracle treats slashes differently when querying versus when used with `.getTables` or `.getColumns`"
- {:arglists '([driver table-name]), :added "0.37.0"}
+ {:arglists '([driver entity-name]), :added "0.37.0"}
dispatch-on-initialized-driver
:hierarchy #'hierarchy)
diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj
index 9073ef746d2..046dd79132d 100644
--- a/src/metabase/driver/postgres.clj
+++ b/src/metabase/driver/postgres.clj
@@ -79,6 +79,12 @@
[driver _feat _db]
(= driver :postgres)))
+(defmethod driver/escape-entity-name-for-metadata :postgres [_driver entity-name]
+ (when entity-name
+ ;; these entities names are used as a pattern for LIKE queries in jdbc
+ ;; so we need to double escape it, first for java, then for sql
+ (str/replace entity-name "\\" "\\\\")))
+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | metabase.driver impls |
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -862,8 +868,8 @@
;; KNOWN LIMITATION: this won't return privileges for foreign tables, calling has_table_privilege on a foreign table
;; result in a operation not supported error
(->> (jdbc/query
- conn-spec
- (str/join
+ conn-spec
+ (str/join
"\n"
["with table_privileges as ("
" select"
diff --git a/test/metabase/driver/postgres_test.clj b/test/metabase/driver/postgres_test.clj
index 9ce2f78d651..c50c460cb10 100644
--- a/test/metabase/driver/postgres_test.clj
+++ b/test/metabase/driver/postgres_test.clj
@@ -213,7 +213,20 @@
(mt/rows (qp/process-query
{:database (u/the-id database)
:type :query
- :query {:source-table (t2/select-one-pk Table :name "presents-and-gifts")}}))))))))))
+ :query {:source-table (t2/select-one-pk Table :name "presents-and-gifts")}}))))))))
+ (testing "Make sure that Schemas / Tables / Fields with backslashes in their names get escaped properly"
+ (mt/with-empty-db
+ (let [conn-spec (sql-jdbc.conn/db->pooled-connection-spec (mt/db))]
+ (doseq [stmt ["CREATE SCHEMA \"my\\schema\";"
+ "CREATE TABLE \"my\\schema\".\"my\\table\" (\"my\\field\" INTEGER);"
+ "INSERT INTO \"my\\schema\".\"my\\table\" (\"my\\field\") VALUES (42);"]]
+ (jdbc/execute! conn-spec stmt))
+ (sync/sync-database! (mt/db) {:scan :schema})
+ (is (= [[42]]
+ (mt/rows (qp/process-query
+ {:database (u/the-id (mt/db))
+ :type :query
+ :query {:source-table (t2/select-one-pk :model/Table :db_id (:id (mt/db)))}})))))))))
(mt/defdataset duplicate-names
[["birds"
diff --git a/test/metabase/driver/sql_jdbc/sync/describe_database_test.clj b/test/metabase/driver/sql_jdbc/sync/describe_database_test.clj
index 0025b6b1d7a..df19f268b3e 100644
--- a/test/metabase/driver/sql_jdbc/sync/describe_database_test.clj
+++ b/test/metabase/driver/sql_jdbc/sync/describe_database_test.clj
@@ -208,3 +208,22 @@
driver/*driver* conn schema (str table-name "_should_not_exist"))))
(is (true? (sql-jdbc.sync.interface/have-select-privilege?
driver/*driver* conn schema table-name))))))))))))))
+
+(deftest sync-table-with-backslash-test
+ (mt/test-drivers #{:postgres} ;; TODO: fix and change this to test on (mt/sql-jdbc-drivers)
+ (testing "table with backslash in name, PKs, FKS are correctly synced"
+ (mt/with-temp-test-data [["human\\race"
+ [{:field-name "humanraceid" :base-type :type/Integer :pk? true}
+ {:field-name "race" :base-type :type/Text}]
+ [[1 "homo sapiens"]]]
+ ["citizen"
+ [{:field-name "citizen\\id" :base-type :type/Integer :pk? true}
+ {:field-name "race\\id" :base-type :type/Integer :fk "human\\race"}]
+ [[1 1]]]]
+ (let [tables (t2/select :model/Table :db_id (:id (mt/db)))
+ field-name->field (t2/select-fn->fn :name identity :model/Field :table_id [:in (map :id tables)])]
+ (is (= #{"human\\race" "citizen"} (set (map :name tables))))
+ (is (= #{"humanraceid" "citizen\\id" "race" "race\\id"}
+ (set (keys field-name->field))))
+ (is (= (get-in field-name->field ["humanraceid" :id])
+ (get-in field-name->field ["race\\id" :fk_target_field_id]))))))))
--
GitLab