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

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

Open
2 tasks done
Vaelek opened this issue Jul 1, 2020 · 27 comments
Open
2 tasks done

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

Vaelek opened this issue Jul 1, 2020 · 27 comments
Labels
dialect: mssql For issues and PRs. Things that involve MSSQL (and do not involve all dialects). type: feature For issues and PRs. For new features. Never breaking changes.

Comments

@Vaelek
Copy link

Vaelek commented Jul 1, 2020

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

  • Sequelize version: 6.2.0
  • Node.js version: 13.8.0
  • Operating System: Win10 Pro 64bit

Issue Template Checklist

How does this problem relate to dialects?

  • I think this problem happens only for the following dialect(s): MSSQL

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

  • Yes, I have the time and know how to start

I have solved it by changing

return `[${identifier.replace(/[[\]']+/g, '')}]`;

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
Copy link
Author

Vaelek commented Jul 21, 2020

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
Copy link

I run into the same problem.

@ghost ghost mentioned this issue May 18, 2021
2 tasks
@ghost
Copy link

ghost commented May 18, 2021

#13277 #6225

kwanhs added a commit to kwanhs/sequelize that referenced this issue Oct 19, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2021

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. 🙂

@github-actions github-actions bot added the stale label Nov 5, 2021
@bkozlik
Copy link

bkozlik commented Nov 9, 2021

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
Copy link
Member

WikiRik commented Nov 9, 2021

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

@WikiRik WikiRik added dialect: mssql For issues and PRs. Things that involve MSSQL (and do not involve all dialects). and removed stale labels Nov 9, 2021
@kwanhs
Copy link

kwanhs commented Nov 9, 2021

@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
Copy link
Member

WikiRik commented Nov 9, 2021

@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
Copy link

kwanhs commented Nov 10, 2021

@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
Copy link

ghost commented Nov 14, 2021

@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
Copy link

ghost commented Nov 29, 2021

@WikiRik Is this a fix pending?

@ghost
Copy link

ghost commented Dec 29, 2021

@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
Copy link
Member

WikiRik commented Dec 29, 2021

@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
Copy link

ghost commented Dec 29, 2021

@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
Copy link
Member

sdepold commented Dec 29, 2021

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
Copy link

ghost commented Dec 30, 2021

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

@ghost
Copy link

ghost commented Jan 21, 2022

@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
Copy link
Member

ephys commented Jan 22, 2022

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
Copy link

ghost commented Jan 29, 2022

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

@ephys ephys added type: feature For issues and PRs. For new features. Never breaking changes. and removed type: bug labels Jan 30, 2022
@ghost
Copy link

ghost commented Jan 30, 2022

@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
Copy link
Member

ephys commented Jan 30, 2022

@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
Copy link

ghost commented Jan 30, 2022

@ephys Yes

@ephys
Copy link
Member

ephys commented Jan 30, 2022

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

@Helcar0123
Copy link

@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
Copy link
Member

ephys commented Mar 4, 2022

@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
Copy link

ghost commented Mar 5, 2022

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?

@ephys ephys changed the title Unable to specify full schema in MSSQL Unable to specify full schema in MSSQL & Postgres Oct 6, 2022
@ephys ephys changed the title Unable to specify full schema in MSSQL & Postgres Unable to specify database + schema in MSSQL & Postgres Oct 6, 2022
@bulldozer2003
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialect: mssql For issues and PRs. Things that involve MSSQL (and do not involve all dialects). type: feature For issues and PRs. For new features. Never breaking changes.
Projects
None yet
Development

No branches or pull requests

10 participants