Permalink
Browse files

reset prepared statement when schema changes imapact statement results.

fixes #3335
  • Loading branch information...
tenderlove committed Oct 18, 2011
1 parent dd27e2e commit 818d285305502cc6191a98400b43633f44394f6e
@@ -279,6 +279,11 @@ def clear
cache.clear
end
+ def delete(sql_key)
+ dealloc cache[sql_key]
+ cache.delete sql_key
+ end
+
private
def cache
@cache[$$]
@@ -999,27 +1004,55 @@ def translate_exception(exception, message)
end
private
- def exec_no_cache(sql, binds)
- @connection.async_exec(sql)
- end
+ FEATURE_NOT_SUPPORTED = "0A000" # :nodoc:
- def exec_cache(sql, binds)
- unless @statements.key? sql
- nextkey = @statements.next_key
- @connection.prepare nextkey, sql
- @statements[sql] = nextkey
+ def exec_no_cache(sql, binds)
+ @connection.async_exec(sql)
end
- key = @statements[sql]
+ def exec_cache(sql, binds)
+ begin
+ stmt_key = prepare_statement sql
+
+ # Clear the queue
+ @connection.get_last_result
+ @connection.send_query_prepared(stmt_key, binds.map { |col, val|
+ type_cast(val, col)
+ })
+ @connection.block
+ @connection.get_last_result
+ rescue PGError => e
+ # Get the PG code for the failure. Annoyingly, the code for

This comment has been minimized.

Show comment
Hide comment
@joevandyk

joevandyk Oct 19, 2011

Contributor

Thanks for doing this!

I was under the assumption that if there's a postgresql error in a transaction that you had to rollback to the nearest savepoint or roll back the entire transaction before you are able to execute more queries.

@joevandyk

joevandyk Oct 19, 2011

Contributor

Thanks for doing this!

I was under the assumption that if there's a postgresql error in a transaction that you had to rollback to the nearest savepoint or roll back the entire transaction before you are able to execute more queries.

This comment has been minimized.

Show comment
Hide comment
@nixme

nixme Sep 17, 2013

this is true and has been biting us in the ass.

@nixme

nixme Sep 17, 2013

this is true and has been biting us in the ass.

This comment has been minimized.

Show comment
Hide comment
@nixme

nixme Sep 17, 2013

I'll file a separate issue.

@nixme

nixme Sep 17, 2013

I'll file a separate issue.

This comment has been minimized.

Show comment
Hide comment
@jordan-brough

jordan-brough Mar 14, 2014

Contributor

For the record, the separate issue filed is #12330. This has been hurting us as well.

@jordan-brough

jordan-brough Mar 14, 2014

Contributor

For the record, the separate issue filed is #12330. This has been hurting us as well.

+ # prepared statements whose return value may have changed is
+ # FEATURE_NOT_SUPPORTED. Check here for more details:
+ # http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/utils/cache/plancache.c#l573
+ code = e.result.result_error_field(PGresult::PG_DIAG_SQLSTATE)
+ if FEATURE_NOT_SUPPORTED == code
+ @statements.delete sql_key(sql)
+ retry
+ else
+ raise e
+ end
+ end
+ end
- # Clear the queue
- @connection.get_last_result
- @connection.send_query_prepared(key, binds.map { |col, val|
- type_cast(val, col)
- })
- @connection.block
- @connection.get_last_result
- end
+ # Returns the statement identifier for the client side cache
+ # of statements
+ def sql_key(sql)
+ "#{schema_search_path}-#{sql}"
+ end
+
+ # Prepare the statement if it hasn't been prepared, return
+ # the statement key.
+ def prepare_statement(sql)
+ sql_key = sql_key(sql)
+ unless @statements.key? sql_key
+ nextkey = @statements.next_key
+ @connection.prepare nextkey, sql
+ @statements[sql_key] = nextkey
+ end
+ @statements[sql_key]
+ end
# The internal PostgreSQL identifier of the money data type.
MONEY_COLUMN_TYPE_OID = 790 #:nodoc:
@@ -56,6 +56,14 @@ def teardown
@connection.execute "DROP SCHEMA #{SCHEMA_NAME} CASCADE"
end
+ def test_schema_change_with_prepared_stmt
+ @connection.exec_query "select * from developers where id = $1", 'sql', [[nil, 1]]
+ @connection.exec_query "alter table developers add column zomg int", 'sql', []
+ @connection.exec_query "select * from developers where id = $1", 'sql', [[nil, 1]]
+ ensure
+ @connection.exec_query "alter table developers drop column if exists zomg", 'sql', []
+ end
+
def test_table_exists?
[Thing1, Thing2, Thing3, Thing4].each do |klass|
name = klass.table_name

2 comments on commit 818d285

@joevandyk

This comment has been minimized.

Show comment
Hide comment
@joevandyk

joevandyk Oct 19, 2011

Contributor

If this works, it should also fix #1892, right?

Contributor

joevandyk replied Oct 19, 2011

If this works, it should also fix #1892, right?

@tenderlove

This comment has been minimized.

Show comment
Hide comment
@tenderlove

tenderlove Oct 19, 2011

Member

Yes, it should. I'll close that ticket.

Member

tenderlove replied Oct 19, 2011

Yes, it should. I'll close that ticket.

Please sign in to comment.