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

scuttle doesn't convert binary values to binary #9

Open
dankohn opened this issue Nov 22, 2014 · 5 comments
Open

scuttle doesn't convert binary values to binary #9

dankohn opened this issue Nov 22, 2014 · 5 comments

Comments

@dankohn
Copy link

dankohn commented Nov 22, 2014

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))
@camertron
Copy link
Owner

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

@camertron
Copy link
Owner

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?

@dankohn
Copy link
Author

dankohn commented Nov 22, 2014

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.

@camertron
Copy link
Owner

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.

@dankohn
Copy link
Author

dankohn commented Nov 30, 2014

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"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants