camertron/scuttle-server

scuttle doesn't convert binary values to binary

Opened this issue · 5 comments

SELECT * FROM studies WHERE studies.boolean_test = 't'

converts to:

Study.select(Arel.star).where(Study.arel_table[:boolean_test].eq('t'))

I believe you should special case t and f so that it converts to:

Study.select(Arel.star).where(Study.arel_table[:boolean_test].eq(true))

Oops, thanks @dankohn! I'll take a look.

Ok, so after a little research, I couldn't find anything saying that 't' and 'f' should be treated as Boolean values. What flavor of SQL are you using, and how would you test for string equality?

The Rails Postgres driver saves boolean values as t and f. I can't find the exact method that does so, but it's easy enough to play with in Rails Console.

http://www.rubydoc.info/docs/rails/ActiveRecord/ConnectionAdapters/Column#FALSE_VALUES-constant

The argument is that one of the main use cases of Scuttle is taking existing SQL generated by strings in Active Record, and running it through Scuttle to see what the Arel equivalent is.

So, while someone could be manually testing against the strings t and f, and that would be a false positive, by far the most likely case is that they were testing against a boolean. Unfortunately, the only way to know for sure would be to know the column type, which Scuttle doesn't have access to.

Hey @dankohn, after doing some playing around in a rails console connected to both a MySQL and a PostgreSQL database, I now better understand how boolean values are treated:

String Integer Boolean
MySQL "1" [varchar(255)] 1 [int(11)] 1 [tinyint(1)]
PostgreSQL "t" [varchar(255)] 1 [integer] "t" [boolean]

This was something I'd never seen before because I've never really used Postgres. It seems awfully strange that Postgres supports a native boolean data type but stores single character strings in them - why use a byte when a single bit will do? As you said, maybe this is a driver-specific choice, not sure.

In any case, I think the best I can do here is to add a check box or something to Scuttle's interface that enables/disables treatment of t and f as boolean values. While it's true that Scuttle's main use case is to process queries generated by ActiveRecord, I can't ignore the fact that not everybody uses the Postgres driver. Since Scuttle can't know the data type of any column or the driver that generated a particular query, it seems as if it could be quite surprising to treat t and f as boolean values by default.

Thanks for doing the research. I agree a checkbox would work best, but
would argue for checking it by default, since Postgres is a popular
database, and it's relatively unlikely to check against the strings "t" and
"f".

It would also be great to automatically replace the postgres quotes at the
same time. E.g.:

pry(main)> Study.pluck(:updated_at)
   (1.3ms)  SELECT "studies"."updated_at" FROM "studies"