Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use longer identifier for DatabaseLimits when using Oracle 12.2 or higher #2128

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean supports_longer_identifier?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It takes the result of ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter#supports_longer_identifier? as argument. Unfortunately no better name has come up yet.

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