fix(postgres): use schema set in sequelize config by default

[nholmes3]

Pull Request Checklist

Please make sure to review and check all of these items:

• Have you added new tests to prevent regressions?
• Does yarn test or yarn test-DIALECT pass with this change (including linting)?
• Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
• Did you update the typescript typings accordingly (if applicable)?
• Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
• Did you follow the commit message conventions explained in CONTRIBUTING.md?

Description Of Change

This is an attempt at fixing a bug where the Postgres Query Generator does not respect a custom schema name if it is provided.

This comment on Issue #248 of sequelize/cli describes the problem in detail.

[WikiRik]

@mike-usa this might be interesting for you

[WikiRik]

Hi @nholmes3, thanks for the PR! Could you add some tests for this? Where you test the normal 'public' schema but also a custom schema

[nholmes3]

Hi @nholmes3, thanks for the PR! Could you add some tests for this? Where you test the normal 'public' schema but also a custom schema

Hey @WikiRik I would be happy to try to add some tests. Do you mind pointing me at the file where they would be most appropriate? Thanks

[ephys]

This change does not exclusively impact postgres. I think these files would be better places to add the tests to: https://github.com/sequelize/sequelize/tree/main/test/unit/sql.

I would say add at test in:

• add-column.test.js
• remove-column.test.js

The test should use a custom sequelize instance which sets a default schema, and ensure the generated SQL uses that schema.

This test also impacts describeTable, so a similar test should be added in https://github.com/sequelize/sequelize/blob/main/test/integration/query-interface/describeTable.test.js

[nholmes3]

This change does not exclusively impact postgres. I think these files would be better places to add the tests to: https://github.com/sequelize/sequelize/tree/main/test/unit/sql.

I would say add at test in:

• add-column.test.js
• remove-column.test.js

The test should use a custom sequelize instance which sets a default schema, and ensure the generated SQL uses that schema.

This test also impacts describeTable, so a similar test should be added in https://github.com/sequelize/sequelize/blob/main/test/integration/query-interface/describeTable.test.js

@ephys I have added tests to add-column and remove-column, and I have also added a unit test covering describeTableQuery in the postgres unit tests. I am a little bit overwhelmed by how to properly add a test to the describeTable integration tests. Do you mind providing some additional guidance?

[ephys]

The describeTable integration test can be skipped because we already have an integration test that checks the query that includes schemas is valid, so the unit test you added is sufficient :)

[ephys]

This looks good to me.

@WikiRik if it's ok for you too, we can merge

[ephys]

Here we go :)
Thank you for your work on this!

[nholmes3]

@WikiRik @ephys Thank you for the help! Do you have any information on when I can expect these changes to be officially released? Thanks!

[WikiRik]

We can make a new alpha release of v7 if you want, but if you want this in v6 we would like to ask you to make a new PR for that.

[nholmes3]

We can make a new alpha release of v7 if you want, but if you want this in v6 we would like to ask you to make a new PR for that.

@WikiRik I would prefer to get this in v6. I checked the documentation for contributing and I don't see any explicit instructions for how to create a pull request for a release. Can you please provide additional guidance?

[WikiRik]

Just make a new branch from v6 (instead of main), implement these changes there as well and make a new PR that points towards v6. You should be able to re-use most of your code there

[github-actions]

🎉 This PR is included in version 7.0.0-alpha.15 🎉

The release is available on:

• npm package (@release-v7 dist-tag)
• GitHub release

Your semantic-release bot 📦🚀