Add a :schema option to allow switching the active schema using 'alter session' #169

Closed
wants to merge 1 commit into
from
@jeffperrin

Makes a schema option available in database.yml like so:

development:
  database: xe
  username: database_user
  password: 1234
  adapter: oracle_enhanced
  host: localhost
  schema: schema_to_use

Use case:

I work in a corporate environment that insists on running the application under a different user/schema than the schema where the application lives (ie; blah_update_user is what we connect with, but the app lives in the blah schema). We could hack around this in Rails by hard-coding schema prefixes in a set_table_name declaration, but this had many implications including making setup of testing databases extremely annoying.

This patch basically just runs alter session set current_schema = schema_name immediately after initializing a connection. We've been using a variation of this patch in several production jruby/rails3/oracle 10/11 applications for the past year without issue.

@jeffperrin

I should add that I ran the test suite under jruby 1.6.7 and rails 3.2.3. There were 12 failures before I made my changes (and the same number of failures afterwards):

rspec ./spec/active_record/connection_adapters/oracle_enhanced_connection_spec.rb:105 # OracleEnhancedConnection create JDBC connection should create new connection using :url and tnsnames alias
rspec ./spec/active_record/connection_adapters/oracle_enhanced_connection_spec.rb:114 # OracleEnhancedConnection create JDBC connection should create new connection using just tnsnames alias
rspec ./spec/active_record/connection_adapters/oracle_enhanced_data_types_spec.rb:585 # OracleEnhancedAdapter timestamp with timezone support / TIMESTAMP WITH TIME ZONE values from ActiveRecord model should return Time value from TIMESTAMP columns
rspec ./spec/active_record/connection_adapters/oracle_enhanced_data_types_spec.rb:599 # OracleEnhancedAdapter timestamp with timezone support / TIMESTAMP WITH TIME ZONE values from ActiveRecord model should return Time value with fractional seconds from TIMESTAMP columns
rspec ./spec/active_record/connection_adapters/oracle_enhanced_data_types_spec.rb:1346 # OracleEnhancedAdapter quoting of NCHAR and NVARCHAR2 columns should create record
rspec ./spec/active_record/connection_adapters/oracle_enhanced_schema_dump_spec.rb:380 # OracleEnhancedAdapter schema dump virtual columns should dump correctly
rspec ./spec/active_record/connection_adapters/oracle_enhanced_schema_dump_spec.rb:396 # OracleEnhancedAdapter schema dump virtual columns with column cache should not change column defaults after several dumps
rspec ./spec/active_record/connection_adapters/oracle_enhanced_schema_statements_spec.rb:1080 # OracleEnhancedAdapter schema definition virtual columns should include virtual columns and not try to update them
rspec ./spec/active_record/connection_adapters/oracle_enhanced_schema_statements_spec.rb:1094 # OracleEnhancedAdapter schema definition virtual columns should add virtual column
rspec ./spec/active_record/connection_adapters/oracle_enhanced_schema_statements_spec.rb:1111 # OracleEnhancedAdapter schema definition virtual columns should add virtual column with explicit type
rspec ./spec/active_record/connection_adapters/oracle_enhanced_schema_statements_spec.rb:1130 # OracleEnhancedAdapter schema definition virtual columns should change virtual column definition
rspec ./spec/active_record/connection_adapters/oracle_enhanced_schema_statements_spec.rb:1151 # OracleEnhancedAdapter schema definition virtual columns should change virtual column type
@yahonda
Collaborator

Thank you for your pull request.

I think some customers have this kind of requirement.
however I do not think it should be merged to the adapter itself, it's not a general one.

In addition it requires this kind of sql statements must be executed to satisfy your pull request.
alter session set current_schema just resolve name space, but it does not make any changes for privileges.

grant select, update, insert delete on blah.foobar to blah_update_user

Then most of your issue can be resolved by creating synonyms at blah_update_user
create synonym foobar on blah.foobar

@jeffperrin

To clarify, what this patch attempts to provide is a feature that mimics MySQL's 'use database' command. The patch is basically composed of two parts:

  1. Use Oracle's equivalent of 'use database': 'alter session set current_schema = 'xxx'' to change the active schema. This removes the need to prefix every query with the name of the schema that you actually want to use. It removes the need to call 'set_table_name' or 'self.table_name' in every model (which makes having a local test and development schema much more difficult than it should be).

  2. Fix some metadata queries in the oracle-enhanced adapter to query the active schema instead of the default schema that is used to connect.

