Fixed
Status Update
No update yet.
Comments
am...@gmail.com
since these are in public API (:/) we need to do this in 1.2
yb...@google.com
since it is already marked as deprecated, we can probably do it by now.
am...@gmail.com
Opening diff shortly
yb...@google.com
Project: platform/frameworks/support
Branch: androidx-master-dev
commit d576cbdc911cba16638a44fd8223391a90a07ef7
Author: Mike Nakhimovich <digitalbuddha@users.noreply.github.com>
Date: Tue Aug 11 09:30:34 2020
[GH] Hide deprecated internal API.
## Proposed Changes
* `RoomDatabase.java` has protected `mCallbacks` field which is leaking in the API docs, we should @Hide it.
## Testing
Test: Ran unit tests locally
## Issues Fixed
Fixes: 76109329
This is an imported pull request fromhttps://github.com/androidx/androidx/pull/61 .
Resolves #61
Github-Pr-Head-Sha: 6440daa3a63752c7f9d5ba2a390248cd85bc634f
GitOrigin-RevId: fe92d8466a59b44b218b6ca3cbd57dcda17992f7
Change-Id: Id599cdf5b02b32bdae0166266fb7da967598fe92
A room/runtime/api/current.ignore
M room/runtime/api/current.txt
M room/runtime/api/public_plus_experimental_current.txt
M room/runtime/api/restricted_current.txt
M room/runtime/src/main/java/androidx/room/RoomDatabase.java
https://android-review.googlesource.com/1396827
Branch: androidx-master-dev
commit d576cbdc911cba16638a44fd8223391a90a07ef7
Author: Mike Nakhimovich <digitalbuddha@users.noreply.github.com>
Date: Tue Aug 11 09:30:34 2020
[GH] Hide deprecated internal API.
## Proposed Changes
* `RoomDatabase.java` has protected `mCallbacks` field which is leaking in the API docs, we should @Hide it.
## Testing
Test: Ran unit tests locally
## Issues Fixed
Fixes: 76109329
This is an imported pull request from
Resolves #61
Github-Pr-Head-Sha: 6440daa3a63752c7f9d5ba2a390248cd85bc634f
GitOrigin-RevId: fe92d8466a59b44b218b6ca3cbd57dcda17992f7
Change-Id: Id599cdf5b02b32bdae0166266fb7da967598fe92
A room/runtime/api/current.ignore
M room/runtime/api/current.txt
M room/runtime/api/public_plus_experimental_current.txt
M room/runtime/api/restricted_current.txt
M room/runtime/src/main/java/androidx/room/RoomDatabase.java
am...@gmail.com
Ok, that's what I thought. Normally one would dynamically create that clause if there is something to filter. We can't do that with Room.
In my case the user can filter by four metadata fields. With Room's auto-expansion each becomes effectively two cases (filter on/off). Additionally they can order by four characteristics as well. Since order can't be made dynamic in any way I'm guaranteed 16 query methods right now. Without the ability for dynamic clauses to be dynamically dropped (null/empty) I resorted to writing a code-gen for the code-gen to generate 256 queries.
In my case the user can filter by four metadata fields. With Room's auto-expansion each becomes effectively two cases (filter on/off). Additionally they can order by four characteristics as well. Since order can't be made dynamic in any way I'm guaranteed 16 query methods right now. Without the ability for dynamic clauses to be dynamically dropped (null/empty) I resorted to writing a code-gen for the code-gen to generate 256 queries.
am...@gmail.com
So is WAI the verdict? As it stands Room is a cute one-trick-pony in the world of ORM (minus the R). Accepting PRs was mentioned in another issue...any chance this source is hitting the interwebs anytime soon?
yb...@google.com
it is WAI as in the example.
requiring you to create 256 queries is definitely not WAI :)
Right now, we have 3 possible paths for dynamic queries:
allow you to pass in SQL, practically opening room for all injection problems + losing verification (of course we can limit it to SQL + params)
create builders (if we can avoid, better)
some 3rd option that does code-gen based on a query. (need to experiment)
I'm not super happy w/ any of these which is partially why the development on this issue is a bit stuck (also, i didn't have much time to focus on it).
any recommendations?
requiring you to create 256 queries is definitely not WAI :)
Right now, we have 3 possible paths for dynamic queries:
allow you to pass in SQL, practically opening room for all injection problems + losing verification (of course we can limit it to SQL + params)
create builders (if we can avoid, better)
some 3rd option that does code-gen based on a query. (need to experiment)
I'm not super happy w/ any of these which is partially why the development on this issue is a bit stuck (also, i didn't have much time to focus on it).
any recommendations?
am...@gmail.com
So the main reason I migrated to Room was not for query formation. I already did that by hand (in fact a lot of it looks like Room code). The Room annotation process is very slick kudos for that. The real reason I switched was for the life-cycle/cursor management, and the allusion towards solving the CursorWindow overflow woes. I really really really wanted to see if it helped with the window crashes so much so that I rigged my own code-gen.
That said I think Google far too often errs on the side of 'people are too dumb to do it right'. The `people are too dumb` assessment usually holds true, but you should never sacrifice functionality for the lowest common denominator. Apps in Android are sand-boxed for a reason. If a user exposes themselves that's their own foolishness; the fact is they can already do that with raw queries.
<spoiler>I'm no SQL expert</spoiler>
1) In that vein, I'd say allow a means to pass SQL. Maybe an @RawQuery that throws caution to the wind.
2) It might be possible to manage a switch in the annotation and allow transparent method extrapolation:
val a:String queryA = "..." <---. (can be verified!)
\
@Query(WHEN :decisionTree [val A: String] ...) thisFirstParamGivesMeControlOverManyQueries( decisionTree: Int);
3) I don't believe that IN() is ever useful or intended, so dumping that associated clause on null/empty would make the lib infinitely more versatile.
4) I think ORDER BY only has very minor vulnerabilities, so a simple string clause in a pinch should work for now *after* ORDER BY.
That said I think Google far too often errs on the side of 'people are too dumb to do it right'. The `people are too dumb` assessment usually holds true, but you should never sacrifice functionality for the lowest common denominator. Apps in Android are sand-boxed for a reason. If a user exposes themselves that's their own foolishness; the fact is they can already do that with raw queries.
<spoiler>I'm no SQL expert</spoiler>
1) In that vein, I'd say allow a means to pass SQL. Maybe an @RawQuery that throws caution to the wind.
2) It might be possible to manage a switch in the annotation and allow transparent method extrapolation:
val a:String queryA = "..." <---. (can be verified!)
\
@Query(WHEN :decisionTree [val A: String] ...) thisFirstParamGivesMeControlOverManyQueries( decisionTree: Int);
3) I don't believe that IN() is ever useful or intended, so dumping that associated clause on null/empty would make the lib infinitely more versatile.
4) I think ORDER BY only has very minor vulnerabilities, so a simple string clause in a pinch should work for now *after* ORDER BY.
yb...@google.com
having another replacement variable comes w/ added confusion though.
e.g. if we wanted to allow you to add unescaped variables, it would need to be denoted differently.
Maybe something like @Query("select * from users ORDER BY {order}") where order will be string replaced by the parameter.
Another alternative is to allow
@RawQuery
fun foo(sql : String, vararg args : Object) : T / List<T>
If we do that though, we would need to enforce T to be an Entity as we cannot verify the query result ahead of time (so we can't know what columns will be returned).
Another possible, safer but a lot verbose solution is to have placeholders like you've mentioned.
e.g.
@RawQuery(
query = "SELECT * FROM users WHERE name LIKE :name AND {whereArg} ORDER BY {order}",
replacements = {
@Replacement(name = "whereArg", default="1"),
@Replacement(name = "order", default="name DESC")
}
)
fun foo(name : String, whereArg : String, order : String)
This does not look like a nice API to me :/.
A third option is to port Room's engine to run on runtime but that is probably too much and too hard to keep consistent in the future.
e.g. if we wanted to allow you to add unescaped variables, it would need to be denoted differently.
Maybe something like @Query("select * from users ORDER BY {order}") where order will be string replaced by the parameter.
Another alternative is to allow
@RawQuery
fun foo(sql : String, vararg args : Object) : T / List<T>
If we do that though, we would need to enforce T to be an Entity as we cannot verify the query result ahead of time (so we can't know what columns will be returned).
Another possible, safer but a lot verbose solution is to have placeholders like you've mentioned.
e.g.
@RawQuery(
query = "SELECT * FROM users WHERE name LIKE :name AND {whereArg} ORDER BY {order}",
replacements = {
@Replacement(name = "whereArg", default="1"),
@Replacement(name = "order", default="name DESC")
}
)
fun foo(name : String, whereArg : String, order : String)
This does not look like a nice API to me :/.
A third option is to port Room's engine to run on runtime but that is probably too much and too hard to keep consistent in the future.
yb...@google.com
oh actually there is 1 more option.
We can enforce @RawQuery to provide a sample. Then we know what the query returns.
e.g.
@RawQuery(sample = "select * from users, pets")
fun query(sql : String, varargs params : Any) : T
Then we can assume the sample and do the resolution based on that and it will be the developer's responsibility to pass down an sql that matches the result of the sample.
It might be a reasonable compromise, wdyt?
We can enforce @RawQuery to provide a sample. Then we know what the query returns.
e.g.
@RawQuery(sample = "select * from users, pets")
fun query(sql : String, varargs params : Any) : T
Then we can assume the sample and do the resolution based on that and it will be the developer's responsibility to pass down an sql that matches the result of the sample.
It might be a reasonable compromise, wdyt?
yb...@google.com
but for parameter replacements we cannot use :name convention because we would need to parse the SQL, which means we would need to ship all of the sqli parsing logic w/ the library.
Alternatively, we can enforce using `?` or force you to pass down a SupportSQLiteQuery
@RawQuery(sample = "select * from users, pets")
fun query(sql : SupportSQLiteQuery) : T
Alternatively, we can enforce using `?` or force you to pass down a SupportSQLiteQuery
@RawQuery(sample = "select * from users, pets")
fun query(sql : SupportSQLiteQuery) : T
am...@gmail.com
1) The additional replacement variable is entirely optional and is a feature of Room. It provides the necessary versatility to expose SQL to advanced users in a manner that that would make strides towards making Room ubiquitous for Android. Users whose toughest use-case is within the current capability (id=:id) would never even need to know of it. For example, your simplest user never needs to know about the ostensibly whacky, but necessary @TypeConverter scoping. I also don't think it was so awful an API. It documented itself.
2) Is there a reason "ORDER BY :order" expansion wouldn't work? One less syntax headache, and I think the ORDER BY versatility would be one of your bigger, safer, easier QOL improvements.
3) I think enforcing some requirements on @RawQuery to ensure fitting into the framework would be a fantastic solution if that means that we take entire responsibility for the actual filtering/ordering. If it's easy on your end then it's the perfect stop-gap until something verifiable could be implemented.
As it stands right now, Room is a bit of a trap for advanced users as many, many hours into it you find there's no path towards your solution which will eventually kill adoption as disclaimers start appearing everywhere. For example, no where in the doc does it mention ORDER is not dynamic, nor does the annotation complain. Even a simple use-case like allowing a user to sort by name/time dead-ends when a dev realizes, "Wait...I can't handle sorting by user-driven, but constant characteristics because someone might try injecting into ORDER BY...how does that apply to me?" You're assuming a responsibility that should be the developer's.
I don't mean to sound critical. But after years of ContentProvider and CursorWindow allocation headaches I want this to succeed, so I'm just noting how important I think this is to get in place ASAP.
------------
I was in the middle of writing this when your formatted rawquery suggestion popped up which is a perfectly acceptable stop-gap imo, I'll include this to clarify what I meant with 'WHEN', though if you see flaws just ignore it:
enum Linker = {AND, OR}
val withRating = "rating = :rating"
val withNone = "id = id"
...
SELECT * FROM users WHERE WHEN :variant [withRating, withKeyword, withBoth, withNone] ORDER BY :order
fun foo (variant: Int, linker: Linker, rating: List<Int>, keywords: List<String>;
This actually solves my use-case in a fairly simple, verifiable, readable(imo) manner. Could it be done better? I have no doubt, but it's an example of extensible capability without blowing up the existing API and maintaining verification. Down the road there might be something more elegant and WHEN gets deprecated, but it would be versatile now, which is sorely lacking in 1.0.0. This is just an off-the-cuff suggestion, any solution that delivers some versatility will pay dividends in the meantime.
----------------
2) Is there a reason "ORDER BY :order" expansion wouldn't work? One less syntax headache, and I think the ORDER BY versatility would be one of your bigger, safer, easier QOL improvements.
3) I think enforcing some requirements on @RawQuery to ensure fitting into the framework would be a fantastic solution if that means that we take entire responsibility for the actual filtering/ordering. If it's easy on your end then it's the perfect stop-gap until something verifiable could be implemented.
As it stands right now, Room is a bit of a trap for advanced users as many, many hours into it you find there's no path towards your solution which will eventually kill adoption as disclaimers start appearing everywhere. For example, no where in the doc does it mention ORDER is not dynamic, nor does the annotation complain. Even a simple use-case like allowing a user to sort by name/time dead-ends when a dev realizes, "Wait...I can't handle sorting by user-driven, but constant characteristics because someone might try injecting into ORDER BY...how does that apply to me?" You're assuming a responsibility that should be the developer's.
I don't mean to sound critical. But after years of ContentProvider and CursorWindow allocation headaches I want this to succeed, so I'm just noting how important I think this is to get in place ASAP.
------------
I was in the middle of writing this when your formatted rawquery suggestion popped up which is a perfectly acceptable stop-gap imo, I'll include this to clarify what I meant with 'WHEN', though if you see flaws just ignore it:
enum Linker = {AND, OR}
val withRating = "rating = :rating"
val withNone = "id = id"
...
SELECT * FROM users WHERE WHEN :variant [withRating, withKeyword, withBoth, withNone] ORDER BY :order
fun foo (variant: Int, linker: Linker, rating: List<Int>, keywords: List<String>;
This actually solves my use-case in a fairly simple, verifiable, readable(imo) manner. Could it be done better? I have no doubt, but it's an example of extensible capability without blowing up the existing API and maintaining verification. Down the road there might be something more elegant and WHEN gets deprecated, but it would be versatile now, which is sorely lacking in 1.0.0. This is just an off-the-cuff suggestion, any solution that delivers some versatility will pay dividends in the meantime.
----------------
am...@gmail.com
Re: # 12
Is that in reference to the varargs? So we package the the sql ourselves in entirety? I think that's fine. I might suggest exposing the parameter expansion methods. I've written them, they're not fun ;-). In a lot ways, that alone is a useful tool in the library.
Is that in reference to the varargs? So we package the the sql ourselves in entirety? I think that's fine. I might suggest exposing the parameter expansion methods. I've written them, they're not fun ;-). In a lot ways, that alone is a useful tool in the library.
yb...@google.com
We've discussed this internally and have an implementation for @RawQuery which can accept either a String or SupportSqlQuery.
e.g. you can do:
@Dao
interface RawDao {
@RawQuery
User getUser(String query);
@RawQuery
User getUserViaQuery(SupportSQLiteQuery query);
}
User user = rawDao.getUser("SELECT * FROM User WHERE id = 3");
SimpleSQLiteQuery query = new SimpleSQLiteQuery("SELECT * FROM User WHERE id = ?",
new Object[]{3});
User user2 = rawDao.getUserViaQuery(query);
All pojos, relations etc are supported.
For observable queries, you need to specify the list of entities.
e.g.
@RawQuery(observedEntities = User.class)
LiveData<User> getUserLiveData(String query);
it is being reviewed now so might change but i think this seems like a reasonable escape hatch w/o cluttering the API w/ builders etc.
Btw, for order, it is not a Room limitation, it is a limitation from SQL.
e.g. you can do:
@Dao
interface RawDao {
@RawQuery
User getUser(String query);
@RawQuery
User getUserViaQuery(SupportSQLiteQuery query);
}
User user = rawDao.getUser("SELECT * FROM User WHERE id = 3");
SimpleSQLiteQuery query = new SimpleSQLiteQuery("SELECT * FROM User WHERE id = ?",
new Object[]{3});
User user2 = rawDao.getUserViaQuery(query);
All pojos, relations etc are supported.
For observable queries, you need to specify the list of entities.
e.g.
@RawQuery(observedEntities = User.class)
LiveData<User> getUserLiveData(String query);
it is being reviewed now so might change but i think this seems like a reasonable escape hatch w/o cluttering the API w/ builders etc.
Btw, for order, it is not a Room limitation, it is a limitation from SQL.
am...@gmail.com
Ahh, didn't even know about the Support query stuff.
Is the support for SupportSQLiteQuery as a parameter new? Looks like it's created with: SupportSQLiteQueryBuilder; I don't see the actual expansion, so just to check, can it expand:
WHERE a IN (?,?,?)
AND b IN(?,?,?)
with 6 bindArgs?
All told, looks great. As far as I can think of it looks like a great way to give devs the control they may need.
I understand that SQL won't bind ORDER, but IIRC the `MeinDao_IMPL` was doing some of the expansion, was it not possible to extract the string there? (sorry I'm not at my dev machine to double-check)
Is the support for SupportSQLiteQuery as a parameter new? Looks like it's created with: SupportSQLiteQueryBuilder; I don't see the actual expansion, so just to check, can it expand:
WHERE a IN (?,?,?)
AND b IN(?,?,?)
with 6 bindArgs?
All told, looks great. As far as I can think of it looks like a great way to give devs the control they may need.
I understand that SQL won't bind ORDER, but IIRC the `MeinDao_IMPL` was doing some of the expansion, was it not possible to extract the string there? (sorry I'm not at my dev machine to double-check)
yb...@google.com
SupportSQLiteQuery is just an interface, SimpleSQLiteQuery is a dummy implementation of it.
It cannot expand any variables because it would require understanding the query.
so you cannot do:
SimpleSQLiteQuery query = new SimpleSQLiteQuery("SELECT * FROM User WHERE id IN (?)",
new Object[]{listOf(3,5,6)});
you must write
SimpleSQLiteQuery query = new SimpleSQLiteQuery("SELECT * FROM User WHERE id IN (?, ?, ?)",
new Object[]{3,5,6});
the expansion for ? is provided by Room because we know at compile time the parameter is a collection.
It cannot expand any variables because it would require understanding the query.
so you cannot do:
SimpleSQLiteQuery query = new SimpleSQLiteQuery("SELECT * FROM User WHERE id IN (?)",
new Object[]{listOf(3,5,6)});
you must write
SimpleSQLiteQuery query = new SimpleSQLiteQuery("SELECT * FROM User WHERE id IN (?, ?, ?)",
new Object[]{3,5,6});
the expansion for ? is provided by Room because we know at compile time the parameter is a collection.
yb...@google.com
am...@gmail.com
What would be the time-frame on this feature. Translation: I'm a willing guinea pig if you guys want to beta it.
yb...@google.com
no promises yet as it didn't even merge yet.
but in the following weeks, we are planning to ship 1.1 alpha.
Translation: really soon but we never promise.
but in the following weeks, we are planning to ship 1.1 alpha.
Translation: really soon but we never promise.
yb...@google.com
it is merged now so next release will have @RawQuery support.
am...@gmail.com
I know the rules, but I'll be that guy (since I waited a whopping two weeks). Any chance an alpha is brewing? Got all the arch pieces in place and trying to decide if I should wait for the clean Room solution, or modify the code-gen to play with paging.
lo...@gmail.com
@#21 There has been a release on January 22nd
am...@gmail.com
Yup, writing tests as we speak. Thanks for the heads up.
ta...@gmail.com
Is there any plan for support RawQuery in the paging library? It would be very nice to pass a custom query and let room handle for limit/offset things.
Like this:
@RawQuery
public abstract DataSource.Factory<Integer, ProjectionModel> getAllRaw(SupportSQLiteQuery sql);
Like this:
@RawQuery
public abstract DataSource.Factory<Integer, ProjectionModel> getAllRaw(SupportSQLiteQuery sql);
am...@gmail.com
There s a bug in getting the paging factory that should be fixed in alpha 3.
On Feb 28, 2018 04:22, <buganizer-system@google.com> wrote:
Replying to this email means your email address will be shared with the
team that works on this product.
https://issuetracker.google.com/issues/71458963
*Changed*
*ta...@gmail.com <ta...@gmail.com> added
<https://issuetracker.google.com/issues/71458963#comment24 >:*
Is there any plan for support RawQuery in the paging library? It would be
very nice to pass a custom query and let room handle for limit/offset
things.
Like this:
@RawQuery
public abstract DataSource.Factory<Integer, ProjectionModel>
getAllRaw(SupportSQLiteQuery sql);
_______________________________
*Reference Info: 71458963 Optional parameters*
component: Android Public Tracker > App Development > Architecture
Components
status: Fixed
reporter: amandra@gmail.com
assignee: yb...@google.com
cc: amandra@gmail.com, an...@google.com
type: Bug P3 S3
Generated by Google IssueTracker notification system
You're receiving this email because you are subscribed to updates on Google
IssueTracker
<https://issuetracker.google.com/issues/71458963 > where you have the roles:
reporter, cc.
On Feb 28, 2018 04:22, <buganizer-system@google.com> wrote:
Replying to this email means your email address will be shared with the
team that works on this product.
*Changed*
*ta...@gmail.com <ta...@gmail.com> added
<
Is there any plan for support RawQuery in the paging library? It would be
very nice to pass a custom query and let room handle for limit/offset
things.
Like this:
@RawQuery
public abstract DataSource.Factory<Integer, ProjectionModel>
getAllRaw(SupportSQLiteQuery sql);
_______________________________
*Reference Info: 71458963 Optional parameters*
component: Android Public Tracker > App Development > Architecture
Components
status: Fixed
reporter: amandra@gmail.com
assignee: yb...@google.com
cc: amandra@gmail.com, an...@google.com
type: Bug P3 S3
Generated by Google IssueTracker notification system
You're receiving this email because you are subscribed to updates on Google
IssueTracker
<
reporter, cc.
Description
Version used:1.0.0
It would be nice if parameters were optional. Without thinking too hard about edge cases it appears the code-gen could reasonably do this. Take for example:
_stringBuilder.append(") AND meta.rating IN (");
final int _inputSize_1 = ratings.size();
StringUtil.appendPlaceholders(_stringBuilder, _inputSize_1);
If the size check was done first it could reasonably generate:
final int _inputSize_1 = ratings.size();
if (_inputSize_1 > 0) { //null check if we're feeling fancy, but i think empty lists suffices
_stringBuilder.append(") AND meta.rating IN (");
StringUtil.appendPlaceholders(_stringBuilder, _inputSize_1);
}
While this is obviously a nice-to-have, I have quite a few dynamic where and order clauses and when your possible combinations starts approaching numbers like 2^8, you need a code-gen to use this code-gen ;)