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

Missing SRID feature of MSSQL and suggested modification #4936

Closed
ChiHang1221 opened this issue Oct 21, 2019 · 2 comments
Closed

Missing SRID feature of MSSQL and suggested modification #4936

ChiHang1221 opened this issue Oct 21, 2019 · 2 comments

Comments

@ChiHang1221
Copy link

ChiHang1221 commented Oct 21, 2019

Issue type:

[ ] question
[x] bug report
[x] feature request
[ ] documentation issue

Database system/driver:

[ ] cordova
[ ] mongodb
[x] mssql
[ ] mysql / mariadb
[ ] oracle
[ ] postgres
[ ] cockroachdb
[ ] sqlite
[ ] sqljs
[ ] react-native
[ ] expo

TypeORM version:

[x] latest
[ ] @next
[ ] 0.x.x (or put your version here)

Steps to reproduce or a small repository showing the problem:
The SRID feature is not properly implemented for MSSQL.
The original implementation cause rejection if SRID constraint is implemented in SQL server.
I added the following code to typeorm/query-builder/UpdateQueryBuilder.js after line 369.
After modification, it works.

Codes:
else if (_this.connection.driver instanceof SqlServerDriver_1.SqlServerDriver && _this.connection.driver.spatialTypes.indexOf(column.type) !== -1) { expression = column.type + "::STGeomFromText(" + _this.connection.driver.createParameter(paramName, parametersCount) + ", " + (column.srid||"0") + ")"; }

@waikuen2010
Copy link
Contributor

We encounter the same issue in our project. The problem actually affect update of geometry column also.

I have submitted a PR #4936 fixing the problem. However the CI fail and I have no idea why. Can somebody help to review and merge the PR.

Currently we have to monkey patch all of our projects to use typeorm with mssql.

@imnotjames
Copy link
Contributor

#5947 was merged which should have fixed this

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

3 participants