google / android-fhir

The Android FHIR SDK is a set of Kotlin libraries for building offline-capable, mobile-first healthcare applications using the HL7® FHIR® standard on Android.
https://google.github.io/android-fhir/
Apache License 2.0
491 stars 291 forks source link

Change Resource - Index foreign key to SQLite ID #98

Closed deepankarb closed 2 years ago

deepankarb commented 4 years ago

Since resourceID + resourceType are 1:1 with SQLite ID, it can be used as foreign key instead. This will save space in index tables.

jingtang10 commented 3 years ago

the index tables don't currently store the sqlite id directly. are you suggesting we add that as an additional column in each index table?

wantingzhang77 commented 2 years ago

Now ResourceType + ResourceID is used for foreign key of these index tables. But there may be some potential problems: ResourceID is a UUID generated locally when resources are saved into the local database. However, this ID may not be unique for remote server as the resources are creating from multiple device and POST to the remote server.

A better way that be used is as following: For the reasons of (1) When inserted into the database, each resource is assigned an SQLite ID(Long) which is the PrimaryKey in the database. This primary key is ensured to be unique. (2) When we search for a resource, we will need the ResourceType(eg. searching for a patient....) So here we proposed to refactor the database by using databaseID +ResourceType, instead of current way of using R esourceID+ResourceType as ForeignKey.

The advantages of using databaseID + ResourceType includes: (1) The SQLite ID is ensured to be unique and immutable. (2) ID as a Long can save spaces in the index tables, compare to using the ResouceId which is a string. This can improve the searching performance as well.

yigit commented 2 years ago

I would actually recommend we go with a local UUID instead as the key (can even ignore resource type). We can have a server id in addition to that to be used for server communications. FYI room 2.4 adds out of the box support for UUID (and saves it as a byte array). And for conflcits, we can simply ignore, it is super super unlikely to happen :)

Also, keep in mind, sqlite ID (which i assume it referes to the auto-generated primary key), is a bit tricky to use. More specifically, if we ever do "insert(onConflict=REPLace)", sqlite will "Delete" the existing item and insert the new one, with a new id. this also means all fkey relationships will be deleted. (not different for UUID except it wouldn't be changing so with defferred fkey checks, we can handle that) Hence, we should also make sure to never use onConflict=REPLACE (instead implement a custom upsert in a transaction).

aditya-07 commented 2 years ago
  1. Local UUID does help engine in allowing users to save Resources created on the device without requiring ID. This is useful when Fhir server allows creating resources with POST only.
  2. In the above case, server assigns an ID to the POSTed Resource and returns it in the response. As such, engine needs to update database with this new Id. PR #775 tackles this issue, but ends up doing multiple db calls as the data is stored in multiple tables as multiple other resources might be referencing the POSTed resource and thus all of them have to be updated in the database.
  3. Using Local UUID can help by reducing required updates to just the ResourceEntity table.
wantingzhang77 commented 2 years ago

Thanks for @yigit and @aditya-07 point out. The solution will be have 2 Local UUID.

jingtang10 commented 2 years ago

suggest we call these IDs uuid and logicalId rather than local and remote.