FlowFuse runs with MS SQL Server as the core DB

[joepavitt]

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

[Steve-Mcl]

Notes from PoC

1. MSSQL does not support function based indexes. We need to create a persisted computed column instead.
   1. This means where we create unique indexs on lower(username) and lower(email) in table Users we need an alternative approach.
   2. The DB approach I took in the POC was to add 2 computed columns and add indexes on those
   3. This means the MSSQL version has 2 extra (computed/virtual) columns vs the PG/SQLite versions
2. MSSQL is strict about cascades that are either self-referencing or might cause cycles or multiple cascade paths
   1. For the POC, these cascades were disabled (approx 11 occurrences)
      1. 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)
      2. We may wish to make changes accross the SQLite and PG models to have common ground
3. MSSQL Index names are per DB not table
   1. This means constrain names, where specified need to be unique
   2. There was only one place in the rollup migration where we had duplicate index names
4. DESC NULLS LAST is not supported. for MSSQL, DESC is the same as DESC NULLS LAST
5. MSSQL does not support bulkCreate with updateOnDuplicate so we need to do this manually
   1. Both the Device and the Project models updateSettings needed rewriting to perform this manually
6. literal statements using TRUE for boolean comparison need to use 1 instead
7. LOCK TABLE "MetaVersions" IN ACCESS EXCLUSIVE MODE equivalent in MSSQL is SELECT * 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, ...

[Steve-Mcl]

Working POC Branch here: https://github.com/FlowFuse/flowfuse/tree/3689-mssql-support

[Steve-Mcl]

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. (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. (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. (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. (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. (test\unit\forge\routes\auth\index_spec.js:128:13) ```

[joepavitt]

Think I've seen in Slack, this is now done?

[Steve-Mcl]

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

[joepavitt]

@zackwasli what's your requirements/status on this now?

[zackwasli]

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.

[joepavitt]

Moving to backlog