Skip to content

Commit

Permalink
pg, default_sequence_name respects schema. Closes #7516.
Browse files Browse the repository at this point in the history
  • Loading branch information
senny committed May 30, 2014
1 parent 6c2b569 commit d6c1205
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 10 deletions.
4 changes: 4 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
* PostgreSQL `default_sequence_name` respects schema. Fixes #7516.

*Yves Senn*

* Fixed `columns_for_distinct` of postgresql adapter to work correctly
with orders without sort direction modifiers.

Expand Down
Expand Up @@ -273,9 +273,9 @@ def client_min_messages=(level)
def default_sequence_name(table_name, pk = nil) #:nodoc:
result = serial_sequence(table_name, pk || 'id')
return nil unless result
result.split('.').last
Utils.extract_schema_qualified_name(result)
rescue ActiveRecord::StatementInvalid
"#{table_name}_#{pk || 'id'}_seq"
PostgreSQL::Name.new(nil, "#{table_name}_#{pk || 'id'}_seq")

This comment has been minimized.

Copy link
@chancancode

chancancode Aug 24, 2014

Member

@senny FYI, this changes the return value of #sequence_name, which is apparently public? (#sequence_name= is public, I guess it makes sense that the reader is public too.)

I noticed this while testing discourse with 4.2. This change broke the seed-fu gem, although I believe it's nolonger an issue for that particular gem on the latest version, because it happens to use interpolation now which calls #to_s on the PostgreSQL::Name.

Not sure if there's any action items here. Some thoughts:

  • Should we call to_s on #sequence_name and other public methods, where people might be expecting strings?
  • Or perhaps we can define to_str on Name so it acts more like strings? (It would work for "'" + sequence_name + "'", but it will still break if you try to call String methods on it)
  • Or perhaps we could delegate everything to #to_s in method missing like AS::Duration?
  • Or perhaps we don't care? Not sure how much of a problem this is in practice.

This comment has been minimized.

Copy link
@senny

senny Aug 25, 2014

Author Member

@chancancode adjusted the PostgreSQL adapter internals to honor the old public interface of default_sequence_name. This will also change sequence_name.

3fe54b3

This comment has been minimized.

Copy link
@chancancode

chancancode Aug 25, 2014

Member

❤️

end

def serial_sequence(table, column)
Expand Down
Expand Up @@ -134,36 +134,34 @@ def test_serial_sequence
end

def test_default_sequence_name
assert_equal 'accounts_id_seq',
assert_equal PostgreSQL::Name.new('public', 'accounts_id_seq'),
@connection.default_sequence_name('accounts', 'id')

assert_equal 'accounts_id_seq',
assert_equal PostgreSQL::Name.new('public', 'accounts_id_seq'),
@connection.default_sequence_name('accounts')
end

def test_default_sequence_name_bad_table
assert_equal 'zomg_id_seq',
assert_equal PostgreSQL::Name.new(nil, 'zomg_id_seq'),
@connection.default_sequence_name('zomg', 'id')

assert_equal 'zomg_id_seq',
assert_equal PostgreSQL::Name.new(nil, 'zomg_id_seq'),
@connection.default_sequence_name('zomg')
end

def test_pk_and_sequence_for
with_example_table do
pk, seq = @connection.pk_and_sequence_for('ex')
assert_equal 'id', pk
expected = PostgreSQL::Name.new("public", @connection.default_sequence_name('ex', 'id'))
assert_equal expected, seq
assert_equal @connection.default_sequence_name('ex', 'id'), seq
end
end

def test_pk_and_sequence_for_with_non_standard_primary_key
with_example_table 'code serial primary key' do
pk, seq = @connection.pk_and_sequence_for('ex')
assert_equal 'code', pk
expected = PostgreSQL::Name.new("public", @connection.default_sequence_name('ex', 'code'))
assert_equal expected, seq
assert_equal @connection.default_sequence_name('ex', 'code'), seq
end
end

Expand Down

0 comments on commit d6c1205

Please sign in to comment.