View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
412PHPOF2Defaultpublic25 Jun 2012 12:225 Jul 2012 21:16
Reporteriarmeanu Assigned To 
PrioritynormalSeverityminorReproducibilityalways
Status confirmedResolutionopen 
Summary412: Set retrieved_ok=false if PHPOF2_DBrow::get() retrieves row where the ID does not match that passed (e.g. due to DB typecasting)
DescriptionCase 1:
   we have a row with an integer primary key ( let's say, 100 )
   we call $row_object->get( '100random_string_here' )
   we get back a valid object ( the row object with id 100 )
   
   expected correct behaviour: return an object with all properties ( including the id ) set to null ( and retrieved_ok = false )

Case 2:
   we have a row with an integer primary key ( let's say, 100 )
   we call $row_object->get( 'random_string_here' )
   we get back an object with id == 'random_string_here' and the rest of the properties set to null

   expected correct behaviour: return an object with all properties ( including the id ) set to null. The id property should never be populated with the parameter given to the get() function ( but with the value coming from the database, and only if it was retrieved ok )
Steps To Reproduceuse the following test script:

<?php
 
$db = Objlib::get( Objlib::TYPE_DB );

require_once 'PHPOF2/DB.php';
require_once 'PHPOF2/DBTable.php';
require_once 'PHPOF2/DBRow.php';

class int_test_obj extends PHPOF2_DBTable
{

    protected $default_row_class = 'int_test_obj_row';
    
}

class int_test_obj_row extends PHPOF2_DBRow
{
    
}

$sql = "CREATE TABLE IF NOT EXISTS `int_test_obj` (
          `id` int(11) NOT NULL AUTO_INCREMENT,
          `property_one` varchar(20) NOT NULL,
          `property_two` varchar(50) NOT NULL,
          PRIMARY KEY (`id`)
        ) ENGINE=InnoDB DEFAULT CHARSET=latin1 AUTO_INCREMENT=1";
$db->query( $sql );
$sql = "INSERT INTO int_test_obj VALUES( '', 'SOME VALUE', 'some other value' )";
$db->query( $sql );
echo "insert sql: \n";
echo $sql . "\n";

$table = new int_test_obj( $db, 'int_test_obj' );
$row = $table->createRowObject();

$sql = 'SELECT max( id ) as max_id FROM int_test_obj';
$r_set = $db->query( $sql );

while( $max_id = $r_set->fetchRow() ) {
    
    echo 'running $row->get( "' . $max_id[0] . 'some dummy text following the id" );' . "...\n";
    $row->get( "{$max_id[0]}zzfdrg" );
    echo $row->id . "\n";
    var_dump( $row->getSimpleObject() );
    
        $row->clear();

    echo 'running $row->get( "some dummy text, no id" );' . "...\n";
    $row->get( "some dummy text, no id" );
    echo $row->id . "\n";
    var_dump( $row->getSimpleObject() );
    
}

?>
TagsNo tags attached.

Activities

iarmeanu

25 Jun 2012 15:30

reporter   ~453

After analysing the issue a bit more, it turns out that the query that gets passed to mySQL server is right ( in the way that it writes something like '... WHERE id='20random_string_here' ), but mySQL automatically casts strings to INTs when applicable [ there might be a way to stop this by enabling/disabling some config options, see http://dev.mysql.com/doc/refman/4.1/en/type-conversion.html for more info ] but either way, PHPOF should populate the id field of the returned object with the id that's coming from the database rather than whatever it gets from the parameter. In Case 2 it should probably nulify the value as well.

timj

25 Jun 2012 19:54

manager   ~454

Hi! Thanks for the report. Interesting points!

1.
a)You're right that the typecasting behaviour of MySQL causes some unexpected results here. There is some way to get a warning if the result of the SELECT is caused by a cast, but I would be reluctant to change the behaviour here (for example to set retrieved_ok=false) after so many years.

b)as regards filling the property of the key with the actual returned value, this seems like a good idea although again it has the risk of breaking expected behaviour. Input from other users would be welcome here!

2. Nullifying the value if failed to retrieve - once again, I think this is a good suggestion but it is changing longstanding behaviour. Again, input sought!

iarmeanu

26 Jun 2012 09:04

reporter   ~455

1.a) I believe that the expected behaviour was always like that. The fact that a matching row is returned is probably something that was not expected by the ( properly written ) code. If somebody is concerned about backward incompatibility ( although, I reiterate, in my opinion, this was always the expected behaviour ), then at least introduce a second parameter to the function, something like strict='true' ( false by default, to be backward compatible ). As for retrieved_ok = false, when using get(), one should always check for retrieved_ok, otherwise there's no guarantee that the object was filled in properly ( since currently, the id is filled in even if the object was not retrieved correctly ).

mkierat

3 Jul 2012 09:23

reporter   ~456

This will probably need to be backwards incompatible.

When the row is loaded it should always have the id that came from the DB - that's a must.

Once that is done a sanity check needs to be done:
a) in PHPOF2 - e.g. if the prikey from the db != the one passed in, then don't set the object/return null/set retrieved_ok=false or something to that extent OR
b) outside of PHPOF2 - where all code now needs to do an extra check after running get()

Option b seems like quite an overhead (and something people will just forget to do), so I would prefer option a.

timj

5 Jul 2012 21:10

manager   ~457

OK, I'm convinced about the need for some additional check here, and the fact that this is unexpected behaviour. Thanks for the clear summary Michał.
Will happily accept a patch that alters the behaviour like this:

- if the primary key of the returned row does not match that which is set in the object when calling get()
 -> don't fill in the rest of the object properties, and
 -> set retrieved_ok to false

Issue History

Date Modified Username Field Change
25 Jun 2012 12:22 iarmeanu New Issue
25 Jun 2012 15:30 iarmeanu Note Added: 453
25 Jun 2012 19:54 timj Note Added: 454
25 Jun 2012 20:07 timj Priority high => normal
25 Jun 2012 20:07 timj Severity major => minor
25 Jun 2012 20:07 timj Status new => acknowledged
26 Jun 2012 09:04 iarmeanu Note Added: 455
3 Jul 2012 09:23 mkierat Note Added: 456
5 Jul 2012 21:10 timj Note Added: 457
5 Jul 2012 21:16 timj Status acknowledged => confirmed
5 Jul 2012 21:16 timj Summary get() called from a row object behaves strangely => Set retrieved_ok=false if PHPOF2_DBrow::get() retrieves row where the ID does not match that passed (e.g. due to DB typecasting)