Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Use the schema_search_path in prepared statements. #3232

Merged
merged 2 commits into from

4 participants

@Juanmcuello

I'm using postgreSQL with multiple schemas.

Rails 3.1 uses prepared statements and does not take into account the 'schema_search_path'. This is a big problem because the statement is prepared once and then the same prepared statement is executed many times no matter if you changed the schema, i.e, the prepared statement is executed ALWAYS for the same schema, the one set when the statement was prepared.

This patch adds the schema search to the sql key to allow the use of prepared statements when changing schemas in postgreSQL,

Discussion here:
https://groups.google.com/forum/#!msg/rubyonrails-core/r2VOKcyUyrs/JQj4MB_lhRMJ

@Juanmcuello Juanmcuello Use the schema_search_path in prepared statements.
To allow the use of prepared statements when changing schemas in
postgres, the schema search path is added to the sql key.
cfc95d8
@tenderlove
Owner

Can you write a test for this? We have schema tests for PG in the active record suite.

@Juanmcuello Juanmcuello refs #3232. Prepared statements and postgreSQL schemas.
Add tests for prepared statements with multiple schemas in
postgreSQL.
ac659b8
@Juanmcuello

Done!

@tenderlove
Owner

Great! Thank you! :heart:

@tenderlove tenderlove merged commit 3088d23 into rails:master
@kevinsapp

Thank you for creating this patch! This problem was driving me crazy until I found your post on the topic. Until this patch is "officially released" in an upcoming version of Rails, I'm working around the problem by calling

ActiveRecord::Base.connection.clear_cache! # HACK: Workaround

...just before setting a new search path. I'm not familiar with the Rails release process, but I hope this fix is included in the next version.

@Juanmcuello

I hope so too! Meanwhile, I'm using the same workaround.

Good to know the patch works for you. :)

@Juanmcuello

@jonleighton, any chance to include this in next 3.1.2 release?

@jonleighton
Collaborator

The RC has already gone out but I am pretty sure this is in there - check the CHANGELOG though.

@jonleighton
Collaborator

Actually, doesn't look like it is in the release, sorry.

It's too late now to put it in I'm afraid. If you'd like it in the next release, please submit a pull request with a backport. Thanks.

@Juanmcuello

After digging a while into git history, I think this is what happened:

In master branch, part of the code introduced by cfc95d8 (included in this pull request) was later modified in 6a28c51.

Instead of porting the two commits to 3.1.2, only the last commit was ported. It was not ported exactly as it was, it was modified instead, and the changes included code from the first commit (???). This changes resulted in commit 818d285.

This pull request also includes tests, but they were not ported to 3.1.2 at all.

So, the problem solved by this pull request seems to be already solved in 3.1.2 by 818d285. The only missing part are tests.

Do I create a pull request to back-port tests?

@jonleighton
Collaborator

Ok, thanks for looking into it :)

If you can create a PR with the tests and a CHANGELOG entry (under 3.1.2) that would be great. Thanks.

@Juanmcuello Juanmcuello referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@Juanmcuello Juanmcuello referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@Juanmcuello Juanmcuello referenced this pull request from a commit in Juanmcuello/rails
@Juanmcuello Juanmcuello Backport #3232 to 3-1-stable.
Use the schema_search_path in prepared statements in postgres.

Only the tests are backported, the fix was already included by
commit 818d285.
b02daec
@Juanmcuello Juanmcuello deleted the Juanmcuello:pg_prepared_statements branch
@ttosch ttosch referenced this pull request from a commit
@Juanmcuello Juanmcuello Backport #3232 to 3-1-stable.
Use the schema_search_path in prepared statements in postgres.

Only the tests are backported, the fix was already included by
commit 818d285.
32d4380
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 5, 2011
  1. @Juanmcuello

    Use the schema_search_path in prepared statements.

    Juanmcuello authored
    To allow the use of prepared statements when changing schemas in
    postgres, the schema search path is added to the sql key.
  2. @Juanmcuello

    refs #3232. Prepared statements and postgreSQL schemas.

    Juanmcuello authored
    Add tests for prepared statements with multiple schemas in
    postgreSQL.
This page is out of date. Refresh to see the latest.
View
7 activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -1035,13 +1035,14 @@ def exec_no_cache(sql, binds)
end
def exec_cache(sql, binds)
- unless @statements.key? sql
+ sql_key = "#{schema_search_path}-#{sql}"
+ unless @statements.key? sql_key
nextkey = @statements.next_key
@connection.prepare nextkey, sql
- @statements[sql] = nextkey
+ @statements[sql_key] = nextkey
end
- key = @statements[sql]
+ key = @statements[sql_key]
# Clear the queue
@connection.get_last_result
View
19 activerecord/test/cases/adapters/postgresql/schema_test.rb
@@ -38,6 +38,10 @@ class Thing4 < ActiveRecord::Base
set_table_name 'test_schema."Things"'
end
+ class Thing5 < ActiveRecord::Base
+ set_table_name 'things'
+ end
+
def setup
@connection = ActiveRecord::Base.connection
@connection.execute "CREATE SCHEMA #{SCHEMA_NAME} CREATE TABLE #{TABLE_NAME} (#{COLUMNS.join(',')})"
@@ -236,6 +240,21 @@ def test_current_schema
end
end
+ def test_prepared_statements_with_multiple_schemas
+
+ @connection.schema_search_path = SCHEMA_NAME
+ Thing5.create(:id => 1, :name => "thing inside #{SCHEMA_NAME}", :email => "thing1@localhost", :moment => Time.now)
+
+ @connection.schema_search_path = SCHEMA2_NAME
+ Thing5.create(:id => 1, :name => "thing inside #{SCHEMA2_NAME}", :email => "thing1@localhost", :moment => Time.now)
+
+ @connection.schema_search_path = SCHEMA_NAME
+ assert_equal 1, Thing5.count
+
+ @connection.schema_search_path = SCHEMA2_NAME
+ assert_equal 1, Thing5.count
+ end
+
def test_schema_exists?
{
'public' => true,
Something went wrong with that request. Please try again.