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

Is it possible to add indexes to views? #17

Open
mfasanya opened this issue Jul 31, 2015 · 8 comments
Open

Is it possible to add indexes to views? #17

mfasanya opened this issue Jul 31, 2015 · 8 comments

Comments

@mfasanya
Copy link

Is it possible to add indexes to views?

@pineapplethief
Copy link

Yeah, ability to add indices like CREATE INDEX on users USING gin(to_tsvector('english', first_name)
in models/.*sql view definition files is a must have IMO

@pineapplethief
Copy link

I see two options here: either pass column names and languages as a hash in a is_view macro, the way unique index is working now, or parse *.sql files looking for first encounter of CREATE INDEX, everything before that goes into view sql, and everything after is executed separately. I like the second way more since a) there is one source of truth on view definition, no need to jump back and forth between AR class and sql file, b) it's more flexible, developer can execute any sql he wants.

@jasoncodes
Copy link
Owner

I agree that this would be a valuable feature.

I am hesitant to add SQL statements to the .sql file as to implement this properly would require an SQL syntax parser. I do not really want to do something ad hoc like spliting after lines ending in a semicolon. Maybe that’d be good enough though. Having everything in the .sql file is appealing.

Another option would be requiring the .sql file to contain valid SQL statements (including a CREATE MATERIALIZED VIEW call). I think I’d rather an ad hoc parser than this though.

@pineapplethief
Copy link

@jasoncodes You could grab anything starting with CREATE [UNIQUE] INDEX and ending with ; and treat that as index query. While pretty simplistic, this approach solves the problem and provides a great deal of flexibility which is so needed, since when using materialized views for full-text search indices could get really tricky, especially with multiple languages of content in some fields. It's not production-usable without indices, and adding indices in separate migrations kind of steal the fun away.

@jasoncodes
Copy link
Owner

Having thought about this some more, I think my preferred solution here is a new after_create_sql option that you can use in the model file. e.g.:

is_view materialized: true, after_create_sql: <<-SQL
  CREATE UNIQUE INDEX ON foos(name);
  CREATE INDEX ON foos USING gin(to_tsvector('english', content));
SQL

This fits in with existing options, is easier to implement, and keeps the .sql file as a single SELECT statement.

If you wish to have the index creation statements in another file, you could use something like File.read('app/models/foo.after.sql') instead of an inline heredoc. Built-in support for an additional file such as this is something I would welcome but would probably not implement initially.

How does this sound?

@pineapplethief
Copy link

Personally, I would rather not fracture definition of the view into 3 files: model file, select query and separate indices query for sake of DRYness. That's why I believe keeping everything in sql file is the best option. You could parse #{table_name} in sql file to make things easier for defining indices, like
CREATE INDEX ON #{table_name} to avoid tedious repetition.

It all boils down to "abstract SQL away" vs. "flexibility and directness of raw SQL" in the end. You can't really eliminate raw sql completely, so why not give users ability to leverage it? Abstracting away CREATE [MATERIALIZED] VIEW [view_name] is nice, but not a deal breaker.

Or maybe you can provide dsl a-la what you have now for unique indices, something along the lines of

is_view materialized: true

full_text_search title: :russian, content: [:russian, :english], subtitle: {english: {using: :gist}}

Problem with DSL approach is of course a need for developer to learn the DSL as well as difficulty covering all use-cases. As I said, full text indices could get quite complex, with sql function calls and different weights of different language indices and so on and so on.

@jasoncodes
Copy link
Owner

I am against creating a new DSL for indexes but could see supporting running a block in a migration-like context which would give access to add_index and any other migration gems you have installed. Not something I want to do now though.

If I were to add support for additional statements in the .sql file, perhaps the first statement needs to be CREATE [MATERIALIZED] VIEW … [WITH NO DATA];, which would allow ActiveRecordViews to just execute this file in its entirety without having to parse it. The more I think about it, the more I do not want to have to parse this file. Enabling this raw mode would most likely be a option to is_view rather than some magic that detects SELECT or CREATE. Not sure what to call the option though.

I am not opposed to also having something like the above but I think I personally would prefer to use an after_create_sql option so I will be implementing that regardless of if (and probably before) the above happens.

@jasoncodes
Copy link
Owner

I’ve revisited this Issue and discussion and I am thinking adding support for a generic “after view create” ActiveSupport callback is the best way to solve this. A “before view drop” callback might be useful too for anything that won’t automatically drop when the view is dropped. The callbacks would be invoke in the context of the model class and allow you create indexes with connection.add_index … and call any other migration API you’d like.

We may have to do something extra to ensure connection is correct in transaction scenarios (e.g. tests) which need without_transaction to maintain consistency. We may just end up passing in the connection as an argument to the callback methods.

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