nullobject/rein

ruby schema does not contain constraint when dumping

ryanong opened this issue · 15 comments

ryanong commented

run migration with any constraint.
schema.rb is never updated with said constraints

nullobject commented

Because rein uses many postgres-specific features, it doesn't really make sense using schema.rb.

If you configure your rails app to use a SQL schema format, then your schema will contain everything you defined in rein:

config.active_record.schema_format = :sql
mvz commented

I have no personal experience with it, but AIUI, the SQL schema format is terrible when put under version control. The schema.rb format apparently does allow for db-specific features (e.g., the using: :btree option for indexes if I'm not mistaken).

Having the option to use schema.rb would be nice.

maxim commented

FWIW this project might serve as a reference on how to extend schema.rb, since it allows creating custom views and doesn't require :sql format: https://github.com/thoughtbot/scenic

danini-the-panini commented

I took a page out of thoughtbot's schema dumper and managed to pull together this little thing:
https://gist.github.com/jellymann/625c2d3cecfde5f8450356ab10185d48

It's only for numericality constraints on PostgreSQL at the moment, but could be a good start for a full-fledged version for rein.

trcarden commented

@nullobject Would you accept a PR generalizing the approach @jellymann across all the other supported constraints etc? I was considering putting in the work but I don't want to waste the time if you don't want it. Let me know

Here is the dumper for scenic: https://github.com/scenic-views/scenic/blob/master/lib/scenic/schema_dumper.rb as well

  • 👍9
Mange commented

I'm also very interested in seeing this.

nruth commented

I have no personal experience with it, but AIUI, the SQL schema format is terrible when put under version control.

I had heard this too, and it put me off for a while, but it works fine in practice. The thing it does make harder is exploring your schema with find/search because key terms you're looking for, like model names, will come up in the table definitions, index definitions, foreign key definitions, constraints, etc, making it a lot more noisy than the ruby dumper. But it's not that bad.

mockdeep commented

It would be great to have schema support. When running rails db:reset in dev mode it loads the schema into the database. If constraints are not in schema.rb they will not show up in dev, taking the local dev environment out of sync with production.

Because rein uses many postgres-specific features, it doesn't really make sense using schema.rb.

There are lots of things that get serialized to schema.rb that are specific to the database implementation. For example, enable_extension calls, as well as data types specific to the database, like hstore and jsonb.

I've worked with structure.sql before and it's kind of a pain. The file keeps changing back and forth when your team has minor differences in their database setup or file system and it is not very easy to navigate as a reference.

mvz commented

I just had the chance look at a structure.sql file, and it seems the formatting has improved: The migration numbers are no longer output on a single line, which would make the diff when adding a single migration much nicer. So, difficult version control of structure.sql is no longer an argument.

mockdeep commented

@mvz the version control problems we saw weren't related to the migration numbers. It also outputs a bunch of settings at the top of the file, and those kept changing back and forth based on the user's system, sometimes just swapping their order.

Out of curiosity, I just ran it on a tiny project and the difference is pretty stark:

schema.rb (54 lines) 
# frozen_string_literal: true

# This file is auto-generated from the current state of the database. Instead
# of editing this file, please use the migrations feature of Active Record to
# incrementally modify your database, and then regenerate this schema definition.
#
# This file is the source Rails uses to define your schema when running `rails
# db:schema:load`. When creating a new database, `rails db:schema:load` tends to
# be faster and is potentially less error prone than running all of your
# migrations from scratch. Old migrations may fail to apply correctly if those
# migrations use external dependencies or application code.
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2019_10_04_222631) do
  # These are extensions that must be enabled in order to support this database
  enable_extension "plpgsql"

  create_table "checks", force: :cascade do |t|
    t.bigint "user_id", null: false
    t.bigint "integration_id", null: false
    t.string "name", null: false
    t.jsonb "data", default: {}, null: false
    t.string "type", null: false
    t.datetime "created_at", precision: 6, null: false
    t.datetime "updated_at", precision: 6, null: false
    t.index ["integration_id"], name: "index_checks_on_integration_id"
    t.index ["user_id", "name"], name: "index_checks_on_user_id_and_name", unique: true
    t.index ["user_id"], name: "index_checks_on_user_id"
  end

  create_table "integrations", force: :cascade do |t|
    t.bigint "user_id", null: false
    t.string "type", null: false
    t.jsonb "data", default: {}, null: false
    t.datetime "created_at", precision: 6, null: false
    t.datetime "updated_at", precision: 6, null: false
    t.index ["type"], name: "index_integrations_on_type"
    t.index ["user_id", "type"], name: "index_integrations_on_user_id_and_type", unique: true
    t.index ["user_id"], name: "index_integrations_on_user_id"
  end

  create_table "users", force: :cascade do |t|
    t.string "email", null: false
    t.string "password_digest", null: false
    t.datetime "created_at", precision: 6, null: false
    t.datetime "updated_at", precision: 6, null: false
    t.index ["email"], name: "index_users_on_email", unique: true
  end

  add_foreign_key "checks", "integrations"
  add_foreign_key "checks", "users"
  add_foreign_key "integrations", "users"
