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

Microsoft SQL Server doesn't support ORDER in sequences #901

Open
wants to merge 1 commit into
base: master
from

Conversation

@danielthegray
Copy link

commented Jul 31, 2019

The CREATE SEQUENCE command of Microsoft SQL Server doesn't support the ORDER clause (see at https://docs.microsoft.com/en-us/sql/t-sql/statements/create-sequence-transact-sql?view=sql-server-2017).

Therefore, it should be ignored for this DB as well. I have tested this change on an SQL Server 2017 (I had an error with the CREATE SEQUENCE command before, and after applying this and installing to my local Maven repo, the problem was gone).

@danielthegray danielthegray force-pushed the danielthegray:fix-sequence-order-mssql branch from ae40844 to 880469b Jul 31, 2019

@danielthegray danielthegray force-pushed the danielthegray:fix-sequence-order-mssql branch from 880469b to b8fba46 Aug 1, 2019

@schrieveslaach

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

Somewhere liqiubase calls ValidationErrors#checkDisallowedField for PostgreSQL and statest that ORDER is not supported. Is it required to call this for MSSQL as well?

@danielthegray

This comment has been minimized.

Copy link
Author

commented Aug 2, 2019

It would depend on the focus you want to have, I guess. Should it ignore the parameter that it doesn't understand, or should it fail explicitly if there is one? If PostgreSQL fails/checks explicitly, why do MySQL/MariaDB and Sysbase get to ignore it? (there might be a valid reason, I'm just asking 😃)

I'm guessing the implications of having an ordered sequence on Oracle become a non-ordered sequence on another DB will be up to each app developer, but I feel that the decision to either fail loudly or ignore the ORDER parameter is an architectural decision that I can't make.

What I did here was simply "follow" a pattern that I had seen was already present in the code. However, if we have conflicting MOs for this kind of thing, I think it would be a good idea to unify them and just pick one.

@schrieveslaach

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2019

I think, we need some guidance through @nvoxland what is the correct approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.