Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
cwhsu1984 opened this issue Oct 11, 2016 · 5 comments
Closed

Comments

@cwhsu1984
Copy link

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
Copy link
Collaborator

treffynnon commented Oct 11, 2016

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
Copy link
Author

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

@treffynnon
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

Fixed in develop

Repository owner locked and limited conversation to collaborators Dec 14, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants