olirice / alembic_utils

An alembic/sqlalchemy extension for migrating sql views, functions, triggers, and policies
https://olirice.github.io/alembic_utils
MIT License
211 stars 43 forks source link

Do not escape plpython functions #69

Closed ddemidov closed 2 years ago

ddemidov commented 2 years ago

This disables escaping of ':' symbols for plpython functions. Without this, the following definition

from textwrap import dedent

PGFunction(
    signature=dedent('''\
        hello(who text)
        '''),
    definition=dedent('''\
        returns text as
        $$
        if who == 'me':
            return 'Hey you!'
        else:
            return 'Hello ' + who
        $$ language plpython3u
        '''),
    schema='public'
    )

gets converted to

CREATE FUNCTION "public"."hello"(who text) returns text as
$$
if who == 'me'\:
    return 'Hey you!'
else\:
    return 'Hello ' + who
$$ language plpython3u

which results in

SyntaxError: unexpected character after line continuation character (<string>, line 3)
codecov-commenter commented 2 years ago

Codecov Report

Merging #69 (b87fc8a) into master (683a447) will decrease coverage by 0.11%. The diff coverage is 80.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##            master      #69      +/-   ##
===========================================
- Coverage   100.00%   99.88%   -0.12%     
===========================================
  Files           18       18              
  Lines          840      842       +2     
===========================================
+ Hits           840      841       +1     
- Misses           0        1       +1     
Flag Coverage Δ
unittests 99.88% <80.00%> (-0.12%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/alembic_utils/pg_function.py 98.33% <80.00%> (-1.67%) :arrow_down:

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 683a447...b87fc8a. Read the comment docs.

olirice commented 2 years ago

thanks for the PR

officially alembic_utils doesn't support non-language sql functions. The plpgsql line was a forced measure to get something working at the office but it is not public/documented.

Since this is a small update, if it were covered by an integration test, I'd merge it and leave the behavior undocumented but that won't be possible without using a custom docker image b/c the base postgres images don't have plpythonu installed.

In this case it might be easier for you to subclass PGFunction and override the __init__ in your application code.

ddemidov commented 2 years ago

Fair enough. Thank you for the subclassing tip, I had not thought of that!