From ce00caa589f0322e9d1e7e7a40daad39fe27f44c Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Mon, 18 Jan 2021 13:40:19 +0900 Subject: [PATCH] Use longer identifier for `DatabaseLimits` when using Oracle 12.2 or higher Follow #1703. Oracle enhanced adapter supports longer identifier by #1703. I encountered the following error when using `rename_table` in Oracle 12c. ```console New table name 'identifier_of_thirty_bytes_or_more' is too long; the limit is 30 characters ``` Because `IDENTIFIER_MAX_LENGTH` constant was fixed to 30 bytes. This PR will use `max_identifier_length` method instead of the constant and accept longer identifier (max 128 bytes) when using Oracle 12.2 or higher. I encountered the error in Rails 6.0. So I'd like to backport to 6.1 and 6.0 stable branches. --- .../oracle_enhanced/connection.rb | 6 +-- .../oracle_enhanced/database_limits.rb | 13 ++--- .../oracle_enhanced/jdbc_connection.rb | 2 +- .../oracle_enhanced/oci_connection.rb | 2 +- .../oracle_enhanced/quoting.rb | 18 +++++-- .../oracle_enhanced/schema_statements.rb | 18 +++---- .../oracle_enhanced_adapter.rb | 8 ++-- .../oracle_enhanced/connection_spec.rb | 21 +++++---- .../oracle_enhanced/quoting_spec.rb | 47 ++++++++++++------- .../oracle_enhanced/schema_statements_spec.rb | 16 +++++-- 10 files changed, 90 insertions(+), 61 deletions(-) diff --git a/lib/active_record/connection_adapters/oracle_enhanced/connection.rb b/lib/active_record/connection_adapters/oracle_enhanced/connection.rb index 8b36c5887..598d6664f 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/connection.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/connection.rb @@ -20,14 +20,14 @@ def self.create(config) private # Used always by JDBC connection as well by OCI connection when describing tables over database link - def describe(name) + def describe(name, supports_longer_identifier) name = name.to_s if name.include?("@") raise ArgumentError "db link is not supported" else default_owner = @owner end - real_name = OracleEnhanced::Quoting.valid_table_name?(name) ? name.upcase : name + real_name = OracleEnhanced::Quoting.valid_table_name?(name, supports_longer_identifier) ? name.upcase : name if real_name.include?(".") table_owner, table_name = real_name.split(".") else @@ -57,7 +57,7 @@ def describe(name) 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']}") + describe("#{result['owner'] && "#{result['owner']}."}#{result['table_name']}", supports_longer_identifier) else [result["owner"], result["table_name"]] end diff --git a/lib/active_record/connection_adapters/oracle_enhanced/database_limits.rb b/lib/active_record/connection_adapters/oracle_enhanced/database_limits.rb index 19901732d..49922163d 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/database_limits.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/database_limits.rb @@ -6,34 +6,31 @@ module ActiveRecord module ConnectionAdapters module OracleEnhanced module DatabaseLimits - # maximum length of Oracle identifiers - IDENTIFIER_MAX_LENGTH = 30 - def table_alias_length #:nodoc: - IDENTIFIER_MAX_LENGTH + max_identifier_length end # the maximum length of a table name def table_name_length - IDENTIFIER_MAX_LENGTH + max_identifier_length end deprecate :table_name_length # the maximum length of a column name def column_name_length - IDENTIFIER_MAX_LENGTH + max_identifier_length end deprecate :column_name_length # the maximum length of an index name # supported by this database def index_name_length - IDENTIFIER_MAX_LENGTH + max_identifier_length end # the maximum length of a sequence name def sequence_name_length - IDENTIFIER_MAX_LENGTH + max_identifier_length end # To avoid ORA-01795: maximum number of expressions in a list is 1000 diff --git a/lib/active_record/connection_adapters/oracle_enhanced/jdbc_connection.rb b/lib/active_record/connection_adapters/oracle_enhanced/jdbc_connection.rb index a15443e95..efb773730 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/jdbc_connection.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/jdbc_connection.rb @@ -457,7 +457,7 @@ def write_lob(lob, value, is_binary = false) end # To allow private method called from `JDBCConnection` - def describe(name) + def describe(name, supports_longer_identifier) super end diff --git a/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb b/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb index 1bd17d407..18916934f 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb @@ -225,7 +225,7 @@ def write_lob(lob, value, is_binary = false) lob.write value end - def describe(name) + def describe(name, supports_longer_identifier) super end diff --git a/lib/active_record/connection_adapters/oracle_enhanced/quoting.rb b/lib/active_record/connection_adapters/oracle_enhanced/quoting.rb index f8be18e94..76fb870e6 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/quoting.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/quoting.rb @@ -38,7 +38,8 @@ def quote_column_name_or_expression(name) #:nodoc: end end - # Names must be from 1 to 30 bytes long with these exceptions: + # Names must be from 1 to 30 bytes (Oracle 12.1 and lower) or from 1 to 128 bytes + # (Oracle 12.2 and higher) long with these exceptions: # * Names of databases are limited to 8 bytes. # * Names of database links can be as long as 128 bytes. # @@ -51,16 +52,23 @@ def quote_column_name_or_expression(name) #:nodoc: # your database character set and the underscore (_), dollar sign ($), # and pound sign (#). # Oracle strongly discourages you from using $ and # in nonquoted identifiers. - NONQUOTED_OBJECT_NAME = /[[:alpha:]][\w$#]{0,29}/ - VALID_TABLE_NAME = /\A(?:#{NONQUOTED_OBJECT_NAME}\.)?#{NONQUOTED_OBJECT_NAME}?\Z/ + def self.nonquoted_object_name(supports_longer_identifier) + @@nonquoted_object_name ||= supports_longer_identifier ? /[[:alpha:]][\w$#]{0,127}/ : /[[:alpha:]][\w$#]{0,29}/ + end + + def self.valid_table_name(supports_longer_identifier) + nonquoted_object_name = nonquoted_object_name(supports_longer_identifier) + + @@valid_table_name ||= /\A(?:#{nonquoted_object_name}\.)?#{nonquoted_object_name}?\z/ + end # unescaped table name should start with letter and # contain letters, digits, _, $ or # # can be prefixed with schema name # CamelCase table names should be quoted - def self.valid_table_name?(name) #:nodoc: + def self.valid_table_name?(name, supports_longer_identifier) #:nodoc: object_name = name.to_s - !!(object_name =~ VALID_TABLE_NAME && !mixed_case?(object_name)) + !!(object_name =~ valid_table_name(supports_longer_identifier) && !mixed_case?(object_name)) end def self.mixed_case?(name) 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 ba5c25416..c7207bc98 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/schema_statements.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/schema_statements.rb @@ -36,7 +36,7 @@ def table_exists?(table_name) else default_owner = current_schema end - real_name = OracleEnhanced::Quoting.valid_table_name?(table_name) ? + real_name = OracleEnhanced::Quoting.valid_table_name?(table_name, supports_longer_identifier?) ? table_name.upcase : table_name if real_name.include?(".") table_owner, table_name = real_name.split(".") @@ -53,7 +53,7 @@ def table_exists?(table_name) end def data_source_exists?(table_name) - (_owner, _table_name) = @connection.describe(table_name) + (_owner, _table_name) = @connection.describe(table_name, supports_longer_identifier?) true rescue false @@ -87,7 +87,7 @@ def synonyms end def indexes(table_name) #:nodoc: - (_owner, table_name) = @connection.describe(table_name) + (_owner, table_name) = @connection.describe(table_name, supports_longer_identifier?) default_tablespace_name = default_tablespace result = select_all(<<~SQL.squish, "SCHEMA", [bind_string("table_name", table_name)]) @@ -253,8 +253,8 @@ def primary_key(*args) end def rename_table(table_name, new_name) #:nodoc: - if new_name.to_s.length > DatabaseLimits::IDENTIFIER_MAX_LENGTH - raise ArgumentError, "New table name '#{new_name}' is too long; the limit is #{DatabaseLimits::IDENTIFIER_MAX_LENGTH} characters" + if new_name.to_s.length > max_identifier_length + raise ArgumentError, "New table name '#{new_name}' is too long; the limit is #{max_identifier_length} characters" end schema_cache.clear_data_source_cache!(table_name.to_s) schema_cache.clear_data_source_cache!(new_name.to_s) @@ -366,7 +366,7 @@ 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) + (_owner, table_name) = @connection.describe(table_name, supports_longer_identifier?) 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') @@ -501,7 +501,7 @@ def change_column_comment(table_name, column_name, comment_or_changes) def table_comment(table_name) #:nodoc: # TODO - (_owner, table_name) = @connection.describe(table_name) + (_owner, table_name) = @connection.describe(table_name, supports_longer_identifier?) select_value(<<~SQL.squish, "SCHEMA", [bind_string("table_name", table_name)]) SELECT /*+ OPTIMIZER_FEATURES_ENABLE('11.2.0.2') */ comments FROM all_tab_comments WHERE owner = SYS_CONTEXT('userenv', 'current_schema') @@ -517,7 +517,7 @@ def table_options(table_name) # :nodoc: def column_comment(table_name, column_name) #:nodoc: # TODO: it does not exist in Abstract adapter - (_owner, table_name) = @connection.describe(table_name) + (_owner, table_name) = @connection.describe(table_name, supports_longer_identifier?) select_value(<<~SQL.squish, "SCHEMA", [bind_string("table_name", table_name), bind_string("column_name", column_name.upcase)]) SELECT /*+ OPTIMIZER_FEATURES_ENABLE('11.2.0.2') */ comments FROM all_col_comments WHERE owner = SYS_CONTEXT('userenv', 'current_schema') @@ -545,7 +545,7 @@ def tablespace(table_name) # get table foreign keys for schema dump def foreign_keys(table_name) #:nodoc: - (_owner, desc_table_name) = @connection.describe(table_name) + (_owner, desc_table_name) = @connection.describe(table_name, supports_longer_identifier?) fk_info = select_all(<<~SQL.squish, "SCHEMA", [bind_string("desc_table_name", desc_table_name)]) SELECT /*+ OPTIMIZER_FEATURES_ENABLE('11.2.0.2') */ r.table_name to_table diff --git a/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb b/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb index 10f1dad5b..104eea814 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb @@ -491,7 +491,7 @@ def prefetch_primary_key?(table_name = nil) table_name = table_name.to_s do_not_prefetch = @do_not_prefetch_primary_key[table_name] if do_not_prefetch.nil? - owner, desc_table_name = @connection.describe(table_name) + owner, desc_table_name = @connection.describe(table_name, supports_longer_identifier?) @do_not_prefetch_primary_key[table_name] = do_not_prefetch = !has_primary_key?(table_name, owner, desc_table_name) end !do_not_prefetch @@ -560,7 +560,7 @@ 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, supports_longer_identifier?) 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, @@ -592,7 +592,7 @@ def clear_table_columns_cache(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, supports_longer_identifier?) 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 @@ -634,7 +634,7 @@ def has_primary_key?(table_name, owner = nil, desc_table_name = nil) #:nodoc: end def primary_keys(table_name) # :nodoc: - (_owner, desc_table_name) = @connection.describe(table_name) + (_owner, desc_table_name) = @connection.describe(table_name, supports_longer_identifier?) 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 diff --git a/spec/active_record/connection_adapters/oracle_enhanced/connection_spec.rb b/spec/active_record/connection_adapters/oracle_enhanced/connection_spec.rb index 7cf040c6a..4ebc1e830 100644 --- a/spec/active_record/connection_adapters/oracle_enhanced/connection_spec.rb +++ b/spec/active_record/connection_adapters/oracle_enhanced/connection_spec.rb @@ -495,53 +495,56 @@ def kill_current_session before(:all) do @conn = ActiveRecord::ConnectionAdapters::OracleEnhanced::Connection.create(CONNECTION_PARAMS) @owner = CONNECTION_PARAMS[:username].upcase + + @oracle12cr2_or_higher = !!@conn.exec( + "select * from product_component_version where product like 'Oracle%' and to_number(substr(version,1,4)) >= 12.2").fetch end it "should describe existing table" do @conn.exec "CREATE TABLE test_employees (first_name VARCHAR2(20))" rescue nil - expect(@conn.describe("test_employees")).to eq([@owner, "TEST_EMPLOYEES"]) + expect(@conn.describe("test_employees", @oracle12cr2_or_higher)).to eq([@owner, "TEST_EMPLOYEES"]) @conn.exec "DROP TABLE test_employees" rescue nil end it "should not describe non-existing table" do - expect { @conn.describe("test_xxx") }.to raise_error(ActiveRecord::ConnectionAdapters::OracleEnhanced::ConnectionException) + expect { @conn.describe("test_xxx", @oracle12cr2_or_higher) }.to raise_error(ActiveRecord::ConnectionAdapters::OracleEnhanced::ConnectionException) end it "should describe table in other schema" do - expect(@conn.describe("sys.dual")).to eq(["SYS", "DUAL"]) + expect(@conn.describe("sys.dual", @oracle12cr2_or_higher)).to eq(["SYS", "DUAL"]) end it "should describe table in other schema if the schema and table are in different cases" do - expect(@conn.describe("SYS.dual")).to eq(["SYS", "DUAL"]) + expect(@conn.describe("SYS.dual", @oracle12cr2_or_higher)).to eq(["SYS", "DUAL"]) end it "should describe existing view" do @conn.exec "CREATE TABLE test_employees (first_name VARCHAR2(20))" rescue nil @conn.exec "CREATE VIEW test_employees_v AS SELECT * FROM test_employees" rescue nil - expect(@conn.describe("test_employees_v")).to eq([@owner, "TEST_EMPLOYEES_V"]) + expect(@conn.describe("test_employees_v", @oracle12cr2_or_higher)).to eq([@owner, "TEST_EMPLOYEES_V"]) @conn.exec "DROP VIEW test_employees_v" rescue nil @conn.exec "DROP TABLE test_employees" rescue nil end it "should describe view in other schema" do - expect(@conn.describe("sys.v_$version")).to eq(["SYS", "V_$VERSION"]) + expect(@conn.describe("sys.v_$version", @oracle12cr2_or_higher)).to eq(["SYS", "V_$VERSION"]) end it "should describe existing private synonym" do @conn.exec "CREATE SYNONYM test_dual FOR sys.dual" rescue nil - expect(@conn.describe("test_dual")).to eq(["SYS", "DUAL"]) + expect(@conn.describe("test_dual", @oracle12cr2_or_higher)).to eq(["SYS", "DUAL"]) @conn.exec "DROP SYNONYM test_dual" rescue nil end it "should describe existing public synonym" do - expect(@conn.describe("all_tables")).to eq(["SYS", "ALL_TABLES"]) + expect(@conn.describe("all_tables", @oracle12cr2_or_higher)).to eq(["SYS", "ALL_TABLES"]) end if defined?(OCI8) context "OCI8 adapter" do it "should not fallback to SELECT-based logic when querying non-existent table information" do expect(@conn).not_to receive(:select_one) - @conn.describe("non_existent") rescue ActiveRecord::ConnectionAdapters::OracleEnhanced::ConnectionException + @conn.describe("non_existent", @oracle12cr2_or_higher) rescue ActiveRecord::ConnectionAdapters::OracleEnhanced::ConnectionException end end end diff --git a/spec/active_record/connection_adapters/oracle_enhanced/quoting_spec.rb b/spec/active_record/connection_adapters/oracle_enhanced/quoting_spec.rb index 2dbeba424..0115289b7 100644 --- a/spec/active_record/connection_adapters/oracle_enhanced/quoting_spec.rb +++ b/spec/active_record/connection_adapters/oracle_enhanced/quoting_spec.rb @@ -64,55 +64,66 @@ class ::TestReservedWord < ActiveRecord::Base; end describe "valid table names" do before(:all) do @adapter = ActiveRecord::ConnectionAdapters::OracleEnhanced::Quoting + + @oracle12cr2_or_higher = !!ActiveRecord::Base.connection.select_value( + "select * from product_component_version where product like 'Oracle%' and to_number(substr(version,1,4)) >= 12.2") end it "should be valid with letters and digits" do - expect(@adapter.valid_table_name?("abc_123")).to be_truthy + expect(@adapter.valid_table_name?("abc_123", @oracle12cr2_or_higher)).to be_truthy end it "should be valid with schema name" do - expect(@adapter.valid_table_name?("abc_123.def_456")).to be_truthy + expect(@adapter.valid_table_name?("abc_123.def_456", @oracle12cr2_or_higher)).to be_truthy end it "should be valid with schema name and object name in different case" do - expect(@adapter.valid_table_name?("TEST_DBA.def_456")).to be_truthy + expect(@adapter.valid_table_name?("TEST_DBA.def_456", @oracle12cr2_or_higher)).to be_truthy end it "should be valid with $ in name" do - expect(@adapter.valid_table_name?("sys.v$session")).to be_truthy + expect(@adapter.valid_table_name?("sys.v$session", @oracle12cr2_or_higher)).to be_truthy end it "should be valid with upcase schema name" do - expect(@adapter.valid_table_name?("ABC_123.DEF_456")).to be_truthy + expect(@adapter.valid_table_name?("ABC_123.DEF_456", @oracle12cr2_or_higher)).to be_truthy end it "should not be valid with two dots in name" do - expect(@adapter.valid_table_name?("abc_123.def_456.ghi_789")).to be_falsey + expect(@adapter.valid_table_name?("abc_123.def_456.ghi_789", @oracle12cr2_or_higher)).to be_falsey end it "should not be valid with invalid characters" do - expect(@adapter.valid_table_name?("warehouse-things")).to be_falsey + expect(@adapter.valid_table_name?("warehouse-things", @oracle12cr2_or_higher)).to be_falsey end it "should not be valid with for camel-case" do - expect(@adapter.valid_table_name?("Abc")).to be_falsey - expect(@adapter.valid_table_name?("aBc")).to be_falsey - expect(@adapter.valid_table_name?("abC")).to be_falsey + expect(@adapter.valid_table_name?("Abc", @oracle12cr2_or_higher)).to be_falsey + expect(@adapter.valid_table_name?("aBc", @oracle12cr2_or_higher)).to be_falsey + expect(@adapter.valid_table_name?("abC", @oracle12cr2_or_higher)).to be_falsey end - it "should not be valid for names > 30 characters" do - expect(@adapter.valid_table_name?("a" * 31)).to be_falsey + it "should not be valid for names over maximum characters" do + if @oracle12cr2_or_higher + expect(@adapter.valid_table_name?("a" * 129, @oracle12cr2_or_higher)).to be_falsey + else + expect(@adapter.valid_table_name?("a" * 31, @oracle12cr2_or_higher)).to be_falsey + end end - it "should not be valid for schema names > 30 characters" do - expect(@adapter.valid_table_name?(("a" * 31) + ".validname")).to be_falsey + it "should not be valid for schema names over maximum characters" do + if @oracle12cr2_or_higher + expect(@adapter.valid_table_name?(("a" * 129) + ".validname", @oracle12cr2_or_higher)).to be_falsey + else + expect(@adapter.valid_table_name?(("a" * 31) + ".validname", @oracle12cr2_or_higher)).to be_falsey + end end it "should not be valid for names that do not begin with alphabetic characters" do - expect(@adapter.valid_table_name?("1abc")).to be_falsey - expect(@adapter.valid_table_name?("_abc")).to be_falsey - expect(@adapter.valid_table_name?("abc.1xyz")).to be_falsey - expect(@adapter.valid_table_name?("abc._xyz")).to be_falsey + expect(@adapter.valid_table_name?("1abc", @oracle12cr2_or_higher)).to be_falsey + expect(@adapter.valid_table_name?("_abc", @oracle12cr2_or_higher)).to be_falsey + expect(@adapter.valid_table_name?("abc.1xyz", @oracle12cr2_or_higher)).to be_falsey + expect(@adapter.valid_table_name?("abc._xyz", @oracle12cr2_or_higher)).to be_falsey end end diff --git a/spec/active_record/connection_adapters/oracle_enhanced/schema_statements_spec.rb b/spec/active_record/connection_adapters/oracle_enhanced/schema_statements_spec.rb index 5b4979684..a19c4c7e6 100644 --- a/spec/active_record/connection_adapters/oracle_enhanced/schema_statements_spec.rb +++ b/spec/active_record/connection_adapters/oracle_enhanced/schema_statements_spec.rb @@ -284,9 +284,19 @@ class ::TestEmployee < ActiveRecord::Base; end end it "should raise error when new table name length is too long" do - expect do - @conn.rename_table("test_employees", "a" * 31) - end.to raise_error(ArgumentError) + if @oracle12cr2_or_higher + expect do + @conn.rename_table("test_employees", "a" * 129) + end.to raise_error(ArgumentError) + + expect do + @conn.rename_table("test_employees", "a" * 31) + end.not_to raise_error + else + expect do + @conn.rename_table("test_employees", "a" * 31) + end.to raise_error(ArgumentError) + end end it "should not raise error when new sequence name length is too long" do