j4mie / idiorm

A lightweight nearly-zero-configuration object-relational mapper and fluent query builder for PHP5.
http://j4mie.github.com/idiormandparis/
2.01k stars 369 forks source link

unexpected behavior and inconsistency of get_last_query and actual query. #307

Closed cwhsu1984 closed 7 years ago

cwhsu1984 commented 8 years ago

Hi,

Assume I have a table named "mytable" which has the following fields:

id  int(11) unsigned
uuid  char(36)
deleted_by varchar(80)

When using idiorm and make a query like

$query = ORM::for_table('mytable')
->where_in('uuid', $uuid)
->where('deleted_by', '')
->find_many();

Assume $uuid = [false] After execution, ORM::get_last_query() shows

SELECT * FROM `mytable` WHERE `uuid` IN ('') AND `deleted_by` = ''

However, when logging actual query from mysql, it shows

SELECT * FROM `mytable` WHERE `uuid` IN (0) AND `deleted_by` = ''

It becomes a disaster for me because I was actually selecting rows to be deleted. And it ends up deleting all my data. There are two issues in this situation:

  1. ORM::get_last_query() must be exactly the same query in mysql for debugging.
  2. I expect the query to be interpreted in the following ways:
SELECT * FROM `mytable` WHERE `uuid` IN ('') AND `deleted_by` = '';   // good
SELECT * FROM `mytable` WHERE `uuid` IN ('false') AND `deleted_by` = ''; // good, I guess
SELECT * FROM `mytable` WHERE `uuid` IN () AND `deleted_by` = ''; // fine, at least I know something is wrong.
SELECT * FROM `mytable` WHERE `uuid` IN (0) AND `deleted_by` = '' // unacceptable, because it gets all rows.
treffynnon commented 8 years ago

This looks like a case of GIGO to me. Suggest:

$query = ORM::for_table('mytable')
->where_in('uuid', $uuid ?: 'false')
->where('deleted_by', '')
->find_many();

or better type cast your input:

$query = ORM::for_table('mytable')
->where_in('uuid', (string) $uuid)
->where('deleted_by', '')
->find_many();
cwhsu1984 commented 8 years ago

What about get_last_query? If I knew the actual query, I would be able to find the exact issue faster.

treffynnon commented 8 years ago

The code that does the query log is an approximation of that provided by PDO/the database: https://github.com/j4mie/idiorm/blob/master/idiorm.php#L435

I think that there could be a pull request to change the Idiorm manual to highlight that this might have edge cases.

The actual query isn't even available to us to log as the database/PDO handles the binding outside of our reach and doesn't pass it back.

cwhsu1984 commented 8 years ago

A highlight to the edge cases would be great, at least the user should know that if they think something is definitely wrong, they should check the general_log in sql themselves. So they don't always trust the get_last_query blindly.

treffynnon commented 7 years ago

Fixed in develop