floor icon indicating copy to clipboard operation
floor copied to clipboard

How do I tell floor that the primary key is unset and should be generated?

Open jaan-c opened this issue 5 years ago • 16 comments

I have a database entity with a field marked as @PrimaryKey(autoGenerate: true) and it should not be nullable, and I was under the assumption that setting it to 0 as default means the field should be generated (a Room behavior), however when I pass the entity to a dao @Insert method Floor uses the id as is, so how do I tell Floor the field should be generated?

jaan-c avatar Mar 31 '21 13:03 jaan-c

As I understand the problem, you're passing 0 value instead of null, in this case, floor autogenerating logic will not work as you expect. Try to use null :)

dkaera avatar Mar 31 '21 14:03 dkaera

I had this problem, I had to change the floor code to not send it to the code when inserting.

To be more dynamic, I modified the year below to work for other cases, good if the floor had something similar.

@Ignore(forInsert: true, forUpdate: false, forDelete: false, forQuery: false)

ErliSoares avatar Mar 31 '21 14:03 ErliSoares

@ErliSoares Sorry, I can't understand how your option is resolving the issue in the topic. Can you provide more explanation, please?

dkaera avatar Mar 31 '21 14:03 dkaera

Sorry, my English is very bad, the idea is that if forInsert is true it will not go on the map to execute the insert, the same applies to forUpdate, forDelete and forQuery.

Example forInsert

Entity

@Entity(tableName: 'estoque')
class EstoqueEntry extends {
  EstoqueEntry({
    int id = 0,
    this.nome = '',
    this.observacao,
    this.ativo = true,
  }) : super(id: id);

  @Ignore(forInsert: true, forUpdate: false, forDelete: false, forQuery: false)
  @PrimaryKey(autoGenerate: true)
  int id;

  String nome;

  bool ativo;
}

Code generated

_estoqueEntryInsertionAdapter = InsertionAdapter(
    floorDatabase.database,
    'estoque',
    (EstoqueEntry item) => <String, Object?>{
          'nome': item.nome,
          'ativo': item.ativo ? 1 : 0,
        }),
_estoqueEntryUpdateAdapter = UpdateAdapter(
    floorDatabase.database,
    'estoque',
    ['id'],
    (EstoqueEntry item) => <String, Object?>{
          'nome': item.nome,
          'ativo': item.ativo ? 1 : 0,
          'id': item.id,
        }),
_estoqueEntryDeletionAdapter = DeletionAdapter(
    floorDatabase.database,
    'estoque',
    ['id'],
    (EstoqueEntry item) => <String, Object?>{
          'nome': item.nome,
          'ativo': item.ativo ? 1 : 0,
          'id': item.id,
        });

Not have 'id': item.id, on InsertionAdapter

Example forQuery

Entity

@Entity(tableName: 'estoque')
class EstoqueEntry extends {
  EstoqueEntry({
    int id = 0,
    this.nome = '',
    this.observacao,
    this.ativo = true,
  }) : super(id: id);

  @PrimaryKey(autoGenerate: true)
  int id;

  String nome;

  @Ignore(forInsert: false, forUpdate: false, forDelete: false, forQuery: true)
  bool ativo;
}

Code generated

  @override
  Future<EstoqueEntry?> findById(int id) async {
    return _queryAdapter.query('SELECT * FROM estoque WHERE id = ?1',
        mapper: (Map<String, Object?> row) => EstoqueEntry(
            id: row['id'] as int,
            nome: row['nome'] as String)
        arguments: [id]);
  }

Not have ativo: (row['ativo'] as int) != 0

I developed a hack on this commit

ErliSoares avatar Mar 31 '21 17:03 ErliSoares

@dkaera but making my primary key nullable will also make it optional when that isn't the case

jaan-c avatar Apr 01 '21 00:04 jaan-c

