luislavena / mysql-gem

MySQL/Ruby Bindings, wrapped as Gem with improved cross-platform support
http://rubyforge.org/projects/mysql-win
Other
53 stars 20 forks source link

Incorrect conversion from ULongLong to Num #11

Open cbandy opened 11 years ago

cbandy commented 11 years ago
  1. mysql_affected_rows returns an unsigned long long, but affected_rows uses INT2NUM
  2. mysql_num_rows returns an unsigned long long, but
    • list_dbs and list_tables store it in an unsigned int
    • num_rows uses INT2NUM
  3. mysql_stmt_affected_rows returns an unsigned long long, but stmt_affected_rows uses INT2NUM
  4. mysql_stmt_num_rows returns an unsigned long long, but stmt_num_rows uses INT2NUM

These should use ULL2NUM, I believe, like insert_id does.

luislavena commented 11 years ago

@cbandy do you have any test that we can add to ensure this works as expected?

cbandy commented 11 years ago

I don't unfortunately. It may be possible to test both num_rows using a row generator, but affected_rows would require a table with that many rows.

Perhaps with a row generator in place, it wouldn't be hard to CREATE TABLE x AS SELECT ... FROM row_generator?

cbandy commented 11 years ago

I tried to write some tests, but they were too expensive. I killed the Mysql#affected_rows test at 7GiB of HDD space, and I killed the Mysql::Result#num_rows test at 6GiB of RAM and swap.

cbandy commented 11 years ago

What about num_rows?

luislavena commented 11 years ago

What about num_rows?

Kinda you forgot about that, right? was nowhere in your patch either :tongue:

Please open a separate issue and will work on that next weekend. Never encountered this issue before.

cbandy commented 11 years ago

I opened a new pull request and called it a "partial fix". I knew exactly what I was doing.

luislavena commented 11 years ago

Sorry, I'm not implying or blaming you.

Simply was not evident from the pull request or this first description.