PMACS / scenic_oracle_enhanced_adapter

OracleEnhanced adapter for Scenic gem.
MIT License
5 stars 2 forks source link

no_data option parameter is required in create_materialized_view #18

Closed timseal closed 4 years ago

timseal commented 5 years ago

Hello esteemed colleagues,

The scenic gem has

Scenic.database.create_materialized_view(
  name,
  sql_definition,
  no_data: no_data(materialized),
)

but we only have two parameters: https://github.com/PMACS/scenic_oracle_enhanced_adapter/blob/ab43cce2bd50472833696f47bd75dde7ec179d6d/lib/scenic/adapters/oracle_enhanced.rb#L33

"Add no_data option to materialized views" is the commit that added the new parameter: https://github.com/scenic-views/scenic/commit/a21c476455f4871dcdf643f209921511365ecb6d

So when we try to create a new materialized view, the following happens:

-- create_view(:wp_mv_dw_proposals, {:materialized=>true})
rails aborted!
StandardError: An error has occurred, all later migrations canceled:

wrong number of arguments (given 3, expected 2)
/home/bthoman/.gem/ruby/2.6.0/gems/scenic_oracle_enhanced_adapter-0.0.2/lib/scenic/adapters/oracle_enhanced.rb:33:in `create_materialized_view'
/home/bthoman/.gem/ruby/2.6.0/gems/scenic-1.5.1/lib/scenic/statements.rb:40:in `create_view'

Maybe we can look at this on infrastructure hackathon day?

RaymondFallon commented 5 years ago

👍 And I think if we're adding parameters to create_materialized_view, we'll need need to add them to drop_materialized_view so that it can still be rolled back correctly in a change method. This mismatch caused a problem previously.

riyengar8 commented 5 years ago

Thanks @timseal and @RaymondFallon . I think I see what you mean. I think we should add the hash parameter to create_materialized_view, but I don't think we can actually do anything with it in regards to WITH NO DATA, as that seems to be a thing that is not supported in Oracle unless I am missing something. I guess having the hash parameter there would allow us to pass any other materialized-view specific options we may want to implement, but a good start would be to add the parameter to the signature and just not do anything with it.

@RaymondFallon - I don't think I'm following what you are saying about adding a param to drop_materialized_view. It looks to me like Scenic will only ever pass one parameter to that method (so even the two parameter signature we have now really never gets used as far as I can tell)

https://github.com/scenic-views/scenic/blob/master/lib/scenic/statements.rb#L65

mattalat commented 5 years ago

Hello friends @timseal and @riyengar8.

I found a DEFERRED option in the Oracle doc for Materialized Views which seems like it might be related? If this isn't related, perhaps we could/should just pass false as a default.

It's also possible that such an option doesn't exist in Oracle since the MySQL docs seem to imply that an MView must always be populated upon creation:

Materialized Views can be refreshed in different kinds. They can be refreshed:

  • never (only once in the beginning, for static data only)
  • on demand (for example once a day, for example after nightly load)
  • immediately (after each statement)

This could be a PG-only option.

@RaymondFallon I think update_materialized_view is called when reverting a view's version rather than drop_materialized_view, unless I have also misunderstood you.

riyengar8 commented 5 years ago

@mattalat - I think you may be on to something. BUILD DEFERRED looks to my amateur eye to be the same sort of functionality as PG's WITH NO DATA.

That said, I bet we (PMACS) won't use that option any time soon, so if we wanted to kick the can on support for it that's fine by me. Or if we want to support it, I'm good with that too.