jasoncodes / activerecord_views

Automatic PostgreSQL database view creation for ActiveRecord
MIT License
35 stars 10 forks source link

Is it possible to add indexes to views? #17

Open mfasanya opened 9 years ago

mfasanya commented 9 years ago

Is it possible to add indexes to views?

pineapplethief commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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.