end
structure.sql (358 lines) 
SET statement_timeout = 0;
SET lock_timeout = 0;
SET client_encoding = 'UTF8';
SET standard_conforming_strings = on;
SELECT pg_catalog.set_config('search_path', '', false);
SET check_function_bodies = false;
SET xmloption = content;
SET client_min_messages = warning;
SET row_security = off;

--
-- Name: plpgsql; Type: EXTENSION; Schema: -; Owner: -
--

CREATE EXTENSION IF NOT EXISTS plpgsql WITH SCHEMA pg_catalog;


--
-- Name: EXTENSION plpgsql; Type: COMMENT; Schema: -; Owner: -
--

COMMENT ON EXTENSION plpgsql IS 'PL/pgSQL procedural language';


SET default_tablespace = '';

SET default_with_oids = false;

--
-- Name: ar_internal_metadata; Type: TABLE; Schema: public; Owner: -
--

CREATE TABLE public.ar_internal_metadata (
    key character varying NOT NULL,
    value character varying,
    created_at timestamp(6) without time zone NOT NULL,
    updated_at timestamp(6) without time zone NOT NULL
);


--
-- Name: check_counts; Type: TABLE; Schema: public; Owner: -
--

CREATE TABLE public.check_counts (
    id bigint NOT NULL,
    check_id bigint NOT NULL,
    value bigint NOT NULL,
    created_at timestamp(6) without time zone NOT NULL,
    updated_at timestamp(6) without time zone NOT NULL
);


--
-- Name: check_counts_id_seq; Type: SEQUENCE; Schema: public; Owner: -
--

CREATE SEQUENCE public.check_counts_id_seq
    START WITH 1
    INCREMENT BY 1
    NO MINVALUE
    NO MAXVALUE
    CACHE 1;


--
-- Name: check_counts_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: -
--

ALTER SEQUENCE public.check_counts_id_seq OWNED BY public.check_counts.id;


--
-- Name: checks; Type: TABLE; Schema: public; Owner: -
--

CREATE TABLE public.checks (
    id bigint NOT NULL,
    user_id bigint NOT NULL,
    integration_id bigint NOT NULL,
    name character varying NOT NULL,
    data jsonb DEFAULT '{}'::jsonb NOT NULL,
    type character varying NOT NULL,
    created_at timestamp(6) without time zone NOT NULL,
    updated_at timestamp(6) without time zone NOT NULL
);


--
-- Name: checks_id_seq; Type: SEQUENCE; Schema: public; Owner: -
--

CREATE SEQUENCE public.checks_id_seq
    START WITH 1
    INCREMENT BY 1
    NO MINVALUE
    NO MAXVALUE
    CACHE 1;


--
-- Name: checks_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: -
--

ALTER SEQUENCE public.checks_id_seq OWNED BY public.checks.id;


