olirice / alembic_utils

An alembic/sqlalchemy extension for migrating sql views, functions, triggers, and policies
MIT License
193 stars 42 forks source link

PGFunction detection of plpgsql doesn't account for language definition wrapped in quotes #88

Closed MattMcFahn closed 2 years ago

MattMcFahn commented 2 years ago

Hey - have been using alembic_utils for a while now for autogenerated migrations on some views and functions in postgres DBs I'm managing. Has been a really useful library, so thanks!

I found a small bug when trying to register a new plpgsql function on my database.

When instantiating a PGFunction object, you're using a is_plpgsql attribute to indicate whether the function is procedural, and escape colons based on this.

You derive the attribute as:

is_plpgsql: bool = "language plpgsql" in normalize_whitespace(definition).lower()

However, it's valid postgres to declare the language of a plpgsql function as:

LANGUAGE 'plpgsql';

It took me a little while to figure out why the revision I was generating failed to run the upgrade script and identify that I needed to change the SQL in my function to just use LANGUAGE plpgsql.

This is probably quite low priority given it's easy to just to make this change in plpgsql functions being managed by alembic_utils, but might be worth handling this case in the PGFunction init statement. Happy to raise a PR from my side if it'd be helpful.

olirice commented 2 years ago

thanks for opening! a PR would be great