Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

db diff creates incoherent migration SQL #653

Closed
trbtm opened this issue Nov 29, 2022 · 2 comments
Closed

db diff creates incoherent migration SQL #653

trbtm opened this issue Nov 29, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@trbtm
Copy link

trbtm commented Nov 29, 2022

Bug report

Describe the bug

The bug is closely related to #496 and seems to have the same cause. pgAdmin/supabase is not creating the tables in right order so that the migration will work when applied. Tables are referenced before they are created. Sometimes the order is correct and sometimes not. It is a matter of running it often enough and hope the order is correct or sorting things manually.

This issue and similar ones (regarding the order of statements) is widely known and well documented since more than one year (supabase/pgadmin4#14 and djrobstep/migra#189 (comment)).

Using the flag --use-migra seems to solve the issue. However as documented here https://supabase.com/blog/supabase-cli "Choosing the best diff tool" and here djrobstep/migra#160 migra might not be the best choice in all cases.

I think its is problematic that there seems to be no diffing solution that works with all relevant use cases and the recommended default solution is broken for the most basic use case.

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. Create a new project on app.subabase.com
  2. Initialise a local version of that project (supabase init, supabase link --project-ref, etc.)
  3. Create a migration of the remote database supabase db remote commit
  4. Start the local instance of the project supabase start
  5. Make changes to local database with tables referencing each other
-- CreateTable
CREATE TABLE "Project" (
    "id" SERIAL NOT NULL,
    "name" TEXT NOT NULL,
    "description" TEXT NOT NULL,
    "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
    "updatedAt" TIMESTAMP(3) NOT NULL,
    "deleted" BOOLEAN NOT NULL,

    CONSTRAINT "Project_pkey" PRIMARY KEY ("id")
);

-- CreateTable
CREATE TABLE "Crew" (
    "id" SERIAL NOT NULL,
    "surname" TEXT,
    "lastname" TEXT,
    "email" TEXT NOT NULL,
    "phone" TEXT,
    "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
    "updatedAt" TIMESTAMP(3) NOT NULL,
    "deleted" BOOLEAN NOT NULL,

    CONSTRAINT "Crew_pkey" PRIMARY KEY ("id")
);

-- CreateTable
CREATE TABLE "CrewOnProjects" (
    "projectId" INTEGER NOT NULL,
    "crewId" INTEGER NOT NULL,
    "assignedAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
    "assignedBy" TEXT NOT NULL,
    "deleted" BOOLEAN NOT NULL,

    CONSTRAINT "CrewOnProjects_pkey" PRIMARY KEY ("projectId","crewId")
);

-- CreateIndex
CREATE UNIQUE INDEX "Crew_email_key" ON "Crew"("email");

-- AddForeignKey
ALTER TABLE "CrewOnProjects" ADD CONSTRAINT "CrewOnProjects_projectId_fkey" FOREIGN KEY ("projectId") REFERENCES "Project"("id") ON DELETE RESTRICT ON UPDATE CASCADE;

-- AddForeignKey
ALTER TABLE "CrewOnProjects" ADD CONSTRAINT "CrewOnProjects_crewId_fkey" FOREIGN KEY ("crewId") REFERENCES "Crew"("id") ON DELETE RESTRICT ON UPDATE CASCADE;

(Created with Prisma)

  1. Run the diff supabase db diff -f apply_prisma_migations and you get
-- This script was generated by the Schema Diff utility in pgAdmin 4
-- For the circular dependencies, the order in which Schema Diff writes the objects is not very sophisticated
-- and may require manual changes to the script to ensure changes are applied in the correct order.
-- Please report an issue for any failure with the reproduction steps.

CREATE TABLE IF NOT EXISTS public._prisma_migrations
(
    id character varying(36) COLLATE pg_catalog."default" NOT NULL,
    checksum character varying(64) COLLATE pg_catalog."default" NOT NULL,
    finished_at timestamp with time zone,
    migration_name character varying(255) COLLATE pg_catalog."default" NOT NULL,
    logs text COLLATE pg_catalog."default",
    rolled_back_at timestamp with time zone,
    started_at timestamp with time zone NOT NULL DEFAULT now(),
    applied_steps_count integer NOT NULL DEFAULT 0,
    CONSTRAINT _prisma_migrations_pkey PRIMARY KEY (id)
)

TABLESPACE pg_default;

ALTER TABLE IF EXISTS public._prisma_migrations
    OWNER to postgres;

GRANT ALL ON TABLE public._prisma_migrations TO anon;

GRANT ALL ON TABLE public._prisma_migrations TO authenticated;

GRANT ALL ON TABLE public._prisma_migrations TO postgres;

GRANT ALL ON TABLE public._prisma_migrations TO service_role;

CREATE TABLE IF NOT EXISTS public."Project"
(
    id integer NOT NULL DEFAULT nextval('"Project_id_seq"'::regclass),
    name text COLLATE pg_catalog."default" NOT NULL,
    description text COLLATE pg_catalog."default" NOT NULL,
    "createdAt" timestamp(3) without time zone NOT NULL DEFAULT CURRENT_TIMESTAMP,
    "updatedAt" timestamp(3) without time zone NOT NULL,
    deleted boolean NOT NULL,
    CONSTRAINT "Project_pkey" PRIMARY KEY (id)
)

TABLESPACE pg_default;

ALTER TABLE IF EXISTS public."Project"
    OWNER to postgres;

GRANT ALL ON TABLE public."Project" TO anon;

GRANT ALL ON TABLE public."Project" TO authenticated;

GRANT ALL ON TABLE public."Project" TO postgres;

GRANT ALL ON TABLE public."Project" TO service_role;

CREATE TABLE IF NOT EXISTS public."CrewOnProjects"
(
    "projectId" integer NOT NULL,
    "crewId" integer NOT NULL,
    "assignedAt" timestamp(3) without time zone NOT NULL DEFAULT CURRENT_TIMESTAMP,
    "assignedBy" text COLLATE pg_catalog."default" NOT NULL,
    deleted boolean NOT NULL,
    CONSTRAINT "CrewOnProjects_pkey" PRIMARY KEY ("projectId", "crewId"),
    CONSTRAINT "CrewOnProjects_crewId_fkey" FOREIGN KEY ("crewId")
        REFERENCES public."Crew" (id) MATCH SIMPLE
        ON UPDATE CASCADE
        ON DELETE RESTRICT,
    CONSTRAINT "CrewOnProjects_projectId_fkey" FOREIGN KEY ("projectId")
        REFERENCES public."Project" (id) MATCH SIMPLE
        ON UPDATE CASCADE
        ON DELETE RESTRICT
)

TABLESPACE pg_default;

ALTER TABLE IF EXISTS public."CrewOnProjects"
    OWNER to postgres;

GRANT ALL ON TABLE public."CrewOnProjects" TO anon;

GRANT ALL ON TABLE public."CrewOnProjects" TO authenticated;

GRANT ALL ON TABLE public."CrewOnProjects" TO postgres;

GRANT ALL ON TABLE public."CrewOnProjects" TO service_role;

CREATE TABLE IF NOT EXISTS public."Crew"
(
    id integer NOT NULL DEFAULT nextval('"Crew_id_seq"'::regclass),
    surname text COLLATE pg_catalog."default",
    lastname text COLLATE pg_catalog."default",
    email text COLLATE pg_catalog."default" NOT NULL,
    phone text COLLATE pg_catalog."default",
    "createdAt" timestamp(3) without time zone NOT NULL DEFAULT CURRENT_TIMESTAMP,
    "updatedAt" timestamp(3) without time zone NOT NULL,
    deleted boolean NOT NULL,
    CONSTRAINT "Crew_pkey" PRIMARY KEY (id)
)

TABLESPACE pg_default;

ALTER TABLE IF EXISTS public."Crew"
    OWNER to postgres;

GRANT ALL ON TABLE public."Crew" TO anon;

GRANT ALL ON TABLE public."Crew" TO authenticated;

GRANT ALL ON TABLE public."Crew" TO postgres;

GRANT ALL ON TABLE public."Crew" TO service_role;
CREATE UNIQUE INDEX IF NOT EXISTS "Crew_email_key"
    ON public."Crew" USING btree
    (email COLLATE pg_catalog."default" ASC NULLS LAST)
    TABLESPACE pg_default;
  1. Stop local supabase supabase stop
  2. Start local supabase supabase start

Expected behavior

The migration created by pgAdmin / supabase should be applied without a problem.

Screenshots

Screenshot 2022-11-29 at 16 31 40

System information

  • OS: macOS
  • Version of supabase-cli: 1.16.4
@trbtm trbtm added the bug Something isn't working label Nov 29, 2022
@peterkogo
Copy link

Luckily I found this issue - dodged a bullet there. Thought I was doing something wrong.

I saw that you are using a forked version with no extra commits of pgadmin. Even though I couldn't find a specific commit maybe this is fixed in upstream?

@sweatybridge
Copy link
Contributor

We have switched the default diff tool to migra since CLI 1.28.0. While it's not perfect, the speed and order correctness makes it better than our fork of pgadmin.

We will continue watching this space for better tools to integrate with. The long term goal over the coming years is to converge on a single diff tool that handles all scenarios.

As we realise this is not an easy task, please help us by continue creating bug reports as you find them. We will look into contributing upstream to migra/schemainspect as well.

@sweatybridge sweatybridge closed this as not planned Won't fix, can't repro, duplicate, stale Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants