Permalink
Browse files

Fix JDBC-39 by forcing Boolean column extraction when metadata indica…

…tes BOOLEAN type.

Since (.getBoolean) seems able to sometimes return a Boolean false that isn't identical to CLojure's false, the result of (.getBoolean) is forced to nil, true or false as appropriate.
Additional tests have been added for MySQL only. I have not been able to reproduce the not-a-boolean situation however, to verify that calling the (pure-boolean) function is appropriate.

This forms the basis for extracting other columns based on their correct, known SQL type, regardless of other field flags.
  • Loading branch information...
1 parent 4e87c30 commit 09057bf8de166de1ade59ca9ccf65e129f6b4f53 @seancorfield seancorfield committed Sep 10, 2012
Showing with 73 additions and 21 deletions.
  1. +19 −3 src/main/clojure/clojure/java/jdbc.clj
  2. +54 −18 src/test/clojure/clojure/java/test_jdbc.clj
@@ -37,7 +37,7 @@ the number of rows affected, except for a single record insert where any
generated keys are returned (as a map)." }
clojure.java.jdbc
(:import [java.net URI]
- [java.sql BatchUpdateException DriverManager PreparedStatement ResultSet SQLException Statement]
+ [java.sql BatchUpdateException DriverManager PreparedStatement ResultSet ResultSetMetaData SQLException Statement]
[java.util Hashtable Map Properties]
[javax.naming InitialContext Name]
[javax.sql DataSource])
@@ -210,6 +210,22 @@ generated keys are returned (as a map)." }
cols
(reduce (fn [unique-cols col-name] (conj unique-cols (make-name-unique unique-cols col-name 1))) [] cols)))
+(defn- pure-boolean
+ "Given a Boolean from the database, map it correctly to Clojure's boolean (or nil)."
+ [b]
+ (cond (nil? b) nil
+ (= b true) true
+ :else false))
+
+(defn- column-extractor
+ "Given a ResultSet and its metadata, return a function that takes a column index and returns the column value.
+ Currently we only special case BOOLEAN but we may special case others over time."
+ [^ResultSet rs ^ResultSetMetaData rsmd]
+ (fn [^Integer i]
+ (condp = (.getColumnType rsmd i)
+ java.sql.Types/BOOLEAN (pure-boolean (.getBoolean rs i))
+ (.getObject rs i))))
+
(defn resultset-seq
"Creates and returns a lazy sequence of maps corresponding to
the rows in the java.sql.ResultSet rs. Based on clojure.core/resultset-seq
@@ -218,13 +234,13 @@ generated keys are returned (as a map)." }
N is a unique integer)."
[^ResultSet rs & {:keys [identifiers]
:or {identifiers *as-key*}}]
- (let [rsmeta (.getMetaData rs)
+ (let [^ResultSetMetaData rsmeta (.getMetaData rs)
idxs (range 1 (inc (.getColumnCount rsmeta)))
keys (->> idxs
(map (fn [^Integer i] (.getColumnLabel rsmeta i)))
make-cols-unique
(map (comp keyword identifiers)))
- row-values (fn [] (map (fn [^Integer i] (.getObject rs i)) idxs))
+ row-values (fn [] (map (column-extractor rs rsmeta) idxs))
;; This used to use create-struct (on keys) and then struct to populate each row.
;; That had the side effect of preserving the order of columns in each row. As
;; part of JDBC-15, this was changed because structmaps are deprecated. We don't
@@ -103,21 +103,31 @@
(for [db test-databases]
@(ns-resolve 'clojure.java.test-jdbc (symbol (str (name db) "-db")))))
+;; Utility to detect MySQL database since we do some
+;; MySQL-specific tests...
+
+(defn- mysql? [db]
+ (let [p (:subprotocol db)]
+ (or (= "mysql" p) (and (string? db) (re-find #"mysql:" db)))))
+
+;; Fixture to drop known tables before each test
+
(defn- clean-up
"Attempt to drop any test tables before we start a test."
[t]
(doseq [db (test-specs)]
- (sql/with-connection db
- (doseq [table [:fruit :fruit2 :veggies :veggies2]]
- (try
- (sql/drop-table table)
- (catch Exception _
- ;; ignore
- )))))
+ (let [tables [:fruit :fruit2 :veggies :veggies2]]
+ (sql/with-connection db
+ (doseq [table (if (mysql? db) (conj tables :booleantest) tables)]
+ (try
+ (sql/drop-table table)
+ (catch Exception _
+ ;; ignore
+ ))))))
(t))
(use-fixtures
- :each clean-up)
+ :each clean-up)
;; We start with all tables dropped and each test has to create the tables
;; necessary for it to do its job, and populate it as needed...
@@ -126,16 +136,14 @@
"Create a standard test table. Must be inside with-connection.
For MySQL, ensure table uses an engine that supports transactions!"
[table db]
- (let [p (:subprotocol db)]
- (sql/create-table
- table
- [:id :int (if (= "mysql" p) "PRIMARY KEY AUTO_INCREMENT" "DEFAULT 0")]
- [:name "VARCHAR(32)" (if (= "mysql" p) "" "PRIMARY KEY")]
- [:appearance "VARCHAR(32)"]
- [:cost :int]
- [:grade :real]
- :table-spec (if (or (= "mysql" p) (and (string? db) (re-find #"mysql:" db)))
- "ENGINE=InnoDB" ""))))
+ (sql/create-table
+ table
+ [:id :int (if (mysql? db) "PRIMARY KEY AUTO_INCREMENT" "DEFAULT 0")]
+ [:name "VARCHAR(32)" (if (mysql? db) "" "PRIMARY KEY")]
+ [:appearance "VARCHAR(32)"]
+ [:cost :int]
+ [:grade :real]
+ :table-spec (if (mysql? db) "ENGINE=InnoDB" "")))
(deftest test-create-table
(doseq [db (test-specs)]
@@ -344,6 +352,34 @@
(is (= 1 (sql/with-query-results res ["SELECT * FROM fruit"] (count res))))))
(is (= 0 (sql/with-query-results res ["SELECT * FROM fruit"] (count res)))))))
+(deftest mysql-boolean-processing
+ (doseq [db (test-specs)]
+ (when (mysql? db)
+ (sql/with-connection db
+ (sql/create-table
+ :booleantest
+ [:id :int "PRIMARY KEY AUTO_INCREMENT"]
+ [:t "BIT(1) DEFAULT b'1'"]
+ [:f "BIT(1) DEFAULT b'0'"]
+ :table-spec "ENGINE=InnoDB")
+ (let [r (sql/insert-records
+ :booleantest
+ {:t 1}
+ {:f 0})]
+ (is (= '({:generated_key 1} {:generated_key 2}) r))
+ (let [rs (sql/with-query-results res
+ ["SELECT * FROM booleantest WHERE id = 1"]
+ (doall res))]
+ (is (= '({:id 1 :t true :f false}) rs))
+ (is (= :valid (if (:t (first rs)) :valid :invalid)))
+ (is (= :valid (if (:f (first rs)) :invalid :valid))))
+ (let [rs (sql/with-query-results res
+ ["SELECT * FROM booleantest WHERE id = 2"]
+ (doall res))]
+ (is (= '({:id 2 :t true :f false}) rs))
+ (is (= :valid (if (:t (first rs)) :valid :invalid)))
+ (is (= :valid (if (:f (first rs)) :invalid :valid)))))))))
+
(deftest test-metadata
(doseq [db (test-specs)]
(when-not (and (map? db) (.endsWith ^String (:subprotocol db) "sqlserver"))

0 comments on commit 09057bf

Please sign in to comment.