FlowFuse
FlowFuse runs with MS SQL Server as the core DB
Investigate the consequences and requirements for running MS SQL Server as the core DB in FlowFuse.
Initial thoughts on approach
The first task to enable MS SQL Server support will be to add the tedious driver to the package.json of the forge application.
Next, the mapping of properties from the YAML to the connection.
Any raw SQL queries will need to be assessed for compatibility.
Any conditional logic testing for SQLite or PG will need to have equivalent SQL Server logic
Notes from PoC
- MSSQL does not support function based indexes. We need to create a persisted computed column instead.
- This means where we create unique indexs on
lower(username)andlower(email)in tableUserswe need an alternative approach. - The DB approach I took in the POC was to add 2 computed columns and add indexes on those
- This means the MSSQL version has 2 extra (computed/virtual) columns vs the PG/SQLite versions
- This means where we create unique indexs on
- MSSQL is strict about cascades that are either self-referencing or might cause cycles or multiple cascade paths
- For the POC, these cascades were disabled (approx 11 occurrences)
- To ensure integrity on MSSQL we will need to assess the impact of these closely and handle them either in model hooks (e.g. beforeDestroy/afterDestroy, beforeUpdate/afterUpdate)
- We may wish to make changes accross the SQLite and PG models to have common ground
- For the POC, these cascades were disabled (approx 11 occurrences)
- MSSQL Index names are per DB not table
- This means constrain names, where specified need to be unique
- There was only one place in the rollup migration where we had duplicate index names
DESC NULLS LASTis not supported. for MSSQL,DESCis the same asDESC NULLS LAST- MSSQL does not support bulkCreate with updateOnDuplicate so we need to do this manually
- Both the Device and the Project models
updateSettingsneeded rewriting to perform this manually
- Both the Device and the Project models
literalstatements usingTRUEfor boolean comparison need to use1insteadLOCK TABLE "MetaVersions" IN ACCESS EXCLUSIVE MODEequivalent in MSSQL isSELECT * FROM "MetaVersions" WITH (TABLOCKX, HOLDLOCK)
The bulk of the remaining work in realising this POC (based on the above) will be in ensuring consistent DB operations. I recommend we do a thorough check of tests around the tables affected, ensuring they specifically check cascades operate as expected. Time est for this piece of work is size 5.
I will push the working POC branch and link it to the issue shortly
Status of POC - based on the branch and above notes:
- FF core runs
- Instances and devices work
- instance logs, audit logs, developer mode, remote editor, settings, etc OK
- All pages display data without error
- Most operations have been glanced and appear to function as expected
- DB maintenance ops not checked
- SSO not checked
- User ops not checked e.g.
- invites, reset password, email change, ...
Working POC Branch here: https://github.com/FlowFuse/flowfuse/tree/3689-mssql-support
UPDATE:
More work pushed to branch. Now 99% complete however there are some decisions and paths chosen that should be carefully considered and discussed with other engineers before the work is committed.
Definitely an 80:20 thing
Branch updates to fix missing awaits, add application logic for modified constraints and various other things, I have managed in local testing to get MSSQL tests down from 100+ failures to:
1363 passing (13m)
24 pending
5 failing
Remaining failing test - click to expand
1363 passing (13m)
24 pending
5 failing
1) Project Type API
Update project type
Prevents duplicate project-type name:
AssertionError: expected Object {
code: 'unexpected_error',
error: 'UQ__ProjectT__72E12F1BE572B557 must be unique'
} to have property error of 'name must be unique' (got 'UQ__ProjectT__72E12F1BE572B557 must be unique')
at Assertion.fail (node_modules\should\cjs\should.js:275:17)
at Assertion.value [as property] (node_modules\should\cjs\should.js:356:19)
at Context.<anonymous> (test\unit\forge\routes\api\projectType_spec.js:192:39)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
2) Stack API
List stacks
"before each" hook for "lists all stacks for a given ProjectType":
The DELETE statement conflicted with the SAME TABLE REFERENCE constraint "FK__ProjectSt__repla__571DF1D5". The conflict occurred in database "flowforge_test", table "dbo.ProjectStacks", column 'replacedBy'.
Error
at Query.run (node_modules\sequelize\lib\dialects\mssql\query.js:107:25)
at C:\Users\Stephen\repos\github\flowfuse\dev-env\packages\flowfuse\node_modules\sequelize\lib\sequelize.js:315:28
at async MSSqlQueryInterface.bulkDelete (node_modules\sequelize\lib\dialects\abstract\query-interface.js:403:12)
at async ProjectStack.destroy (node_modules\sequelize\lib\model.js:1838:16)
at async Context.<anonymous> (test\unit\forge\routes\api\stack_spec.js:462:13)
3) Team Devices API
Team Devices
Supports sorting the devices
by instance->application name:
AssertionError: expected Array [ undefined, 'application-1', 'application-1', 'application-2' ] to match Array [ 'application-1', 'application-1', 'application-2', undefined ]
not matched properties: '0' (undefined), '2' ('application-1'), '3' ('application-2')
matched properties: '1'
at Assertion.fail (node_modules\should\cjs\should.js:275:17)
at Assertion.value [as match] (node_modules\should\cjs\should.js:356:19)
at Context.<anonymous> (test\unit\forge\routes\api\teamDevices_spec.js:217:77)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
4) Team Devices API
Team Devices
Supports sorting the devices
by instance name:
AssertionError: expected Array [ undefined, 'instance-2', 'project1', 'project1' ] to match Array [ 'instance-2', 'project1', 'project1', undefined ]
not matched properties: '0' (undefined), '1' ('instance-2'), '3' ('project1')
matched properties: '2'
at Assertion.fail (node_modules\should\cjs\should.js:275:17)
at Assertion.value [as match] (node_modules\should\cjs\should.js:356:19)
at Context.<anonymous> (test\unit\forge\routes\api\teamDevices_spec.js:244:74)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
5) Accounts API
Register User
rejects duplicate username:
AssertionError: expected 'UQ__Users__F3DBC5724C6B4EBE must be unique' to match /Username or email not available/
at Assertion.fail (node_modules\should\cjs\should.js:275:17)
at Assertion.value [as match] (node_modules\should\cjs\should.js:356:19)
at expectRejection (test\unit\forge\routes\auth\index_spec.js:51:42)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async Context.<anonymous> (test\unit\forge\routes\auth\index_spec.js:128:13)
Think I've seen in Slack, this is now done?
No, it is at a point where we can state it is viable (with some more work to fix remaining tests) but it does require some engineering discussion and agreement on direction and overall acceptance (including ramifications on relationships & cascades changes in order to adopt MSSQL DB as a backend choice)
there was some feedback of nervousness supporting another DB which i do not fully disagree.
It was deliberately raised in slack first
The potential customer has not shared the date where they need this by. It would be required for the project that they are undertaking for their client, but we are still in the proposal stages. The potential customer has decided on FlowFuse, but they need buy in from their client before signing a contract with us. I imagine this integration will be needed shortly after that once they begin their initial deployment. If I had to estimate, we have a month or so to go before implementation. But even then, this may not be needed on day 1 of the implementation.
Sorry for the vagueness, but this is everything that we know at this point and it is a work in progress. We have a technical call with them later this week and I will try to get more clarity.
