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

NOT NULL in triggers/functions #263

Closed
BranislavBajuzik opened this issue Aug 7, 2023 · 3 comments
Closed

NOT NULL in triggers/functions #263

BranislavBajuzik opened this issue Aug 7, 2023 · 3 comments

Comments

@BranislavBajuzik
Copy link

Hello.

I am using this tool to lint my Postgres migrations (definitely prevented several headaches on deploy) and I found a false-positive:

BEGIN;
-- Create trigger api_user_update on model user
            CREATE OR REPLACE FUNCTION "public"._pgtrigger_should_ignore(
                trigger_name NAME
            )
            RETURNS BOOLEAN AS $$
                DECLARE
                    _pgtrigger_ignore TEXT[];
                BEGIN
                    IF _pgtrigger_ignore IS NOT NULL THEN
                        SELECT trigger_name = ANY(_pgtrigger_ignore)
                 -- And so on
COMMIT;

The migration/sql is courtesy of django-pgtrigger.

This gets flagged because of the if statement, which I think should not trip up the detection. It gets flagged here:

re.search("(?<!DROP )NOT NULL", sql)

Some of my hand-written migrations also get flagged for this reason.

If I also may, I suggest that the .startswith() checks should be .strip().lower().startswith() to deal with possible future regressions because of indented/hand-written migrations

@David-Wobrock
Copy link
Collaborator

Hey @BranislavBajuzik

Thanks for taking interesting in the library 🙏

Indeed, the checks are a bit raw sometimes. Feel free to suggest a PR! 😄
Else I can try to find some times in the next weeks/months

@kekekekule
Copy link
Contributor

@BranislavBajuzik @David-Wobrock I've suggested a PR here: #273

@David-Wobrock
Copy link
Collaborator

Addressed in #282.

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

No branches or pull requests

3 participants