I think this fixes two troublesome use cases with using the adapter in an enterprise environment:

  1. The case where the developer does not have control over schemas and their permissions. I want my Rails application to be able to run in another schema without having to make requests to DBA's to add permissions and/or create synonyms.

  2. The local development scenario where we connect with two different schemas for test and development. If we have to alter our models with 'set_table_name' declarations in order for the application to run in production we have to hack around this for local development.

The patch fixes these two scenarios. Yes, there are ways around this issue without the patch. But the patch turns the solution into a very simple 'schema: blah' declaration in the users database.yml.

@wndxlori

While it may not be a "general" case, it is not an uncommon Oracle configuration. I have run into this at two different client sites with large data warehouse Oracle instances.

@ericgascoine

While I agree that this is not the "general" case, I can only conclude that it's something in the water that requires Western Canadian Oracle DBAs to set up enterprise Oracle instances in this format.

I've come across 3 clients that use Oracle in this fashion (I'm a developer consultant in Calgary).

I've seen the following approaches:
1. The use of the set_table_name solution, usually with code to change the values for Dev, Test and Prod.
2. The use of public/private synonyms
3. Two different implementations of this approach (with Jeff's being one, and certainly the cleanest).

From my experience, I think that Jeff's solution is the cleanest and easiest to maintain. It does require that the developer be aware of the switch of schema and the possible problems with permissions, but at least they can then manage the development process in a succinct and obvious fashion.

This patch definitely has my vote.

@wndxlori

Not just Western Canadian, Eric. One of my clients is in Denver...

@ctreatma

This patch has my vote, too. My office has been maintaining a similar patch on our fork of the adapter, for similar reasons. In environments where web applications run under a different Oracle user than the application schema owner, adding a :schema option to the config is a much cleaner change than the alternatives.

As for the concern that it is only useful to a small part of the oracle-enhanced user base: people who don't need to use the schema option can leave it out. The only effect of merging this patch is that the adapter supports more environments without requiring a fork.

@tamersalama

This request has my vote too - definitely useful.

@reejosamuel

+1 for this, i searched the code to find where to set current schema

@heilja

It is a bit short sighted to say this request does not merit inclusion. Organizations are being asked to increase the security of their applications. Thus, having this feature allows developers and dba's to separate sql code deployment and application operations from schema ownership. Having role separation is key to improving security, and allows for better audit and security compliance.

@cjbj

+1

@unclebilly

I also agree that this is a valuable feature and I would like to see it get merged. I'm not sure it should be so heavy-handed, though... perhaps the alter session should not happen unless the config[:schema] exists? That would at least prevent this change from breaking the adapter for people who haven't specified schema in their database.yml. In other words, this behavior should be opt-in - no? @jeffperrin

@jeffperrin

The behaviour is opt-in, see: https://github.com/rsim/oracle-enhanced/pull/169/files#diff-ca7c8c425714bbfad966883480dd2943R18

That said, this PR is 3 years old now and would need updating if it was to be mergeable. I'm no longer using Oracle at this point either, so someone else would have to do the work.

@barendt

We've been maintaining our own patch that we apply to the adapter that adds the schema option. If this concept is something that has a chance of being accepted I'd be happy to get an updated PR together.

@net1957

+1 for this
At this time we use synonyms but it's always difficult because the DBA has more work to do, it's one more task to do and change management is more difficult.

@yahonda
Collaborator

I understand there are some needs for this feature and here are what I'm thinking about:

  • Create another gem whose supports_migrations? returns false. Not implementing this feature to Oracle enhanced adapter itself.

  • I guess many of those who needs this feature are not using Rails migration rake db:migrate. Then it looks difficult (for me) to implement Oracle enhanced unit tests for this feature since unit tests are working like this - creating new tables, do some tests then drop tables created.

  • It overrides Oracle enhanced adapterSYS_CONTEXT('userenv', 'session_user') with SYS_CONTEXT('userenv', 'current_schema')

@barendt

I'm not sure about the others, but in our case we do use rake db:migrate. We use a RAILS_ENV for migrations that is different from the one we use when serving up the application over the web. The reason for this is that we're using Oracle VPD to implement some additional row-level filtering at the database layer - so we run migrations against one schema and then use the VPD to populate another schema with database objects to serve up to our web users.

For what it's worth, the patch we're using to add support for a schema option looks like this: master...PMACS:schema_patch

@yahonda
Collaborator

#742 has been merged to master. It enables schema options. Thanks guys providing your use cases.

@yahonda yahonda closed this Dec 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment