uo-lca / CalRecycleLCA

CalRecycle Used Oil LCA Online Tool
Other
0 stars 0 forks source link

DB auto-generation of primary keys #107

Closed bkuczenski closed 9 years ago

bkuczenski commented 9 years ago

All of the tables involved in the Param computation should have auto-generated primary keys, except:

Scenario Param ConservationParam

ConservationParam uses a DependencyParamID as a primary key- it is defined by context.

for Scenario and Param, the creation of a new entity is going to have to involve querying the database to find the maximal ID and incrementing it.

All other tables will have auto-generated primary keys after pending DB update.

uo-lca commented 9 years ago

My recommendation: Handle all Matlab generated IDs the same as ILCD UUIDs. That is, map external IDs in CSVs to internal IDs generated by the database. Advantage: Consistent overall approach and can be done without impacting Web API (external ID need not be exposed) and LCA_Data files. Changing the schema after general release will be hard so if this is the long-term solution, we should implement it now. Issue can be assigned to me.

uo-lca commented 9 years ago

Found this on stack overflow: http://stackoverflow.com/questions/25317799/how-to-switch-between-databasegeneratedoption-identity-computed-and-none-at-run

If I understand the answer correctly, we may be able to develop an alternate DbContext configuration for loading CSV files where ID generation is disabled. If this issue is assigned to me, I will try that approach.

uo-lca commented 9 years ago

Will attempt to address the issue using stackoverflow answer above. If that does not work, we decided to not support CSV loading in the future.

uo-lca commented 9 years ago

I tried the following: Remove [DatabaseGenerated(DatabaseGeneratedOption.None)] from Scenario.ScenarioID Default is DatabaseGeneratedOption.Identity Added EnableIdGeneration switch to EntityDataModel that would allow loader to change the property when model is created. Database initialization sets EnableIdGeneration to true, so ID is created as Identity type. CSV loader sets EnableIdGeneration to false.

This cause SQL Server error: Cannot insert explicit value for identity column in table 'Scenario' when IDENTITY_INSERT is set to OFF

So there is no magic. The schema needs to match the model. I could try implementing database migration to change the column type after the load. Information about that is in one of the responses to Brandon's stack overflow question (http://romiller.com/2013/04/30/ef6-switching-identity-onoff-with-a-custom-migration-operation/). That does not look any easier than doing ID mapping.

bkuczenski commented 9 years ago

thanks for checking- I think the proper thing to do here is: 1) write the GET routines for ScenarioDetailResource, and export current scenarios as JSONs 2) write the POST routines for ScenarioDetailResource components 3) set all scenario tables to be db-generated and re-import them. and then use only the API thereafter.

uo-lca commented 9 years ago

I am not able to set the seed value of a table as described here: http://stackoverflow.com/questions/23967881/ The DBCC command has no effect, and the Alter Table command causes SQL Exception. I am able to change the seed value in SQL Server table designer. I had it generate the SQL to do that. The result was long and complicated - the table and references to the table are dropped, then the table is created with the new seed and the references to the table are recreated. I believe that the primary key type cannot be changed without dropping the foreign key references.

Currently, only ScenarioID has a starting value (0) that does not match SQL Server's default seed value (1). I recommend that ScenarioID start at 1, understanding that the back end has some logic for scenarioID = 0. Instead of hard-coding the Model Base Case ScenarioID, a constant could be defined (e.g. BASE_CASE_SCENARIO_ID = 1)

uo-lca commented 9 years ago

Current plan in progress: All entity tables, except for Classification, will have a database generated ID column. Existing CSVs do not need to change because the IDs in those files match what is generated in a new database. Exception: ScenarioID in the CSVs start at 0. The CSV loader transforms those IDs by adding 1. The CSV loader will verify that the database generated ID matches the expected ID, and exit with error in case a mismatch is detected. The Classification table is excluded because the CSV for that is loaded in a different way for performance optimization. Changing the ClassificationID type in the future should not be difficult since there are no foreign key references to Classification.

uo-lca commented 9 years ago

I have finished working on this. Please review and let me know if I may close the issue.