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 for variable whitespace in “CHECK” enum constraints #289

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Dsolnik
Copy link

@Dsolnik Dsolnik commented Sep 28, 2023

allow for multiple spaces before the check keyword and any amount of white space (including 0) after the check keyword but before the in keyword:

For example, if your constraints was CHECK IN('a', 'b', 'c'), this would not be recognized.

Dsolnik and others added 2 commits September 28, 2023 18:22
Allow for spaces between enum options
@coveralls
Copy link

coveralls commented Sep 28, 2023

Coverage Status

coverage: 97.639%. remained the same when pulling c07f3a0 on Dsolnik:patch-1 into c68d2a8 on agronholm:master.

@agronholm
Copy link
Owner

I don't think the changes you made do what you think they do.

@agronholm
Copy link
Owner

Basically all your changes do is allow any amount of whitespace before and after the IN.

@Dsolnik
Copy link
Author

Dsolnik commented Sep 28, 2023

Tired Thursday apologies.

Yes, this PR allows for multiple spaces before the check keyword and any amount of white space (including 0) after the check keyword but before the in keyword.

@agronholm
Copy link
Owner

Just what kind of CHECK constraint did you have a problem with that wouldn't work with the existing code? I have a hard time understanding.

@Dsolnik
Copy link
Author

Dsolnik commented Sep 28, 2023

My constraint was “CHECK IN('a', 'b', 'c')”, notice the lack of spaces after the IN

@Dsolnik Dsolnik changed the title Update generators.py Allow for variable whitespace in “CHECK” enum constraints Sep 28, 2023
@agronholm
Copy link
Owner

I thought the RDBMS would normalize these. What RDBMS was this on?

@Dsolnik
Copy link
Author

Dsolnik commented Sep 28, 2023 via email

@agronholm
Copy link
Owner

Ah, that answers my question. I guess it's only PostgreSQL that normalizes those then.

@agronholm
Copy link
Owner

I will need an accompanying test to ensure that the PR fixes what it says it fixes (and won't break in the future). Meanwhile I'll experiment with sqlite (I don't know how to operate SQL Server).

@Dsolnik
Copy link
Author

Dsolnik commented Sep 28, 2023 via email

@cameronclaero
Copy link

cameronclaero commented Oct 3, 2023

@Dsolnik would you also be able to add a fix for #291? In this instance the spaces were are the start and end of the constraint eg " CAP_RESTRICT_LEVEL IN ('NONE', 'WARN', 'RESTRICT') ". I was in the process of creating a pr but looks like you are already making changes ;)

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.

4 participants