feat(dialect): snowflake dialect support by jesse23 · Pull Request #13406 · sequelize/sequelize

jesse23 · 2021-07-29T21:55:34Z

Pull Request check-list

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

Description of change

snowflake dialect support.

Supported by Dynatrace and Snowflake
Contributed by jesse23, josecolella, cmp11290, cincyelite22, jdanielmyers

Note

All our use cases has been covered by unit test and integration test.
We have limited use cases so yes it might not have 100% coverage - that could be improved by follow up PR if possible

How to run integration test

Snowflake account is required
Run command below:

DIALECT=snowflake SEQ_ACCOUNT=<snowflake-account.us-east-1> SEQ_USER=<snowflake-user-name> SEQ_PW=<snowflake-password> SEQ_ROLE=<snowflake-role> SEQ_DB=<DB_NAME> SEQ_SCHEMA=<DV_SCHEMA> SEQ_WH=<WHAREHOUSE_NAME> npm run test-integration

Link with existing defect:

#11979

jesse23 · 2021-08-04T00:51:30Z

Pipeline is green.... Let me know if any other actions needed...thx @Keimeno

wbourne0

Haven't looked at everything, but here's some feedback for now.

Edit: also - it looks like you copied and pasted the query generator from mysql. A lot of the functions seem untouched, maybe you should just extend the mysql query generator instead of the abstract one.

jesse23 · 2021-08-05T13:22:22Z

@allawesome497 Thx! Yes it is fork from mySQL Dialect and most of use cases we are using are adopted (there are some practices more close to pgsql, not mysql) and tested in snowflake integration test....

I will fix most of the missing name changing and ping u back.

Also regarding to extend the mysql query generator I will take a look and let u know.

Thx again!

jesse23 · 2021-08-06T14:19:45Z

@allawesome497 All comments addressed. Could u please review it again and let me know anything still needs update? Thx!

Regarding the query-generator, we can refactor it later after we have most of the use cases ( especially the complex one) covered.

wbourne0

Still haven't looked through everything, but here's some more feedback.

Ideally this would be broken up into multiple prs - e.g. connection manager -> queryGenerator -> queryInterface.

jesse23 · 2021-08-11T19:06:21Z

Still haven't looked through everything, but here's some more feedback.

Ideally this would be broken up into multiple prs - e.g. connection manager -> queryGenerator -> queryInterface.

Thx will try to catch it up today or tomorow.

Regarding to the PR split, it might be good to promote something working (with the real server) as 1st commit...sorry for the heavy load of review.

I could take responsibility for adding more cases and issue fixes.

wbourne0 · 2021-08-11T23:01:24Z

Regarding to the PR split, it might be good to promote something working (with the real server) as 1st commit

Agreed. There should probably be a process for adding dialects added to the contributing docs.

Ideally this would be broken up into multiple prs but since there isn't any established guidelines for this it's probably fine as is.

jesse23 · 2021-08-13T18:48:20Z

@allawesome497 All addressed - thanks for the review!

Let me know anything needs to be update and fixed.

jesse23 · 2021-09-10T17:47:40Z

@Keimeno @allawesome497 Hi could we have another review and proceed this PR? Thanks...

wbourne0 · 2021-09-20T21:22:41Z

@Keimeno @allawesome497 Hi could we have another review and proceed this PR? Thanks...

Haven't taken the time to look at sequelize prs since there isn't any active core maintainers- so even if its approved, I don't see it getting merged any time soon. I'll let you know if the situation changes.

sdepold · 2021-11-04T17:36:54Z

@jesse23 Could you please ping me on slack?

sdepold · 2021-11-22T07:38:37Z

We are going to have our monthly sequelize sync tomorrow and will discuss the matter of how to deal with dialect support requests there: https://www.meetup.com/eBay-Europe-Technology/events/282125063/

Feel free to drop by and participate in the discussion.

jesse23 · 2021-11-22T14:11:52Z

We are going to have our monthly sequelize sync tomorrow and will discuss the matter of how to deal with dialect support requests there: https://www.meetup.com/eBay-Europe-Technology/events/282125063/

