Identity column should be named "_id" in Android SQLite

[esteewhy]

I've tried to use CursorLoader + ContentProvider and immediately hit the wall with the following problem:

06-25 09:51:51.772: E/AndroidRuntime(284): Uncaught handler: thread main exiting due to uncaught exception 06-25 09:51:51.902: E/AndroidRuntime(284): java.lang.IllegalArgumentException: column '_id' does not exist 06-25 09:51:51.902: E/AndroidRuntime(284): at android.database.AbstractCursor.getColumnIndexOrThrow(AbstractCursor.java:314) 06-25 09:51:51.902: E/AndroidRuntime(284): at android.database.CursorWrapper.getColumnIndexOrThrow(CursorWrapper.java:99) 06-25 09:51:51.902: E/AndroidRuntime(284): at android.support.v4.widget.CursorAdapter.swapCursor(CursorAdapter.java:344) 06-25 09:51:51.902: E/AndroidRuntime(284): at android.support.v4.widget.SimpleCursorAdapter.swapCursor(SimpleCursorAdapter.java:326) ...

This seems to be because ActiveAndroid names primary key field as "Id". Android's SQLite however, expects it to be BaseColumns._ID ("_id"). I did the replacement all over ActiveAndroid sources and then my app started working as exepcted.

(Sorry for no patch at the moment: having troubles with github networking..)

PS: Also, wanted to bring to your attention another "user experience" issue i'm facing at the moment (a conceptual one, so no separate bug report yet):

When creating new entity it's handy to use object initializers like this:

MyEntity myEntity = new MyEntity() {{ myProperty = ""; }};

The problem with the code above is that anonymous class is generated, which, in turn, pollutes entity cache with some junk like "MyOuterClass$1", which in turns leads to malformed SQL DDL stmt being executed (something like CREATE TABLE without actual table name). Have no idea how to "fix" it at the moment, it asks for some brainstorming..

[schulzp]

Whoops, must have overlooked this one, my bad. ;-) Have you issued a pull request for your commit yet?

[esteewhy]

Not to worry, i'm not even sure i've identified a "right" fix to the problem :)

Basically what i did just replaced all references to field "Id" with "_ID" => now works fine with loaders / adapters. But maybe the author has something to say about it. Also, this solution isn't backward compatible, so existing code / db will inevitably break.

No, i didn't pull requests (because: 1. Not sure how to, 2. Anticipating more interesting things to find and fix), but my fork is up to date with my mods, so i suppose i can contribute it (only need to learn now).

[schulzp]

Alright, if u'r fine with my solution (which basically does the same as yours, except it's limited to the _ID issue including tests, however) I'd say one pull request is enough. Fixing only one problem per request makes it easier for the maintainers to grasp what's been changed.

[pardom-zz]

Before this is done, consideration must be made on how to upgrade from 'Id" to "_id". Should this be a one-time script provided to users of AA? Should AA automatically migrate? The later solution seems like it would add a lot of overhead to the startup time. The first solution will probably be missed by a lot of people updating.

[schulzp]

Thanks for considering the switch to _ID, it's good to see some progress on this. Another solution would be a meta-data called AA_PK_ALIAS which if set to _id causes the primary key column to be aliased through SQL like SELECT id AS _id FROM model.

Finally, my (#87) Model.Columns is not an implementation of BaseColumns it merely provides an abstraction/alias for BaseColumns._ID which improves the independence of AA from underlying APIs.

[uudashr]

Is there any progress regarding to this issue? What are we waiting for?

[pardom-zz]

We're waiting for a consensus on clean and understandable migration path.

[vonloxley]

If this is done, then AA should handle the migration automatically. This is however a rather expensive operation, since Sqlite doesn’t support column renames. StackOverflow has the right way to do the sqlite-side: http://stackoverflow.com/a/805508/366385

[esteewhy]

I'd vote for a breaking change (i.e.: "script" solution) because AA is at the beginning of it's adoption curve and i believe the public here is advanced enough to help themselves:)

[dgmltn]

Why not allow the user to specify the id field in the table annotations? i.e.

@Table(name = "Items", id = BaseColumns._ID)
public class Item extends Model {...}

For backwards compatibility, the default id = "Id". Those needing to migrate their tables can follow the SO script.

[pardom-zz]

@dgmltn I like this.

[vsigler]

Good idea! This makes it both backwards-compatible and easy to use. Although this could also be a global setting, maybe in application metadata like the db name. In case you have many models, this would spare you from having to put this setting to every single one.

[pardom-zz]

Yeah, putting it in the table definition would make it more difficult to map joins and whatnot. It will probably go in the config, which can be defined as meta-data.

https://github.com/pardom/ActiveAndroid/blob/basecolumns-id/src/com/activeandroid/Configuration.java

[zorfling]

Hi guys, is this moving forward any time soon?

[kramer65]

On the readme page about the contentprovider I see that the contentprovider only works if this issue is fixed. Will a fix be merged anytime soon?

We would welcome that very very (very!) much!

[dgmltn]

@kramer65 @zorfling if you don't want to wait and don't have to do any db upgrades, you can just replace every occurrence of "Id" in the code with "_id" or BaseColumns._ID in your local copy. There are about 8 occurrences, maybe 9, in the code.