From b915b11cca558eb99b7c2621c4457491d4bdb43b Mon Sep 17 00:00:00 2001 From: Yaw Boakye Date: Fri, 8 Jul 2016 03:22:37 +0000 Subject: [PATCH] For `PostgreSQL >= 9.4` use `gen_random_uuid()` Since 9.4, PostgreSQL recommends using `pgcrypto`'s `gen_random_uuid()` to generate version 4 UUIDs instead of the functions in the `uuid-ossp` extension. These changes uses the appropriate UUID function depending on the underlying PostgreSQL server's version, while maintaining `uuid_generate_v4()` in older migrations. --- activerecord/CHANGELOG.md | 5 ++ .../postgresql/schema_definitions.rb | 17 ++--- .../connection_adapters/postgresql_adapter.rb | 4 ++ .../active_record/migration/compatibility.rb | 8 +++ .../cases/adapters/postgresql/uuid_test.rb | 71 +++++++++++++++---- .../test/schema/postgresql_specific_schema.rb | 1 + 6 files changed, 85 insertions(+), 21 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index d0cc808133..0b9a2f9943 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,8 @@ +* For PostgreSQL >= 9.4 use `pgcrypto`'s `gen_random_uuid()` instead of + `uuid-ossp`'s UUID generation function. + + *Yuji Yaginuma*, *Yaw Boakye* + * Introduce `Model#reload_` to bring back the behavior of `Article.category(true)` where `category` is a singular association. diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb index a11dbe7dce..5d689c2dc3 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb @@ -11,11 +11,12 @@ module ActiveRecord # t.timestamps # end # - # By default, this will use the +uuid_generate_v4()+ function from the - # +uuid-ossp+ extension, which MUST be enabled on your database. To enable - # the +uuid-ossp+ extension, you can use the +enable_extension+ method in your - # migrations. To use a UUID primary key without +uuid-ossp+ enabled, you can - # set the +:default+ option to +nil+: + # By default, this will use the +gen_random_uuid()+ function from the + # +pgcrypto+ extension (only PostgreSQL >= 9.4), or +uuid_generate_v4()+ + # function from the +uuid-ossp+ extension. To enable the appropriate + # extension, which is a requirement, you can use the +enable_extension+ + # method in your migrations. To use a UUID primary key without any of + # of extensions, you can set the +:default+ option to +nil+: # # create_table :stuffs, id: false do |t| # t.primary_key :id, :uuid, default: nil @@ -23,15 +24,15 @@ module ActiveRecord # t.timestamps # end # - # You may also pass a different UUID generation function from +uuid-ossp+ - # or another library. + # You may also pass a custom stored procedure that returns a UUID or use a + # different UUID generation function from another library. # # Note that setting the UUID primary key default value to +nil+ will # require you to assure that you always provide a UUID value before saving # a record (as primary keys cannot be +nil+). This might be done via the # +SecureRandom.uuid+ method and a +before_save+ callback, for instance. def primary_key(name, type = :primary_key, **options) - options[:default] = options.fetch(:default, "uuid_generate_v4()") if type == :uuid + options[:default] = options.fetch(:default, "gen_random_uuid()") if type == :uuid super end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 710b5cd887..140ad4827a 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -315,6 +315,10 @@ module ActiveRecord postgresql_version >= 90300 end + def supports_pgcrypto_uuid? + postgresql_version >= 90400 + end + def get_advisory_lock(lock_id) # :nodoc: unless lock_id.is_a?(Integer) && lock_id.bit_length <= 63 raise(ArgumentError, "Postgres requires advisory lock ids to be a signed 64 bit integer") diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index 04e538baa5..ae45ac7157 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -103,6 +103,14 @@ module ActiveRecord end class V5_0 < V5_1 + def create_table(table_name, options = {}) + if ActiveRecord::Base.connection.adapter_name == "PostgreSQL" + if options[:id] == :uuid && !options[:default] + options[:default] = "uuid_generate_v4()" + end + end + super + end end class V4_2 < V5_0 diff --git a/activerecord/test/cases/adapters/postgresql/uuid_test.rb b/activerecord/test/cases/adapters/postgresql/uuid_test.rb index 9a59691737..ab0815631f 100644 --- a/activerecord/test/cases/adapters/postgresql/uuid_test.rb +++ b/activerecord/test/cases/adapters/postgresql/uuid_test.rb @@ -9,6 +9,10 @@ module PostgresqlUUIDHelper def drop_table(name) connection.drop_table name, if_exists: true end + + def uuid_function + connection.supports_pgcrypto_uuid? ? "gen_random_uuid()" : "uuid_generate_v4()" + end end class PostgresqlUUIDTest < ActiveRecord::PostgreSQLTestCase @@ -21,6 +25,7 @@ class PostgresqlUUIDTest < ActiveRecord::PostgreSQLTestCase setup do enable_extension!("uuid-ossp", connection) + enable_extension!("pgcrypto", connection) if connection.supports_pgcrypto_uuid? connection.create_table "uuid_data_type" do |t| t.uuid "guid" @@ -31,19 +36,27 @@ class PostgresqlUUIDTest < ActiveRecord::PostgreSQLTestCase drop_table "uuid_data_type" end - def test_change_column_default - @connection.add_column :uuid_data_type, :thingy, :uuid, null: false, default: "uuid_generate_v1()" - UUIDType.reset_column_information - column = UUIDType.columns_hash["thingy"] - assert_equal "uuid_generate_v1()", column.default_function + if ActiveRecord::Base.connection.supports_pgcrypto_uuid? + def test_uuid_column_default + connection.add_column :uuid_data_type, :thingy, :uuid, null: false, default: "gen_random_uuid()" + UUIDType.reset_column_information + column = UUIDType.columns_hash["thingy"] + assert_equal "gen_random_uuid()", column.default_function + end + else + def test_change_column_default + connection.add_column :uuid_data_type, :thingy, :uuid, null: false, default: "uuid_generate_v1()" + UUIDType.reset_column_information + column = UUIDType.columns_hash["thingy"] + assert_equal "uuid_generate_v1()", column.default_function - @connection.change_column :uuid_data_type, :thingy, :uuid, null: false, default: "uuid_generate_v4()" - - UUIDType.reset_column_information - column = UUIDType.columns_hash["thingy"] - assert_equal "uuid_generate_v4()", column.default_function - ensure - UUIDType.reset_column_information + connection.change_column :uuid_data_type, :thingy, :uuid, null: false, default: "uuid_generate_v4()" + UUIDType.reset_column_information + column = UUIDType.columns_hash["thingy"] + assert_equal "uuid_generate_v4()", column.default_function + ensure + UUIDType.reset_column_information + end end def test_data_type_of_uuid_types @@ -155,7 +168,7 @@ class PostgresqlUUIDGenerationTest < ActiveRecord::PostgreSQLTestCase # to test dumping tables which columns have defaults with custom functions connection.execute <<-SQL CREATE OR REPLACE FUNCTION my_uuid_generator() RETURNS uuid - AS $$ SELECT * FROM uuid_generate_v4() $$ + AS $$ SELECT * FROM #{uuid_function} $$ LANGUAGE SQL VOLATILE; SQL @@ -164,11 +177,16 @@ class PostgresqlUUIDGenerationTest < ActiveRecord::PostgreSQLTestCase t.string "name" t.uuid "other_uuid_2", default: "my_uuid_generator()" end + + connection.create_table("pg_uuids_3", id: :uuid) do |t| + t.string "name" + end end teardown do drop_table "pg_uuids" drop_table "pg_uuids_2" + drop_table "pg_uuids_3" connection.execute "DROP FUNCTION IF EXISTS my_uuid_generator();" end @@ -206,6 +224,33 @@ class PostgresqlUUIDGenerationTest < ActiveRecord::PostgreSQLTestCase assert_match(/\bcreate_table "pg_uuids_2", id: :uuid, default: -> { "my_uuid_generator\(\)" }/, schema) assert_match(/t\.uuid "other_uuid_2", default: -> { "my_uuid_generator\(\)" }/, schema) end + + def test_schema_dumper_for_uuid_primary_key_default + schema = dump_table_schema "pg_uuids_3" + if connection.supports_pgcrypto_uuid? + assert_match(/\bcreate_table "pg_uuids_3", id: :uuid, default: -> { "gen_random_uuid\(\)" }/, schema) + else + assert_match(/\bcreate_table "pg_uuids_3", id: :uuid, default: -> { "uuid_generate_v4\(\)" }/, schema) + end + end + + if ActiveRecord::Base.connection.supports_pgcrypto_uuid? + def test_schema_dumper_for_uuid_primary_key_default_in_legacy_migration + migration = Class.new(ActiveRecord::Migration[4.2]) do + def version; 101 end + def migrate(x) + create_table("pg_uuids_4", id: :uuid) + end + end.new + ActiveRecord::Migrator.new(:up, [migration]).migrate + + schema = dump_table_schema "pg_uuids_4" + assert_match(/\bcreate_table "pg_uuids_4", id: :uuid, default: -> { "uuid_generate_v4\(\)" }/, schema) + ensure + drop_table "pg_uuids_4" + end + else + end end end diff --git a/activerecord/test/schema/postgresql_specific_schema.rb b/activerecord/test/schema/postgresql_specific_schema.rb index f00b858ea6..15ba2d67ab 100644 --- a/activerecord/test/schema/postgresql_specific_schema.rb +++ b/activerecord/test/schema/postgresql_specific_schema.rb @@ -1,6 +1,7 @@ ActiveRecord::Schema.define do enable_extension!("uuid-ossp", ActiveRecord::Base.connection) + enable_extension!("pgcrypto", ActiveRecord::Base.connection) if ActiveRecord::Base.connection.supports_pgcrypto_uuid? create_table :uuid_parents, id: :uuid, force: true do |t| t.string :name