Sorry for the misunderstanding, guys. I've lost the strict restriction about the id should not be nullable ... hehe :) Basically, if you wanna use autogenerated id functionality it implies that id could be nullable to say the inner framework logic to handle this case and generate id if it's required. @vitusortner please, correct me if I'm wrong. Otherwise, if you set the id to 0 it means that the value has already set and shouldn't be autogenerated.

dkaera avatar Apr 01 '21 12:04 dkaera

@ErliSoares Ignore feature looks very attractive, I think would be great to create MR to the main repo, if you do not mind :)

In your sample, what will be if we will use replace insert for an entity that has already placed in DB? Will it be replaced or will add with a new id?

dkaera avatar Apr 01 '21 12:04 dkaera

Sorry, but I am not able to create a MR with the quality that the project deserves, my english is bad and my knowledge in the tests needs to be improved.

Do not get to test, but I believe it would give an error, the primary key parameter would not be in the map with the values, but it would have ? In SQL. The primary key fields in the update could not be ignored, it would have to raise an exception when generating the code.

ForUpdate would be used in fields that can never be updated, in the code below for example.

@Ignore(forInsert: false, forUpdate: true, forDelete: false, forQuery: false)
String createTime;

@ErliSoares Ignore feature looks very attractive, I think would be great to create MR to the main repo, if you do not mind :)

In your sample, what will be if we will use replace insert for an entity that has already placed in DB? Will it be replaced or will add with a new id?

ErliSoares avatar Apr 01 '21 14:04 ErliSoares

@ErliSoares don't worry about your English level, I can’t brag about it high level too... haha... XD Anyway, we have one language which is understandable to both of us - it's Dart )))) So I would recommend you to push this proposal with the @Ignore feature to the discussions thread, perhaps it attracts the community ;)

dkaera avatar Apr 01 '21 15:04 dkaera

The main problem currently is that even if autoGenerate is true, the insert query should allow an override of it, which is currently done by autogenerating if the id is null and using the given id if it is not null.

Using 0 instead of null for indicating autogenerate should be feasible, although it's not backwards compatible (afaict). Maybe we can add an annotation parameter for it...

mqus avatar Apr 01 '21 21:04 mqus

@ErliSoares don't worry about your English level, I can’t brag about it high level too... haha... XD Anyway, we have one language which is understandable to both of us - it's Dart )))) So I would recommend you to push this proposal with the @ignore feature to the discussions thread, perhaps it attracts the community ;)

Thank you very much for the encouragement, I have implemented a few more resources in my free time, when I finish the resources that suits me I will start to improve my code and do the MR, it will take a little time and I have little free time.

ErliSoares avatar Apr 03 '21 14:04 ErliSoares

@dkaera would you mind joining the project's Slack? I'd like to chat with you as you've been so actively contributing to the project! You can find the Slack here.

vitusortner avatar Apr 05 '21 14:04 vitusortner

Conceptually, setting a field that is a primary key to null is wrong. Erli's idea is not a bad one.

tulioccalazans avatar Apr 13 '21 18:04 tulioccalazans

The main problem currently is that even if autoGenerate is true, the insert query should allow an override of it, which is currently done by autogenerating if the id is null and using the given id if it is not null.

Using 0 instead of null for indicating autogenerate should be feasible, although it's not backwards compatible (afaict). Maybe we can add an annotation parameter for it...

The problem is that the example (at least) shows a non nullable Id field. I believe that majority of the use cases are such that someone doesn't want to assign an Id by themselves but want the system to generate it.

MrCsabaToth avatar Jun 05 '21 04:06 MrCsabaToth

Conceptually, setting a field that is a primary key to null is wrong. Erli's idea is not a bad one.

It should be a nullable field and optional constructor parameter in which case I don't even have to pass it. Ok, it's implicitly null. That's how I used it before sound null safety, I'm migrating my projects over right now.

MrCsabaToth avatar Jun 05 '21 06:06 MrCsabaToth