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

Fix multiple primary keys results in syntax error (duplicate "NOT NULL") #12237

Merged
merged 1 commit into from May 9, 2020
Merged

Conversation

soryy708
Copy link
Contributor

@soryy708 soryy708 commented May 9, 2020

Pull Request check-list

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

  • [V] Does npm run test or npm run test-DIALECT pass with this change (including linting)?
  • [V] Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
  • [V] 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)?
  • [V] Did you follow the commit message conventions explained in CONTRIBUTING.md?

Description of change

Fixes #12194

@soryy708 soryy708 changed the title Fix duplicate NOT NULL Fix multiple primary keys results in syntax error (duplicate "NOT NULL") May 9, 2020
@codecov
Copy link

codecov bot commented May 9, 2020

Codecov Report

Merging #12237 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #12237   +/-   ##
=======================================
  Coverage   96.33%   96.33%           
=======================================
  Files          95       95           
  Lines        9118     9120    +2     
=======================================
+ Hits         8784     8786    +2     
  Misses        334      334           
Impacted Files Coverage Δ
lib/dialects/sqlite/query-generator.js 96.63% <100.00%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c1ce2b...ec818b8. Read the comment docs.

@sushantdhiman sushantdhiman merged commit a35ec01 into sequelize:master May 9, 2020
4 checks passed
@soryy708
Copy link
Contributor Author

soryy708 commented May 10, 2020

@sushantdhiman When will a version of sequelize be published that includes this bugfix?
Would you agree that being unable to make a composite key out of a column that has a "not null" constraint (what this PR fixed) is a critical bug?

@sushantdhiman
Copy link
Contributor

sushantdhiman commented May 10, 2020

@soryy708 Master branch is currently tracking v6 beta, I am not sure when next beta release will be shipped.

If you want this in v5 stable release, please submit this PR for v5 branch. It will get auto released upon merge :)

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

Successfully merging this pull request may close these issues.

SQLite dialect: composite key with "NOT NULL" constraint results in syntax error
2 participants