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

feat(snowflake): Add support for QueryGenerator#tableExistsQuery #15087

Merged

Conversation

sfc-gh-dgadomski
Copy link
Contributor

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 additional PR to merge fix from #14930 (already merged into v7) into v6 branch

Starting from version 6.20.0 a new method tableExistsQuery is introduced and used by the sequelizesync method. Unfortunately this method is implemented in all dialects except snowflake. This causes snowflake dialect basically unusable. This PR fixes the issue by adding the implementation of tableExistsQuery in snowlake dialect.

Copy link
Member

@WikiRik WikiRik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! I'm rerunning the docs job in the hope that it was just flaky

Copy link
Member

@WikiRik WikiRik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, the docs are not flaky

2022-10-04T11:29:45.2598963Z parse: /home/runner/work/sequelize/sequelize/src/dialects/snowflake/query-generator.js
2022-10-04T11:29:45.2771163Z �[31mwarning: could not parse the following code. if you want to use ECMAScript proposals, see https://esdoc.org/manual/feature.html#ecmascript-proposal�[32m
2022-10-04T11:29:45.2772577Z /home/runner/work/sequelize/sequelize/src/dialects/snowflake/query-generator.js
2022-10-04T11:29:45.2773073Z 171|   }
2022-10-04T11:29:45.2773753Z 172| 
2022-10-04T11:29:45.2774175Z 173|   tableExistsQuery(table) {
2022-10-04T11:29:45.2774561Z 174|     const tableName = table.tableName ?? table;
2022-10-04T11:29:45.2774955Z 175|     const schema = table.schema;
2022-10-04T11:29:45.2775188Z 176| 

Can you look into this?

@ephys
Copy link
Member

ephys commented Oct 4, 2022

I don't think docs are ever flaky. esdoc has been left unmaintained for a long time and doesn't support newer syntax such as ??. Thankfully we've migrated to TypeDoc in v7

@sdepold
Copy link
Member

sdepold commented Oct 4, 2022

Since we now all approved the pr 😂 should we merge it or is the mentioned ?? Operator a Blocker?

@ephys
Copy link
Member

ephys commented Oct 4, 2022

I was waiting on the CI to be done, then forgot :)

@ephys ephys merged commit a44772e into sequelize:v6 Oct 4, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2022

🎉 This PR is included in version 6.24.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

None yet

4 participants