Feel free to drop by and participate in the discussion.

Thanks - yes will attend. FYI @cincyelite22

sdepold · 2021-11-28T13:03:36Z

+ * 
+ * @private
+ */
+const snowflakeReservedWords = 'account,all,alter,and,any,as,between,by,case,cast,check,column,connect,connections,constraint,create,cross,current,current_date,current_time,current_timestamp,current_user,database,delete,distinct,drop,else,exists,false,following,for,from,full,grant,group,gscluster,having,ilike,in,increment,inner,insert,intersect,into,is,issue,join,lateral,left,like,localtime,localtimestamp,minus,natural,not,null,of,on,or,order,organization,qualify,regexp,revoke,right,rlike,row,rows,sample,schema,select,set,some,start,table,tablesample,then,to,trigger,true,try_cast,union,unique,update,using,values,view,when,whenever,where,with'.split(',');


Would be really cool if this could be moved over to the snowflake specific dialect files

@sdepold This is currently following the same practice as what postgres is doing for reserved words. Do you have a recommendation on where this can neatly be injected without refactoring all of the calls to this function?

@sdepold I will follow up in the dialect separation work with postgres together.

jesse23 · 2021-11-28T15:07:38Z

@cincyelite22 Please follow up. Thx.

Keimeno · 2021-12-02T21:33:31Z

+    strategy:
+      fail-fast: false
+      matrix:
+        node-version: [10, 12]


Please change the highest tested node version from 12 to 16, we recently changed this

* fix(dialect): fix Snowflake query syntax * fix(dialect): fix offset-limit test * fix(dialect): fix offset-limit test Co-authored-by: Maria Lee <maria.lee@dynatrace.com>

sdepold · 2021-12-03T11:52:42Z

Docs seems to be broken now. But we are getting closer :)

…mote

jmikolay22 · 2021-12-03T20:02:05Z

@sdepold Should be ready now for another review.

sdepold · 2021-12-03T20:45:12Z

I think you'll have to update with master one more time

