Microsoft SQL Server support #124
Comments
|
Hi @leroydev. Nice to hear from you. I can't actually remember why I removed SQL Server support. I think one of the tests failed on MS SQL and I thought "well no-one is using MS SQL anyway, so might as well drop it." Seems I was wrong! I've just created a branch to run the tests for MS SQL on Travis: https://travis-ci.org/overlookmotel/sequelize-hierarchy/builds/253938155 Let's see what fails... |
|
@overlookmotel I'm using PostgreSQL at the moment, but having MSSQL open as an option would be nice. Tests are failing at the moment because of a connection timeout:
If there's incompatibilities, I'd be willing to try and fix them. I've got MSSQL running locally anyways. :) |
|
I updated the MS SQL credentials, coping those in Sequelize. But it's still failing (https://travis-ci.org/overlookmotel/sequelize-hierarchy/builds/254123040). There isn't a public instance of MS SQL to run against any more, it seems. Looks like Sequelize only runs it's MS SQL tests on Appveyor and Appveyor provides the MS SQL instance (see https://github.com/sequelize/sequelize/blob/master/appveyor-setup.ps1). So for sequelize-hierarchy to support MS SQL would require setting up Appveyor, which I have no idea how to do. Further, I suspect changes would be required to the sequelize-hierarchy codebase to some of the raw SQL queries in e.g. the I suspect that very few people use (or want to use) MS SQL so I'm not sure whether all of the above work is justified. Certainly, for me it's not as I never use MS SQL. However, if you really want it, and are prepared to do the legwork to make it work, then please be my guest. I guess the first step would be to run the tests against your local MS SQL instance and see if tests fail. If you do that, please let me know the results. |
|
@overlookmotel I ran the tests in my MSSQL instance in Azure and got this. |
|
@aguerere Thanks for running the tests. Looks to me like everything is functional. The failing tests appear to be due to Sequelize with MS SQL returning a different error from what you get with e.g. MySQL. I'd say actually this is a bug in Sequelize. The errors that get returned for a foreign key constraint violation should be consistent regardless of what database you're using. At a guess the error is not being converted to a If you have the appetite to get this fixed, I'd suggest best thing is to raise an issue on Sequelize repo and (even better) see if you guys, since you're using MS SQL, can make a fix. If my guess about what's wrong is correct, it should be pretty trivial to fix. Then we can run sequelize-hierarchy tests again and make sure everything passes. But the good news is that it looks like sequelize-hierarchy should work fine on MS SQL. |
|
If either of you are willing to try to fix Sequelize for this and set up / tell me how to set up Appveyor to run the tests on MS SQL, I'd be willing to officially support MS SQL from here on. Please advise - otherwise I'm afraid I'm not going to have time to help further with this. |
|
Hello, I like to use with MS SQL, please let me know it is working now. Also is this going to work with sequelize v5? Thanks. |
|
@nengine v2.0.0 just released does support Sequelize v5. I have no plans to support MS SQL. It's not popular, so doesn't seem worth the time to figure out implementation. If you want to put in the time to do it, I'll be happy to receive a PR, but please be aware that I'm not going to spend time on it myself answering questions etc. You'd have to do the legwork. |
|
Hello, Thank you. I did on pg instead and got the following errors. Any suggestion is appreciated. folder.js var { pg, DataTypes, Model } = require('./pg_driver');
class Folder extends Model {}
Folder.init({
name: { type: DataTypes.STRING },
parentId: {
type: DataTypes.INTEGER,
hierarchy: true
}
}, {
sequelize: pg
})
module.exports = Folder;app.js await Folder.sync()
await Folder.isHierarchy()
var fa = await Folder.create({name: 'a'})
var fab = await Folder.create({name: 'ab', parentId: 1})
var fac = await Folder.create({name: 'ac', parentId: 1})
const folders = await Folder.findAll({ hierarchy: true });
console.log(folders)error Executing (default): CREATE TABLE IF NOT EXISTS "Folders" ("id" SERIAL , "name" VARCHAR(255), "parentId" INTEGER REFERENCES "Folders" ("id") ON DELETE RESTRICT ON UPDATE CASCADE, "createdAt" TIMESTAMP WITH TIME ZONE NOT NULL, "updatedAt" TIMESTAMP WITH TIME ZONE NOT NULL, "hierarchyLevel" INTEGER, PRIMARY KEY ("id"));
Executing (default): SELECT i.relname AS name, ix.indisprimary AS primary, ix.indisunique AS unique, ix.indkey AS indkey, array_agg(a.attnum) as column_indexes, array_agg(a.attname) AS column_names, pg_get_indexdef(ix.indexrelid) AS definition FROM pg_class t, pg_class i, pg_index ix, pg_attribute a WHERE t.oid = ix.indrelid AND i.oid = ix.indexrelid AND a.attrelid = t.oid AND t.relkind = 'r' and t.relname = 'Folders' GROUP BY i.relname, ix.indexrelid, ix.indisprimary, ix.indisunique, ix.indkey ORDER BY i.relname;
{ SequelizeAssociationError: You have used the alias children in two separate associations. Aliased associations must have unique aliases.
at new Association (C:\Users\nash\workspace\nodeapps\oma-dpo\node_modules\sequelize\lib\associations\base.js:106:13)
at new HasMany (C:\Users\nash\workspace\nodeapps\oma-dpo\node_modules\sequelize\lib\associations\has-many.js:19:5)
at Function.hasMany (C:\Users\nash\workspace\nodeapps\oma-dpo\node_modules\sequelize\lib\associations\mixin.js:34:25)
at Function.isHierarchy (C:\Users\nash\workspace\nodeapps\oma-dpo\node_modules\sequelize-hierarchy\lib\modelExtends.js:101:9)
at Object.exports.Tree (C:\Users\nash\workspace\nodeapps\oma-dpo\seed\data.js:48:16) name: 'SequelizeAssociationError' }
|
|
@nengine Hi. Can you please open this as a separate issue, as it has nothing to do with Microsoft SQL Server? Also please specify what version of sequelize-hierarchy and sequelize, and also provide the code from |
For me personally, it's not really clear if sequelize-hierarchy supports Microsoft SQL Server or not (and if not, why it's not supported).
In the README is the following text:
But there's also instructions on how to test with MSSQL:
Those two give off a bit of a conflicting message in my opinion.
I did find the commit in which MSSQL support was removed, but it unfortunately doesn't include a reason for the removal.
It might be good to add to the README why MSSQL isn't supported anymore, as I'm now wondering if it's not being tested for compatibility or if it's really not working with MSSQL.
The text was updated successfully, but these errors were encountered: