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

Support for Microsoft SQL Server NEWSEQUENTIALID() #11387

Open
2 of 6 tasks
Palpie opened this issue Sep 5, 2019 · 3 comments
Open
2 of 6 tasks

Support for Microsoft SQL Server NEWSEQUENTIALID() #11387

Palpie opened this issue Sep 5, 2019 · 3 comments
Labels
status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action type: feature For issues and PRs. For new features. Never breaking changes.

Comments

@Palpie
Copy link

Palpie commented Sep 5, 2019

Issue Description

When working with an MSSQL database you can have sequential UUID using NEWSEQUENTIALID() as default value. This doesn't seem to work with create() in sequelize. The table is generated successfully using:

id: {
    type: 'uniqueidentifier',
    primaryKey: true,
    defaultValue: Sequelize.literal('newsequentialid()'),
},

But when I try to insert using sequelize create method the generate SQL is:

INSERT INTO [Company] ([id],[unitId],[name]) OUTPUT INSERTED.* VALUES (newsequentialid(),@0,@1);

It results in an error, because newsequentialid() is only supposed to be used in a default constraint: SequelizeDatabaseError: The newsequentialid() built-in function can only be used in a DEFAULT expression for a column of type 'uniqueidentifier' in a CREATE TABLE or ALTER TABLE statement. It cannot be combined with other operators to form a complex scalar expression.

If I work with an existing database and remove the field defaultValue the create works. The id is generated as it should if I look in the database, but sequelize seems to ignore it in the output from the insert statement.

Is your feature request related to a problem? Please describe.

We have an existing database with all id columns as sequential UUID. With sequelize we couldn't insert data and get the generated ID back. This made me look in the the table generation. Both of these problem makes it hard for us to use sequelize in our project.

Describe the solution you'd like

Support to not use the defaultValue in the insert statement, but only in the table creation and not for every insert statement. Also don't remove output from insert statements for the id.

Perhaps a field option generated: true to tell sequelize that the field shouldn't be used in the insert statement and should always be taken from the output after the insert.

Why should this be in Sequelize

When the default value is added as a part of the table definition it shouldn't be necessary to send it with every insert statement.

Describe alternatives/workarounds you've considered

Considered, but not reasonable solutions are:

  • Use something in the added data to get the value from the database. This will get me the ID, but there is too high risk for it not to work.
  • Change the whole database to use int id columns.

Additional context

Add any other context or screenshots about the feature request here.

Issue Template Checklist

Is this issue dialect-specific?

  • No. This issue is relevant to Sequelize as a whole.
  • Yes. This issue only applies to the following dialect(s): MSSQL

Would you be willing to resolve this issue by submitting a Pull Request?

  • Yes, I have the time and I know how to start.
  • Yes, I have the time but I don't know how to start, I would need guidance.
  • No, I don't have the time, although I believe I could do it if I had the time...
  • No, I don't have the time and I wouldn't even know how to start.
@papb
Copy link
Member

papb commented Sep 5, 2019

Hello! Thanks for the detailed request. It makes sense to me.

One question: what SQL query would you like sequelize to have produced instead? Is it the following?

INSERT INTO [Company] ([unitId],[name]) OUTPUT INSERTED.* VALUES (@0,@1);

Just to make sure we are in the same track (also by the way I am not very familiar with MSSQL)

@papb papb added status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action type: feature For issues and PRs. For new features. Never breaking changes. labels Sep 5, 2019
@Palpie
Copy link
Author

Palpie commented Sep 6, 2019

Yes, just like that. The output will be the full inserted row, including the generated UUID.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2021

This issue has been automatically marked as stale because it has been open for 7 days without activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment or remove the "stale" label. 🙂

@github-actions github-actions bot added the stale label Nov 7, 2021
@WikiRik WikiRik removed the stale label Nov 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action type: feature For issues and PRs. For new features. Never breaking changes.
Projects
None yet
Development

No branches or pull requests

3 participants