--
-- Name: integrations; Type: TABLE; Schema: public; Owner: -
--

CREATE TABLE public.integrations (
    id bigint NOT NULL,
    user_id bigint NOT NULL,
    type character varying NOT NULL,
    data jsonb DEFAULT '{}'::jsonb NOT NULL,
    created_at timestamp(6) without time zone NOT NULL,
    updated_at timestamp(6) without time zone NOT NULL
);


--
-- Name: integrations_id_seq; Type: SEQUENCE; Schema: public; Owner: -
--

CREATE SEQUENCE public.integrations_id_seq
    START WITH 1
    INCREMENT BY 1
    NO MINVALUE
    NO MAXVALUE
    CACHE 1;


--
-- Name: integrations_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: -
--

ALTER SEQUENCE public.integrations_id_seq OWNED BY public.integrations.id;


--
-- Name: schema_migrations; Type: TABLE; Schema: public; Owner: -
--

CREATE TABLE public.schema_migrations (
    version character varying NOT NULL
);


--
-- Name: users; Type: TABLE; Schema: public; Owner: -
--

CREATE TABLE public.users (
    id bigint NOT NULL,
    email character varying NOT NULL,
    password_digest character varying NOT NULL,
    created_at timestamp(6) without time zone NOT NULL,
    updated_at timestamp(6) without time zone NOT NULL
);


--
-- Name: users_id_seq; Type: SEQUENCE; Schema: public; Owner: -
--

CREATE SEQUENCE public.users_id_seq
    START WITH 1
    INCREMENT BY 1
    NO MINVALUE
    NO MAXVALUE
    CACHE 1;


--
-- Name: users_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: -
--

ALTER SEQUENCE public.users_id_seq OWNED BY public.users.id;


--
-- Name: id; Type: DEFAULT; Schema: public; Owner: -
--

ALTER TABLE ONLY public.check_counts ALTER COLUMN id SET DEFAULT nextval('public.check_counts_id_seq'::regclass);


--
-- Name: id; Type: DEFAULT; Schema: public; Owner: -
--

ALTER TABLE ONLY public.checks ALTER COLUMN id SET DEFAULT nextval('public.checks_id_seq'::regclass);


--
-- Name: id; Type: DEFAULT; Schema: public; Owner: -
--

ALTER TABLE ONLY public.integrations ALTER COLUMN id SET DEFAULT nextval('public.integrations_id_seq'::regclass);


--
-- Name: id; Type: DEFAULT; Schema: public; Owner: -
--

ALTER TABLE ONLY public.users ALTER COLUMN id SET DEFAULT nextval('public.users_id_seq'::regclass);


--
-- Name: ar_internal_metadata_pkey; Type: CONSTRAINT; Schema: public; Owner: -
--

ALTER TABLE ONLY public.ar_internal_metadata
    ADD CONSTRAINT ar_internal_metadata_pkey PRIMARY KEY (key);


--
-- Name: check_counts_pkey; Type: CONSTRAINT; Schema: public; Owner: -
--

ALTER TABLE ONLY public.check_counts
    ADD CONSTRAINT check_counts_pkey PRIMARY KEY (id);


--
-- Name: checks_pkey; Type: CONSTRAINT; Schema: public; Owner: -
--

ALTER TABLE ONLY public.checks
    ADD CONSTRAINT checks_pkey PRIMARY KEY (id);


--
-- Name: integrations_pkey; Type: CONSTRAINT; Schema: public; Owner: -
--

ALTER TABLE ONLY public.integrations
    ADD CONSTRAINT integrations_pkey PRIMARY KEY (id);


--
-- Name: schema_migrations_pkey; Type: CONSTRAINT; Schema: public; Owner: -
--

ALTER TABLE ONLY public.schema_migrations
    ADD CONSTRAINT schema_migrations_pkey PRIMARY KEY (version);


--
-- Name: users_pkey; Type: CONSTRAINT; Schema: public; Owner: -
--

ALTER TABLE ONLY public.users
    ADD CONSTRAINT users_pkey PRIMARY KEY (id);


