stripe / pg-schema-diff

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

Add support for FK switching between non-castable types #73

Closed sidyakinian closed 7 months ago

sidyakinian commented 9 months ago

Description

This is a draft PR for adding support for FK switching between non-castable types. Draft PR because:

  1. For now it only handles the case of bigint -> timestamp.
  2. The logic should probably be structured a lot better.

Motivation

Solves https://github.com/stripe/pg-schema-diff/issues/51.

Testing

Docker test pass. (There are 4 errors though, but they were there before the changes as well)

Considering this test case of migration:

CREATE TABLE events (
    timestamp [BIGINT -> TIMESTAMP] PRIMARY KEY,
    replicated_timestamp BIGINT
);

CREATE TABLE effects (
    name VARCHAR(26),
    timestamp [BIGINT -> TIMESTAMP],
    FOREIGN KEY (timestamp) REFERENCES public.events(timestamp)
);

Before implementing the changes, planning the migration returns an error:

Error: generating plan: validating migration plan: running migration plan: executing migration statement: {ALTER TABLE "public"."effects" ALTER COLUMN "timestamp" SET DATA TYPE timestamp without time zone using to_timestamp("timestamp" / 1000) 3s 3s [ACQUIRES_ACCESS_EXCLUSIVE_LOCK: This will completely lock the table while the data is being re-written for a duration of time that scales with the size of your data. The values previously stored as BIGINT will be translated into a TIMESTAMP value via the PostgreSQL to_timestamp() function. This translation will assume that the values stored in BIGINT represent a millisecond epoch value.]}: ERROR: foreign key constraint "effects_timestamp_fkey" cannot be implemented (SQLSTATE 42804) 

After the changes, here's the generated plan:

1. ALTER TABLE "public"."effects" DROP CONSTRAINT "effects_timestamp_fkey";
        -- Statement Timeout: 3s

3. ALTER TABLE "public"."effects" ALTER COLUMN "timestamp" SET DATA TYPE timestamp without time zone using to_timestamp("timestamp" / 1000);
        -- Statement Timeout: 3s
        -- Hazard ACQUIRES_ACCESS_EXCLUSIVE_LOCK: This will completely lock the table while the data is being re-written for a duration of time that scales with the size of your data. The values previously stored as BIGINT will be translated into a TIMESTAMP value via the PostgreSQL to_timestamp() function. This translation will assume that the values stored in BIGINT represent a millisecond epoch value.

4. ANALYZE "effects" ("timestamp");
        -- Statement Timeout: 20m0s
        -- Lock Timeout: 3s
        -- Hazard IMPACTS_DATABASE_PERFORMANCE: Running analyze will read rows from the table, putting increased load on the database and consuming database resources. It won't prevent reads/writes to the table, but it could affect performance when executing queries.

5. ALTER TABLE "public"."events" ALTER COLUMN "timestamp" SET DATA TYPE timestamp without time zone using to_timestamp("timestamp" / 1000);
        -- Statement Timeout: 3s
        -- Hazard ACQUIRES_ACCESS_EXCLUSIVE_LOCK: This will completely lock the table while the data is being re-written for a duration of time that scales with the size of your data. The values previously stored as BIGINT will be translated into a TIMESTAMP value via the PostgreSQL to_timestamp() function. This translation will assume that the values stored in BIGINT represent a millisecond epoch value.

6. ANALYZE "events" ("timestamp");
        -- Statement Timeout: 20m0s
        -- Lock Timeout: 3s
        -- Hazard IMPACTS_DATABASE_PERFORMANCE: Running analyze will read rows from the table, putting increased load on the database and consuming database resources. It won't prevent reads/writes to the table, but it could affect performance when executing queries.

7. ALTER TABLE "public"."effects" ADD CONSTRAINT "effects_timestamp_fkey" FOREIGN KEY ("timestamp") REFERENCES events("timestamp");
        -- Statement Timeout: 3s
        -- Hazard ACQUIRES_SHARE_ROW_EXCLUSIVE_LOCK: This will lock writes to the owning table and referenced table while the constraint is being added. Instead, consider adding the constraint as NOT VALID and validating it later.
CLAassistant commented 9 months ago

CLA assistant check
All committers have signed the CLA.

bplunkett-stripe commented 7 months ago

I'm going to close this, since it's stale at this point. Feel free to re-open if you're still working on it!