Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use msnodesqlv8 driver with typeorm #8063

Open
xiaoweiliu945 opened this issue Aug 10, 2021 · 9 comments
Open

Use msnodesqlv8 driver with typeorm #8063

xiaoweiliu945 opened this issue Aug 10, 2021 · 9 comments

Comments

@xiaoweiliu945
Copy link

Issue Description

Tests break when using msnodesqlv8 driver

Expected Behavior

Expect all test pass

Actual Behavior

Steps to Reproduce

  1. First issue is I have to replace this.mssql = PlatformTools.load("mssql") with this.mssql = PlatformTools.load("mssql/msnodesqlv8") in order to use msnodesqlv8 driver. Seesrc/driver/sqlserver/SqlServerDriver.ts

wonder if there is a fix for this so I don't need this workaround.

    protected loadDependencies(): void {
        try {
            this.mssql = PlatformTools.load("mssql/msnodesqlv8");

        } catch (e) { // todo: better error for browser env
            throw new DriverPackageNotInstalledError("SQL Server", "mssql");
  1. I use msnodesqlv8 and I have the following tests failure:
test/functional/query-builder/count/query-builder-count.ts 
test/github-issues/134/issue-134.ts
test/github-issues/1716/issue-1716.ts
test/github-issues/2199/issue-2199.ts
test/github-issues/2518/issue-2518.ts
test/github-issues/4220/issue-4220.ts
test/other-issues/mssql-add-column-with-default-value/mssql-add-column-with-default-value.ts

My Environment

Dependency Version
Operating System MacOS 10.13.6
Node.js version 14.17.3
Typescript version 3.7.2
TypeORM version 0.2.34

Additional Context

Relevant Database Driver(s)

DB Type Reproducible
aurora-data-api no
aurora-data-api-pg no
better-sqlite3 no
cockroachdb no
cordova no
expo no
mongodb no
mysql no
nativescript no
oracle no
postgres no
react-native no
sap no
sqlite no
sqlite-abstract no
sqljs no
sqlserver Yes

Are you willing to resolve this issue by submitting a Pull Request?

  • ✖️ Yes, I have the time, and I know how to start.
  • ✅Yes, I have the time, but I don't know how to start. I would need guidance.
  • ✖️ No, I don’t have the time, but I can support (using donations) development.
  • ✖️ No, I don’t have the time and I’m okay to wait for the community / maintainers to resolve this issue.
@imnotjames
Copy link
Contributor

Indulge me in this because I truly don't know.

Why?

@xiaoweiliu945
Copy link
Author

@imnotjames wonder typeorm support msnodesqlv8
Because typeorm use mssql and mssql uses tediousjs as default driver, while claiming msnodesqlv8 is an alternative driver here:
https://github.com/tediousjs/node-mssql

But when I want to use typeorm with msnodesqlv8, I have the above issues.

I basically refer to this issue: #5830, force typeorm to use msnodesqlv8 driver by using this.mssql = PlatformTools.load("mssql/msnodesqlv8")

Then I got 7 failed test cases, here I only ran mssql tests.

@imnotjames
Copy link
Contributor

We don't officially support msnodesqlv8 as of now.

As far as I can tell this is similar to writing a new driver because of some differences in where the parameter types need to come from, etc.

@imnotjames
Copy link
Contributor

imnotjames commented Aug 11, 2021

If you're requesting a new driver I can update this issue as such. I don't see how else we'd handle this.

Feel free to include information on how the tests are failing, though.

@xiaoweiliu945
Copy link
Author

@imnotjames

oh I think this is more like a bug fix.

Because typeorm is using driver:mssql and mssql support both tedious and msnodesqlv8, so technically msnodesqlv8 should work with typeorm, and I also saw you provided example code here using msnodesqlv8: #5830 (comment).

I am thinking those failed 7 test scenarios are something need to be addressed or fixed. Not sure if I should call it a new feature, it might just be extra bug fix for driver:mssql

@imnotjames
Copy link
Contributor

imnotjames commented Aug 11, 2021

I did not provide any sample code there.

Still need more info on what the tests are doing that are failing.

I have a good feeling that they're not interoperable and there's issues that are not trivial to make work between the two underlying drivers even if the compatibility layer exists via the mssql package.

@dsbert
Copy link
Contributor

dsbert commented Apr 4, 2022

One reason to use the other driver - it supports Trusted_Connection, where the default driver does not.

@TBG-FR
Copy link

TBG-FR commented Jul 7, 2022

Is someone still working on that ? Are there many people interested ? I had to drop TypeORM on a specific project because I needed that driver, but making the right changes to implement it could be interesting 👀

@dsbert
Copy link
Contributor

dsbert commented Jul 7, 2022

Is someone still working on that ? Are there many people interested ? I had to drop TypeORM on a specific project because I needed that driver, but making the right changes to implement it could be interesting 👀

It's possible to make it work with a few minor changes in versions < 0.3.0. I think the latest release of TypeORM does include some of these changes and may support it out of the box. However, there are some issues with the driver itself. For example - tediousjs/node-mssql#1385

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants