Added tests for symbols valued slots, and better printer/reader bindings
[clsql.git] / sql / oodml.lisp
index 3c65919ea8db867b080898d54ab1e95974b56cbf..169fa89d70d6f0fe17e834749e9a27f337cfe2f8 100644 (file)
   "Turns key class and slot-def into a sql-expression representing the
    table and column it comes from
 
-   used by things like generate-selection-list, update-slot-from-record"
+   used by things like make-select-list, update-slot-from-record"
   (when (key-or-base-slot-p slotdef)
     (sql-expression :attribute (database-identifier slotdef database)
                     :table (database-identifier vclass database))))
 
-;;
-;; Function used by 'find-all'
-;;
-
-(defun generate-selection-list (vclass)
-  (let* ((sels nil)
-         (this-class vclass)
-         (slots (if (normalizedp vclass)
-                    (labels ((getdslots ()
-                               (let ((sl (ordered-class-direct-slots this-class)))
-                                 (cond (sl)
-                                       (t
-                                        (setf this-class
-                                              (car (class-direct-superclasses this-class)))
-                                        (getdslots))))))
-                      (getdslots))
-                    (ordered-class-slots this-class))))
-    (dolist (slotdef slots)
-      (let ((res (generate-attribute-reference this-class slotdef)))
-        (when res
-          (push (cons slotdef res) sels))))
-    (if sels
-        sels
-        (error "No slots of type :base in view-class ~A" (class-name vclass)))))
-
-
-
-(defun generate-retrieval-joins-list (vclass retrieval-method)
-  "Returns list of immediate join slots for a class."
-  (let ((join-slotdefs nil))
-    (dolist (slotdef (ordered-class-slots vclass) join-slotdefs)
-      (when (and (eq :join (view-class-slot-db-kind slotdef))
-                 (eq retrieval-method (gethash :retrieval (view-class-slot-db-info slotdef))))
-        (push slotdef join-slotdefs)))))
-
-(defun generate-immediate-joins-selection-list (vclass)
-  "Returns list of immediate join slots for a class."
-  (let (sels)
-    (dolist (joined-slot (generate-retrieval-joins-list vclass :immediate) sels)
-      (let* ((join-class-name (gethash :join-class (view-class-slot-db-info joined-slot)))
-             (join-class (when join-class-name (find-class join-class-name))))
-        (dolist (slotdef (ordered-class-slots join-class))
-          (let ((res (generate-attribute-reference join-class slotdef)))
-            (when res
-              (push (cons slotdef res) sels))))))
-    sels))
+(defun get-join-slots (class &optional retrieval-method)
+  "Returns list of join slots for a class.
+
+   if a retrieval method is specified only return slots of that type
+   if the retrieval method is T, nil or :all return all join slots"
+  (assert (member retrieval-method '(nil t :all :immediate :deferred)))
+  (setf class (to-class class))
+  (let ((all? (member retrieval-method '(nil t :all))))
+    (loop for slot in (ordered-class-slots class)
+          when (and (join-slot-p slot)
+                    (or all? (eql (join-slot-retrieval-method slot) retrieval-method)))
+          collect slot)))
 
 (defun immediate-join-slots (class)
-  (generate-retrieval-joins-list class :immediate))
+  (get-join-slots class :immediate))
 
 (defmethod choose-database-for-instance ((obj standard-db-object) &optional database)
   "Determine which database connection to use for a standard-db-object.
 
 (defun update-auto-increments-keys (class obj database)
   " handle pulling any autoincrement values into the object
-   if normalized and we now that all the "
+    Also handles normalized key chaining"
   (let ((pk-slots (keyslots-for-class class))
         (table (view-table class))
         new-pk-value)
-    (labels ((do-update (slot)
-               (when (and (null (easy-slot-value obj slot))
-                          (auto-increment-column-p slot database))
-                 (update-slot-from-db-value
-                  obj slot
-                  (or new-pk-value
-                      (setf new-pk-value
-                            (database-last-auto-increment-id
-                             database table slot))))))
+    (labels ((do-update (slot &aux (val (easy-slot-value obj slot)))
+               (if val
+                   (setf new-pk-value val)
+                   (update-slot-from-db-value
+                    obj slot
+                    (or new-pk-value
+                        (setf new-pk-value
+                              (database-last-auto-increment-id
+                               database table slot))))))
+             ;; NB: This interacts very strangely with autoincrement keys
+             ;; (see changelog 2014-01-30)
              (chain-primary-keys (in-class)
                "This seems kindof wrong, but this is mostly how it was working, so
                   its here to keep the normalized code path working"
            (insert-records :into table-sql
                            :av-pairs avps
                            :database database)
+           ;; also handles normalized-class key chaining
            (update-auto-increments-keys view-class obj database)
            ;; we dont set view database here, because there could be
            ;; N of these for each call to update-record-from-* because
    the public api"
   (update-record-from-slots obj slot :database database))
 
-(defun view-classes-and-storable-slots (class)
+(defmethod view-classes-and-storable-slots (class &key to-database-p)
   "Get a list of all the tables we need to update and the slots on them
 
    for non normalized classes we return the class and all its storable slots
 
    for normalized classes we return a list of direct slots and the class they
    came from for each normalized view class
+
+   to-database-p is provided so that we can read / write different data
+   to the database in different circumstances
+   (specifically clsql-helper:dirty-db-slots-mixin which only updates slots
+    that have changed )
   "
   (setf class (to-class class))
   (let* (rtns)
     (labels ((storable-slots (class)
                (loop for sd in (slots-for-possibly-normalized-class class)
-                     when (key-or-base-slot-p sd)
+                     when (and (key-or-base-slot-p sd)
+                               ;; we dont want to insert/update auto-increments
+                               ;; but we do read them
+                               (not (and to-database-p (auto-increment-column-p sd))))
                      collect sd))
              (get-classes-and-slots (class &aux (normalizedp (normalizedp class)))
                (let ((slots (storable-slots class)))
   "Makes sure that if a class has unfilled slots that claim to have a default,
    that we retrieve those defaults from the database
 
-   TODO: use update slots-from-record instead to batch this!"
+   TODO: use update-slots-from-record (doesnt exist) instead to batch this!"
   (loop for class-and-slots in (listify classes-and-slots)
         do (loop for slot in (slot-defs class-and-slots)
                  do (when (and (slot-has-default-p slot)
    view-database slot on the object is nil then the object is assumed to be
    new and is inserted"
   (let ((database (choose-database-for-instance obj database))
-        (classes-and-slots (view-classes-and-storable-slots obj)))
+        (classes-and-slots (view-classes-and-storable-slots obj :to-database-p t)))
     (loop for class-and-slots in classes-and-slots
           do (%update-instance-helper class-and-slots obj database))
     (setf (slot-value obj 'view-database) database)
         (signal-no-database-error database))))
 
 (defmethod update-instance-from-records ((instance standard-db-object)
-                                         &key (database *default-database*)
-                                         this-class)
-  (let* ((view-class (or this-class (class-of instance)))
-         (pclass (car (class-direct-superclasses view-class)))
-         (pres nil))
-    (when (normalizedp view-class)
-      (setf pres (update-instance-from-records instance :database database
-                                               :this-class pclass)))
-    (let* ((view-table (sql-expression :table (view-table view-class)))
-           (vd (choose-database-for-instance instance database))
-           (view-qual (key-qualifier-for-instance instance :database vd
-                                                           :this-class view-class))
-           (sels (generate-selection-list view-class))
-           (res nil))
-      (cond (view-qual
-             (setf res (apply #'select (append (mapcar #'cdr sels)
-                                               (list :from  view-table
-                                                     :where view-qual
-                                                     :result-types nil
-                                                     :database vd))))
-             (when res
-              (setf (slot-value instance 'view-database) vd)
-               (get-slot-values-from-view instance (mapcar #'car sels) (car res))))
-            (pres)
-            (t nil)))))
+                                         &key (database *default-database*))
+  "Updates a database object with the current values stored in the database
+
+   TODO: Should this update immediate join slots similar to build-objects?
+         Can we just call build-objects?, update-objects-joins?
+  "
+
+  (let* ((classes-and-slots (view-classes-and-storable-slots
+                             instance :to-database-p nil))
+         (vd (choose-database-for-instance instance database)))
+    (labels ((do-update (class-and-slots)
+               (let* ((select-list (make-select-list class-and-slots
+                                                     :do-joins-p nil
+                                                     :database database))
+                      (view-table (sql-table select-list))
+                      (view-qual (key-qualifier-for-instance
+                                  instance :database vd
+                                  :this-class (view-class select-list)))
+                      (res (when view-qual
+                             (first
+                              (apply #'select
+                                     (append (full-select-list select-list)
+                                             (list :from view-table
+                                                   :where view-qual
+                                                   :result-types nil
+                                                   :database vd)))))))
+                 (when res
+                   (setf (slot-value instance 'view-database) vd)
+                   (get-slot-values-from-view instance (slot-list select-list) res))
+                 )))
+      (loop for class-and-slots in classes-and-slots
+            do (do-update class-and-slots)))))
 
 
 (defmethod get-slot-value-from-record ((instance standard-db-object)
             (error "No view-table for class ~A"  classname))
           (sql-expression :table (view-table class))))
 
-
-(defmethod database-get-type-specifier (type args database db-type)
-  (declare (ignore type args database db-type))
-  (format nil "VARCHAR(~D)" *default-string-length*))
-
-(defmethod database-get-type-specifier ((type (eql 'integer)) args database db-type)
-  (declare (ignore database db-type))
-  (if args
-      (format nil "INT(~A)" (car args))
-      "INT"))
-
 (deftype tinyint ()
   "An 8-bit integer, this width may vary by SQL implementation."
   'integer)
 
-(defmethod database-get-type-specifier ((type (eql 'tinyint)) args database db-type)
-  (declare (ignore args database db-type))
-  "INT")
-
 (deftype smallint ()
   "An integer smaller than a 32-bit integer. this width may vary by SQL implementation."
   'integer)
 
-(defmethod database-get-type-specifier ((type (eql 'smallint)) args database db-type)
-  (declare (ignore args database db-type))
-  "INT")
-
 (deftype mediumint ()
   "An integer smaller than a 32-bit integer, but may be larger than a smallint. This width may vary by SQL implementation."
   'integer)
 
-(defmethod database-get-type-specifier ((type (eql 'mediumint)) args database db-type)
-  (declare (ignore args database db-type))
-  "INT")
-
 (deftype bigint ()
   "An integer larger than a 32-bit integer, this width may vary by SQL implementation."
   'integer)
 
-(defmethod database-get-type-specifier ((type (eql 'bigint)) args database db-type)
-  (declare (ignore args database db-type))
-  "BIGINT")
-
 (deftype varchar (&optional size)
   "A variable length string for the SQL varchar type."
   (declare (ignore size))
   'string)
 
-(defmethod database-get-type-specifier ((type (eql 'varchar)) args
-                                        database db-type)
-  (declare (ignore database db-type))
-  (if args
-      (format nil "VARCHAR(~A)" (car args))
-      (format nil "VARCHAR(~D)" *default-string-length*)))
-
-(defmethod database-get-type-specifier ((type (eql 'string)) args database db-type)
-  (declare (ignore database db-type))
-  (if args
-      (format nil "CHAR(~A)" (car args))
-      (format nil "VARCHAR(~D)" *default-string-length*)))
-
 (deftype universal-time ()
   "A positive integer as returned by GET-UNIVERSAL-TIME."
   '(integer 1 *))
 
-(defmethod database-get-type-specifier ((type (eql 'universal-time)) args database db-type)
-  (declare (ignore args database db-type))
-  "BIGINT")
-
-(defmethod database-get-type-specifier ((type (eql 'wall-time)) args database db-type)
-  (declare (ignore args database db-type))
-  "TIMESTAMP")
-
-(defmethod database-get-type-specifier ((type (eql 'date)) args database db-type)
-  (declare (ignore args database db-type))
-  "DATE")
-
-(defmethod database-get-type-specifier ((type (eql 'duration)) args database db-type)
-  (declare (ignore database args db-type))
-  "VARCHAR")
-
-(defmethod database-get-type-specifier ((type (eql 'money)) args database db-type)
-  (declare (ignore database args db-type))
-  "INT8")
+(deftype generalized-boolean ()
+  "A type which outputs a SQL boolean value, though any lisp type can be stored in the slot."
+  t)
 
 #+ignore
 (deftype char (&optional len)
   "A lisp type for the SQL CHAR type."
   `(string ,len))
 
-(defmethod database-get-type-specifier ((type (eql 'float)) args database db-type)
-  (declare (ignore database db-type))
-  (if args
-      (format nil "FLOAT(~A)" (car args))
-      "FLOAT"))
-
-(defmethod database-get-type-specifier ((type (eql 'long-float)) args database db-type)
-  (declare (ignore database db-type))
-  (if args
-      (format nil "FLOAT(~A)" (car args))
-      "FLOAT"))
-
-(deftype generalized-boolean ()
-  "A type which outputs a SQL boolean value, though any lisp type can be stored in the slot."
-  t)
-
-(defmethod database-get-type-specifier ((type (eql 'boolean)) args database db-type)
-  (declare (ignore args database db-type))
-  "BOOL")
-
-(defmethod database-get-type-specifier ((type (eql 'generalized-boolean)) args database db-type)
+(defmethod database-get-type-specifier ((type string) args database (db-type t))
+  "Pass through the literal type as defined in the type string"
   (declare (ignore args database db-type))
-  "BOOL")
-
-(defmethod database-get-type-specifier ((type (eql 'number)) args database db-type)
-  (declare (ignore database db-type))
-  (cond
-    ((and (consp args) (= (length args) 2))
-     (format nil "NUMBER(~D,~D)" (first args) (second args)))
-    ((and (consp args) (= (length args) 1))
-     (format nil "NUMBER(~D)" (first args)))
+  type)
+
+
+(defmethod database-get-type-specifier ((type symbol) args database db-type)
+  (case type
+    (char (if args
+              (format nil "CHAR(~D)" (first args))
+              "CHAR(1)"))
+    ((varchar string symbol keyword)
+     (if args
+         (format nil "VARCHAR(~A)" (car args))
+         (format nil "VARCHAR(~D)" *default-string-length*)))
+    ((longchar text) "text")
+    (integer (if args
+                 (format nil "INT(~A)" (car args))
+                 "INT"))
+    ((tinyint smallint mediumint) "INT")
+    ((long-float float)
+     (if args
+         (format nil "FLOAT(~A)" (car args))
+         "FLOAT"))
+    ((bigint universal-time) "BIGINT")
+    (number
+     (cond
+       ((and (consp args) (= (length args) 2))
+        (format nil "NUMBER(~D,~D)" (first args) (second args)))
+       ((and (consp args) (= (length args) 1))
+        (format nil "NUMBER(~D)" (first args)))
+       (t
+        "NUMBER")))
+    (wall-time "TIMESTAMP")
+    (date "DATE")
+    (duration "VARCHAR")
+    (money "INT8")
+    ((boolean generalized-boolean) "BOOL")
+    (t (warn "Could not determine a valid ~A type specifier for ~A ~A ~A, defaulting to VARCHAR "
+             db-type type args database)
+     (format nil "VARCHAR(~D)" *default-string-length*))))
+
+(defun print-readable-symbol (in &aux (*package* (find-package :keyword))
+                                 (*print-readably* t))
+  (prin1-to-string in))
+
+(defmethod database-output-sql-as-type
+    (type val database db-type
+     &aux
+     (*print-circle* t) (*print-array* t)
+     (*print-length* nil) (*print-base* #10r10))
+  (declare (ignore database))
+  (cond 
+    ((null type) val)
+    ((member type '(boolean generalized-boolean))
+     ;; booleans handle null differently
+     (case db-type
+       ;; done here so it can be done once
+       ((:mssql :mysql) (if val 1 0))
+       (otherwise (if val "t" "f"))))
+    ((null val)
+     (when (next-method-p)
+       (call-next-method)))
     (t
-     "NUMBER")))
-
-(defmethod database-get-type-specifier ((type (eql 'char)) args database db-type)
-  (declare (ignore database db-type))
-  (if args
-      (format nil "CHAR(~D)" (first args))
-      "CHAR(1)"))
-
-
-(defmethod database-output-sql-as-type (type val database db-type)
-  (declare (ignore type database db-type))
-  val)
-
-(defmethod database-output-sql-as-type ((type (eql 'list)) val database db-type)
-  (declare (ignore database db-type))
-  (progv '(*print-circle* *print-array*) '(t t)
-    (let ((escaped (prin1-to-string val)))
-      (substitute-char-string
-       escaped #\Null " "))))
-
-(defmethod database-output-sql-as-type ((type (eql 'symbol)) val database db-type)
-  (declare (ignore database db-type))
-  (if val
-      (concatenate 'string
-                   (package-name (symbol-package val))
-                   "::"
-                   (symbol-name val))
-      ""))
-
-(defmethod database-output-sql-as-type ((type (eql 'keyword)) val database db-type)
-  (declare (ignore database db-type))
-  (if val
-      (symbol-name val)
-      ""))
-
-(defmethod database-output-sql-as-type ((type (eql 'vector)) val database db-type)
-  (declare (ignore database db-type))
-  (progv '(*print-circle* *print-array*) '(t t)
-    (prin1-to-string val)))
-
-(defmethod database-output-sql-as-type ((type (eql 'array)) val database db-type)
-  (declare (ignore database db-type))
-  (progv '(*print-circle* *print-array*) '(t t)
-    (prin1-to-string val)))
-
-(defmethod database-output-sql-as-type ((type (eql 'boolean)) val database db-type)
-  (declare (ignore database db-type))
-  (if val "t" "f"))
-
-(defmethod database-output-sql-as-type ((type (eql 'generalized-boolean)) val database db-type)
-  (declare (ignore database db-type))
-  (if val "t" "f"))
-
-(defmethod database-output-sql-as-type ((type (eql 'string)) val database db-type)
-  (declare (ignore database db-type))
-  val)
-
-(defmethod database-output-sql-as-type ((type (eql 'char)) val database db-type)
-  (declare (ignore database db-type))
-  (etypecase val
-    (character (write-to-string val))
-    (string val)))
-
-(defmethod database-output-sql-as-type ((type (eql 'float)) val database db-type)
-  (declare (ignore database db-type))
-  (if (eq (type-of val) 'null)
-      nil
-      (let ((*read-default-float-format* (type-of val)))
-       (format nil "~F" val))))
+     (case type
+       ((or symbol keyword)
+        (print-readable-symbol val))
+       (string val)
+       (char (etypecase val
+               (character (write-to-string val))
+               (string val)))
+       (float (format nil "~F" val))
+       ((list vector array)
+        (prin1-to-string val))
+       (otherwise
+        (if (next-method-p)
+            (call-next-method)
+            val))))))
+
+
+(defmethod read-sql-value :around
+    (val type database db-type
+     ;; never eval while reading values, always read base 10
+     &aux *read-eval* (*read-base* #10r10))
+  (declare (ignore db-type))
+  (cond
+    ;; null value or type
+    ((or (equalp "nil" val) (eql 'null val)) nil) 
+    
+    ;; no specified type or already the right type
+    ((or (null type)
+         (ignore-errors (typep val type)))
+     val)
+
+    ;; actually convert
+    (t 
+     (let ((res (handler-bind
+                    ;; all errors should be converted to sql-value-conversion-error
+                    ((error (lambda (c)
+                              (when *debugger-hook*
+                                (invoke-debugger c))
+                              (unless (typep c 'sql-value-conversion-error)
+                                (error-converting-value val type database)))))
+                  (call-next-method))))
+       ;; if we didnt get the right type after converting, we should probably
+       ;; error right away
+       (maybe-error-converting-value
+        res val type database)))))
 
 (defmethod read-sql-value (val type database db-type)
-  (declare (ignore database db-type))
-  (cond
-    ((null type) val) ;;we have no desired type, just give the value
-    ((typep val type) val) ;;check that it hasn't already been converted.
-    ((typep val 'string) (read-from-string val)) ;;maybe read will just take care of it?
-    (T (error "Unable to read-sql-value ~a as type ~a" val type))))
-
-(defmethod read-sql-value (val (type (eql 'string)) database db-type)
-  (declare (ignore database db-type))
-  val)
-
-(defmethod read-sql-value (val (type (eql 'varchar)) database db-type)
-  (declare (ignore database db-type))
-  val)
-
-(defmethod read-sql-value (val (type (eql 'char)) database db-type)
-  (declare (ignore database db-type))
-  (schar val 0))
-
-(defmethod read-sql-value (val (type (eql 'keyword)) database db-type)
-  (declare (ignore database db-type))
-  (when (< 0 (length val))
-    (intern (symbol-name-default-case val)
-            (find-package '#:keyword))))
-
-(defmethod read-sql-value (val (type (eql 'symbol)) database db-type)
-  (declare (ignore database db-type))
-  (when (< 0 (length val))
-    (unless (string= val (symbol-name-default-case "NIL"))
-      (read-from-string val))))
-
-(defmethod read-sql-value (val (type (eql 'integer)) database db-type)
-  (declare (ignore database db-type))
-  (etypecase val
-    (string
-     (unless (string-equal "NIL" val)
-       (parse-integer val)))
-    (number val)))
-
-(defmethod read-sql-value (val (type (eql 'smallint)) database db-type)
-  (declare (ignore database db-type))
-  (etypecase val
-    (string
-     (unless (string-equal "NIL" val)
-       (parse-integer val)))
-    (number val)))
-
-(defmethod read-sql-value (val (type (eql 'bigint)) database db-type)
-  (declare (ignore database db-type))
-  (etypecase val
-    (string
-     (unless (string-equal "NIL" val)
-       (parse-integer val)))
-    (number val)))
-
-(defmethod read-sql-value (val (type (eql 'float)) database db-type)
-  (declare (ignore database db-type))
-  ;; writing 1.0 writes 1, so we we *really* want a float, must do (float ...)
-  (etypecase val
-    (string (float (read-from-string val)))
-    (float val)))
-
-(defmethod read-sql-value (val (type (eql 'double-float)) database db-type)
-  (declare (ignore database db-type))
-  ;; writing 1.0 writes 1, so if we *really* want a float, must do (float ...)
-  (etypecase val
-    (string (float
-            (let ((*read-default-float-format* 'double-float))
-              (read-from-string val))
-            1.0d0))
-    (double-float val)
-    (float (coerce val 'double-float))))
-
-(defmethod read-sql-value (val (type (eql 'boolean)) database db-type)
-  (declare (ignore database db-type))
-  (equal "t" val))
-
-(defmethod read-sql-value (val (type (eql 'generalized-boolean)) database db-type)
-  (declare (ignore database db-type))
-  (equal "t" val))
-
-(defmethod read-sql-value (val (type (eql 'number)) database db-type)
-  (declare (ignore database db-type))
-  (etypecase val
-    (string
-     (unless (string-equal "NIL" val)
-       (read-from-string val)))
-    (number val)))
-
-(defmethod read-sql-value (val (type (eql 'universal-time)) database db-type)
-  (declare (ignore database db-type))
-  (unless (eq 'NULL val)
-    (etypecase val
-      (string
-       (parse-integer val))
-      (number val))))
-
-(defmethod read-sql-value (val (type (eql 'wall-time)) database db-type)
-  (declare (ignore database db-type))
-  (unless (eq 'NULL val)
-    (parse-timestring val)))
-
-(defmethod read-sql-value (val (type (eql 'date)) database db-type)
-  (declare (ignore database db-type))
-  (unless (eq 'NULL val)
-    (parse-datestring val)))
-
-(defmethod read-sql-value (val (type (eql 'duration)) database db-type)
-  (declare (ignore database db-type))
-  (unless (or (eq 'NULL val)
-              (equal "NIL" val))
-    (parse-timestring val)))
+  ;; errors, nulls and preconverted types are already handled in around
+  (typecase type
+    (symbol
+     (case type
+       ((string varchar) val)
+       (char (string (schar val 0)))
+       ((or keyword symbol)
+        (read-from-string val))
+       ((smallint mediumint bigint integer universal-time)
+        (parse-integer val))
+       ((double-float float)
+        ;; ensure that whatever we got is coerced to a float of the correct
+        ;; type (eg: 1=>1.0d0)
+        (float
+         (etypecase val
+           (string (let ((*read-default-float-format*
+                           (ecase type
+                             (float 'single-float)
+                             (double-float 'double-float))))
+                     (read-from-string val)))
+           ;; maybe wrong type of float
+           (float val)) 
+         (if (eql type 'double-float) 1.0d0 1.0s0)))
+       (number (read-from-string val))
+       ((boolean generalized-boolean)
+        (if (member val '(nil t))
+            val
+            (etypecase val
+              (string
+               (when (member val '("1" "t" "true" "y") :test #'string-equal)
+                 t))
+              (number (not (zerop val))))))
+       ((wall-time duration) (parse-timestring val))
+       (date (parse-datestring val))
+       (t (call-next-method))))
+    (t (typecase val
+         (string (read-from-string val))
+         (t (error-converting-value val type database))))))
 
 ;; ------------------------------------------------------------
 ;; Logic for 'faulting in' :join slots
   "The default value to use for the MAX-LEN keyword argument to
   UPDATE-OBJECT-JOINS.")
 
-(defun update-objects-joins (objects &key (slots t) (force-p t)
-                             class-name (max-len
-                                         *default-update-objects-max-len*))
-  "Updates from the records of the appropriate database tables
-the join slots specified by SLOTS in the supplied list of View
-Class instances OBJECTS.  SLOTS is t by default which means that
-all join slots with :retrieval :immediate are updated. CLASS-NAME
-is used to specify the View Class of all instance in OBJECTS and
-default to nil which means that the class of the first instance
-in OBJECTS is used. FORCE-P is t by default which means that all
-join slots are updated whereas a value of nil means that only
-unbound join slots are updated. MAX-LEN defaults to
-*DEFAULT-UPDATE-OBJECTS-MAX-LEN* and when non-nil specifies that
-UPDATE-OBJECT-JOINS may issue multiple database queries with a
-maximum of MAX-LEN instances updated in each query."
+(defun %update-objects-joins-slot-defs (class slot-names)
+  "Get the slot definitions for the joins slots specified as slot-names
+   if slot-names is :immediate, :deferred or (or :all t) return all of
+   that type of slot definitions"
+  (setf class (to-class class))
+  (when (eq t slot-names) (setf slot-names :all))
+  (etypecase slot-names
+    (null nil)
+    (keyword
+     ;; slot-names is the retrieval type of the join-slot or :all
+     (get-join-slots class slot-names))
+    ((or symbol list)
+     (loop for slot in (listify slot-names)
+           for def = (find-slot-by-name class slot)
+           when (and def (join-slot-p def))
+           collecting def
+           unless (and def (join-slot-p def))
+           do (warn "Unable to find join slot named ~S in class ~S." slot class)))))
+
+(defun get-joined-objects (objects slotdef &key force-p
+                                           (batch-size *default-update-objects-max-len*))
+  "Given a list of objects and a join slot-def get the objects that need to be
+   joined to the input objects
+
+   we will query in batches as large as batch-size"
+  (when (join-slot-p slotdef)
+    (let* ((slot-name (to-slot-name slotdef))
+           (join-class (join-slot-class-name slotdef))
+           (home-key (join-slot-info-value slotdef :home-key))
+           (foreign-key (join-slot-info-value slotdef :foreign-key))
+           (foreign-key-values
+             (remove-duplicates
+              (loop for object in (listify objects)
+                    for hk = (slot-value object home-key)
+                    when (or force-p
+                             (not (slot-boundp object slot-name)))
+                    collect hk)
+              :test #'equal)))
+      ;; we want to retrieve at most batch-size objects per query
+      (flet ((fetch (keys)
+               (find-all
+                (list join-class)
+                :where (make-instance
+                        'sql-relational-exp
+                        :operator 'in
+                        :sub-expressions (list (sql-expression :attribute foreign-key)
+                                               keys))
+                :result-types :auto
+                :flatp t)))
+        (if (null batch-size)
+            (fetch foreign-key-values)
+            (loop
+              for keys = (pop-n foreign-key-values batch-size)
+              while keys
+              nconcing (fetch keys)))))))
+
+(defun %object-joins-from-list (object slot joins force-p )
+  "Given a list of objects that we are trying to join to, pull the correct
+   ones for this object"
+  (when (or force-p (not (slot-boundp object (to-slot-name slot))))
+    (let ((home-key (join-slot-info-value slot :home-key))
+          (foreign-key (join-slot-info-value slot :foreign-key)))
+      (loop for join in joins
+            when (equal (slot-value join foreign-key)
+                        (slot-value object home-key))
+            collect join))))
+
+(defun update-objects-joins (objects &key (slots :immediate) (force-p t)
+                                     class-name (max-len *default-update-objects-max-len*))
+  "Updates from the records of the appropriate database tables the join slots
+   specified by SLOTS in the supplied list of View Class instances OBJECTS.
+
+   A simpler method of causing a join-slot to be requeried is to set it to
+   unbound, then request it again.  This function has efficiency gains where
+   join-objects are shared among the `objects` (querying all join-objects,
+   then attaching them appropriately to each of the `objects`)
+
+   SLOTS can be one of:
+
+    * :immediate (DEFAULT) - refresh join slots created with :retrieval :immediate
+    * :deferred - refresh join slots created with :retrieval :deferred
+    * :all,t - refresh all join slots regardless of :retrieval
+    * list of symbols - which explicit slots to refresh
+    * a single symobl - what slot to refresh
+
+   CLASS-NAME is used to specify the View Class of all instance in OBJECTS and
+   default to nil which means that the class of the first instance in OBJECTS
+   is used.
+
+   FORCE-P is t by default which means that all join slots are updated whereas
+   a value of nil means that only unbound join slots are updated.
+
+   MAX-LEN defaults to *DEFAULT-UPDATE-OBJECTS-MAX-LEN* When non-nil this is
+   essentially a batch size for the max number of objects to query from the
+   database at a time.  If we need more than max-len we loop till we have all
+   the objects"
   (assert (or (null max-len) (plusp max-len)))
   (when objects
-    (unless class-name
-      (setq class-name (class-name (class-of (first objects)))))
+    (defaulting class-name (class-name (class-of (first objects))))
     (let* ((class (find-class class-name))
-           (class-slots (ordered-class-slots class))
-           (slotdefs
-            (if (eq t slots)
-                (generate-retrieval-joins-list class :deferred)
-                (remove-if #'null
-                           (mapcar #'(lambda (name)
-                                       (let ((slotdef (find name class-slots :key #'slot-definition-name)))
-                                         (unless slotdef
-                                           (warn "Unable to find slot named ~S in class ~S." name class))
-                                         slotdef))
-                                   slots)))))
-      (dolist (slotdef slotdefs)
-        (let* ((dbi (view-class-slot-db-info slotdef))
-               (slotdef-name (slot-definition-name slotdef))
-               (foreign-key (gethash :foreign-key dbi))
-               (home-key (gethash :home-key dbi))
-               (object-keys
-                (remove-duplicates
-                 (if force-p
-                     (mapcar #'(lambda (o) (slot-value o home-key)) objects)
-                     (remove-if #'null
-                                (mapcar
-                                 #'(lambda (o) (if (slot-boundp o slotdef-name)
-                                                   nil
-                                                   (slot-value o home-key)))
-                                 objects)))))
-               (n-object-keys (length object-keys))
-               (query-len (or max-len n-object-keys)))
-
-          (do ((i 0 (+ i query-len)))
-              ((>= i n-object-keys))
-            (let* ((keys (if max-len
-                             (subseq object-keys i (min (+ i query-len) n-object-keys))
-                             object-keys))
-                   (results (unless (gethash :target-slot dbi)
-                              (find-all (list (gethash :join-class dbi))
-                                        :where (make-instance 'sql-relational-exp
-                                                              :operator 'in
-                                                              :sub-expressions (list (sql-expression :attribute foreign-key)
-                                                                                     keys))
-                                        :result-types :auto
-                                        :flatp t)) ))
-
-              (dolist (object objects)
-                (when (or force-p (not (slot-boundp object slotdef-name)))
-                  (let ((res (if results
-                                 (remove-if-not #'(lambda (obj)
-                                                    (equal obj (slot-value
-                                                                object
-                                                                home-key)))
-                                                results
-                                                :key #'(lambda (res)
-                                                         (slot-value res
-                                                                     foreign-key)))
-
-                                 (progn
-                                   (when (gethash :target-slot dbi)
-                                     (fault-join-target-slot class object slotdef))))))
-                    (when res
-                      (setf (slot-value object slotdef-name)
-                            (if (gethash :set dbi) res (car res)))))))))))))
+           (slotdefs (%update-objects-joins-slot-defs class slots)))
+      (loop for slotdef in slotdefs
+            ;; all the joins we will need for *all* the objects
+            ;; which then get filtered below for each object
+            for joins = (unless (join-slot-info-value slotdef :target-slot)
+                          (get-joined-objects objects slotdef
+                                              :force-p force-p :batch-size max-len))
+            do (loop for object in objects
+                     for these-joins = ;; the joins just for this object (filtered from above)
+                                       ;; or retrieved via fault-join-target-slot
+                        (or (%object-joins-from-list object slotdef joins force-p)
+                            (when (join-slot-info-value slotdef :target-slot)
+                              (fault-join-target-slot class object slotdef)))
+                     ;; when this object has joined-objects copy them in to the correct slot
+                     do (when these-joins
+                          (setf (easy-slot-value object slotdef)
+                                (if (join-slot-info-value slotdef :set)
+                                    these-joins
+                                    (first these-joins))))))))
   (values))
 
 (defun fault-join-slot-raw (class object slot-def)
@@ -983,8 +882,6 @@ maximum of MAX-LEN instances updated in each query."
         (select jc :where jq :flatp t :result-types nil
                 :database (choose-database-for-instance object))))))
 
-
-
 (defun fault-join-slot (class object slot-def)
   (let* ((dbi (view-class-slot-db-info slot-def))
          (ts (gethash :target-slot dbi))
@@ -1091,29 +988,79 @@ maximum of MAX-LEN instances updated in each query."
           collect tbl into rtn
           finally (return rtn))))
 
-(defun full-select-list ( classes )
-  "Returns a list of sql-ref of things to select for the given classes
 
-   THIS NEEDS TO MATCH THE ORDER OF build-objects
+(defclass select-list ()
+  ((view-class :accessor view-class :initarg :view-class :initform nil)
+   (select-list :accessor select-list :initarg :select-list :initform nil)
+   (slot-list :accessor slot-list :initarg :slot-list :initform nil)
+   (joins :accessor joins :initarg :joins :initform nil)
+   (join-slots :accessor join-slots :initarg :join-slots :initform nil))
+  (:documentation
+   "Collects the classes, slots and their respective sql representations
+    so that update-instance-from-recors, find-all, build-objects can share this
+    info and calculate it once.  Joins are select-lists for each immediate join-slot
+    but only if make-select-list is called with do-joins-p"))
+
+(defmethod view-table ((o select-list))
+  (view-table (view-class o)))
 
-   TODO: this used to include order-by and distinct as more things to select.
-    distinct seems to always be used in a boolean context, so it doesnt seem
-    like appending it to the select makes any sense
+(defmethod sql-table ((o select-list))
+  (sql-expression :table (view-table o)))
+
+(defmethod filter-select-list ((c clsql-sys::standard-db-object)
+                               (sl clsql-sys::select-list)
+                               database)
+  sl)
+
+(defun make-select-list (class-and-slots &key (do-joins-p nil)
+                                         (database *default-database*))
+  "Make a select-list for the current class (or class-and-slots) object."
+  (let* ((class-and-slots
+           (etypecase class-and-slots
+             (class-and-slots class-and-slots)
+             ((or symbol standard-db-class)
+              ;; find the first class with slots for us to select (this should be)
+              ;; the first of its classes / parent-classes with slots
+              (first (reverse (view-classes-and-storable-slots
+                               (to-class class-and-slots)
+                                :to-database-p nil))))))
+         (class (view-class class-and-slots))
+         (join-slots (when do-joins-p (immediate-join-slots class))))
+    (multiple-value-bind (slots sqls)
+        (loop for slot in (slot-defs class-and-slots)
+              for sql = (generate-attribute-reference class slot)
+              collect slot into slots
+              collect sql into sqls
+              finally (return (values slots sqls)))
+      (unless slots
+        (error "No slots of type :base in view-class ~A" (class-name class)))
+      (let ((sl (make-instance
+                 'select-list
+                 :view-class class
+                 :select-list sqls
+                 :slot-list slots
+                 :join-slots join-slots
+                 ;; only do a single layer of join objects
+                 :joins (when do-joins-p
+                          (loop for js in join-slots
+                                collect (make-select-list
+                                         (join-slot-class js)
+                                         :do-joins-p nil
+                                         :database database))))))
+        (filter-select-list (make-instance class) sl database)
+        sl))))
+
+(defun full-select-list ( select-lists )
+  "Returns a list of sql-ref of things to select for the given classes
 
-    We also used to remove duplicates, but that seems like it would make
-    filling/building objects much more difficult so skipping for now...
+   THIS NEEDS TO MATCH THE ORDER OF build-objects
   "
-  (setf classes (mapcar #'to-class (listify classes)))
-  (mapcar
-   #'cdr
-   (loop for class in classes
-         appending (generate-selection-list class)
-         appending
-            (loop for join-slot in (immediate-join-slots class)
-                  for join-class = (join-slot-class-name join-slot)
-                  appending (generate-selection-list join-class)))))
-
-(defun build-objects (classes row database &optional existing-instances)
+  (loop for s in (listify select-lists)
+        appending (select-list s)
+        appending (loop for join in (joins s)
+                        appending (select-list join))))
+
+(defun build-objects (select-lists row database &optional existing-instances)
   "Used by find-all to build objects.
 
    THIS NEEDS TO MATCH THE ORDER OF FULL-SELECT-LIST
@@ -1130,21 +1077,23 @@ maximum of MAX-LEN instances updated in each query."
       would be multiple rows per object, but we would return an object per row
    "
   (setf existing-instances (listify existing-instances))
-  (loop for class in classes
-        for existing = (pop existing-instances)
-        for object = (or existing
-                         (make-instance class :view-database database))
-        do (loop for (slot . _) in (generate-selection-list class)
-                 do (update-slot-from-db-value object slot (pop row)))
-        do (loop for join-slot in (immediate-join-slots class)
-                 for join-class = (join-slot-class-name join-slot)
-                 for join-object =
-                     (setf
-                      (easy-slot-value object join-slot)
+  (loop
+    for select-list in select-lists
+    for class = (view-class select-list)
+    for existing = (pop existing-instances)
+    for object = (or existing
+                     (make-instance class :view-database database))
+    do (loop for slot in (slot-list select-list)
+             do (update-slot-from-db-value object slot (pop row)))
+    do (loop for join-slot in (join-slots select-list)
+             for join in (joins select-list)
+             for join-class = (view-class join)
+             for join-object =
+                (setf (easy-slot-value object join-slot)
                       (make-instance join-class))
-                 do (loop for (slot . _) in (generate-selection-list join-class)
-                          do (update-slot-from-db-value join-object slot (pop row))))
-        do (when existing (instance-refreshed object))
+             do (loop for slot in (slot-list join)
+                      do (update-slot-from-db-value join-object slot (pop row))))
+    do (when existing (instance-refreshed object))
         collect object))
 
 (defun find-all (view-classes
@@ -1180,7 +1129,9 @@ maximum of MAX-LEN instances updated in each query."
            (loop for class in sclasses
                  appending (loop for slot in (immediate-join-slots class)
                                  collect (join-slot-qualifier class slot))))
-         (full-select-list (full-select-list sclasses))
+         (select-lists (loop for class in sclasses
+                             collect (make-select-list class :do-joins-p t :database database)))
+         (full-select-list (full-select-list select-lists))
          (where (clsql-ands (append (listify where) (listify join-where))))
          #|
           (_ (format t "~&sclasses: ~W~%ijc: ~W~%tables: ~W~%"
@@ -1195,7 +1146,7 @@ maximum of MAX-LEN instances updated in each query."
          (return-objects
            (loop for row in rows
                  for old-objs = (pop instances)
-                 for objs = (build-objects sclasses row database
+                 for objs = (build-objects select-lists row database
                                            (when refresh old-objs))
                  collecting (if flatp
                                 (delist-if-single objs)