Skip to content

Commit

Permalink
Fix longer sequence name detection for serial columns (rails#28339)
Browse files Browse the repository at this point in the history
We already found the longer sequence name, but we could not consider
whether it was the sequence name created by serial type due to missed a
max identifier length limitation. I've addressed the sequence name
consideration to respect the max identifier length.

Fixes rails#28332.
  • Loading branch information
kamipo committed Oct 15, 2017
1 parent 976f9c3 commit af9c170
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 4 deletions.
6 changes: 6 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
* Fix longer sequence name detection for serial columns.

Fixes #28332.

*Ryuta Kamizono*

* MySQL: Don't lose `auto_increment: true` in the `db/schema.rb`.

Fixes #30894.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class Column
# +default+ is the type-casted default value, such as +new+ in <tt>sales_stage varchar(20) default 'new'</tt>.
# +sql_type_metadata+ is various information about the type of the column
# +null+ determines if this column allows +NULL+ values.
def initialize(name, default, sql_type_metadata = nil, null = true, table_name = nil, default_function = nil, collation = nil, comment: nil)
def initialize(name, default, sql_type_metadata = nil, null = true, table_name = nil, default_function = nil, collation = nil, comment: nil, **)
@name = name.freeze
@table_name = table_name
@sql_type_metadata = sql_type_metadata
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ class PostgreSQLColumn < Column #:nodoc:
delegate :array, :oid, :fmod, to: :sql_type_metadata
alias :array? :array

def initialize(*, max_identifier_length: 63, **)
super
@max_identifier_length = max_identifier_length
end

def serial?
return unless default_function

Expand All @@ -13,8 +18,23 @@ def serial?
end
end

protected
attr_reader :max_identifier_length

private
def sequence_name_from_parts(table_name, column_name, suffix)
over_length = [table_name, column_name, suffix].map(&:length).sum + 2 - max_identifier_length

if over_length > 0
column_name_length = [(max_identifier_length - suffix.length - 2) / 2, column_name.length].min
over_length -= column_name.length - column_name_length
column_name = column_name[0, column_name_length - [over_length, 0].min]
end

if over_length > 0
table_name = table_name[0, table_name.length - over_length]
end

"#{table_name}_#{column_name}_#{suffix}"
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,8 @@ def new_column_from_field(table_name, field) # :nondoc:
table_name,
default_function,
collation,
comment: comment.presence
comment: comment.presence,
max_identifier_length: max_identifier_length
)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,10 +364,11 @@ def extensions
end

# Returns the configured supported identifier length supported by PostgreSQL
def table_alias_length
def max_identifier_length
@max_identifier_length ||= query_value("SHOW max_identifier_length", "SCHEMA").to_i
end
alias index_name_length table_alias_length
alias table_alias_length max_identifier_length
alias index_name_length max_identifier_length

# Set the authorized user for this session
def session_auth=(user)
Expand Down
32 changes: 32 additions & 0 deletions activerecord/test/cases/adapters/postgresql/serial_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,4 +119,36 @@ def test_schema_dump_with_collided_sequence_name
assert_match %r{t\.bigserial\s+"bar_baz_id",\s+null: false$}, output
end
end

class LongerSequenceNameDetectionTest < ActiveRecord::PostgreSQLTestCase
include SchemaDumpingHelper

def setup
@table_name = "long_table_name_to_test_sequence_name_detection_for_serial_cols"
@connection = ActiveRecord::Base.connection
@connection.create_table @table_name, force: true do |t|
t.serial :seq
t.bigserial :bigseq
end
end

def teardown
@connection.drop_table @table_name, if_exists: true
end

def test_serial_columns
columns = @connection.columns(@table_name)
columns.each do |column|
assert_equal :integer, column.type
assert column.serial?
end
end

def test_schema_dump_with_long_table_name
output = dump_table_schema @table_name
assert_match %r{create_table "#{@table_name}", force: :cascade}, output
assert_match %r{t\.serial\s+"seq",\s+null: false$}, output
assert_match %r{t\.bigserial\s+"bigseq",\s+null: false$}, output
end
end
end

0 comments on commit af9c170

Please sign in to comment.