diff --git a/lib/active_record/connection_adapters/oracle_enhanced/connection.rb b/lib/active_record/connection_adapters/oracle_enhanced/connection.rb index 61bb4f84e..8b36c5887 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/connection.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/connection.rb @@ -36,25 +36,25 @@ def describe(name) sql = <<~SQL.squish SELECT owner, table_name, 'TABLE' name_type FROM all_tables - WHERE owner = '#{table_owner}' - AND table_name = '#{table_name}' + WHERE owner = :table_owner + AND table_name = :table_name UNION ALL SELECT owner, view_name table_name, 'VIEW' name_type FROM all_views - WHERE owner = '#{table_owner}' - AND view_name = '#{table_name}' + WHERE owner = :table_owner + AND view_name = :table_name UNION ALL SELECT table_owner, table_name, 'SYNONYM' name_type FROM all_synonyms - WHERE owner = '#{table_owner}' - AND synonym_name = '#{table_name}' + WHERE owner = :table_owner + AND synonym_name = :table_name UNION ALL SELECT table_owner, table_name, 'SYNONYM' name_type FROM all_synonyms WHERE owner = 'PUBLIC' - AND synonym_name = '#{real_name}' + AND synonym_name = :real_name SQL - if result = _select_one(sql) + if result = _select_one(sql, "CONNECTION", [table_owner, table_name, table_owner, table_name, table_owner, table_name, real_name]) case result["name_type"] when "SYNONYM" describe("#{result['owner'] && "#{result['owner']}."}#{result['table_name']}") @@ -92,14 +92,23 @@ def _oracle_downcase(column_name) # Returns a record hash with the column names as keys and column values # as values. + # binds is a array of native values in contrast to ActiveRecord::Relation::QueryAttribute def _select_one(arel, name = nil, binds = []) - result = select(arel) - result.first if result + cursor = prepare(arel) + cursor.bind_params(binds) + cursor.exec + columns = cursor.get_col_names.map do |col_name| + _oracle_downcase(col_name) + end + row = cursor.fetch + columns.each_with_index.map { |x, i| [x, row[i]] }.to_h if row + ensure + cursor.close end # Returns a single value from a record def _select_value(arel, name = nil, binds = []) - if result = _select_one(arel) + if result = _select_one(arel, name, binds) result.values.first end end diff --git a/lib/active_record/connection_adapters/oracle_enhanced/schema_statements.rb b/lib/active_record/connection_adapters/oracle_enhanced/schema_statements.rb index 90204003d..ba5c25416 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/schema_statements.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/schema_statements.rb @@ -367,12 +367,12 @@ def index_name(table_name, options) #:nodoc: # Will always query database and not index cache. def index_name_exists?(table_name, index_name) (_owner, table_name) = @connection.describe(table_name) - result = select_value(<<~SQL.squish, "SCHEMA") + result = select_value(<<~SQL.squish, "SCHEMA", [bind_string("table_name", table_name), bind_string("index_name", index_name.to_s.upcase)]) SELECT /*+ OPTIMIZER_FEATURES_ENABLE('11.2.0.2') */ 1 FROM all_indexes i WHERE i.owner = SYS_CONTEXT('userenv', 'current_schema') AND i.table_owner = SYS_CONTEXT('userenv', 'current_schema') - AND i.table_name = '#{table_name}' - AND i.index_name = '#{index_name.to_s.upcase}' + AND i.table_name = :table_name + AND i.index_name = :index_name SQL result == 1 end diff --git a/lib/active_record/connection_adapters/oracle_enhanced/structure_dump.rb b/lib/active_record/connection_adapters/oracle_enhanced/structure_dump.rb index 1142900d0..56ccd8b7d 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/structure_dump.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/structure_dump.rb @@ -30,11 +30,11 @@ def structure_dump #:nodoc: tables.each do |table_name| virtual_columns = virtual_columns_for(table_name) if supports_virtual_columns? ddl = +"CREATE#{ ' GLOBAL TEMPORARY' if temporary_table?(table_name)} TABLE \"#{table_name}\" (\n" - columns = select_all(<<~SQL.squish, "SCHEMA") + columns = select_all(<<~SQL.squish, "SCHEMA", [bind_string("table_name", table_name)]) SELECT /*+ OPTIMIZER_FEATURES_ENABLE('11.2.0.2') */ column_name, data_type, data_length, char_used, char_length, data_precision, data_scale, data_default, nullable FROM all_tab_columns - WHERE table_name = '#{table_name}' + WHERE table_name = :table_name AND owner = SYS_CONTEXT('userenv', 'current_schema') ORDER BY column_id SQL @@ -174,10 +174,10 @@ def structure_dump_table_comments(table_name) def structure_dump_column_comments(table_name) comments = [] - columns = select_values(<<~SQL.squish, "SCHEMA") + columns = select_values(<<~SQL.squish, "SCHEMA", [bind_string("table_name", table_name)]) SELECT /*+ OPTIMIZER_FEATURES_ENABLE('11.2.0.2') */ column_name FROM all_tab_columns WHERE owner = SYS_CONTEXT('userenv', 'current_schema') - AND table_name = '#{table_name}' ORDER BY column_id + AND table_name = :table_name ORDER BY column_id SQL columns.each do |column| @@ -218,11 +218,11 @@ def structure_dump_db_stored_code #:nodoc: SQL all_source.each do |source| ddl = +"CREATE OR REPLACE \n" - texts = select_all(<<~SQL.squish, "all source at structure dump") + texts = select_all(<<~SQL.squish, "all source at structure dump", [bind_string("source_name", source["name"]), bind_string("source_type", source["type"])]) SELECT /*+ OPTIMIZER_FEATURES_ENABLE('11.2.0.2') */ text FROM all_source - WHERE name = '#{source['name']}' - AND type = '#{source['type']}' + WHERE name = :source_name + AND type = :source_type AND owner = SYS_CONTEXT('userenv', 'current_schema') ORDER BY line SQL @@ -323,12 +323,12 @@ def execute_structure_dump(string) # Called only if `supports_virtual_columns?` returns true # return [{'column_name' => 'FOOS', 'data_default' => '...'}, ...] def virtual_columns_for(table) - select_all(<<~SQL.squish, "SCHEMA") + select_all(<<~SQL.squish, "SCHEMA", [bind_string("table_name", table.upcase)]) SELECT /*+ OPTIMIZER_FEATURES_ENABLE('11.2.0.2') */ column_name, data_default FROM all_tab_cols WHERE virtual_column = 'YES' AND owner = SYS_CONTEXT('userenv', 'current_schema') - AND table_name = '#{table.upcase}' + AND table_name = :table_name SQL end diff --git a/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb b/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb index eadaf6b1a..10f1dad5b 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb @@ -562,7 +562,7 @@ def default_tablespace def column_definitions(table_name) (owner, desc_table_name) = @connection.describe(table_name) - select_all(<<~SQL.squish, "SCHEMA") + select_all(<<~SQL.squish, "SCHEMA", [bind_string("owner", owner), bind_string("table_name", desc_table_name)]) SELECT /*+ OPTIMIZER_FEATURES_ENABLE('11.2.0.2') */ 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, @@ -575,8 +575,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 = '#{owner}' - AND cols.table_name = #{quote(desc_table_name)} + WHERE cols.owner = :owner + AND cols.table_name = :table_name AND cols.hidden_column = 'NO' AND cols.owner = comments.owner AND cols.table_name = comments.table_name @@ -594,19 +594,19 @@ def clear_table_columns_cache(table_name) def pk_and_sequence_for(table_name, owner = nil, desc_table_name = nil) #:nodoc: (owner, desc_table_name) = @connection.describe(table_name) - seqs = select_values(<<~SQL.squish, "SCHEMA") + seqs = select_values_forcing_binds(<<~SQL.squish, "SCHEMA", [bind_string("owner", owner), bind_string("sequence_name", default_sequence_name(desc_table_name))]) select /*+ OPTIMIZER_FEATURES_ENABLE('11.2.0.2') */ us.sequence_name from all_sequences us - where us.sequence_owner = '#{owner}' - and us.sequence_name = upper(#{quote(default_sequence_name(desc_table_name))}) + where us.sequence_owner = :owner + and us.sequence_name = upper(:sequence_name) SQL # changed back from user_constraints to all_constraints for consistency - pks = select_values(<<~SQL.squish, "SCHEMA") + pks = select_values_forcing_binds(<<~SQL.squish, "SCHEMA", [bind_string("owner", owner), bind_string("table_name", desc_table_name)]) SELECT /*+ OPTIMIZER_FEATURES_ENABLE('11.2.0.2') */ cc.column_name FROM all_constraints c, all_cons_columns cc - WHERE c.owner = '#{owner}' - AND c.table_name = #{quote(desc_table_name)} + WHERE c.owner = :owner + AND c.table_name = :table_name AND c.constraint_type = 'P' AND cc.owner = c.owner AND cc.constraint_name = c.constraint_name @@ -636,7 +636,7 @@ def has_primary_key?(table_name, owner = nil, desc_table_name = nil) #:nodoc: def primary_keys(table_name) # :nodoc: (_owner, desc_table_name) = @connection.describe(table_name) - pks = select_values(<<~SQL.squish, "SCHEMA", [bind_string("table_name", desc_table_name)]) + pks = select_values_forcing_binds(<<~SQL.squish, "SCHEMA", [bind_string("table_name", desc_table_name)]) SELECT /*+ OPTIMIZER_FEATURES_ENABLE('11.2.0.2') */ cc.column_name FROM all_constraints c, all_cons_columns cc WHERE c.owner = SYS_CONTEXT('userenv', 'current_schema') @@ -666,7 +666,7 @@ def columns_for_distinct(columns, orders) #:nodoc: end def temporary_table?(table_name) #:nodoc: - select_value(<<~SQL.squish, "SCHEMA", [bind_string("table_name", table_name.upcase)]) == "Y" + select_value_forcing_binds(<<~SQL.squish, "SCHEMA", [bind_string("table_name", table_name.upcase)]) == "Y" SELECT /*+ OPTIMIZER_FEATURES_ENABLE('11.2.0.2') */ temporary FROM all_tables WHERE table_name = :table_name and owner = SYS_CONTEXT('userenv', 'current_schema') SQL @@ -764,6 +764,22 @@ def bind_string(name, value) ActiveRecord::Relation::QueryAttribute.new(name, value, Type::OracleEnhanced::String.new) end + # call select_values using binds even if surrounding SQL preparation/execution is done + # with conn.unprepared_statement (like AR.to_sql) + def select_values_forcing_binds(arel, name, binds) + # remove possible force of unprepared SQL during dictionary access + unprepared_statement_forced = prepared_statements_disabled_cache.include?(object_id) + prepared_statements_disabled_cache.delete(object_id) if unprepared_statement_forced + + select_values(arel, name, binds) + ensure + # Restore unprepared_statement setting for surrounding SQL + prepared_statements_disabled_cache.add(object_id) if unprepared_statement_forced + end + + def select_value_forcing_binds(arel, name, binds) + single_value_from_rows(select_values_forcing_binds(arel, name, binds)) + end + ActiveRecord::Type.register(:boolean, Type::OracleEnhanced::Boolean, adapter: :oracleenhanced) ActiveRecord::Type.register(:json, Type::OracleEnhanced::Json, adapter: :oracleenhanced) end 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 38c77d811..8fda9bc6a 100644 --- a/spec/active_record/connection_adapters/oracle_enhanced_adapter_spec.rb +++ b/spec/active_record/connection_adapters/oracle_enhanced_adapter_spec.rb @@ -577,9 +577,29 @@ class TestLog < ActiveRecord::Base schema_define do drop_table :test_posts, if_exists: true create_table :test_posts + + drop_table :users, if_exists: true + create_table :users, force: true do |t| + t.string :name + t.integer :group_id + end + + drop_table :groups, if_exists: true + create_table :groups, force: true do |t| + t.string :name + end end + class ::TestPost < ActiveRecord::Base end + + class User < ActiveRecord::Base + belongs_to :group + end + + class Group < ActiveRecord::Base + has_one :user + end end before(:each) do @@ -594,6 +614,8 @@ class ::TestPost < ActiveRecord::Base after(:all) do schema_define do drop_table :test_posts + drop_table :users + drop_table :groups end Object.send(:remove_const, "TestPost") ActiveRecord::Base.clear_cache! @@ -610,15 +632,15 @@ class ::TestPost < ActiveRecord::Base expect(@logger.logged(:debug).last).to match(/\["table_name", "TEST_POSTS"\]/) end - it "should return content from columns without bind usage" do + it "should return content from columns witt bind usage" do expect(@conn.columns("TEST_POSTS").length).to be > 0 - expect(@logger.logged(:debug).last).not_to match(/:table_name/) - expect(@logger.logged(:debug).last).not_to match(/\["table_name", "TEST_POSTS"\]/) + expect(@logger.logged(:debug).last).to match(/:table_name/) + expect(@logger.logged(:debug).last).to match(/\["table_name", "TEST_POSTS"\]/) end - it "should return pk and sequence from pk_and_sequence_for without bind usage" do + it "should return pk and sequence from pk_and_sequence_for with bind usage" do expect(@conn.pk_and_sequence_for("TEST_POSTS").length).to eq 2 - expect(@logger.logged(:debug).last).not_to match(/\["table_name", "TEST_POSTS"\]/) + expect(@logger.logged(:debug).last).to match(/\["table_name", "TEST_POSTS"\]/) end it "should return pk from primary_keys with bind usage" do @@ -626,6 +648,11 @@ class ::TestPost < ActiveRecord::Base expect(@logger.logged(:debug).last).to match(/\["table_name", "TEST_POSTS"\]/) end + it "should not raise missing IN/OUT parameter like issue 1687 " do + # "to_sql" enforces unprepared_statement including dictionary access SQLs + expect { User.joins(:group).to_sql }.not_to raise_exception + end + it "should return false from temporary_table? with bind usage" do expect(@conn.temporary_table?("TEST_POSTS")).to eq false expect(@logger.logged(:debug).last).to match(/:table_name/)