mysql2 icon indicating copy to clipboard operation
mysql2 copied to clipboard

Prepared statements don't support true/false arguments properly

Open jeremyevans opened this issue 7 years ago • 4 comments

It results in BigDecimal 0 for both:

client.prepare('SELECT ? as a').execute(true).to_a.first[:a]
# /usr/local/lib/ruby/gems/2.3/gems/mysql2-0.4.4/lib/mysql2/statement.rb:8: warning: :cache_rows is forced for prepared statements (if not streaming)
# => #<BigDecimal:88803c38,'0.0',9(18)>
client.prepare('SELECT ? as a').execute(false).to_a.first[:a]
# /usr/local/lib/ruby/gems/2.3/gems/mysql2-0.4.4/lib/mysql2/statement.rb:8: warning: :cache_rows is forced for prepared statements (if not streaming)
# => #<BigDecimal:7e32c4a8,'0.0',9(18)>

I think the best behavior would be to treat true as 1 and false as 0, though I could understand raising an exception if you don't want to handle them.

jeremyevans avatar Jul 14 '16 16:07 jeremyevans

In this case, MySQL has no way of knowing that you intended a boolean, in fact there is no boolean type. http://dev.mysql.com/doc/refman/5.7/en/numeric-type-overview.html

MySQL can be asked to evaluate a value as true or false, where 0 is false and all other values are true. The values are numeric, and absent a column type to narrow down the field width, all numeric types fall back to BigDecimal as the most generic container.

sodabrew avatar Jul 14 '16 17:07 sodabrew

I am aware that MySQL does not have a boolean type. However, using 0 for false and 1 for true seems to be reasonable, as the :cast_booleans=>true setting returns 0 as false and 1 as true. Certainly returning 0 for both is quite surprising, wouldn't you agree?

I think handling T_TRUE/T_FALSE in the switch (https://github.com/brianmario/mysql2/blob/master/ext/mysql2/statement.c#L252) would make the most sense if you want to support true/false.

I think adding an else block that raises an exception makes sense (https://github.com/brianmario/mysql2/blob/master/ext/mysql2/statement.c#L336). Turns out this isn't a true/false issue, returning BigDecimal 0 is how all unhandled types are currently handled. If you pass Object.new to Statement#execute, you get the same thing. That is likely to hide bugs, and I doubt is it behavior that users' want.

jeremyevans avatar Jul 14 '16 18:07 jeremyevans

So, two separate issues, if I understand correctly:

  • Providing true or false into a Statement#execute should be cast to integers 0 or 1.
  • MySQL returning an unhandled type should raise an exception rather than cast to BigDecimal. This is actually a weird one, I thought unhandled types came back as Strings.

sodabrew avatar Jul 14 '16 18:07 sodabrew

Correct. Maybe true/false should only be handled if the client has :cast_booleans=>true, but I wouldn't have a problem making it unconditional.

jeremyevans avatar Jul 14 '16 19:07 jeremyevans