DanCardin / sqlalchemy-declarative-extensions

Library to declare additional kinds of objects not natively supported by SqlAlchemy/Alembic.
https://sqlalchemy-declarative-extensions.readthedocs.io/en/latest/
Apache License 2.0
31 stars 5 forks source link

False-postitive anaysis of the the multi-line PostgreSQL functions #40

Closed YaraslauZhylko closed 1 year ago

YaraslauZhylko commented 1 year ago

Function definition:

Function(
    "on_update_timestamp",
    """
    BEGIN
        NEW.updated_at = CLOCK_TIMESTAMP();
        RETURN NEW;
    END
    """,
    returns="trigger",
    language="plpgsql",
)

Generated Alembic command:

op.execute("""CREATE FUNCTION on_update_timestamp() RETURNS trigger LANGUAGE plpgsql AS $$
    BEGIN
        NEW.updated_at = CLOCK_TIMESTAMP();
        RETURN NEW;
    END
    $$;""")

The statement is correct and is successfully applied.


But when trying to generate a new migration, the function is incorrectly considered to be modified:

def upgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    op.execute("""CREATE OR REPLACE FUNCTION on_update_timestamp() RETURNS trigger LANGUAGE plpgsql AS $$
        BEGIN
            NEW.updated_at = CLOCK_TIMESTAMP();
            RETURN NEW;
        END
        $$;""")
    # ### end Alembic commands ###

def downgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    op.execute("""CREATE OR REPLACE FUNCTION on_update_timestamp() RETURNS trigger LANGUAGE plpgsql AS $$
            BEGIN
                NEW.updated_at = CLOCK_TIMESTAMP();
                RETURN NEW;
            END
            $$;""")
    # ### end Alembic commands ###

As you can see, the only difference is in the number of leading spaces.

This is most probably caused by the indentation of the Alembic code and the way Python handles multi-line strings.


To mitigate that on my side I have a custom function:

def oneliner(multiliner: str) -> str:
    return " ".join(map(str.strip, multiliner.split("\n"))).strip()

that I use to wrap function definitions:

Function(
    "on_update_timestamp",
    oneliner(
        """
        BEGIN
            NEW.updated_at = CLOCK_TIMESTAMP();
            RETURN NEW;
        END
        """
    ),
    returns="trigger",
    language="plpgsql",
)

The resulting Alembic command looks like that:

op.execute("""CREATE FUNCTION on_update_timestamp() RETURNS trigger LANGUAGE plpgsql AS $$BEGIN NEW.updated_at = CLOCK_TIMESTAMP(); RETURN NEW; END$$;""")

As the final function has no new lines and no indentation, trying to generate a new migration does not cause false-positive changes.


The potential fix would be to use the same oneliner() function to pre-process both the Python definition and the returned PostgreSQL definition before comparing them and making a decision on whether the function has to be re-created.

DanCardin commented 1 year ago

Thanks for the report! The Function/Trigger implementations are under utilized in comparison to some of the other features, so much appreciated.

Fix should be released in v0.6.5

YaraslauZhylko commented 1 year ago

@DanCardin The release seems to work fine.

No false-positive migrations generated. Even if I reformat the multi-line function code to make it look prettier (as long as the relative indentation of the function body is maintained).

But if I change the relative indentation in the function definition, a re-create command is generated as expected.

Thanks a lot!