WHERE clause in DB_DataObjects::update()

Hi all,

None of the keys on my tables are detected when I run
createTables.php. 
That is a problem I'll deal with later but it has
highlighted another 
problem in the DB_DataObjects::update() method.

DB_DataObjects::update() calls _build_condition() which
generates the 
"where" clause based on:

   1. the keys for the table, or
   2. all the columns in the table in the case where no keys
have been
      specified.

_build_condition() does not take into account the column
which is being 
updated - that is, it will add a "where" condition
for the column whose 
data has changed. Obviously that condition will never be
true.

I have made a few small changes which are in the patch
below.

Regards,

    Michael



Index: DataObject.php
============================================================
=======
RCS file: /repository/pear/DB_DataObject/DataObject.php,v
retrieving revision 1.422
diff -u -r1.422 DataObject.php
--- DataObject.php      11 Nov 2006 04:05:02 -0000     
1.422
+++ DataObject.php      15 Nov 2006 00:57:31 -0000
 -1169,6
+1169,7 
             $this->raiseError("update:No table
definition for 
{$this->__table}",
DB_DATAOBJECT_ERROR_INVALIDCONFIG);
             return false;
         }
+        $update_cols = array();
         $datasaved = 1;
         $settings  = '';
         $this->_connect();
 -1186,6
+1187,9 
             // dont write things that havent changed..
             if (($dataObject !== false) &&
isset($dataObject->$k) && 
($dataObject->$k === $this->$k)) {
                 continue;
+            } else {
+                // Add column to $update_cols so that it
isn't included 
in the "where" condition
+                $update_cols[] = $k;
             }

             // - dont write keys to left.!!!
 -1256,7
