schultek / stormberry

Access your postgres database effortlessly from dart code.
https://pub.dev/packages/stormberry
MIT License
68 stars 17 forks source link

[Unexpected] Insert updates existing entry #37

Closed ynnob closed 1 year ago

ynnob commented 1 year ago

If the insert statement runs into a conflict with the primary key it will update the conflicting entry with the new information.

The generated query looks like:

  @override
  Future<void> insert(Database db, List<UserInsertRequest> requests) async {
    if (requests.isEmpty) return;

    await db.query(
      'INSERT INTO "users" ( "id", "email", "password_hash", "register_date", "email_confirmed" )\n'
      'VALUES ${requests.map((r) => '( ${registry.encode(r.id)}, ${registry.encode(r.email)}, ${registry.encode(r.passwordHash)}, ${registry.encode(r.registerDate)}, ${registry.encode(r.emailConfirmed)} )').join(', ')}\n'
      'ON CONFLICT ( "id" ) DO UPDATE SET "email" = EXCLUDED."email", "password_hash" = EXCLUDED."password_hash", "register_date" = EXCLUDED."register_date", "email_confirmed" = EXCLUDED."email_confirmed"',
    );
  }

This is unexpected and potentially dangerous. When not using auto increment an missplaced id would override existing data. For example: A table uses guid as their primary keys. A new user registers and generates the same uuid. Instead of an error the query just updates the existing user and therefore links all refrence tables to a new person while loosing the previous user at the same time.

To update a entry of the database the method update should be used exclusivly.

schultek commented 1 year ago

Yes, i think there are still a few of those oversights.

Should the method then simply fail?

ynnob commented 1 year ago

Yes or make it optional with an annotation of the model to set @UpdateOnInsert for example.

But that's just my opinion.

schultek commented 1 year ago

The on conflict clause is now removed from insert statements in v0.10.0