From b53d06b5637722c5159f5221ee7f15f08092fb6d Mon Sep 17 00:00:00 2001 From: Yasuo Honda Date: Thu, 26 Apr 2018 22:24:37 +0000 Subject: [PATCH] Use SQL literals for `column_definitions` and `pk_and_sequence_for` since these methods called via `to_sql` method under `unprepared_statement` Fix #1678 Starting from Oracle enhanced adapter 5.2.0.beta1 most of the queries for the dictionary, such as all_sequences use bind variables by #1498 . It should help shared memory usage however, which eventually means Oracle enhanced adapter 5.2+ only supports `prepared_statements: true` as Rails default configuration but it does not support `prepared_statements: false`. Even if `prepared_statements: true`, `to_sql` uses `unprepared_statement` to generate SQL statement. If the connection already knows which table is associated with which model object in advance, `to_sql` works fine but when `to_sql` executed first the connection needs to know these relation then call column_definitions` and `pk_and_sequence_for` methods at least. --- .../oracle_enhanced_adapter.rb | 22 +++++++++---------- .../oracle_enhanced_adapter_spec.rb | 10 ++++----- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb b/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb index c92c84df4..17a401bc0 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb @@ -525,9 +525,9 @@ def default_tablespace end def column_definitions(table_name) - (_owner, desc_table_name) = @connection.describe(table_name) + (owner, desc_table_name) = @connection.describe(table_name) - select_all(<<-SQL.strip.gsub(/\s+/, " "), "Column definitions", [bind_string("table_name", desc_table_name)]) + select_all(<<-SQL.strip.gsub(/\s+/, " "), "Column definitions") SELECT cols.column_name AS name, cols.data_type AS sql_type, cols.data_default, cols.nullable, cols.virtual_column, cols.hidden_column, cols.data_type_owner AS sql_type_owner, @@ -540,8 +540,8 @@ def column_definitions(table_name) DECODE(data_type, 'NUMBER', data_scale, NULL) AS scale, comments.comments as column_comment FROM all_tab_cols cols, all_col_comments comments - WHERE cols.owner = SYS_CONTEXT('userenv', 'current_schema') - AND cols.table_name = :table_name + WHERE cols.owner = '#{owner}' + AND cols.table_name = #{quote(desc_table_name)} AND cols.hidden_column = 'NO' AND cols.owner = comments.owner AND cols.table_name = comments.table_name @@ -553,21 +553,21 @@ def column_definitions(table_name) # Find a table's primary key and sequence. # *Note*: Only primary key is implemented - sequence will be nil. def pk_and_sequence_for(table_name, owner = nil, desc_table_name = nil) #:nodoc: - (_owner, desc_table_name) = @connection.describe(table_name) + (owner, desc_table_name) = @connection.describe(table_name) - seqs = select_values(<<-SQL.strip.gsub(/\s+/, " "), "Sequence", [bind_string("sequence_name", default_sequence_name(desc_table_name).upcase)]) + seqs = select_values(<<-SQL.strip.gsub(/\s+/, " "), "Sequence") select us.sequence_name from all_sequences us - where us.sequence_owner = SYS_CONTEXT('userenv', 'current_schema') - and us.sequence_name = :sequence_name + where us.sequence_owner = '#{owner}' + and us.sequence_name = upper(#{quote(default_sequence_name(desc_table_name))}) SQL # changed back from user_constraints to all_constraints for consistency - pks = select_values(<<-SQL.strip.gsub(/\s+/, " "), "Primary Key", [bind_string("table_name", desc_table_name)]) + pks = select_values(<<-SQL.strip.gsub(/\s+/, " "), "Primary Key") SELECT cc.column_name FROM all_constraints c, all_cons_columns cc - WHERE c.owner = SYS_CONTEXT('userenv', 'current_schema') - AND c.table_name = :table_name + WHERE c.owner = '#{owner}' + AND c.table_name = #{quote(desc_table_name)} AND c.constraint_type = 'P' AND cc.owner = c.owner AND cc.constraint_name = c.constraint_name diff --git a/spec/active_record/connection_adapters/oracle_enhanced_adapter_spec.rb b/spec/active_record/connection_adapters/oracle_enhanced_adapter_spec.rb index 661a869a2..c31577f27 100644 --- a/spec/active_record/connection_adapters/oracle_enhanced_adapter_spec.rb +++ b/spec/active_record/connection_adapters/oracle_enhanced_adapter_spec.rb @@ -480,15 +480,15 @@ class ::TestPost < ActiveRecord::Base expect(@conn.table_exists?("NOT_EXISTING")).to eq false end - it "should return content from columns with bind usage" do + it "should return content from columns without bind usage" do expect(@conn.columns("TEST_POSTS").length).to be > 0 - expect(@logger.logged(:debug).last).to match(/:table_name/) - expect(@logger.logged(:debug).last).to match(/\["table_name", "TEST_POSTS"\]/) + expect(@logger.logged(:debug).last).not_to match(/:table_name/) + expect(@logger.logged(:debug).last).not_to match(/\["table_name", "TEST_POSTS"\]/) end - it "should return pk and sequence from pk_and_sequence_for with bind usage" do + it "should return pk and sequence from pk_and_sequence_for without bind usage" do expect(@conn.pk_and_sequence_for("TEST_POSTS").length).to eq 2 - expect(@logger.logged(:debug).last).to match(/\["table_name", "TEST_POSTS"\]/) + expect(@logger.logged(:debug).last).not_to match(/\["table_name", "TEST_POSTS"\]/) end it "should return pk from primary_keys with bind usage" do