Skip to content

Commit

Permalink
Use longer identifier for DatabaseLimits when using Oracle 12.2 or …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
koic committed Jan 19, 2021
1 parent 000bfb4 commit ce00caa
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
18 changes: 13 additions & 5 deletions lib/active_record/connection_adapters/oracle_enhanced/quoting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(".")
Expand All @@ -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
Expand Down Expand Up @@ -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)])
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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')
Expand All @@ -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')
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading

0 comments on commit ce00caa

Please sign in to comment.