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.

Sign up for GitHub

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

Jump to bottom

CREATE SEQUENCE for decimal (sql server) #26566

Merged
merged 1 commit into from
Nov 8, 2021
Merged

CREATE SEQUENCE for decimal (sql server) #26566

merged 1 commit into from
Nov 8, 2021

Conversation

bobbyangers
Copy link
Contributor

  • I've read the guidelines for contributing and seen the walkthrough
  • I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
  • The code builds and tests pass locally (also verified by our automated build checks)
  • Commit messages follow this format:
        Summary of the changes
        - Detail 1
        - Detail 2

        Fixes #bugnumber
  • Tests for the changes have been added (for bug fixes / features)
  • Code follows the same patterns and style as existing code in this repo

@dnfadmin
Copy link

dnfadmin commented Nov 8, 2021 •
edited
Loading

CLA assistant check
All CLA requirements met.

@bobbyangers bobbyangers marked this pull request as ready for review November 8, 2021 01:42
@bobbyangers bobbyangers force-pushed the dev/rangers-sequence-decimal branch from 56d1a60 to 861ac25 Compare November 8, 2021 01:45
@bobbyangers bobbyangers changed the title fix #26562 CREATE SEQUENCE for decimal - fix #26562 Nov 8, 2021
@bobbyangers bobbyangers changed the title CREATE SEQUENCE for decimal - fix #26562 CREATE SEQUENCE for decimal (sql server) - fix #26562 Nov 8, 2021
@bobbyangers bobbyangers changed the title CREATE SEQUENCE for decimal (sql server) - fix #26562 CREATE SEQUENCE for decimal (sql server) Nov 8, 2021
@bobbyangers bobbyangers force-pushed the dev/rangers-sequence-decimal branch from 861ac25 to b452b54 Compare November 8, 2021 14:40
roji
roji reviewed Nov 8, 2021
View reviewed changes
Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

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

Thanks, this looks almost ready - just another small note on testing.

test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs Outdated Show resolved Hide resolved
@bobbyangers bobbyangers force-pushed the dev/rangers-sequence-decimal branch from b452b54 to 3f76dd6 Compare November 8, 2021 17:54
@bobbyangers
Summary of the changes
5c09c24 
- Generated SQL >> CREATE SEQUENCE wrong when using decimal
   was generating (18, 2) which SQL returned an error
- Added tests for int, long, short, byte, decimal

Fixes dotnet#26562
@bobbyangers bobbyangers force-pushed the dev/rangers-sequence-decimal branch from 3f76dd6 to 5c09c24 Compare November 8, 2021 18:13
@bobbyangers bobbyangers requested a review from roji November 8, 2021 19:01
roji
roji reviewed Nov 8, 2021
View reviewed changes
Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

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

Thanks @bobbyangers!

@roji roji merged commit dbc86fd into dotnet:main Nov 8, 2021
@bobbyangers bobbyangers deleted the dev/rangers-sequence-decimal branch November 8, 2021 23:08
@ajcvickers ajcvickers mentioned this pull request Nov 18, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajcvickers ajcvickers ajcvickers left review comments

@roji roji roji approved these changes

Assignees
No one assigned
Labels
community-contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bobbyangers @dnfadmin @ajcvickers @roji