support sending back rows changed #90
Conversation
| @@ -86,7 +87,7 @@ private static String removeSuffix(String str, String[] suffixesToRemove) { | |||
| SQLiteDatabase database = openDatabase(databaseName); | |||
| try { | |||
| Cursor cursor = database.rawQuery("SELECT name FROM sqlite_master WHERE type=?", | |||
| new String[] { "table" }); | |||
| new String[]{"table"}); | |||
|
Check the Travis build errors. I think they're lint related, which I don't have a great way to extract from Travis-CI at the moment. I just re-run |
|
Seems my answer on original post was lost or I forget to push the button:( Why don't you just check for no colums or count -1 in resulting cursor ? (if cursor null then it's an error) If no column then start new simple query 'select changes(), last_insert_rowid()' and return a MaxtrixCursor with 1 line / 1 row containing the data. (Can handle one last case when no change no row id for create database for example). This sounds cleaner and can handle more things like create XXX, drop, ... |
|
@Tolriq yeah I thought about doing that. |
|
A select without rows have the columns filled, so does not trigger this. From the other problem about database mode change, I suppose that the database is opened / closed on each query / access. So changes and last insert are the values corresponding to previous query so rule would be in order :
This covers all cases and is simple to follow and maintain. Of course this does not work if open / close assumption is wrong. |
|
what would be returned for last_insert_id or select_changes for statements other than update/insert/delete? IE what is the return type for Im just trying to figure out the value in the return from them to do an additional round trip. |
|
They return 0 if no query have made changes or insert, that why my previous post talked about != 0 to handle all known cases. And yes that why I suggested using MatrixCursor that are valid cursors so will be correctly handled by Stetho and Inspector UI. |
|
I think if they are congruent changes doing the smart thing and handling the return in the correct way rather than a second round trip to the database. |
| @@ -129,7 +187,13 @@ private SQLiteDatabase openDatabase(String databaseName) throws SQLiteException | |||
| } | |||
|
|
|||
| public interface ExecuteResultHandler<T> { | |||
| public T handleResult(Cursor result) throws SQLiteException; | |||
| public T handleRawQuery(Cursor result) throws SQLiteException; | |||
There was a problem hiding this comment.
This shouldn't be Cursor; it implies that the cursor came from database.rawQuery. Instead, prefer to parse changes(), last_insert_row_id() into two longs (or whatever type they are?)
|
Looks good except for nits. Please update the commit message of the rebased diff to include the summary in the pull request and "Closes #..." at the bottom. I recently learned that the pull request's metadata is not used for this feature, it must be in the commit itself. In general it's better to have the meta data in the commit anyway so that it is visible in the git history later. |
|
I was planning on squashing and was going to update the commit message to have |
Previously we would only return a result for sql commands that were selct statements. This diff makes `DatabasePeerManager` aware of what commands are being executed and returns a result set appropriate to that command. Note: We are returning a result for all commands (including ones that do not normally contain results like CREATE TABLE). We do this so that the inspector interface will not remove the command. Closes facebookarchive#82
support sending back rows changed
This solves #82. We use the prefix of the first 3 characters to determine what type of request it is. Since we only have access to "columns" and "error" text use the column to display the type and the value to display the count.
