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

Views with multi-schema joins in postgres #10

Open
mcls opened this issue Aug 28, 2015 · 6 comments
Open

Views with multi-schema joins in postgres #10

mcls opened this issue Aug 28, 2015 · 6 comments

Comments

@mcls
Copy link

mcls commented Aug 28, 2015

I have an issue when defining views with joins between two postgres schemas (e.g. public and client_a).

Suppose we have this view definition:

CREATE VIEW my_view AS SELECT plans.name, clients.id
FROM clients 
JOIN public.plans ON plans.id = clients.plan_id

Then then the following SQL is stored in db/schema.rb (notice missing public prefix for plans):

SELECT plans.name, clients.id
FROM clients 
JOIN plans ON plans.id = clients.plan_id

I'm using this in combination with the apartment gem which loads schema.rb file for every tenant it creates. It also creates all tables for every schema/tenant, even if they are only used in the public schema.

The query to get the views:

SELECT pg_get_viewdef(oid)
FROM pg_class
WHERE relkind = 'v'
AND relname = '#{view_name}'

Not sure how to proceed yet. The views generated by migrations are correct.
Basically I want the "exluded model tables" from apartment to be prefixed with public in the db/schema.rb while the others use search_path.

@mcls
Copy link
Author

mcls commented Aug 28, 2015

Currently using this to transform the SQL received from view_definition to work around it:

class TransformViewDefinitionSQL
  include Procto.call

  # @param [String] view_sql The sql string to transform
  def initialize(view_sql)
    @view_sql = view_sql.freeze
  end

  # @return [String] the transformed SQL
  def call
    pattern = "(#{excluded_table_names.join('|')})"
    view_sql.gsub(/(FROM|JOIN) [(]*#{pattern}\b/, '\1 public.\2')
  end

  protected

  attr_reader :view_sql

  def excluded_table_names
    @excluded_table_names ||= Apartment.excluded_models
      .map(&:constantize)
      .map(&:table_name)
      .map { |t| t.split('.', 2).last }
  end
end

Hackish, but it's the simplest solution I've found.

@ronen
Copy link
Member

ronen commented Aug 28, 2015

@mcls coincidentally, there's a new gem schema_plus_multischema on its way (courtesy of @stenver) that starts improving ActiveRecord's behavior with multiple schemas. Hopefully it'll be ready in a couple of days. (For a preview as of this moment, take a look at this branch, which is currently PR'd to be pulled into master)

But I don't think that gem will immediately solve your problem.

Also, I'm not quite clear on whether what you want is something that's specific to your use of views with the apartment gem, or whether more broadly schema_plus_views should strive to handle cross-schema views more elegantly.

As for how to do it...

I don't know offhand whether there's any way to coerce pg_get_viewdef(oid) to return all the table names appropriately qualified. (Maybe by fiddling with the search path before making the query?)

If there isn't a way to get pg_get_viewdef(oid) to return the properly qualified table names, then some sort of rewriting along the lines you're doing would be the only choice.

I'm embarrassed that when I wrote schema_plus_views, I didn't wrap a middleware stack around the various methods such as views and view_definitions. That'd be the cleanest way to let you add custom rewriting as a hook on view_definitions if something custom is needed. I'd be happy to add the middleware stacks if you need them. (I should add them anyway, because somebody somebody will need them!)

@mcls
Copy link
Author

mcls commented Aug 29, 2015

Thanks for the quick reply.

I'm a fan of your middleware suggestion. I was mostly checking if I didn't miss any obvious solutions. The issue is can't be solved by schema_plus_views on its own, because the excluded tables are only known by Apartment.

We'll be using a simple hack for now because of time constraints. But I'll be revisiting this issue in a couple of weeks.

RE the pg_get_viewdef:
I did experiment in psql with change the search_path before doing pg_get_viewdef.
That does work, but it requires that the other schema is already present (and switching to it) during the schema.rb dump.

I'll keep an eye on schema_plus_multischema.

@stenver
Copy link

stenver commented Aug 29, 2015

@mcls, im not completely sure, but you might be able to experiment with something like this using schema_plus_multischema.

When doing schema dump, then before dumping, set schema search path by asking all the schemas from the db:

schemas = ActiveRecord::Base.connection.execute( <<-SQL # Get schemas
  SELECT schema_name FROM information_schema.schemata;
SQL
).map do |row|  # Map schema names
  row["schema_name"] 
end.select do |schema|  # Remove psql system schemas
  schema.slice(0, 3) != "pg_"
end.join(',') # Join results into correct format

ActiveRecord::Base.connection.schema_search_path = schemas

It should now find all the tables in all schemas. In addition, the tablenames should have their schemas prefixed. Not sure if it will do the trick, but worth the shot.

@ronen
Copy link
Member

ronen commented Sep 1, 2015

@mcls

I'm a fan of your middleware suggestion.

Cool. I've just released 0.3.0 of schema_plus_views with the middleware stacks in place.

I was mostly checking if I didn't miss any obvious solutions.

Nothing obvious to me either, anyway :)

The issue is can't be solved by schema_plus_views on its own, because the excluded tables are only known by Apartment. We'll be using a simple hack for now because of time constraints. But I'll be revisiting this issue in a couple of weeks.

This issue would presumably arise for anybody using apartment & views? Maybe you could make a gem
for using views with apartment -- it could depend on both apartment and schema_plus_views, and add the necessary middleware.

@stenver

... asking all the schemas from the db:

makes me think that schema_plus_multischema should have a method 'schemas' that returns all schemas (possibly with options to include/exclude pg_ and standard ones vs user-defined ones)

@stenver
Copy link

stenver commented Sep 1, 2015

@ronen Thats a good idea. I hope I can make some time in the near future to implement it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants