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

Some SQL statements generate from supabase db diff are ordered in an unexecutable way #496

Closed
dshukertjr opened this issue Oct 2, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@dshukertjr
Copy link
Member

Bug report

Describe the bug

Sorry if I'm missing something obvious, but SQL statements produced by supabase db diff seems to be ordered in an unexecutable way. For example, create table statement might be placed after a function definition that depends on the table. Using v1.5.4 of the CLI.

To Reproduce

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

  1. Start Supabase locally with supabase start
  2. Go to local console and run the following SQL
SQL statements
-- USERS
CREATE TYPE public.user_status AS ENUM ('ONLINE', 'OFFLINE');
CREATE TABLE public.users (
  username text primary key,
  data jsonb DEFAULT null,
  age_range int4range DEFAULT null,
  status user_status DEFAULT 'ONLINE'::public.user_status,
  catchphrase tsvector DEFAULT null,
  interests text[] DEFAULT null
);
ALTER TABLE public.users REPLICA IDENTITY FULL; -- Send "previous data" to supabase 
COMMENT ON COLUMN public.users.data IS 'For unstructured data and prototyping.';

-- CHANNELS
CREATE TABLE public.channels (
  id bigint GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
  data jsonb DEFAULT null,
  slug text
);
ALTER TABLE public.users REPLICA IDENTITY FULL; -- Send "previous data" to supabase
COMMENT ON COLUMN public.channels.data IS 'For unstructured data and prototyping.';

-- MESSAGES
CREATE TABLE public.messages (
  id bigint GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
  data jsonb DEFAULT null,
  message text,
  username text REFERENCES users NOT NULL,
  channel_id bigint REFERENCES channels NOT NULL,
  inserted_at timestamp with time zone default timezone('utc'::text, now()) not null
);
ALTER TABLE public.messages REPLICA IDENTITY FULL; -- Send "previous data" to supabase
COMMENT ON COLUMN public.messages.data IS 'For unstructured data and prototyping.';

-- REACTIONS
CREATE TABLE public.reactions (
  id bigint GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
  emoji TEXT NOT NULL,
  message_id bigint REFERENCES messages NOT NULL,
  created_at timestamp with time zone default timezone('utc'::text, now()) not null
);

-- STORED FUNCTION
CREATE FUNCTION public.get_status(name_param text)
RETURNS user_status AS $$
  SELECT status from users WHERE username=name_param;
$$ LANGUAGE SQL IMMUTABLE;

CREATE FUNCTION public.get_username_and_status(name_param text)
RETURNS TABLE(username text, status user_status) AS $$
  SELECT username, status from users WHERE username=name_param;
$$ LANGUAGE SQL IMMUTABLE;

CREATE FUNCTION public.void_func() 
RETURNS void AS $$
$$ LANGUAGE SQL;
  1. Run supabase db diff create_tables -f create_tables
  2. Run supabase db reset, which fails, because order of the SQL statements.
SQL statements that get generated
-- 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 OR REPLACE FUNCTION public.get_status(
	name_param text)
    RETURNS user_status
    LANGUAGE 'sql'
    COST 100
    IMMUTABLE PARALLEL UNSAFE
AS $BODY$
  SELECT status from users WHERE username=name_param;
$BODY$;

ALTER FUNCTION public.get_status(text)
    OWNER TO postgres;

GRANT EXECUTE ON FUNCTION public.get_status(text) TO PUBLIC;

GRANT EXECUTE ON FUNCTION public.get_status(text) TO anon;

GRANT EXECUTE ON FUNCTION public.get_status(text) TO authenticated;

GRANT EXECUTE ON FUNCTION public.get_status(text) TO postgres;

GRANT EXECUTE ON FUNCTION public.get_status(text) TO service_role;

CREATE OR REPLACE FUNCTION public.void_func(
	)
    RETURNS void
    LANGUAGE 'sql'
    COST 100
    VOLATILE PARALLEL UNSAFE
AS $BODY$
$BODY$;

ALTER FUNCTION public.void_func()
    OWNER TO postgres;

GRANT EXECUTE ON FUNCTION public.void_func() TO PUBLIC;

GRANT EXECUTE ON FUNCTION public.void_func() TO anon;

GRANT EXECUTE ON FUNCTION public.void_func() TO authenticated;

GRANT EXECUTE ON FUNCTION public.void_func() TO postgres;

GRANT EXECUTE ON FUNCTION public.void_func() TO service_role;

CREATE OR REPLACE FUNCTION public.get_username_and_status(
	name_param text)
    RETURNS TABLE(username text, status user_status) 
    LANGUAGE 'sql'
    COST 100
    IMMUTABLE PARALLEL UNSAFE
    ROWS 1000

AS $BODY$
  SELECT username, status from users WHERE username=name_param;
$BODY$;

ALTER FUNCTION public.get_username_and_status(text)
    OWNER TO postgres;

GRANT EXECUTE ON FUNCTION public.get_username_and_status(text) TO PUBLIC;

GRANT EXECUTE ON FUNCTION public.get_username_and_status(text) TO anon;

GRANT EXECUTE ON FUNCTION public.get_username_and_status(text) TO authenticated;

GRANT EXECUTE ON FUNCTION public.get_username_and_status(text) TO postgres;

GRANT EXECUTE ON FUNCTION public.get_username_and_status(text) TO service_role;

CREATE TABLE IF NOT EXISTS public.reactions
(
    id bigint NOT NULL GENERATED BY DEFAULT AS IDENTITY ( INCREMENT 1 START 1 MINVALUE 1 MAXVALUE 9223372036854775807 CACHE 1 ),
    emoji text COLLATE pg_catalog."default" NOT NULL,
    message_id bigint NOT NULL,
    created_at timestamp with time zone NOT NULL DEFAULT timezone('utc'::text, now()),
    CONSTRAINT reactions_pkey PRIMARY KEY (id),
    CONSTRAINT reactions_message_id_fkey FOREIGN KEY (message_id)
        REFERENCES public.messages (id) MATCH SIMPLE
        ON UPDATE NO ACTION
        ON DELETE NO ACTION
)

TABLESPACE pg_default;

ALTER TABLE IF EXISTS public.reactions
    OWNER to postgres;

GRANT ALL ON TABLE public.reactions TO anon;

GRANT ALL ON TABLE public.reactions TO authenticated;

GRANT ALL ON TABLE public.reactions TO postgres;

GRANT ALL ON TABLE public.reactions TO service_role;

CREATE TABLE IF NOT EXISTS public.users
(
    username text COLLATE pg_catalog."default" NOT NULL,
    data jsonb,
    age_range int4range,
    status user_status DEFAULT 'ONLINE'::user_status,
    catchphrase tsvector,
    interests text[] COLLATE pg_catalog."default",
    CONSTRAINT users_pkey PRIMARY KEY (username)
)

TABLESPACE pg_default;

ALTER TABLE IF EXISTS public.users
    OWNER to postgres;

GRANT ALL ON TABLE public.users TO anon;

GRANT ALL ON TABLE public.users TO authenticated;

GRANT ALL ON TABLE public.users TO postgres;

GRANT ALL ON TABLE public.users TO service_role;

COMMENT ON COLUMN public.users.data
    IS 'For unstructured data and prototyping.';

CREATE TABLE IF NOT EXISTS public.messages
(
    id bigint NOT NULL GENERATED BY DEFAULT AS IDENTITY ( INCREMENT 1 START 1 MINVALUE 1 MAXVALUE 9223372036854775807 CACHE 1 ),
    data jsonb,
    message text COLLATE pg_catalog."default",
    username text COLLATE pg_catalog."default" NOT NULL,
    channel_id bigint NOT NULL,
    inserted_at timestamp with time zone NOT NULL DEFAULT timezone('utc'::text, now()),
    CONSTRAINT messages_pkey PRIMARY KEY (id),
    CONSTRAINT messages_channel_id_fkey FOREIGN KEY (channel_id)
        REFERENCES public.channels (id) MATCH SIMPLE
        ON UPDATE NO ACTION
        ON DELETE NO ACTION,
    CONSTRAINT messages_username_fkey FOREIGN KEY (username)
        REFERENCES public.users (username) MATCH SIMPLE
        ON UPDATE NO ACTION
        ON DELETE NO ACTION
)

TABLESPACE pg_default;

ALTER TABLE IF EXISTS public.messages
    OWNER to postgres;

GRANT ALL ON TABLE public.messages TO authenticated;

GRANT ALL ON TABLE public.messages TO anon;

GRANT ALL ON TABLE public.messages TO service_role;

GRANT ALL ON TABLE public.messages TO postgres;

COMMENT ON COLUMN public.messages.data
    IS 'For unstructured data and prototyping.';

CREATE TABLE IF NOT EXISTS public.channels
(
    id bigint NOT NULL GENERATED BY DEFAULT AS IDENTITY ( INCREMENT 1 START 1 MINVALUE 1 MAXVALUE 9223372036854775807 CACHE 1 ),
    data jsonb,
    slug text COLLATE pg_catalog."default",
    CONSTRAINT channels_pkey PRIMARY KEY (id)
)

TABLESPACE pg_default;

ALTER TABLE IF EXISTS public.channels
    OWNER to postgres;

GRANT ALL ON TABLE public.channels TO anon;

GRANT ALL ON TABLE public.channels TO authenticated;

GRANT ALL ON TABLE public.channels TO postgres;

GRANT ALL ON TABLE public.channels TO service_role;

COMMENT ON COLUMN public.channels.data
    IS 'For unstructured data and prototyping.';

-- Type: user_status

-- DROP TYPE IF EXISTS public.user_status;

CREATE TYPE public.user_status AS ENUM
    ('ONLINE', 'OFFLINE');

ALTER TYPE public.user_status
    OWNER TO postgres;

Expected behavior

After running supabase db diff to generate the migration file, we should be able to run supabase db reset.

System information

  • OS: macOS
  • Version of Supabase CLI: v1.5.4
@dshukertjr dshukertjr added the bug Something isn't working label Oct 2, 2022
@sweatybridge
Copy link
Contributor

sweatybridge commented Oct 2, 2022

Hello @dshukertjr , could you try diffing with --use-migra experimental flag? We found that it covers some cases in which pgadmin fails. The full command is:

supabase db diff --use-migra create_tables -f create_tables

Here's the generated output on my local dev:

SQL statements
create type "public"."user_status" as enum ('ONLINE', 'OFFLINE');

create table "public"."channels" (
    "id" bigint generated by default as identity not null,
    "data" jsonb,
    "slug" text
);


create table "public"."messages" (
    "id" bigint generated by default as identity not null,
    "data" jsonb,
    "message" text,
    "username" text not null,
    "channel_id" bigint not null,
    "inserted_at" timestamp with time zone not null default timezone('utc'::text, now())
);


create table "public"."reactions" (
    "id" bigint generated by default as identity not null,
    "emoji" text not null,
    "message_id" bigint not null,
    "created_at" timestamp with time zone not null default timezone('utc'::text, now())
);


create table "public"."users" (
    "username" text not null,
    "data" jsonb,
    "age_range" int4range,
    "status" user_status default 'ONLINE'::user_status,
    "catchphrase" tsvector,
    "interests" text[]
);


CREATE UNIQUE INDEX channels_pkey ON public.channels USING btree (id);

CREATE UNIQUE INDEX messages_pkey ON public.messages USING btree (id);

CREATE UNIQUE INDEX reactions_pkey ON public.reactions USING btree (id);

CREATE UNIQUE INDEX users_pkey ON public.users USING btree (username);

alter table "public"."channels" add constraint "channels_pkey" PRIMARY KEY using index "channels_pkey";

alter table "public"."messages" add constraint "messages_pkey" PRIMARY KEY using index "messages_pkey";

alter table "public"."reactions" add constraint "reactions_pkey" PRIMARY KEY using index "reactions_pkey";

alter table "public"."users" add constraint "users_pkey" PRIMARY KEY using index "users_pkey";

alter table "public"."messages" add constraint "messages_channel_id_fkey" FOREIGN KEY (channel_id) REFERENCES channels(id) not valid;

alter table "public"."messages" validate constraint "messages_channel_id_fkey";

alter table "public"."messages" add constraint "messages_username_fkey" FOREIGN KEY (username) REFERENCES users(username) not valid;

alter table "public"."messages" validate constraint "messages_username_fkey";

alter table "public"."reactions" add constraint "reactions_message_id_fkey" FOREIGN KEY (message_id) REFERENCES messages(id) not valid;

alter table "public"."reactions" validate constraint "reactions_message_id_fkey";

set check_function_bodies = off;

CREATE OR REPLACE FUNCTION public.get_status(name_param text)
 RETURNS user_status
 LANGUAGE sql
 IMMUTABLE
AS $function$
  SELECT status from users WHERE username=name_param;
$function$
;

CREATE OR REPLACE FUNCTION public.get_username_and_status(name_param text)
 RETURNS TABLE(username text, status user_status)
 LANGUAGE sql
 IMMUTABLE
AS $function$
  SELECT username, status from users WHERE username=name_param;
$function$
;

CREATE OR REPLACE FUNCTION public.void_func()
 RETURNS void
 LANGUAGE sql
AS $function$
$function$
;

We are leaning towards defaulting to migra in the future if it works for most people. So please also let us know if you find anything missing from migra generated diff. For eg. #420 (comment)

@sweatybridge
Copy link
Contributor

Feel free to reopen if this is still a blocker for you.

@dshukertjr
Copy link
Member Author

Thanks! Using --use-migra flag worked like a charm!

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

2 participants