pushtorefresh / storio

Reactive API for SQLiteDatabase and ContentResolver.
Apache License 2.0
2.55k stars 182 forks source link

WhereArgs in Query.Builder has no love for booleans #684

Open ZakTaccardi opened 8 years ago

ZakTaccardi commented 8 years ago

whereArgs doesn't like booleans (at least if provided from Kotlin).

had to create an extension function to convert true -> int (1) or false -> int (0)

return Query.builder()
                    .table(TABLE)
                    .where("$COLUMN_ID = ? AND $COLUMN_IS_COMPLETE = ?")
                    .whereArgs(businessId, true.toInt())
                    .build()

Obviously this occurs because SQLite does not have a concept of booleans, and we use integers for this instead.

nikitin-da commented 8 years ago

Hi, @ZakTaccardi Yes, it is quite simple to check type of argument and replace with integer. But it is not single place where we should change it. Will such replacement be obvious for users who write resolvers manually. Should we fail early if they for some reasons use another converting scheme (not such as 0-false, 1-true)? /cc @artem-zinnatullin

artem-zinnatullin commented 8 years ago

Hm, I think it's a good idea for Query, but not sure about RawQuery. What are your thoughts on that guys?

On 28 Sep 2016 2:46 pm, "Dmitrii Nikitin" notifications@github.com wrote:

Hi, @ZakTaccardi https://github.com/ZakTaccardi Yes, it is quite simple to check type of argument and replace with integer. But it is not single place where we should change it. Will such replacement be obvious for users who write resolvers manually. Should we fail early if they for some reasons use another converting scheme (not such as 0-false, 1-true)? /cc @artem-zinnatullin https://github.com/artem-zinnatullin

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pushtorefresh/storio/issues/684#issuecomment-250261970, or mute the thread https://github.com/notifications/unsubscribe-auth/AA7B3Cnc0-hjuiIDyFoXEbWJlUePCa8lks5qurYZgaJpZM4KEnAq .

nikitin-da commented 8 years ago

What is the difference between Query and RawQuery from this point of view? Of cause if we use only generated resolvers - there are no any problems with Query, otherwise we haven't full control on this converting.

artem-zinnatullin commented 8 years ago

Well, I mean, Query is abstract over RawQuery so it can be ok to do such processing, but RawQuery should always be as low-level as possible to allow any type of SQL.

@nikitin-da not sure I got what you said about resolvers :)

cc @thevery /others

nikitin-da commented 8 years ago

I mean, if user will decided to make resolver like this:

new DefaultPutResolver<Tweet>() {
    @Override
    protected ContentValues mapToContentValues(Tweet tweet) {
        ContentValues cv = new ContentValues();
        cv.put("read", tweet.wasRead() ? 1 : -1);
        return contentValues;
    }
}

And after then will put wasRead as argument of Query by mistake directly, it will not be easy to find bug in such case.

artem-zinnatullin commented 8 years ago

Ah, I see. Yeah, @ZakTaccardi SQLite is kind of "interesting" regarding type-safety :)

Basically, the type of the column you use in CREATE TABLE is not a strict requirement, but "hint" for SQLite to treat it as this type during some operations. But in fact you can store any type of data in any column, so with auto-convert feature we can prevent user from doing some queries he/she would like to do!