--
-- Name: index_check_counts_on_check_id; Type: INDEX; Schema: public; Owner: -
--

CREATE INDEX index_check_counts_on_check_id ON public.check_counts USING btree (check_id);


--
-- Name: index_checks_on_integration_id; Type: INDEX; Schema: public; Owner: -
--

CREATE INDEX index_checks_on_integration_id ON public.checks USING btree (integration_id);


--
-- Name: index_checks_on_user_id; Type: INDEX; Schema: public; Owner: -
--

CREATE INDEX index_checks_on_user_id ON public.checks USING btree (user_id);


--
-- Name: index_checks_on_user_id_and_name; Type: INDEX; Schema: public; Owner: -
--

CREATE UNIQUE INDEX index_checks_on_user_id_and_name ON public.checks USING btree (user_id, name);


--
-- Name: index_integrations_on_type; Type: INDEX; Schema: public; Owner: -
--

CREATE INDEX index_integrations_on_type ON public.integrations USING btree (type);


--
-- Name: index_integrations_on_user_id; Type: INDEX; Schema: public; Owner: -
--

CREATE INDEX index_integrations_on_user_id ON public.integrations USING btree (user_id);


--
-- Name: index_integrations_on_user_id_and_type; Type: INDEX; Schema: public; Owner: -
--

CREATE UNIQUE INDEX index_integrations_on_user_id_and_type ON public.integrations USING btree (user_id, type);


--
-- Name: index_users_on_email; Type: INDEX; Schema: public; Owner: -
--

CREATE UNIQUE INDEX index_users_on_email ON public.users USING btree (email);


--
-- Name: fk_rails_85a57b9170; Type: FK CONSTRAINT; Schema: public; Owner: -
--

ALTER TABLE ONLY public.check_counts
    ADD CONSTRAINT fk_rails_85a57b9170 FOREIGN KEY (check_id) REFERENCES public.checks(id);


--
-- Name: fk_rails_99d169fb59; Type: FK CONSTRAINT; Schema: public; Owner: -
--

ALTER TABLE ONLY public.integrations
    ADD CONSTRAINT fk_rails_99d169fb59 FOREIGN KEY (user_id) REFERENCES public.users(id);


--
-- Name: fk_rails_b84f0d51f7; Type: FK CONSTRAINT; Schema: public; Owner: -
--

ALTER TABLE ONLY public.checks
    ADD CONSTRAINT fk_rails_b84f0d51f7 FOREIGN KEY (integration_id) REFERENCES public.integrations(id);


--
-- Name: fk_rails_d09d3671b2; Type: FK CONSTRAINT; Schema: public; Owner: -
--

ALTER TABLE ONLY public.checks
    ADD CONSTRAINT fk_rails_d09d3671b2 FOREIGN KEY (user_id) REFERENCES public.users(id);


--
-- PostgreSQL database dump complete
--

SET search_path TO "$user", public;

INSERT INTO "schema_migrations" (version) VALUES
('20190913200312'),
('20191004201245'),
('20191004222631'),
('20200412213655');

On a much larger project it was 699 lines in schema.rb and 3485 lines in structure.sql.

mvz commented

those kept changing back and forth based on the user's system, sometimes just swapping their order.

Ouch, that's also nasty.

nruth commented

It's manageable: use the same dbms version, docker, and/or have the person reviewing PRs re-migrate and commit a canonical version before merging.
But yes it would be nice to opt-in to that extra noise and work.

If there's a scope/support concern, could it be a new / extension gem?

mockdeep commented

It's manageable: use the same dbms version, docker, and/or have the person reviewing PRs re-migrate and commit a canonical version before merging.

In other words, It's manageable by adding a whole lot of extra complexity to our development process. This feels like the tail wagging the dog.

If there's a scope/support concern, could it be a new / extension gem?

The gem would have to depend on rein, since it would still be making use of rein features in schema.rb.

danini-the-panini commented

I've put the code from my gist into a PR: #55

Added a simple test, too.