Hi @raff this error https://pastebin.com/HDiizpud is preventing this line https://github.com/openmrs/openmrs-core/blob/master/api/src/main/resources/liquibase-update-to-2.0.xml#L9077 from executing, any advice on how I can bypass that?
In order to bypass that you need to temporarily drop the report_definition_uuid foreign key on reporting_report_design table, run the changeset and restore the fk.
Thanks. That is what I had done but thought there could be other ways
Well, I don’t think we ever discussed how to handle updates in core in case they conflict with table definitions in modules. I can’t recall it happening in the past so maybe we could just address it in core this time by having two changesets, which drop the fk and recreate it.
@mseaton, thoughts?
Looking at this with more context…
- The reporting module is included in the reference application
- Lots of people use the reporting module outside the refapp also
- anybody with the reference application and/or who uses the reporting module will be unable to upgrade from platform 1.x to 2.x without manually dropping and readding a db constraint
If I’m understanding this right, I vote that we should provide a workaround in openmrs-core, even though this means making the core liquibase changesets slightly aware of the reporting module. (E.g. if the constraint is there, drop it and set a flag. If the flag was set, recreate it after.)
After dropping that key for tempral upgrade and later try to restore it via
ALTER TABLE reporting_report_design **
** ADD CONSTRAINT report_definition_uuid for reporting_report_design
** FOREIGN KEY(report_definition_uuid)**
** REFERENCES serialized_object(uuid);**
I get a this error, is it that the report_definition_uuid need to be made unique?
@darius here seems to be a twist of things. I just ran my sdk and installed ref app 2.6.1 which runs on paltform 2.0.5 and something funny, the table structure of the reporting_report_design does NOT have a constraint report_definition_uuid_for_reporting_report_design it only has it as an index. see the attached for review
.
My question is, and maybe @mseaton and @raff can help me, is it possible that we no longer need such foreign_key created on this table? am confused. Seems we only need index for this table and NOTforeign_key constraint? what do others structure look like @ssmusoke
I have just checked here to see where such constraint is created and found none, only the index is created. If so, does it imply that by dropping that constraint manually suffice thereby having no need to restore it?
Critically looking into this issue, I pecked through the reporting module version 0.7.6 which is currently running on our platform 1.9.2(which was upgraded to 1.11.5) and found out the following:
-
The constraint in question was introduced here
-
Later, the column datatypes were modified and changed here to point to the uuids(That is what we still have in the upgraded version from 1.9.2 to 1.11.5)
-
Then it was dropped on the subsequent line. Looks like our database never hit this line and therefore retaining what is in the second bullet.
Questions: and maybe get it from @mseaton and @wyclif, is there any potential data loss, disconnections and system malfunctions if that constraint is removed per the flow of events? What was its intended for in the first place? and why is it NOT available in subsequent versions? was that redundant?
cc @dkayiwa
@ningosi, from what I recall, we intentionally removed any constraint that directly ties a ReportDefinition uuid to the serialized_object table, due to the introduction of In-Memory Reports and “DefinitionLibraries” which enable ReportDefinitions to be looked up by uuid.
Looks like the relevant tickets are these
Introduced here:
Attempted to fix and clean up here:
Mike
Thanks mike, this is clear. So what this means is that if I drop the foregn key manually and never restore it all will be fine? I tried writing this **select * from reporting_report_design ** join serialized_object on (serialized_object.uuid = reporting_report_design.report_definition_uuid) on the relations and I got 50 matching rows. The use case here is that reports were being created on the reporting framework UI by that time unlike now where we manage them in java code.
@ningosi, yes I think that would make sense. If this matches what the database looks like on a clean install, then that should confirm it.
Mike