From 56b37147e6b039153c330f8b1c73a6c11e84bd2c Mon Sep 17 00:00:00 2001 From: Victor Miao Date: Mon, 10 May 2021 12:01:20 -0700 Subject: [PATCH 1/5] DEV-7136 #time 2h Find and apply code changes from Rails ActiveRecord. --- .../redshift/schema_definitions.rb | 13 +++++++------ .../redshift/schema_statements.rb | 10 +++++----- .../connection_adapters/redshift_adapter.rb | 4 ++-- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/active_record/connection_adapters/redshift/schema_definitions.rb b/lib/active_record/connection_adapters/redshift/schema_definitions.rb index 10d2cf0..5cba449 100644 --- a/lib/active_record/connection_adapters/redshift/schema_definitions.rb +++ b/lib/active_record/connection_adapters/redshift/schema_definitions.rb @@ -30,23 +30,23 @@ module ColumnMethods # 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 = {}) + def primary_key(name, type = :primary_key, **options) return super unless type == :uuid options[:default] = options.fetch(:default, 'uuid_generate_v4()') options[:primary_key] = true column name, type, options end - def json(name, options = {}) + def json(name, **options) column(name, :json, options) end - def jsonb(name, options = {}) + def jsonb(name, **options) column(name, :jsonb, options) end end - class ColumnDefinition < Struct.new(:name, :type, :options, :sql_type) + class ColumnDefinition = Struct.new(:name, :type, :options, :sql_type) do def primary_key? options[:primary_key] end @@ -56,6 +56,7 @@ def primary_key? def #{option_name} options[:#{option_name}] end + def #{option_name}=(value) options[:#{option_name}] = value end @@ -68,8 +69,8 @@ class TableDefinition < ActiveRecord::ConnectionAdapters::TableDefinition private - def create_column_definition(*args) - Redshift::ColumnDefinition.new(*args) + def create_column_definition(name, type, options) + Redshift::ColumnDefinition.new(name, type, options) end end diff --git a/lib/active_record/connection_adapters/redshift/schema_statements.rb b/lib/active_record/connection_adapters/redshift/schema_statements.rb index 62e5881..9ab9007 100644 --- a/lib/active_record/connection_adapters/redshift/schema_statements.rb +++ b/lib/active_record/connection_adapters/redshift/schema_statements.rb @@ -168,7 +168,7 @@ def create_table(table_name, **options) super end - def drop_table(table_name, options = {}) + def drop_table(table_name, **options) execute "DROP TABLE#{' IF EXISTS' if options[:if_exists]} #{quote_table_name(table_name)}#{' CASCADE' if options[:force] == :cascade}" end @@ -252,7 +252,7 @@ def create_schema schema_name end # Drops the schema for the given schema name. - def drop_schema(schema_name, options = {}) + def drop_schema(schema_name, **options) execute "DROP SCHEMA#{' IF EXISTS' if options[:if_exists]} #{quote_schema_name(schema_name)} CASCADE" end @@ -320,13 +320,13 @@ def rename_table(table_name, new_name) execute "ALTER TABLE #{quote_table_name(table_name)} RENAME TO #{quote_table_name(new_name)}" end - def add_column(table_name, column_name, type, options = {}) #:nodoc: + def add_column(table_name, column_name, type, **options) #:nodoc: clear_cache! super end # Changes the column of a table. - def change_column(table_name, column_name, type, options = {}) + def change_column(table_name, column_name, type, **options) clear_cache! quoted_table_name = quote_table_name(table_name) sql_type = type_to_sql(type, options[:limit], options[:precision], options[:scale]) @@ -373,7 +373,7 @@ def rename_column(table_name, column_name, new_column_name) #:nodoc: execute "ALTER TABLE #{quote_table_name(table_name)} RENAME COLUMN #{quote_column_name(column_name)} TO #{quote_column_name(new_column_name)}" end - def add_index(table_name, column_name, options = {}) #:nodoc: + def add_index(table_name, column_name, **options) #:nodoc: end def remove_index!(table_name, index_name) #:nodoc: diff --git a/lib/active_record/connection_adapters/redshift_adapter.rb b/lib/active_record/connection_adapters/redshift_adapter.rb index fab74fd..66eec87 100644 --- a/lib/active_record/connection_adapters/redshift_adapter.rb +++ b/lib/active_record/connection_adapters/redshift_adapter.rb @@ -733,8 +733,8 @@ def construct_coder(row, coder_class) coder_class.new(oid: row["oid"].to_i, name: row["typname"]) end - def create_table_definition(*args) # :nodoc: - Redshift::TableDefinition.new(self, *args) + def create_table_definition(name, **options) # :nodoc: + Redshift::TableDefinition.new(self, name, **options) end end end From 3ece56f7603b98062151064428dabf106b0c6a8c Mon Sep 17 00:00:00 2001 From: Victor Miao Date: Mon, 10 May 2021 12:04:25 -0700 Subject: [PATCH 2/5] DEV-7136 #time 1m Remove 'class' statement for new ColumnDefinition syntax. --- .../connection_adapters/redshift/schema_definitions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_record/connection_adapters/redshift/schema_definitions.rb b/lib/active_record/connection_adapters/redshift/schema_definitions.rb index 5cba449..181d00d 100644 --- a/lib/active_record/connection_adapters/redshift/schema_definitions.rb +++ b/lib/active_record/connection_adapters/redshift/schema_definitions.rb @@ -46,7 +46,7 @@ def jsonb(name, **options) end end - class ColumnDefinition = Struct.new(:name, :type, :options, :sql_type) do + ColumnDefinition = Struct.new(:name, :type, :options, :sql_type) do def primary_key? options[:primary_key] end From 1f023b3742b142e5d207bd1f7976c2d724aeeaf6 Mon Sep 17 00:00:00 2001 From: Victor Miao Date: Mon, 10 May 2021 16:10:13 -0700 Subject: [PATCH 3/5] DEV-7136 #time 2.5h Resolve issue with incorrect handling of 'limit' migration parameter, by adding to the NATIVE_DATABASE_TYPES and handling 'int' as type instead of just 'integer'. Bump version. --- activerecord6-redshift-adapter.gemspec | 6 +++--- .../redshift/schema_statements.rb | 7 +++++-- .../connection_adapters/redshift_adapter.rb | 14 +++++++++----- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/activerecord6-redshift-adapter.gemspec b/activerecord6-redshift-adapter.gemspec index 6dcc22f..35b7c71 100644 --- a/activerecord6-redshift-adapter.gemspec +++ b/activerecord6-redshift-adapter.gemspec @@ -1,14 +1,14 @@ Gem::Specification.new do |s| s.platform = Gem::Platform::RUBY s.name = 'activerecord6-redshift-adapter' - s.version = '1.2.0' + s.version = '1.2.1' s.summary = 'Amazon Redshift adapter for ActiveRecord ' s.description = 'Amazon Redshift _makeshift_ adapter for ActiveRecord 6.' s.license = 'MIT' - s.author = ['Nancy Foen', 'Minero Aoki', 'iamdbc', 'Quentin Rousseau'] + s.author = ['Nancy Foen', 'Minero Aoki', 'iamdbc', 'Quentin Rousseau', 'vmeow'] s.email = 'fantast.d@gmail.com' - s.homepage = 'https://github.com/kwent/activerecord6-redshift-adapter' + s.homepage = 'https://github.com/ValorWaterAnalytics/activerecord6-redshift-adapter' s.files = Dir.glob(['LICENSE', 'README.md', 'lib/**/*.rb']) s.require_path = 'lib' diff --git a/lib/active_record/connection_adapters/redshift/schema_statements.rb b/lib/active_record/connection_adapters/redshift/schema_statements.rb index 9ab9007..2852275 100644 --- a/lib/active_record/connection_adapters/redshift/schema_statements.rb +++ b/lib/active_record/connection_adapters/redshift/schema_statements.rb @@ -5,7 +5,10 @@ class SchemaCreation < SchemaCreation private def visit_ColumnDefinition(o) - super + o.sql_type = type_to_sql(o.type, **o.options) + column_sql = +"#{quote_column_name(o.name)} #{o.sql_type}" + add_column_options!(column_sql, column_options(o)) unless o.type == :primary_key + column_sql end def add_column_options!(sql, options) @@ -426,7 +429,7 @@ def index_name_length # Maps logical Rails types to PostgreSQL-specific data types. def type_to_sql(type, limit: nil, precision: nil, scale: nil, **) case type.to_s - when 'integer' + when 'integer', 'int' return 'integer' unless limit case limit diff --git a/lib/active_record/connection_adapters/redshift_adapter.rb b/lib/active_record/connection_adapters/redshift_adapter.rb index 66eec87..0ec30d1 100644 --- a/lib/active_record/connection_adapters/redshift_adapter.rb +++ b/lib/active_record/connection_adapters/redshift_adapter.rb @@ -77,13 +77,17 @@ class RedshiftAdapter < AbstractAdapter primary_key: "bigint identity primary key", string: { name: "varchar" }, text: { name: "varchar" }, + varchar: { name: "varchar" }, + smallint: { name: "smallint" }, + int: { name: "integer" }, integer: { name: "integer" }, + bigint: { name: "bigint" }, float: { name: "float" }, decimal: { name: "decimal" }, datetime: { name: "timestamp" }, + timestamptz: { name: "timestamptz" }, time: { name: "time" }, date: { name: "date" }, - bigint: { name: "bigint" }, boolean: { name: "boolean" }, serial: { name: "integer" }, bigserial: { name: "bigint" }, @@ -350,10 +354,10 @@ def get_oid_type(oid, fmod, column_name, sql_type = '') # :nodoc: } end - def initialize_type_map(m) # :nodoc: - register_class_with_limit m, 'int2', Type::Integer - register_class_with_limit m, 'int4', Type::Integer - register_class_with_limit m, 'int8', Type::Integer + def initialize_type_map(m = type_map) # :nodoc: + m.register_type "int2", Type::Integer.new(limit: 2) + m.register_type "int4", Type::Integer.new(limit: 4) + m.register_type "int8", Type::Integer.new(limit: 8) m.alias_type 'oid', 'int2' m.register_type 'float4', Type::Float.new m.alias_type 'float8', 'float4' From 8d4e1d17291905020fb5192bd1c64596b0a1dc2c Mon Sep 17 00:00:00 2001 From: Victor Miao Date: Mon, 10 May 2021 16:20:20 -0700 Subject: [PATCH 4/5] DEV-7136 #time 5m Add 'limit' parameter to NATIVE_DATABASE_TYPES integer objects to prevent the 'limit' being written out by SchemaDumper, as it is unnecessary. --- .../connection_adapters/redshift_adapter.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/active_record/connection_adapters/redshift_adapter.rb b/lib/active_record/connection_adapters/redshift_adapter.rb index 0ec30d1..f7068a4 100644 --- a/lib/active_record/connection_adapters/redshift_adapter.rb +++ b/lib/active_record/connection_adapters/redshift_adapter.rb @@ -78,10 +78,10 @@ class RedshiftAdapter < AbstractAdapter string: { name: "varchar" }, text: { name: "varchar" }, varchar: { name: "varchar" }, - smallint: { name: "smallint" }, - int: { name: "integer" }, - integer: { name: "integer" }, - bigint: { name: "bigint" }, + smallint: { name: "smallint", limit: 2 }, + int: { name: "integer", limit: 4 }, + integer: { name: "integer", limit: 4 }, + bigint: { name: "bigint", limit: 8 }, float: { name: "float" }, decimal: { name: "decimal" }, datetime: { name: "timestamp" }, @@ -89,8 +89,8 @@ class RedshiftAdapter < AbstractAdapter time: { name: "time" }, date: { name: "date" }, boolean: { name: "boolean" }, - serial: { name: "integer" }, - bigserial: { name: "bigint" }, + serial: { name: "integer", limit: 4 }, + bigserial: { name: "bigint", limit: 8 }, } OID = Redshift::OID #:nodoc: From bcf32355f1178d3d0d9dbb7b51f194c0f397cbf7 Mon Sep 17 00:00:00 2001 From: Victor Miao Date: Mon, 10 May 2021 16:25:15 -0700 Subject: [PATCH 5/5] DEV-7136 #time 5m Explicitly handle 'smallint' and 'bigint' sql types, to prevent a limit clause being added. --- .../connection_adapters/redshift/schema_statements.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/active_record/connection_adapters/redshift/schema_statements.rb b/lib/active_record/connection_adapters/redshift/schema_statements.rb index 2852275..bb08511 100644 --- a/lib/active_record/connection_adapters/redshift/schema_statements.rb +++ b/lib/active_record/connection_adapters/redshift/schema_statements.rb @@ -429,6 +429,10 @@ def index_name_length # Maps logical Rails types to PostgreSQL-specific data types. def type_to_sql(type, limit: nil, precision: nil, scale: nil, **) case type.to_s + when 'smallint' + return 'smallint' + when 'bigint' + return 'bigint' when 'integer', 'int' return 'integer' unless limit