Bug on 3.1.1 : causes spaces in a constant, where it shouldn't

[AndroidDeveloperLB]

This is what the generated file had:

public static final LongProperty ID = new LongProperty(TABLE_MODEL_NAME, "_id", PRIMARY KEY AUTOINCREMENT);

As you can see, "PRIMARY KEY AUTOINCREMENT" doesn't have "_" , and instead it has spaces. This causes this error: Error:(31, 92) error: ')' expected Error:(31, 110) error: expected

Here's the spec file:

@TableModelSpec(className = "ExperimentEntity", tableName = "experiments", tableConstraint = "UNIQUE(experimentId)")
public class ExperimentSpec {
    public String experimentId, experimentJson;
    public int experimentVersion, experimentLevel;
}

Reverting back to version 3.1.0 fixed the issue.

[jdkoren]

On 3.1.1 I ran ./gradlew :squidb-android-sample:clean :squidb-android-sample:assembleDebug and checked the generated models in samples/squidb-sample-core/build/generated/source/apt/debug/. The build succeeds without any compilation errors.

Here is what's generated for the ID property in both:

public static final LongProperty ID = new LongProperty(TABLE_MODEL_NAME, "_id",
        "PRIMARY KEY AUTOINCREMENT");

This looks as expected and follows the create table syntax (expand the column-def and column-constraint sections). There is no underscore in any of that syntax.

Have you tried cleaning and generating the model files again?

[sbosley]

I was actually able to reproduce this. We'll push up a fix shortly.

[AndroidDeveloperLB]

I don't understand. Have you noticed the bug or not?

[sbosley]

@AndroidDeveloperLB yes, I found the bug. We deprecated the default ID property in 3.1.0, but kept generating it with a warning for backwards compatibility reasons. 3.1.1 broke the backwards compatibility handling. Our unit tests didn't catch it because all of our test model specs had been updated to remove the deprecated behavior.

Note that the spaces are correct (there should be no underscores), it's the lack of quotes that's the bug. It should be "PRIMARY KEY AUTOINCREMENT". I've opened PR 209 to fix the issue and we'll release a 3.1.2 later today with the fix.

[AndroidDeveloperLB]

oh ok, makes sense. Please remove version 3.1.1 so that others won't use it.

[AndroidDeveloperLB]

So it got fixed and now 3.1.2 is available?

[sbosley]

The fix has been merged. 3.1.2 will be released within the hour.

[AndroidDeveloperLB]

Nice. Thank you!

[sbosley]

3.1.2 has been released. Thanks for reporting this bug!

[AndroidDeveloperLB]

On 3.1.2, I get now this warning, even though it succeeds compiling:

C:.../ExperimentSpec.java Warning:(6, 8) Model class com.....ExperimentEntity is currently generating an integer primary key ID property to act as an alias to the table's rowid. Future versions of SquiDB will remove this default property for the sake of better support for arbitrary primary keys. If you are using the ID property, you should update your model spec by explicitly declaring a field, named id with column name '_id' and annotated with @PrimaryKey C:...\RemindMeSpec.java Warning:(6, 8) Model class com.....RemindMeEntity is currently generating an integer primary key ID property to act as an alias to the table's rowid. Future versions of SquiDB will remove this default property for the sake of better support for arbitrary primary keys. If you are using the ID property, you should update your model spec by explicitly declaring a field, named id with column name '_id' and annotated with @PrimaryKey

What does it mean?

[sbosley]

Yes, earlier I mentioned that we deprecated the default ID in 3.1.0 -- this is that deprecation warning. See this wiki page for instructions on future proofing.

[jdkoren]

We are making improvements to how we handle primary keys; in particular, we are removing restrictions about how you can declare a primary key for your model (and what gets generated as a result). The details of this are explained further in this wiki page. (edit--same page @sbosley linked to, different section).

[AndroidDeveloperLB]

Can you please tell me what should I change? Should I just add this for each spec class:

   @PrimaryKey
    @ColumnSpec(name = "_id")
    long id;

?

Why can't it be the default behavior?

[jdkoren]

Add a field like this one:

// This replaces the legacy ID property that was generated by default 
// with one that is semantically identical
@PrimaryKey
@ColumnSpec(name = "_id")
long id;

[AndroidDeveloperLB]

Can I set it to always have it by default? This is already the standard way to set DB tables on Android. Probably all DB tables that Google created on Android has this field as primary key...

[jdkoren]

We may re-introduce this as a default in some way, but at the moment we are trying to add more flexibility in the declaration of primary keys. Until then we prefer that you add the lines above, which shouldn't require you to do anything in onUpgrade() since it generates the same code as before (alternatively, you could leave it out and just ignore the code generator's warning message that you encountered earlier--this will also result in the same code being generated).

Android tends to use the _id for framework providers, but there's nothing really enforcing that you do this as well. CursorAdapter is the only class I've seen that looks for it explicitly, but with RecyclerView becoming the standard and no sign of any Cursor-based adapter for it in the framework, I don't expect that kind of approach to make a resurgence.

[AndroidDeveloperLB]

OK, thank you