sequelize / sequelize

Feature-rich ORM for modern Node.js and TypeScript, it supports PostgreSQL (with JSON and JSONB support), MySQL, MariaDB, SQLite, MS SQL Server, Snowflake, Oracle DB (v6), DB2 and DB2 for IBM i.
https://sequelize.org/
MIT License
29.48k stars 4.27k forks source link

Unable to specify database + schema in MSSQL & Postgres #12449

Open Vaelek opened 4 years ago

Vaelek commented 4 years ago

Issue Description

when using the schema option of a Model, a database cannot specified.

What are you doing?

On a table model defining schema: "SomeDatabase.dbo"

What do you expect to happen?

The query to be executed as [...] FROM [SomeDatabase].[dbo].[Tablename]

What is actually happening?

The query is executed as [...] FROM [SomeDatabase.dbo].[Tablename] Naturally this fails.

Environment

Issue Template Checklist

How does this problem relate to dialects?

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

I have solved it by changing https://github.com/sequelize/sequelize/blob/945af927db60e964eba7e344665ed0594ee63231/lib/dialects/abstract/query-generator/helpers/quote.js#L68

to

let cleanIdentifier = identifier.replace(/[[\]']+/g, '');
return cleanIdentifier.endsWith('.dbo') ? cleanIdentifier.split('.').map(part => `[${part}]`).join('.') : `[${cleanIdentifier}]`;

This allows me to specify SomeDatabase.dbo or [SomeDatabase].[dbo] or even [SomeDatabase.dbo] and it will appear in the query correctly as [SomeDatabase].[dbo].[TableName] The ternary handles field names so [blah].[id] AS [blah.id] doesn't end up as [blah].[id] AS [blah].[id]

I'm sure there is a more elegant way, and this assumes all tables will be on the dbo schema.

I have seen several mentions in other issues as well as I think the docs which state both that the schema can be used to select a different database from the connection, as well as that 2 instances of Sequelize must be used to support 2 databases. Assuming everything about the connection is the same except for the database name, the latter is true only because of this issue.

Vaelek commented 4 years ago

After trying to do this "properly" and use 2 connections, I find that this was brought up several years ago and apparently swept under the rug in #6225. Making a real effort to not monkey patch but it is becoming increasingly difficult. Is there no legit way to properly join tables from 2 MSSQL databases with Sequelize?

hoangcao21 commented 3 years ago

I run into the same problem.

ghost commented 3 years ago

13277 #6225

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has been open for 7 days without activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment or remove the "stale" label. 🙂

bkozlik commented 2 years ago

Hi folks, been looking forward to this issue getting resolved for a few months now. Checking in today, I noticed that @kwanhs committed the fix to his own fork of Sequelize a few weeks ago but the fix has not been incorporated into the official Sequelize package. I'm not sure what the next steps are in your merge process are but if you need a volunteer to copy and paste @kwanhs 's update into the official package I'd be happy to jump in so we can finally close this family of issues.

WikiRik commented 2 years ago

Hi! If you could make a PR to this repo with the suggested change and some tests that would be great.

kwanhs commented 2 years ago

@bkozlik please feel free to. I'm new to git and this is the first time I fork a repo. I haven't really tested out any bugs but just made this quick fix for my company's project.

WikiRik commented 2 years ago

@kwanhs thanks for your honesty. How does the fork work at your project? Did you encounter any unwanted issues already? Or did it function exactly as you expected?

kwanhs commented 2 years ago

@WikiRik Everything works fine till now. Haven't met any errors/issues.

I have been using the fork for making plain select queries and also with joins. Also tried on connecting to tables under two schema of the same database, and views under the same schema of the same database. Of course, it works for tables under same schema as well.

ghost commented 2 years ago

@WikiRik I have been using this solution in several projects for over half a year. This works great with both one database and several. Please merge these fix

ghost commented 2 years ago

@WikiRik Is this a fix pending?

ghost commented 2 years ago

@sdepold Since version 6.12.0-beta.3 this does not work anymore, because removed lib\dialects\abstract\query-generator\helpers. Can you add official cross schema support for MS SQL?

WikiRik commented 2 years ago

@sdepold Since version 6.12.0-beta.3 this does not work anymore, because removed lib\dialects\abstract\query-generator\helpers. Can you add official cross schema support for MS SQL?

Are you importing that directly?

@kwanhs sorry for not getting back to you earlier. Can you try to rebase your fork on the main branch and open a Pull Request to this repository?

ghost commented 2 years ago

@WikiRik

@sdepold Начиная с версии 6.12.0-beta.3это больше не работает, т.к. удалено lib\dialects\abstract\query-generator\helpers. Можете ли вы добавить официальную поддержку кросс-схемы для MS SQL?

Вы импортируете это напрямую?

Before yes, now it is not possible with version 6.12.0-beta.3

sdepold commented 2 years ago

Could you paste your code?

We usually don't recommend requiring specifics files from the code base since it is likely to break. We usually only ever guarantee the exposed properties to remain the same but it's sort of up to us to put the code in whatever file we like. Recently we have started to migrate some files to typescript and hence moved stuff around a bit.

ghost commented 2 years ago

@sdepold @WikiRik You can see an example of how it works

ghost commented 2 years ago

@sdepold Can we expect cross schema support for MS SQL? The lack of this support brings more and more inconveniences, and with the release of the latest versions, it has already become impossible.

ephys commented 2 years ago

Unfortunately we can't include that workaround as-is as it assumes and special cases the dbo schema but any other schema name could be used.

Maybe this can be resolved by adding a mssql-only database option to specify the database. Without it, schema: 'SomeDatabase.dbo' would be ambiguous and could be interpreted as both [SomeDatabase].[dbo] and [SomeDatabase.dbo]

A lot more work but PR welcome

ghost commented 2 years ago

@ephys This is a good decision. But unfortunately I can't figure out how to do it. Hope someone can add this solution.

ghost commented 2 years ago

@ephys Will add. Using postgresql, you do not need to specify an additional database, just write the name of the schema. I think that for mssql it would also be more correct not to write the database name additionally.

ephys commented 2 years ago

@alex-jss do you mean that you don't specify the database name at all and postgres infers in which database the table is?

ghost commented 2 years ago

@ephys Yes

ephys commented 2 years ago

If not specifying the schema in mssql doesn't work I think that's on the database's end and not sequelize

Helcar0123 commented 2 years ago

@ephys I need to use cross schema for mssql too. Unfortunately I don't know how to apply your solution. I use sequelize 6.17.0. Please make corrections to sequelize for cross schema support for mssql.

ephys commented 2 years ago

@Helcar0123 cross-schema is already supported in mssql, just set the schema option when calling YourModel.init or Sequelize.define. This thread is about specifying in which database the table is ; which is not currently supported and requires someone willing to open a PR implementing the feature.

ghost commented 2 years ago

cross-schema is already supported in mssql, just set the schema option when calling YourModel.init or Sequelize.define.

@ephys Can you show an example how it should be?

bulldozer2003 commented 1 year ago

Is there any way to implement this using a hook as a temporary fix in my local codebase?