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

Allow optional keywords in create table unique constraints #2077

Merged
merged 7 commits into from Dec 11, 2021
Merged

Allow optional keywords in create table unique constraints #2077

merged 7 commits into from Dec 11, 2021

Conversation

kayman-mk
Copy link
Contributor

@kayman-mk kayman-mk commented Dec 9, 2021

Brief summary of the change made

Allows optional keywords and index name in unique constraints of create table.

CREATE TABLE a(
    a INT NOT NULL,
    UNIQUE KEY (a),
    UNIQUE INDEX (a),
    UNIQUE KEY idx_a(a),
    UNIQUE idx_b(a),
    UNIQUE (a),
);
  • the index name is optional
  • key and index keywords are optional

At least MySql and PostgreSql are using this syntax. I didn't found anything in SQL-99 but the implementation was already present in dialect_mysql.py

Are there any other side effects of this change that we should be aware of?

No.

Pull Request checklist

  • Please confirm you have completed any of the necessary steps below.

  • Included test cases to demonstrate any code changes, which may be one or more of the following:

    • .yml rule test cases in test/fixtures/rules/std_rule_cases.
    • .sql/.yml parser test cases in test/fixtures/dialects (note YML files can be auto generated with python test/generate_parse_fixture_yml.py or by running tox locally).
    • Full autofix test cases in test/fixtures/linter/autofix.
    • Other.
  • Added appropriate documentation for the change.

  • Created GitHub issues for any relevant followup/future enhancements if appropriate.

@kayman-mk kayman-mk changed the title Allow optional keywords in create tableconstraints Allow optional keywords in create table unique constraints Dec 9, 2021
@kayman-mk kayman-mk closed this Dec 9, 2021
@kayman-mk kayman-mk reopened this Dec 9, 2021
@kayman-mk kayman-mk marked this pull request as ready for review December 9, 2021 13:04
@jpy-git
Copy link
Contributor

jpy-git commented Dec 9, 2021

@kayman-mk I'm conscious that this PR is impacting the same area of the code as #2076 that we've just merged in. Can you update your fork and deal with any potential merge conflicts before I review?

@kayman-mk
Copy link
Contributor Author

@jpy-git Branch updated. No conflicts found.

@codecov
Copy link

codecov bot commented Dec 9, 2021

Codecov Report

Merging #2077 (923fed5) into main (982aef1) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main     #2077   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          148       148           
  Lines        10599     10603    +4     
=========================================
+ Hits         10599     10603    +4     
Impacted Files Coverage Δ
src/sqlfluff/dialects/dialect_mysql.py 100.00% <100.00%> (ø)

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 982aef1...923fed5. Read the comment docs.

@jpy-git
Copy link
Contributor

jpy-git commented Dec 9, 2021

So I can see this implementation in the mysql docs: https://dev.mysql.com/doc/refman/8.0/en/create-table.html#:~:text=%5BCONSTRAINT%20%5Bsymbol%5D%5D%20UNIQUE%20%5BINDEX%20%7C%20KEY%5D

However reading the postgres docs the syntax for unique constraints seems to be different?: https://www.postgresql.org/docs/14/sql-createtable.html

Copy link
Contributor

@jpy-git jpy-git left a comment

Choose a reason for hiding this comment

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

@kayman-mk can you move this logic to mysql specific please?

src/sqlfluff/dialects/dialect_ansi.py Outdated Show resolved Hide resolved
src/sqlfluff/dialects/dialect_ansi.py Outdated Show resolved Hide resolved
src/sqlfluff/dialects/dialect_mysql.py Outdated Show resolved Hide resolved
@kayman-mk kayman-mk marked this pull request as draft December 11, 2021 10:07
@kayman-mk kayman-mk marked this pull request as ready for review December 11, 2021 10:21
@jpy-git
Copy link
Contributor

jpy-git commented Dec 11, 2021

@kayman-mk great work, thanks for adding this! 🥇

For some reason I don't have the usual option to update the branch for the latest changes in main 🤷
image

Could you just pull in the latest changes from main and then we can get this merged in? Thanks 😄

@jpy-git jpy-git merged commit 31c1b1f into sqlfluff:main Dec 11, 2021
@tunetheweb
Copy link
Member

For some reason I don't have the usual option to update the branch for the latest changes in main 🤷

When people open a PR there’s a tick box to allow maintainers to alter the branch. Most people leave it ticked to allow small changes and updates to be applied.

@jpy-git
Copy link
Contributor

jpy-git commented Dec 11, 2021

@tunetheweb ah makes sense thanks! 😄

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.

None yet

3 participants