stripe / pg-schema-diff

Go library for diffing Postgres schemas and generating SQL migrations
MIT License
364 stars 26 forks source link

Add support for stored procedures #159

Open bheemvennapureddy opened 2 months ago

bheemvennapureddy commented 2 months ago

I did try to add a STORED PROCEDURE but the plan didn't generate a diff with the new file

Navbryce commented 2 months ago

It should. Do you have the SQL for reference, pg dump of your schema, and the command you are running to generate the diff?

bheemvennapureddy commented 2 months ago
-- ------------ Write DROP-PROCEDURE-stage scripts -----------

-- DROP ROUTINE IF EXISTS dbo."Usp_Archive"(IN INTEGER, IN INTEGER, INOUT INT, INOUT REFCURSOR);

-- ------------ Write CREATE-DATABASE-stage scripts -----------

--CREATE SCHEMA IF NOT EXISTS dbo;

-- ------------ Write CREATE-PROCEDURE-stage scripts -----------

CREATE OR REPLACE PROCEDURE dbo."Usp_Archive"(IN par_count INTEGER, IN par_numdays_to_keep INTEGER, INOUT return_code INT DEFAULT 0, INOUT p_refcur REFCURSOR DEFAULT NULL)
AS 
$BODY$
DECLARE
    var_cutoff TIMESTAMP WITHOUT TIME ZONE DEFAULT timezone('UTC', CURRENT_TIMESTAMP(6)) - (par_numdays_to_keep::NUMERIC || ' days')::INTERVAL;
BEGIN
    CREATE TEMPORARY TABLE "VISITKEYS_USP_ARCHIVE"
    ("VISIT_KEY" BIGINT);
    CREATE TEMPORARY TABLE "REQUESTKEYS_USP_ARCHIVE"
    ("REQUEST_KEY" BIGINT);
    RAISE NOTICE 'selecting Visits';
    INSERT INTO "VISITKEYS_USP_ARCHIVE"
    SELECT
        "VISIT_KEY"
        FROM dbo."Visit"
        WHERE "DATE_ENTERED_UTC" < var_cutoff
        LIMIT 250;
    RAISE NOTICE 'selecting Requests';
    INSERT INTO "REQUESTKEYS_USP_ARCHIVE"
    SELECT
        "REQUEST_KEY"
        FROM dbo."Request"
        WHERE "VISIT_KEY" IN (SELECT "VISIT_KEY" FROM "VISITKEYS_USP_ARCHIVE");
    RAISE NOTICE 'inserting into Visit';
    INSERT INTO webanalyticsarchivecandev_dbo."Visit" ("VISIT_KEY", "CHANNEL", "DATE_ENTERED_UTC", "DATE_SESSION_END_UTC", "DESCRIPTION", "KEYWORDS", "LINK_TYPE", "SEARCH_QUERY", "SUB_CHANNEL", "USER_AGENT", "VISITOR_KEY", "LOAN_APPLICATION_KEY")
    SELECT
        "VISIT_KEY", "CHANNEL", "DATE_ENTERED_UTC", "DATE_SESSION_END_UTC", "DESCRIPTION", "KEYWORDS", "LINK_TYPE", "SEARCH_QUERY", "SUB_CHANNEL", "USER_AGENT", "VISITOR_KEY", "LOAN_APPLICATION_KEY"
        FROM dbo."Visit"
        WHERE "VISIT_KEY" IN (SELECT "VISIT_KEY" FROM "VISITKEYS_USP_ARCHIVE");
    RAISE NOTICE 'inserting into Request';
    INSERT INTO webanalyticsarchivecandev_dbo."Request" ("REQUEST_KEY", "VISIT_KEY", "URL_KEY", "REQUEST_TYPE", "IP_ADDRESS", "DATE_ENTERED_UTC", "NOTE", "DEVICE_KEY", "SERVER_NAME")
    SELECT
        "REQUEST_KEY", "VISIT_KEY", "URL_KEY", "REQUEST_TYPE", "IP_ADDRESS", "DATE_ENTERED_UTC", "NOTE", "DEVICE_KEY", "SERVER_NAME"
        FROM dbo."Request"
        WHERE "VISIT_KEY" IN (SELECT "VISIT_KEY" FROM "VISITKEYS_USP_ARCHIVE");
    RAISE NOTICE 'inserting into RequestDetail';
    INSERT INTO webanalyticsarchivecandev_dbo."RequestDetail" ("REQUEST_DETAIL_KEY", "REQUEST_KEY", "NAME", "VALUE")
    SELECT
        "REQUEST_DETAIL_KEY", "REQUEST_KEY", "NAME", "VALUE"
        FROM dbo."RequestDetail"
        WHERE "REQUEST_KEY" IN (SELECT "REQUEST_KEY" FROM "REQUESTKEYS_USP_ARCHIVE");
    RAISE NOTICE 'deleting from LastFocusedField';
    DELETE FROM dbo."LastFocusedField"
        WHERE "REQUEST_KEY" IN (SELECT "REQUEST_KEY" FROM "REQUESTKEYS_USP_ARCHIVE");
    RAISE NOTICE 'deleting from RequestDetail';
    DELETE FROM dbo."RequestDetail"
        WHERE "REQUEST_KEY" IN (SELECT "REQUEST_KEY" FROM "REQUESTKEYS_USP_ARCHIVE");
    RAISE NOTICE 'deleting from ValidationError';
    DELETE FROM dbo."ValidationError"
        WHERE "REQUEST_KEY" IN (SELECT "REQUEST_KEY" FROM "REQUESTKEYS_USP_ARCHIVE");
    RAISE NOTICE 'deleting from Request';
    DELETE FROM dbo."Request"
        WHERE "REQUEST_KEY" IN (SELECT "REQUEST_KEY" FROM "REQUESTKEYS_USP_ARCHIVE");
    RAISE NOTICE 'deleting from Visit';
    DELETE FROM dbo."Visit"
        WHERE "VISIT_KEY" IN (SELECT "VISIT_KEY" FROM "VISITKEYS_USP_ARCHIVE");
    OPEN p_refcur FOR
    SELECT
        COUNT(1)
        FROM "VISITKEYS_USP_ARCHIVE";
    return_code := 0;
    DROP TABLE IF EXISTS "VISITKEYS_USP_ARCHIVE";
    DROP TABLE IF EXISTS "REQUESTKEYS_USP_ARCHIVE";
    RETURN;