* feat(typescript): create alpha release with ts * ci: add other ts versions to ci (#13686) * fix: wrong interface used within mixin (#13685) * fix(increment): fix key value broken query (#12985) Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com> * fix(upsert): fall back to DO NOTHING if no update key values provided (#13594) * fix(types): add instance member declaration (#13684) * fix(types): add instance member declaration * test(types): add static/instance members test cases Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com> * Create aws-lambda.md (#12642) * fix(docs): add aws-lamda route (#13693) * fix(types): allow override json function with custom return type (#13694) * fix(types): allow override to json function with custom return type * fix(types): remove automatic linter changes Co-authored-by: sander-mol <SMol@thepeoplegroup.nl> * ci(docs): add doc generation to checks (#13704) * docs: add logo (#13700) * docs: add logo * fix(docs): logo not show up (#13699) * fix(build): markdownlint * docs(readme): use internal link * docs(index.md): use internal link * docs(index): update logo rendering in docs * Center logo and headline Co-authored-by: Sascha Depold <sascha@depold.com> Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com> * test: fix mocha (#13707) Co-authored-by: Rik Smale <WikiRik@users.noreply.github.com> Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com> * test: fix failing stack trace test (#13708) * test: fix failing tests (#13709) * test: fix failing tests due to minification Removes esbuild minification which was causing tests to fail and some changed behavior. * Revert "fix(upsert): fall back to DO NOTHING if no update key values provided (#13594)" This reverts commit 4071378. * fix(upsert): fall back to DO NOTHING if no update key values provided (#13711) * fix(upsert): fall back to DO NOTHING if no update key values provided (#13594) * fix: remove erroneous .length in _.isEmpty * refactor: use includes instead of or operators (#13706) * docs(contributing): add section on adding/updating deps (#13715) * docs(contributing): add Node versions Fixes #13714 * docs(contributing): add section on adding/updating deps * fix(types): add Col to where Ops (#13717) * fix(types): add Col to where Ops * fix(types): tests * fix(data-types): unnecessary warning when getting data with DATE dataTypes (#13712) * fix(data-types): unnecessary error when getting data with DATE dataTypes * fix(data-types): date stringify mariadb * fix(types): add missing schema field to sequelize options Fixes #12606 Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com> * fix(example): fix coordinates format as per GeoJson (#13718) * fix(example): fix coordinates format as per GeoJson * Update data-types.js Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com> * ci(node): use Node 16 instead of 12 (#13703) * ci(node): add Node 14 and 16 to DB tests * ci(node): move linting, test typing and release to Node 16 * ci(docs): use Node 16 for docs * ci(node): run tests on Node 10 and 16 Co-authored-by: Rik Smale <WikiRik@users.noreply.github.com> * refactor(postgres): move `clientMinMessages` from general to pg options (#13720) * refactor(postgres): move `clientMinMessages` from general to pg options * refactor(postgres): address review comments * refactor(postgres): address review comments * refactor(postgres): fix pipeline * fix(dialect): try to fix flaky test * fix(dialect): try to fix flaky test Co-authored-by: Jesse Peng <jesse.peng@dynatrace.com> * refactor(build): use rm instead of rmdir for Node 14 and up (#13702) * fix(build): refactor rmdir to rm * refactor(build): only use rm in Node 14 and up * refactor(build): move consts inside function * refactor(build): fix definition Co-authored-by: Rik Smale <WikiRik@users.noreply.github.com> Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com> * docs(postgres): warn about deprecated clientMinMessage option (#13727) * docs(postgres): warn about deprecated clientMinMessage option * docs(deprecation-warning): fix typo in deprecation warning * docs(deprecation-warning): fix typo in deprecation warning * meta(deps): upgrade (dev)deps (#13729) Co-authored-by: Rik Smale <WikiRik@users.noreply.github.com> * test(integration): mark and fix flaky test (#13735) * feat(dialect): snowflake dialect support (#13406) Co-authored-by: Mohamed El Mahallawy <mmahalwy@gmail.com> Co-authored-by: bparan <bparan@softserveinc.com> Co-authored-by: Matthew Blasius <matthew.blasius@expel.io> Co-authored-by: WeRDyin <heyin223@gmail.com> Co-authored-by: Marco Gonzalez <marcogrcr@gmail.com> Co-authored-by: Constantin Metz <constantin@metzworld.com> Co-authored-by: Sander Mol <Sandermol95@hotmail.com> Co-authored-by: sander-mol <SMol@thepeoplegroup.nl> Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com> Co-authored-by: Fauzan <fncolon@pm.me> Co-authored-by: Rik Smale <WikiRik@users.noreply.github.com> Co-authored-by: AllAwesome497 <47748690+AllAwesome497@users.noreply.github.com> Co-authored-by: manish kakoti <madguy02@users.noreply.github.com> Co-authored-by: Marco Kerwitz <marco@kerwitz.com> Co-authored-by: Jesse Peng <vijcp@outlook.com> Co-authored-by: Jesse Peng <jesse.peng@dynatrace.com>

josecolella · 2021-12-04T15:41:03Z

🥳

github-actions · 2021-12-04T18:31:00Z

🎉 This PR is included in version 6.12.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions · 2021-12-17T19:25:58Z

🎉 This PR is included in version 6.12.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

jesse23 changed the title ~~snowflake dialect support~~ feat(dialect): snowflake dialect support Jul 30, 2021

Keimeno added hard For issues and PRs. type: feature DEPRECATED: replace with the "feature" issue type labels Jul 30, 2021

wbourne0 requested changes Aug 5, 2021

View reviewed changes

jesse23 mentioned this pull request Aug 6, 2021

Test PR for final promote Snowflake-Labs/sequelize-snowflake#8

Merged

jesse23 requested a review from wbourne0 August 8, 2021 20:29

wbourne0 reviewed Aug 11, 2021

View reviewed changes

jesse23 force-pushed the snowflake-final-promote branch 4 times, most recently from 11702c0 to ce72c6b Compare August 13, 2021 18:46

jesse23 requested a review from wbourne0 August 13, 2021 18:47

jesse23 force-pushed the snowflake-final-promote branch 3 times, most recently from 191c708 to 56a0957 Compare August 13, 2021 19:19

jesse23 force-pushed the snowflake-final-promote branch from 56a0957 to 1109f58 Compare September 10, 2021 17:44

jmikolay22 force-pushed the snowflake-final-promote branch 2 times, most recently from c585fe3 to 7fec7cc Compare October 21, 2021 22:48

github-actions Bot added the stale label Oct 29, 2021

jmikolay22 force-pushed the snowflake-final-promote branch from 7fec7cc to 1ef2c36 Compare October 29, 2021 20:22

sdepold self-assigned this Nov 2, 2021

github-actions Bot removed the stale label Nov 23, 2021

sdepold requested changes Nov 28, 2021

View reviewed changes

Keimeno reviewed Dec 2, 2021

View reviewed changes

jdanielmyers and others added 10 commits December 2, 2021 21:45

Add snowflake explanation to readme

133083c

feat(dialect): working snowflake connection

21ed033

feat(dialect): working user practice

fbafb32

feat(dialect): verify date processing in snowflake. SPN-17166 (#4)

3224b05

feat(dialect): unit test. SPN-17555 (#5)

9c2b82b

feat(dialect): snowflake dialect support

6707530

fix(dialect): fix Snowflake query syntax (#17)

9a25b92

* fix(dialect): fix Snowflake query syntax * fix(dialect): fix offset-limit test * fix(dialect): fix offset-limit test Co-authored-by: Maria Lee <maria.lee@dynatrace.com>

feat(dialect): clean up test code for snowflake

23f2f75

feat(dialect): update yarn lock

1dbcc8c

feat(snowflake): fix tests

7eb548a

jmikolay22 force-pushed the snowflake-final-promote branch from 69210c9 to 7eb548a Compare December 3, 2021 02:58

Merge remote-tracking branch 'upstream/main' into snowflake-final-pro…

d8e164b

…mote

sdepold approved these changes Dec 3, 2021

View reviewed changes

sdepold merged commit ad68a5e into sequelize:main Dec 3, 2021

github-actions Bot added the released on @v6-beta label Dec 4, 2021

github-actions Bot added the released label Dec 17, 2021

aliatsis pushed a commit to creditiq/sequelize that referenced this pull request Jun 2, 2022

feat(dialect): snowflake dialect support (sequelize#13406)

c858cb4

Uh oh!

Conversation

jesse23 commented Jul 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request check-list

Description of change

Note

How to run integration test

Link with existing defect:

Uh oh!

jesse23 commented Aug 4, 2021

Uh oh!

wbourne0 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jesse23 commented Aug 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jesse23 commented Aug 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wbourne0 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jesse23 commented Aug 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wbourne0 commented Aug 11, 2021

Uh oh!

jesse23 commented Aug 13, 2021

Uh oh!

jesse23 commented Sep 10, 2021

Uh oh!

wbourne0 commented Sep 20, 2021

Uh oh!

sdepold commented Nov 4, 2021

Uh oh!

sdepold commented Nov 22, 2021

Uh oh!

jesse23 commented Nov 22, 2021

Uh oh!

sdepold Nov 28, 2021

Choose a reason for hiding this comment

Uh oh!

jmikolay22 Nov 29, 2021

Choose a reason for hiding this comment

Uh oh!

jesse23 Dec 2, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jesse23 commented Nov 28, 2021

Uh oh!

Keimeno Dec 2, 2021

Choose a reason for hiding this comment

Uh oh!

sdepold commented Dec 3, 2021

jesse23 commented Jul 29, 2021 •

edited

Loading

wbourne0 left a comment •

edited

Loading

jesse23 commented Aug 5, 2021 •

edited

Loading

jesse23 commented Aug 6, 2021 •

edited

Loading

jesse23 commented Aug 11, 2021 •

edited

Loading

jmikolay22 commented Dec 3, 2021 •

edited

Loading