-
Notifications
You must be signed in to change notification settings - Fork 460
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
Support for ActiveRecord schema format :sql
in migrations
#16
base: development
Are you sure you want to change the base?
Support for ActiveRecord schema format :sql
in migrations
#16
Conversation
Hey thanks for the PR. My only issue is for people using the schema based strategy. I'd want the adapter to throw an appropriate exception if someone has configured Apartment to use schemas AND the sql format. And sorry I'll get to your other PR asap |
Agreed, I'll add that logic so that an exception is raised in that instance. |
Hi, I'm currently using both |
I haven't actually tested this out myself, but my understanding was that the generated schema.sql file would have the postgresql schema name embedded into the table names when it's generated. So if you had a CREATE TABLE public.users ... This is a problem because Apartment uses the schema to populate a new I've never personally tried the sql format though, can you verify what I'm saying above to be true or false? |
Well, here is a glimpse of how it looks/works, it set's the --
-- PostgreSQL database dump
--
SET statement_timeout = 0;
SET client_encoding = 'UTF8';
SET standard_conforming_strings = on;
SET check_function_bodies = false;
SET client_min_messages = warning;
--
-- Name: da; Type: SCHEMA; Schema: -; Owner: -
--
CREATE SCHEMA da;
--
-- Name: SCHEMA da; Type: COMMENT; Schema: -; Owner: -
--
COMMENT ON SCHEMA da IS 'standard public schema';
--
-- Name: hstore; Type: SCHEMA; Schema: -; Owner: -
--
CREATE SCHEMA hstore;
CREATE EXTENSION hstore SCHEMA hstore;
SET search_path = da, pg_catalog;
--
-- Name: _final_mode(anyarray); Type: FUNCTION; Schema: da; Owner: -
--
CREATE FUNCTION _final_mode(anyarray) RETURNS anyelement
LANGUAGE sql IMMUTABLE
AS $_$
SELECT a
FROM unnest($1) a
GROUP BY 1
ORDER BY COUNT(1) DESC, 1
LIMIT 1;
$_$;
--
-- Name: mode(anyelement); Type: AGGREGATE; Schema: da; Owner: -
--
CREATE AGGREGATE mode(anyelement) (
SFUNC = array_append,
STYPE = anyarray,
INITCOND = '{}',
FINALFUNC = _final_mode
);
SET default_tablespace = '';
SET default_with_oids = false;
--
-- Name: advert_candidate_collector_fails; Type: TABLE; Schema: da; Owner: -; Tablespace:
--
CREATE TABLE advert_candidate_collector_fails (
id integer NOT NULL,
exception_message text,
); |
@NielsKSchjoedt Did you create this file, or was it dumped when running migrations? |
Rereading @NielsKSchjoedt comment, I guess it could be possible to parse the schema search path and insert the new apartments schema prior to creating the new database schema. I don't think this is a full proof solution though, as subsequent migrations, when dumped out, would include the schemas of apartments, which would be loaded when adding additional apartments. |
It was created during migration (nothing is changed except for the fact that I manually enhanced it to add |
I don't think I'm understanding your setup as your question seems contradictory:
AFAIK, the schema formats are mutually exclusive, so if the format is set to
The apartment implementation can only be configured to use one method (database or schema). If you have multiple databases, each with identical schema's then this pull request will work for you.
In the multi-schema scenario, with a schema format of Hope this helps... |
Hmm okay, so just to clerify myself on my setup: |
hey @virtualstaticvoid in order for me to accept this can you make a few changes? This gem doesn't have a hard dependency on Rails, so your change would break any other Rack based apps using Apartment. What I'd like to see is this as a config option that one can pass into Also if you can rebase from dev so it can be auto merged that'd be great. |
Cool, I'll have a go at improving it. |
Ok, I've rebased off the As a side note, a default for |
I just posted an issue that relates to this. Has there been any progress on getting this implemented? |
hey sorry, this kind of fell by the wayside. I'll take a look at this PR hopefully soon and get it merged in. |
That would be much appreciated! Keep up the good work |
I've rebased off of the latest development branch, and pushed to a new branch sql_migration_2 on my clone. If you want, I can update this pull request, or you can cherry-pick the commits when you are ready. |
@virtualstaticvoid I just took your changes (virtualstaticvoid/apartment@27431faaf5f50d8b37dc5b384c6083be6828f98a) for a test spin. Am I right that it doesn't support creation of databases with
Schema format is configured to How are you creating new schemas and setting up the test database? |
Ok, so you create your Rails application and Postgres database as usual. Next specify the EDIT: Then after adding migrations, you can apply them to each database by running The test database is created when you run This pull request adds support for the (BTW: the revision you referred to is the current HEAD on development of this repo). |
Thanks @virtualstaticvoid. The git ref was obviously incorrect - I wasn't testing your changes :) Your instructions are clear. The issue now is that we're on Postgres, which appears not to be supported: I'm guessing that's because Postgres dumps It would be great to support this also. Any ideas where to start? |
Ah apologies, I forgot about that important fact. I've corrected my comment above. |
@JonasNielsen So a possible solution, albeit inelegant, would be to parse the |
Thanks @virtualstaticvoid I actually just started hacking on that. I'll share the results in this issue. |
Also, a further complication, is that Rails would need to be (monkey) patched, so that when migrations are executed, and it runs See activerecord/lib/active_record/tasks/postgresql_database_tasks.rb |
Perhaps the solution is the generate a secondary structure.sql file for this purpose, instead of parsing the existing one. The Looking at the |
Any plans to proceed w/ this? |
So, turns out that apartment doesn't support structure.sql (yet) (influitive/apartment/pull/16), so we must go back to schema.rb The only way to make apartment work with hstore is to have a persistent schema, that has the hstore extension installed. I've added this step to the installation procedure. It is not optimal, but makes everything work.
Bah. This just cost me hours trying to figure out why tests wouldn't run after switching to the I understand the complexity, but I'm curious how everyone handles custom SQL commands with new tenants? I have a migration that creates some materialized postgresql views and it doesn't work with schema.rb. Should I just call a command.execute after a tenant is created? |
Hey guys, so what is the recommended way of running custom SQL functions during migrations in multiple-schemas? The current method |
Enable support for the
:sql
schema format in Rails. See ActiveRecord::Base.schema_format for details onschema_format
.For example, this is useful for Postgres databases using
hstore
.NOTE
This method currently does not work for multi-schema databases, since the schema names are usually embedded in the
structure.sql
file.