Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change Resource - Index foreign key to SQLite ID #98

Closed
deepankarb opened this issue Jul 23, 2020 · 6 comments · Fixed by #1159
Closed

Change Resource - Index foreign key to SQLite ID #98

deepankarb opened this issue Jul 23, 2020 · 6 comments · Fixed by #1159
Assignees
Labels
P1 High priority issue type:bug Something isn't working

Comments

@deepankarb
Copy link
Contributor

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
Copy link
Collaborator

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?

@fredhersch fredhersch added P1 High priority issue alpha type:bug Something isn't working and removed alpha labels May 11, 2021
@wantingzhang77
Copy link
Contributor

wantingzhang77 commented Jan 27, 2022

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
Copy link
Contributor

yigit commented Jan 27, 2022

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
Copy link
Collaborator

  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 Changes to update the resource id and its references. #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
Copy link
Contributor

wantingzhang77 commented Feb 1, 2022

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

  • One is called resourceLocalId, used by index table.
  • Another one, which is just the original logicalID is resourceId. This will be set as the ID that returned from the server.

@jingtang10
Copy link
Collaborator

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

FHIR engine library automation moved this from To do to Done Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 High priority issue type:bug Something isn't working
Projects
Archived in project
7 participants