AutoIncrement triggered by 0 value id.

[thornley-touchstar]

DBFlow Version: 4.1.2
Issue Kind (Bug, Question, Feature): Question

Description:

Regarding auto increment support, I noticed that you are using '0' as a signifier for triggering an object's id column to increment automatically. If the id value is predefined in the model/object it must be > 0 to prevent the incrementation.

This behaviour seems to be defined specifically in the ModelAdapter<> in the method "hasAutoIncrement" which might be better called something like "requiresAutoIncrement" with the logic negated, as it is really determining if the column is not predefined and needs to be be auto incremented. I also noticed the method treats null with an exception, which leads me to my question...

What do you think about changing this behaviour so that (rather than 0) if you (optionally?) autoincremented this based on the id being null? The point is that zero is technically a valid "predefined" id, and my particular use case actually requires this. I discovered this after finding that all predefined rows that I insert (for data sync) which contain a preset id of '0' got autoincremented, unexpectedly.

I think this might be better handled as you transition the codebase to Kotlin as you should be able to better detect whether or not the id type is Int? or Int. But it could be addressed either way via annotations...

Just curious what your thoughts are regarding this, and if my understanding is in error. I am happy to help contribute a solution if you agree.

Thanks in advance! What a great library this is :)

[agrosner]

From SQLITE docs:

If the table has never before contained any data, then a ROWID of 1 is used.

What use case would require you to use 0 as the id if its an autoincrementing column? You probably would want to use a regular non autoincrementing column in the DB for anything but ids that get autogenerated for you.

[agrosner]

We already warn you in the processor if you use a nullable type:

warning: *==========*
w: 

w:   Attempting to use nullable field type on an autoincrementing column. To suppress or remove this warning switch to java primitive, add @android.support.annotation.NonNull,@org.jetbrains.annotation.NotNull, or in Kotlin don't make it nullable. Check the column $columnName on $Table
w:   *==========*

but dont explicitly forbid it, especially if you want to use null for objects that you will fill with real value prior to saving to DB.

[thornley-touchstar]

This was more about allowing the column (if marked auto-incrementing) to be seeded with data that contains a 0, sqlite does not forbid this. You are correct it does semantically default to a starting id of 1, which is fine in this scenario and probably the reason why DBFlow works this way (there isn't much choice).

The use case for this is data sync, so say for example I import a table from a backend data service that has the id's pre-generated, but we also allow for additional rows to be added internally (using the auto-increment feature. The backend generated data can contain a valid primary key id that is zero, which would get inserted to the database in DBFlow as an auto incremented id (and not explicitly zero). This obviously causes issues when FK's are depending on the id of 0. I just figured it would be cleaner to allow for nulls, however it doesn't seem to be the case in DBFlow due to the hasAutoIncrement() method in ModelAdapter<>: -

    public boolean hasAutoIncrement(TModel model) {
        Number id = getAutoIncrementingId(model);
        //noinspection ConstantConditions
        if (id == null) {
            throw new IllegalStateException("An autoincrementing column field cannot be null.");
        }
        return id.longValue() > 0;
    }

I might be missing something as I am a newbie with DBFlow but I couldn't get it to work due to the method above. I didn't use any annotations, and just to clarify I am using Kotlin which the preprocessor warning you pointed out says simply 'don't make it nullable' ;)

To be honest with you, to work around this we simply managed the id's internally and don't use the auto-increment feature at this stage, however if DBFlow were to allow using "null" as the flag as opposed to "0", this might be a better approach as at the sql level, primary keys are typically by definition not-null.

This might be something to think about supporting formally with the shift to Kotlin.

[agrosner]

I see so something like this:

 public boolean hasAutoIncrement(TModel model) {
        Number id = getAutoIncrementingId(model);
        return id != null && id.longValue() > 0;
    }

would be beneficial? I think that makes sense.

[agrosner]

so when its null we treat it also as if its missing

[agrosner]

in develop, try it out with commit 7d0d039f1a instead of version 4.2.2 until 4.2.3 is released.

[thornley-touchstar]

Awesome thanks! :)