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

How to add a function with an " text[] default array['text', 'text2']"? #106

Open jankatins opened 1 year ago

jankatins commented 1 year ago

I've a functions which looks like this:

compare_commits_per_table = PGFunction(
    schema=metadata_schema_name,
    signature="""
compare_commits_per_table(schema_name text,
                          table_name text,
                          first_branch_id bigint, first_commit_id bigint,
                          second_branch_id bigint, second_commit_id bigint,
                          id_columns text[],
                          ignore_columns text[] default array ['branch_id', 'commit_id_from', 'commit_id_to'],
                          out table_diff jsonb)""",
    definition="""
language 'plpgsql' as
$func$
...
$func$""",
)

The resulting error during alembic revision --autogenerate -m "Add diff functions" is

sqlalchemy.exc.ProgrammingError: (psycopg2.errors.SyntaxError) syntax error at or near "'commit_id_from'"
LINE 1: ...bigint, id_columns text[], ignore_columns text[], 'commit_id...
                                                             ^

[SQL: DROP FUNCTION "branch_metadata"."compare_commits_per_table"(schema_name text, table_name text, first_branch_id bigint, first_commit_id bigint, second_branch_id bigint, second_commit_id bigint, id_columns text[], ignore_columns text[], 'commit_id_from', 'commit_id_to'], out table_diff jsonb) ]
(Background on this error at: https://sqlalche.me/e/20/f405)

https://github.com/olirice/alembic_utils/blob/master/src/alembic_utils/pg_function.py#L90

For now I have a workaround to set the default to NULL and use coalesce( ignore_columns, array ['branch_id', 'commit_id_from', 'commit_id_to'])later, but it would have been nice to actually to use the regular default value.

olirice commented 1 year ago

There isn't currently a good solution for this.

The clearest path forward would be to improve the parameter parser at the line you referenced above

If you have bandwidth to submit a PR I'd be very happy to see this improve

andressommerhoff commented 3 months ago

Hi, are there any updates on this issue? I encountered the same problem where Alembic interprets a comma in the default definition as the beginning of a new parameter, instead of recognizing it as part of the content within "[" and "]". I also unsuccessfully tried using a string formatted as '{1,2,3}' and casting it, hoping that the parser might handle commas within a string differently than in a standard array declaration. Unfortunately, it behaved the same way.

If there have been no updates on this issue, I might find some time to take a look at the code myself. Could you kindly point out where you think I should start, particularly in the pg_function parser? I briefly reviewed the code in pg_function.py and replaceable_entity.py, but it wasn't immediately clear to me where the parser that processes the signature is involved.

olirice commented 3 months ago

Sure, this would be a good place to start https://github.com/olirice/alembic_utils/blob/3711ec78d0d8513f7cc75a31b9ab71bc54182802/src/alembic_utils/pg_function.py#L90-L94