+1260,7 
             $this->debug("got keys as
".serialize($keys),3);
         }
         if ($dataObject !== true) {
-            $this->_build_condition($items,$keys);
+           
$this->_build_condition($items,$keys,$update_cols);
         } else {
             // prevent wiping out of data!
             if (empty($this->_query['condition'])) {

if you do this:

$oldDO = clone($do);
$do->someval = "xxx";
$do->update($oldDO);
it will only send the differences.

if you are in a situation where you have no keys, then
DB_DATAOBJECT_WHEREADD_ONLY, is the only way to do this, in
which case,
it's unlikely you have a previous dataobject, or the result
of the call
could be ambiguous.

Regards
Alan

Michael Henry wrote:
> Hi all,
>
> None of the keys on my tables are detected when I run
> createTables.php. That is a problem I'll deal with
later but it has
> highlighted another problem in the
DB_DataObjects::update() method.
>
> DB_DataObjects::update() calls _build_condition() which
generates the
> "where" clause based on:
>
>   1. the keys for the table, or
>   2. all the columns in the table in the case where no
keys have been
>      specified.
>
> _build_condition() does not take into account the
column which is
> being updated - that is, it will add a
"where" condition for the
> column whose data has changed. Obviously that condition
will never be
> true.
>
> I have made a few small changes which are in the patch
below.
>
> Regards,
>
>    Michael
>
>
>
> Index: DataObject.php
>
============================================================
=======
> RCS file:
/repository/pear/DB_DataObject/DataObject.php,v
> retrieving revision 1.422
> diff -u -r1.422 DataObject.php
> --- DataObject.php      11 Nov 2006 04:05:02 -0000     
1.422
> +++ DataObject.php      15 Nov 2006 00:57:31 -0000
>  -1169,6 +1169,7 
>             $this->raiseError("update:No table
definition for
> {$this->__table}",
DB_DATAOBJECT_ERROR_INVALIDCONFIG);
>             return false;
>         }
> +        $update_cols = array();
>         $datasaved = 1;
>         $settings  = '';
>         $this->_connect();
>  -1186,6 +1187,9 
>             // dont write things that havent changed..
>             if (($dataObject !== false) &&
isset($dataObject->$k) &&
> ($dataObject->$k === $this->$k)) {
>                 continue;
> +            } else {
> +                // Add column to $update_cols so that
it isn't
> included in the "where" condition
> +                $update_cols[] = $k;
>             }
>
>             // - dont write keys to left.!!!
>  -1256,7 +1260,7 
>             $this->debug("got keys as
".serialize($keys),3);
>         }
>         if ($dataObject !== true) {
> -            $this->_build_condition($items,$keys);
> +           
$this->_build_condition($items,$keys,$update_cols);
>         } else {
>             // prevent wiping out of data!
>             if (empty($this->_query['condition'])) {
>
>

-- 
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub
.php

Alan Knowles wrote:
> if you do this:
>
> $oldDO = clone($do);
> $do->someval = "xxx";
> $do->update($oldDO);
> it will only send the differences.
>   
Yep, this is what I am doing. The SET clause is fine; it's
just the 
WHERE clause that has a condition for the field whose data
has changed.

> if you are in a situation where you have no keys, then
> DB_DATAOBJECT_WHEREADD_ONLY, is the only way to do
this,
OK. Could you add this to the documentation for
DB_DataObject::update()? 
I've spent a couple of days trying to figure out why this
isn't behaving 
as I'd expect.
>  in which case,
> it's unlikely you have a previous dataobject, or the
result of the call
> could be ambiguous.
>   
I do have a previous dataobject because I know how to
uniquely identify 
my rows even though the unique/primary keys haven't been
detected by 
createTables.php. This raises another point: In the case
where a 
dataobject has been passed to update() the values from that
object 
should be used in the generation of the where clause. ie.
the fields 
from $this dataobject have changed so the "where"
clause using those 
values will fail.

Regards,

    Michael

> Regards
> Alan
>
> Michael Henry wrote:
>   
>> Hi all,
>>
>> None of the keys on my tables are detected when I
run
>> createTables.php. That is a problem I'll deal with
later but it has
>> highlighted another problem in the
DB_DataObjects::update() method.
>>
>> DB_DataObjects::update() calls _build_condition()
which generates the
>> "where" clause based on:
>>
>>   1. the keys for the table, or
>>   2. all the columns in the table in the case where
no keys have been
>>      specified.
>>
>> _build_condition() does not take into account the
column which is
>> being updated - that is, it will add a
"where" condition for the
>> column whose data has changed. Obviously that
condition will never be
>> true.
>>
>> I have made a few small changes which are in the
patch below.
>>
>> Regards,
>>
>>    Michael
>>
>>
>>
>> Index: DataObject.php
>>
============================================================
=======
>> RCS file:
/repository/pear/DB_DataObject/DataObject.php,v
>> retrieving revision 1.422
>> diff -u -r1.422 DataObject.php
>> --- DataObject.php      11 Nov 2006 04:05:02 -0000 
    1.422
>> +++ DataObject.php      15 Nov 2006 00:57:31 -0000
>>  -1169,6 +1169,7 
>>             $this->raiseError("update:No
table definition for
>> {$this->__table}",
DB_DATAOBJECT_ERROR_INVALIDCONFIG);
>>             return false;
>>         }
>> +        $update_cols = array();
>>         $datasaved = 1;
>>         $settings  = '';
>>         $this->_connect();
>>  -1186,6 +1187,9 
>>             // dont write things that havent
changed..
>>             if (($dataObject !== false) &&
isset($dataObject->$k) &&
>> ($dataObject->$k === $this->$k)) {
>>                 continue;
>> +            } else {
>> +                // Add column to $update_cols so
that it isn't
>> included in the "where" condition
>> +                $update_cols[] = $k;
>>             }
>>
>>             // - dont write keys to left.!!!
>>  -1256,7 +1260,7 
>>             $this->debug("got keys as
".serialize($keys),3);
>>         }
>>         if ($dataObject !== true) {
>> -           
$this->_build_condition($items,$keys);
>> +           
$this->_build_condition($items,$keys,$update_cols);
>>         } else {
>>             // prevent wiping out of data!
>>             if
(empty($this->_query['condition'])) {
>>
>>
>>     
>
>   

-- 
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub
.php

Looking at the code, I suspect the correct behavior is to
error out..
        if ($dataObject !== true) {
            $this->_build_condition($items,$keys);
        } else {
            // prevent wiping out of data!
            if (empty($this->_query['condition'])) {
                 $this->raiseError("update: global
table update not
available
                        do $do->whereAdd('1=1'); if you
really want to
do that.
                    ",
DB_DATAOBJECT_ERROR_INVALIDARGS);
                return false;
            }
        }

changed to.
         if (($dataObject !== true) && $keys) {
            $this->_build_condition($items,$keys);
        } else {
            // prevent wiping out of data!
            if (empty($this->_query['condition'])) {
                 $this->raiseError("update: global
table update not
available
                        do $do->whereAdd('1=1'); if you
really want to
do that.
                    ",
DB_DATAOBJECT_ERROR_INVALIDARGS);
                return false;
            }
        }
That way it forces you to either generate the keys (define
the keys() 
method/ fix the ini etc.), or use
DB_DATAOBJECT_WHEREADD_ONLY.

Regards
Alan



Michael Henry wrote:
>
> Alan Knowles wrote:
>> if you do this:
>>
>> $oldDO = clone($do);
>> $do->someval = "xxx";
>> $do->update($oldDO);
>> it will only send the differences.
>>   
> Yep, this is what I am doing. The SET clause is fine;
it's just the
> WHERE clause that has a condition for the field whose
data has changed.
>
>> if you are in a situation where you have no keys,
then
>> DB_DATAOBJECT_WHEREADD_ONLY, is the only way to do
this,
> OK. Could you add this to the documentation for
> DB_DataObject::update()? I've spent a couple of days
trying to figure
> out why this isn't behaving as I'd expect.
>>  in which case,
>> it's unlikely you have a previous dataobject, or
the result of the call
>> could be ambiguous.
>>   
> I do have a previous dataobject because I know how to
uniquely
> identify my rows even though the unique/primary keys
haven't been
> detected by createTables.php. This raises another
point: In the case
> where a dataobject has been passed to update() the
values from that
> object should be used in the generation of the where
clause. ie. the
> fields from $this dataobject have changed so the
"where" clause using
> those values will fail.
>
> Regards,
>
>    Michael
>
>> Regards
>> Alan
>>
>> Michael Henry wrote:
>>  
>>> Hi all,
>>>
>>> None of the keys on my tables are detected when
I run
>>> createTables.php. That is a problem I'll deal
with later but it has
>>> highlighted another problem in the
DB_DataObjects::update() method.
>>>
>>> DB_DataObjects::update() calls
_build_condition() which generates the
>>> "where" clause based on:
>>>
>>>   1. the keys for the table, or
>>>   2. all the columns in the table in the case
where no keys have been
>>>      specified.
>>>
>>> _build_condition() does not take into account
the column which is
>>> being updated - that is, it will add a
"where" condition for the
>>> column whose data has changed. Obviously that
condition will never be
>>> true.
>>>
>>> I have made a few small changes which are in
the patch below.
>>>
>>> Regards,
>>>
>>>    Michael
>>>
>>>
>>>
>>> Index: DataObject.php
>>>
============================================================
=======
>>> RCS file:
/repository/pear/DB_DataObject/DataObject.php,v
>>> retrieving revision 1.422
>>> diff -u -r1.422 DataObject.php
>>> --- DataObject.php      11 Nov 2006 04:05:02
-0000      1.422
>>> +++ DataObject.php      15 Nov 2006 00:57:31
-0000
>>>  -1169,6 +1169,7 
>>>            
$this->raiseError("update:No table definition for
>>> {$this->__table}",
DB_DATAOBJECT_ERROR_INVALIDCONFIG);
>>>             return false;
>>>         }
>>> +        $update_cols = array();
>>>         $datasaved = 1;
>>>         $settings  = '';
>>>         $this->_connect();
>>>  -1186,6 +1187,9 
>>>             // dont write things that havent
changed..
>>>             if (($dataObject !== false)
&& isset($dataObject->$k) &&
>>> ($dataObject->$k === $this->$k)) {
>>>                 continue;
>>> +            } else {
>>> +                // Add column to $update_cols
so that it isn't
>>> included in the "where" condition
>>> +                $update_cols[] = $k;
>>>             }
>>>
>>>             // - dont write keys to left.!!!
>>>  -1256,7 +1260,7 
>>>             $this->debug("got keys as
".serialize($keys),3);
>>>         }
>>>         if ($dataObject !== true) {
>>> -           
$this->_build_condition($items,$keys);
>>> +           
$this->_build_condition($items,$keys,$update_cols);
>>>         } else {
>>>             // prevent wiping out of data!
>>>             if
(empty($this->_query['condition'])) {
>>>
>>>
>>>     
>>
>>   
>

-- 
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub
.php