END;
$BODY$
LANGUAGE plpgsql;

here is the stored proc

command used

➜  WebAnalytics git:(Add_postgres_db_migrations) ✗ pg-schema-diff apply --dsn "postgresql://postgres:mysecretpassword@localhost:5432/webanalytics" --schema-dir WebAnalytics.Database.Postgres/db/Scripts/Create --allow-hazards HAS_UNTRACKABLE_DEPENDENCIES --skip-confirm-prompt
Schema matches expected. No plan generated
bheemvennapureddy commented 2 months ago
Screenshot 2024-08-26 at 11 09 32 PM

if i use two folders i run into this issue

 WebAnalytics git:(Add_postgres_db_migrations) ✗ pg-schema-diff apply --dsn "postgresql://postgres:mysecretpassword@localhost:5432/webanalytics" --schema-dir WebAnalytics.Database.Postgres/db/Scripts/Create --allow-hazards HAS_UNTRACKABLE_DEPENDENCIES --skip-confirm-prompt  --schema-dir WebAnalytics.Database.Postgres/db/Procedures
Error: generating plan: getting new schema: running DDL: ERROR: schema "dbo" does not exist (SQLSTATE 3F000)

if i keep them in one folder then it doesn't recognize the stored proc

Navbryce commented 2 months ago

Okay, I was mistaken. We do not support stored procedures. I was under the impression that function support == stored procedure support (and that stored procedures are just syntactic sugar for functions). This is only partially true. In the interim, you could structure your migrations to always create or replace your stored procedures at the end.

bheemvennapureddy commented 2 months ago

In the interim, you could structure your migrations to always create or replace your stored procedures at the end.

Can you elaborate on what you mean by that?

bplunkett-stripe commented 2 months ago

Can you elaborate on what you mean by that? You can plan your migration with --insert-statement that create or replace your stored procedures every time. Insert statement lets you inject migration statements into your migration plan.

Unfortunately, the offset is in terms of index from the start instead of the end. I might add support for "negative indexes" in the future, such that the offset can also be in terms of a relative position from the end of the migration statements.

bheemvennapureddy commented 2 months ago

So for now i can't do stored procedures ?

bplunkett-stripe commented 2 months ago

So for now i can't do stored procedures ? You can do them using the workaround I suggested, but otherwise, the tooling does not support them yet. I can look into adding support for them.

bheemvennapureddy commented 2 months ago

So for now i can't do stored procedures ?

You can do them using the workaround I suggested, but otherwise, the tooling does not support them yet. I can look into adding support for them.

But isn't the workaround to run them from start rather than at the end ?

bplunkett-stripe commented 2 months ago

But isn't the workaround to run them from start rather than at the end ?

It's a bit situation dependent, but generally makes sense to run at the end after any adds/alters that may occur.

bheemvennapureddy commented 1 month ago

Do you anticipate supporting stored procedures any time soon?

bplunkett-stripe commented 1 month ago

I can try to knock it out soon-ish. Stored procedures shouldn't be too bad to implement. They're definitely a bit on the trickier end, however.

Notably, we can't track the dependencies of pg/plsql functions. Postgres doesn't even track those dependencies.

bheemvennapureddy commented 1 week ago

@bplunkett-stripe will you be able to knock this down ?

bheemvennapureddy commented 6 days ago

any thoughts on this @Navbryce ?

Navbryce commented 6 days ago

I was looking into this today! Yes, I can try to get it done this week. I have a prototype for a continuing refactor I'm also working on, but I can try to prioritize this first.

bheemvennapureddy commented 5 days ago

Appreciate that. Thanks

bheemvennapureddy commented 1 day ago

Are you planning to make the changes so that the stored proc runs everytime or similar to any changes made to SQL file in general ?

bplunkett-stripe commented 1 day ago

Are you planning to make the changes so that the stored proc runs everytime or similar to any changes made to SQL file in general ?

Stored procedures don't have dependency tracking in Postgres except at initilization. So it will probably look like

...Delete old stored procedures...
...
...
All other migration statements
...
...
...
...
Create new/alter existing stored procedures