Sqlite: Make AUTOINCREMENT accurate

[smitpatel]

Turns out SqlServer:Identity is also not first class. It is computed based on ValueGenerationStrategy and ValueGenerated flag.
Aligned Sqlite to do the same to compute AutoIncrement only when applicable

Resolves #10228

[smitpatel]

@Pilchie for RC1 approval

[Pilchie]

Approved for RC1 (pending CI completion)

[ghost]

Hello @smitpatel!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[bricelam]

This is good, but I want to do more for #10228. In SqlServer, we have SqlServerValueGenerationStrategy.Identity and None you can use to turn on and off IDENTITY in the model. You currently can't turn off AUTOINCREMENT in the same way.

[smitpatel]

I can add SqliteValueGenerationStrategy.AutoIncrement/None
Should I go that route?

Also UseAutoIncrement() API.

[smitpatel]

Side question, can you set a column as auto increment which is not int PK?

[bricelam]

Side question, can you set a column as auto increment which is not int PK?

No, it must be a non-composite integer primary key.

[smitpatel]

So should there be an API, UseAutoIncrement on key (which verifies that it is non-composite, int PK only when setting) and use that info to generate the column syntax?

[bricelam]

There are several ways the ValueGenerated.OnAdd could be satisifed on SQLite:

• INTEGER PRIMARY KEY (rowid alias)
• INTEGER PRIMARY KEY AUTOINCREMENT (rowid alias without value reuse)
• DEFAULT
• TRIGGER...AFTER UPDATE
• Virtual table modules
• Client-side

[bricelam]

• Fluent API (and an annotation) that also sets ValueGeneratedOnAdd

modelBuilder.Entity<MyEntity>().Property(e => e.Id).UseAutoincrement();
modelBuilder.Entity<MyOtherEntity>().Property(e => e.Id).UseAutoincrement(false);

• A convention to add it by default to non-composite integer primary key columns

• Validation to make sure you don't set it to true on a non-integer or composite primary key, or non-primary key property

[bricelam]

At least that matches SQL Server. We might need a design meeting to discuss whether it should be on the property or primary key

[smitpatel]

Needs-design? out of 5.0? Merge this (or leave this) and keep issue open to discuss design for next release?

[bricelam]

Merge this--it fixes a bug. Keep the issue open to iterate on later. I'll copy my notes into the issue.

[bricelam]

I don't think there was ever a functional bug here--just some extra clutter in the Migration. Migrations SQL generation wouldn't try to use the annotation unless it was on an integer primary key property