Re: [patch] Updated Patch for fault-join-target-slot and update-objects-joins

Aleksandar Bakic wrote:
>>>Attached is an updated version of my previous patch which also fixes the 
>>>problems with update-objects-joins when using the :target-slot attribute.
>>
>>Thanks for the patch, Drew.
>>
>>I've applied it and released CLSQL 3.3.3 with your fix.
> 
> 
> Hi,
> 
> What should happen if the target slot is not a join slot at all, but a regular
> one? I am getting an error at this point in fault-join-target-slot:
> 
> 	 (tdbi (view-class-slot-db-info
> 		(find ts (class-slots (find-class jc)) 
> 		      :key #'slot-definition-name))) 
> 	 (retrieval (gethash :retrieval tdbi)) ; tdbi is nil

What is it that you are trying to do? :target-slot is specifically for 
many-to-many joins, and i can't see it making sense outside of them. it 
would probably be trivial to simply return the sql-slot-value of the 
:target-slot when it is not a join slot, but i can't see what value this 
gains.

FWIW, although the patch corrects 'can't find class' issue, f-j-t-s 
still doesn't work for my purposes as it doesn't fill in the primary 
keys (when using :deferred, none of the instances actually come from the 
database. when using :immediate (which i never use) the target class has 
"real" instances, but the join-class (hosting the many-to-many) just 
fills the joined keys, but does not get the actual rows from the 
database. Both of these behaviors i find completely useless. A 'regular' 
join slot returns 'real' instances that i can manipulate and 
UPDATE-RECORDS-FROM-INSTACE, where a many-to-many returns these 'pseudo' 
instances that do me no good at all.

I've modified my clsql f-j-t-s locally to use the following definition :

(defun fault-join-target-slot (class object slot-def)
   (let* ((dbi (view-class-slot-db-info slot-def))
	 (ts (gethash :target-slot dbi))
	 (jc  (gethash :join-class dbi))
	 (foo (warn "~A ~A " (class-slots (find-class jc)) ts ))
	 (jc-view-table (view-table (find-class jc)))
	 (tdbi (view-class-slot-db-info
		(find ts (class-slots (find-class jc))
		      :key #'slot-definition-name)))
	 (retrieval (gethash :retrieval tdbi))
	 (tsc (gethash :join-class tdbi))
	 (ts-view-table (view-table (find-class tsc)))
	 (jq (join-qualifier class object slot-def))
	 (key (slot-value object (gethash :home-key dbi))))

     (when jq
       (let ((res
		(find-all (list jc)
			  :inner-join (sql-expression :table ts-view-table)
			  :on (sql-operation
			       '==
			       (sql-expression
				:attribute (gethash :foreign-key tdbi)
				:table ts-view-table)
			       (sql-expression
				:attribute (gethash :home-key tdbi)
				:table jc-view-table))
			  :where jq
			  :result-types :auto
			  :database (view-database object))))
	   (mapcar #'(lambda (i)
		       (setf (slot-value (car i) 'view-database) (view-database object))
		       (cons (car
			      (select tsc
				      :where (sql-= (sql-slot-value tsc (gethash :foreign-key tdbi))
						    (slot-value (car i)  (gethash :home-key tdbi)))
				      :flatp t))
			     i))
		   res)))))


This 'works', although it hits the database a lot. I'd rather do a join, 
but clsql seems to offer no way of doing what i need to do (although my 
clsql-internals-fu is quite possibly not up to the task). I'm still 
hacking on this locally, but am using the above in production (with 
great success i might add).

The behavior of f-j-t-s as i've patched it locally, to me, seems the be 
the right-thing (tm). It is consistent with the rest of the OODML 
interface, and removes a major source of confusion. I'm not sure what 
behavior CommonSQL specified here, but the current behavior simply can't 
be right.

Does anybody actually use :join-slot with the current behavior and can 
justify keeping it that way? if not, i'd like to see my version, or 
something that returns the same instances, in 3.3.4. I'll even volunteer 
to update the documentation to remove any confusion about :target-slot :)

Thoughts?

drewc