From c41874c471b93f792aba3a2847b92d3a021ac466 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20Busqu=C3=A9?= Date: Thu, 18 Apr 2019 12:27:51 +0200 Subject: [PATCH] Update to work with decoupled alias options Co-authored-by: Nikita Shilnikov --- lib/rom/sql/attribute.rb | 25 +++--- lib/rom/sql/function.rb | 2 +- lib/rom/sql/mapper_compiler.rb | 2 +- lib/rom/sql/migration/schema_diff.rb | 4 +- lib/rom/sql/schema/attributes_inferrer.rb | 4 +- lib/rom/sql/schema/index_dsl.rb | 6 +- .../auto_migrations/indexes_spec.rb | 4 +- .../auto_migrations/managing_columns_spec.rb | 10 +-- spec/integration/commands/create_spec.rb | 2 +- spec/integration/commands/update_spec.rb | 2 +- .../schema/inferrer/postgres_spec.rb | 77 ++++++++++--------- spec/spec_helper.rb | 6 +- spec/support/helpers.rb | 17 ++-- spec/unit/function_spec.rb | 14 ++++ 14 files changed, 102 insertions(+), 73 deletions(-) diff --git a/lib/rom/sql/attribute.rb b/lib/rom/sql/attribute.rb index 49e8f2c8d..0223ddc39 100644 --- a/lib/rom/sql/attribute.rb +++ b/lib/rom/sql/attribute.rb @@ -26,8 +26,6 @@ def self.[](*args) fetch_or_store(args) { new(*args) } end - option :extensions, type: Types::Hash, default: -> { TypeExtensions[type] } - # Return a new attribute with an alias # # @example @@ -36,8 +34,10 @@ def self.[](*args) # @return [SQL::Attribute] # # @api public - def aliased(name) - super.meta(name: meta.fetch(:name, name), sql_expr: sql_expr.as(name)) + def aliased(alias_name) + super.with(name: name || alias_name).meta( + sql_expr: sql_expr.as(alias_name) + ) end alias_method :as, :aliased @@ -46,7 +46,7 @@ def aliased(name) # @api public def canonical if aliased? - meta(alias: nil, sql_expr: nil) + with(alias: nil).meta(sql_expr: nil) else self end @@ -66,8 +66,8 @@ def qualified(table_alias = nil) case sql_expr when Sequel::SQL::AliasedExpression, Sequel::SQL::Identifier, Sequel::SQL::QualifiedIdentifier - type = meta(qualified: table_alias || true) - type.meta(sql_expr: type.to_sql_name) + attr = meta(qualified: table_alias || true) + attr.meta(sql_expr: attr.to_sql_name) else raise QualifyError, "can't qualify #{name.inspect} (#{sql_expr.inspect})" end @@ -292,11 +292,11 @@ def sql_literal(ds) def to_sql_name @_to_sql_name ||= if qualified? && aliased? - Sequel.qualify(table_name, name).as(meta[:alias]) + Sequel.qualify(table_name, name).as(self.alias) elsif qualified? Sequel.qualify(table_name, name) elsif aliased? - Sequel.as(name, meta[:alias]) + Sequel.as(name, self.alias) else Sequel[name] end @@ -315,7 +315,7 @@ def indexed end # @api private - def meta_ast + def meta_options_ast meta = super meta[:index] = true if indexed? meta @@ -420,6 +420,11 @@ def table_name end end + # @api private + def extensions + TypeExtensions[type] + end + memoize :joined, :to_sql_name, :table_name, :canonical end end diff --git a/lib/rom/sql/function.rb b/lib/rom/sql/function.rb index 9f01b58d6..f171372af 100644 --- a/lib/rom/sql/function.rb +++ b/lib/rom/sql/function.rb @@ -48,7 +48,7 @@ def sql_literal(ds) # @api private def name - meta[:alias] || super + self.alias || super end # @see Attribute#qualified diff --git a/lib/rom/sql/mapper_compiler.rb b/lib/rom/sql/mapper_compiler.rb index a00f1d799..d0b68d725 100644 --- a/lib/rom/sql/mapper_compiler.rb +++ b/lib/rom/sql/mapper_compiler.rb @@ -7,7 +7,7 @@ def visit_attribute(node) name, _, meta = node if meta[:wrapped] - [name, from: meta[:alias]] + [name, from: self.alias] else [name] end diff --git a/lib/rom/sql/migration/schema_diff.rb b/lib/rom/sql/migration/schema_diff.rb index cea6b9a7a..e5026523f 100644 --- a/lib/rom/sql/migration/schema_diff.rb +++ b/lib/rom/sql/migration/schema_diff.rb @@ -75,8 +75,8 @@ def primary_key? attr.primary_key? end - def unwrap(type) - type.optional? ? SQL::Attribute[type.right].meta(type.meta) : type + def unwrap(attr) + attr.optional? ? SQL::Attribute[attr.right, attr.options].meta(attr.meta) : attr end end diff --git a/lib/rom/sql/schema/attributes_inferrer.rb b/lib/rom/sql/schema/attributes_inferrer.rb index 157c60d74..a157144e3 100644 --- a/lib/rom/sql/schema/attributes_inferrer.rb +++ b/lib/rom/sql/schema/attributes_inferrer.rb @@ -25,10 +25,10 @@ def call(schema, gateway) inferred = columns.map do |(name, definition)| type = type_builder.(definition) - attr_class.new(type.meta(name: name, source: schema.name)) if type + attr_class.new(type.meta(source: schema.name), name: name) if type end.compact - missing = columns.map(&:first) - inferred.map { |attr| attr.meta[:name] } + missing = columns.map(&:first) - inferred.map { |attr| attr.name } [inferred, missing] end diff --git a/lib/rom/sql/schema/index_dsl.rb b/lib/rom/sql/schema/index_dsl.rb index 6195a1990..b521b7af6 100644 --- a/lib/rom/sql/schema/index_dsl.rb +++ b/lib/rom/sql/schema/index_dsl.rb @@ -26,8 +26,10 @@ def index(*attributes, **options) end # @api private - def call(schema_name, types) - attributes = types.map { |type| attr_class.new(type).meta(source: schema_name) } + def call(schema_name, attrs) + attributes = attrs.map do |attr| + attr_class.new(attr[:type], attr[:options] || {}).meta(source: schema_name) + end registry.map { |attr_names, options| build_index(attributes, attr_names, options) diff --git a/spec/integration/auto_migrations/indexes_spec.rb b/spec/integration/auto_migrations/indexes_spec.rb index aa319070a..7868a47f4 100644 --- a/spec/integration/auto_migrations/indexes_spec.rb +++ b/spec/integration/auto_migrations/indexes_spec.rb @@ -50,12 +50,12 @@ def indexdef(index) [:attribute, [:id, [:nominal, [Integer, {}]], - primary_key: true, source: :users]], + primary_key: true, source: :users, alias: nil]], [:attribute, [:name, [:nominal, [String, {}]], index: true, - source: :users]], + source: :users, alias: nil]], ]) expect(migrated_schema.indexes.first). diff --git a/spec/integration/auto_migrations/managing_columns_spec.rb b/spec/integration/auto_migrations/managing_columns_spec.rb index 39a0d5455..6f4b6f02b 100644 --- a/spec/integration/auto_migrations/managing_columns_spec.rb +++ b/spec/integration/auto_migrations/managing_columns_spec.rb @@ -37,8 +37,8 @@ [:attribute, [:id, [:nominal, [Integer, {}]], - primary_key: true, source: :users]], - [:attribute, [:name, [:nominal, [String, {}]], source: :users]], + primary_key: true, source: :users, alias: nil]], + [:attribute, [:name, [:nominal, [String, {}]], source: :users, alias: nil]], [:attribute, [:email, [:sum, @@ -48,7 +48,7 @@ ]], [:nominal, [String, {}]], {}]], - source: :users]] + source: :users, alias: nil]] ]) end end @@ -65,7 +65,7 @@ expect(attributes[1].to_ast) .to eql( - [:attribute, [:name, [:nominal, [String, {}]], source: :users]] + [:attribute, [:name, [:nominal, [String, {}]], source: :users, alias: nil]] ) expect(attributes[2].to_ast) .to eql( @@ -78,7 +78,7 @@ ]], [:nominal, [String, {}]], {}]], - source: :users]] + source: :users, alias: nil]] ) end end diff --git a/spec/integration/commands/create_spec.rb b/spec/integration/commands/create_spec.rb index d1d2a9235..aae7db70f 100644 --- a/spec/integration/commands/create_spec.rb +++ b/spec/integration/commands/create_spec.rb @@ -31,7 +31,7 @@ def self.[](input) conf.relation(:profiles) do schema(:users, infer: true) do - attribute :name, Types::String.meta(alias: 'login') + attribute :name, Types::String, alias: :login end end diff --git a/spec/integration/commands/update_spec.rb b/spec/integration/commands/update_spec.rb index 58a7c1133..f2109b33d 100644 --- a/spec/integration/commands/update_spec.rb +++ b/spec/integration/commands/update_spec.rb @@ -32,7 +32,7 @@ def by_name(name) conf.relation(:profiles) do schema(:users, infer: true) do - attribute :name, Types::String.meta(alias: 'login') + attribute :name, Types::String, alias: :login end def by_name(name) diff --git a/spec/integration/schema/inferrer/postgres_spec.rb b/spec/integration/schema/inferrer/postgres_spec.rb index c7d200464..2339cdb4a 100644 --- a/spec/integration/schema/inferrer/postgres_spec.rb +++ b/spec/integration/schema/inferrer/postgres_spec.rb @@ -82,44 +82,47 @@ end context 'inferring db-specific attributes' do + let(:expected) do + attributes( + id: define_attribute(:id, ROM::SQL::Types::PG::UUID, primary_key: true), + big: define_attribute(:big, ROM::SQL::Types::Integer.optional, source: source), + json_data: define_attribute(:json_data, ROM::SQL::Types::PG::JSON.optional, source: source), + jsonb_data: define_attribute(:jsonb_data, ROM::SQL::Types::PG::JSONB.optional, source: source), + money: define_attribute(:money, ROM::SQL::Types::PG::Money, source: source), + decimal: define_attribute(:decimal, ROM::SQL::Types::Decimal, source: source), + tags: define_attribute(:tags, ROM::SQL::Types::PG::Array('text').optional, source: source), + tag_ids: define_attribute(:tag_ids, ROM::SQL::Types::PG::Array('bigint').optional, source: source), + ip: define_attribute(:ip, ROM::SQL::Types::PG::IPAddress.optional, source: source), + color: define_attribute(:color, ROM::SQL::Types::String.enum(*colors).optional, source: source), + subnet: define_attribute(:subnet, ROM::SQL::Types::PG::IPAddress.optional, source: source), + hw_address: define_attribute(:hw_address, ROM::SQL::Types::String.optional, source: source), + center: define_attribute(:center, ROM::SQL::Types::PG::Point.optional, source: source), + page: define_attribute(:page, ROM::SQL::Types::PG::XML.optional, source: source), + mapping: define_attribute(:mapping, ROM::SQL::Types::PG::HStore.optional, source: source), + line: define_attribute(:line, ROM::SQL::Types::PG::Line.optional, source: source), + circle: define_attribute(:circle, ROM::SQL::Types::PG::Circle.optional, source: source), + box: define_attribute(:box, ROM::SQL::Types::PG::Box.optional, source: source), + lseg: define_attribute(:lseg, ROM::SQL::Types::PG::LineSegment.optional, source: source), + polygon: define_attribute(:polygon, ROM::SQL::Types::PG::Polygon.optional, source: source), + path: define_attribute(:path, ROM::SQL::Types::PG::Path.optional, source: source), + ltree_path: define_attribute(:ltree_path, ROM::SQL::Types::PG::LTree.optional, source: source), + created_at: define_attribute(:created_at, ROM::SQL::Types::Time.optional, source: source), + datetime: define_attribute(:datetime, ROM::SQL::Types::Time.optional, source: source), + datetime_tz: define_attribute(:datetime_tz, ROM::SQL::Types::Time.optional, source: source), + flag: define_attribute(:flag, ROM::SQL::Types::Bool, source: source), + int4range: define_attribute(:int4range, ROM::SQL::Types::PG::Int4Range.optional, source: source), + int8range: define_attribute(:int8range, ROM::SQL::Types::PG::Int8Range.optional, source: source), + numrange: define_attribute(:numrange, ROM::SQL::Types::PG::NumRange.optional, source: source), + tsrange: define_attribute(:tsrange, ROM::SQL::Types::PG::TsRange.optional, source: source), + tstzrange: define_attribute(:tstzrange, ROM::SQL::Types::PG::TsTzRange.optional, source: source), + daterange: define_attribute(:daterange, ROM::SQL::Types::PG::DateRange.optional, source: source) + ) + end + it 'can infer attributes for dataset' do - expect(schema.to_h). - to eql( - attributes( - id: ROM::SQL::Types::PG::UUID.meta(name: :id, primary_key: true), - big: ROM::SQL::Types::Integer.optional.meta(name: :big), - json_data: ROM::SQL::Types::PG::JSON.optional.meta(name: :json_data), - jsonb_data: ROM::SQL::Types::PG::JSONB.optional.meta(name: :jsonb_data), - money: ROM::SQL::Types::PG::Money.meta(name: :money), - decimal: ROM::SQL::Types::Decimal.meta(name: :decimal), - tags: ROM::SQL::Types::PG::Array('text').optional.meta(name: :tags), - tag_ids: ROM::SQL::Types::PG::Array('bigint').optional.meta(name: :tag_ids), - ip: ROM::SQL::Types::PG::IPAddress.optional.meta(name: :ip), - color: ROM::SQL::Types::String.enum(*colors).optional.meta(name: :color), - subnet: ROM::SQL::Types::PG::IPAddress.optional.meta(name: :subnet), - hw_address: ROM::SQL::Types::String.optional.meta(name: :hw_address), - center: ROM::SQL::Types::PG::Point.optional.meta(name: :center), - page: ROM::SQL::Types::PG::XML.optional.meta(name: :page), - mapping: ROM::SQL::Types::PG::HStore.optional.meta(name: :mapping), - line: ROM::SQL::Types::PG::Line.optional.meta(name: :line), - circle: ROM::SQL::Types::PG::Circle.optional.meta(name: :circle), - box: ROM::SQL::Types::PG::Box.optional.meta(name: :box), - lseg: ROM::SQL::Types::PG::LineSegment.optional.meta(name: :lseg), - polygon: ROM::SQL::Types::PG::Polygon.optional.meta(name: :polygon), - path: ROM::SQL::Types::PG::Path.optional.meta(name: :path), - ltree_path: ROM::SQL::Types::PG::LTree.optional.meta(name: :ltree), - created_at: ROM::SQL::Types::Time.optional.meta(name: :created_at), - datetime: ROM::SQL::Types::Time.optional.meta(name: :datetime), - datetime_tz: ROM::SQL::Types::Time.optional.meta(name: :datetime_tz), - flag: ROM::SQL::Types::Bool.meta(name: :flag), - int4range: ROM::SQL::Types::PG::Int4Range.optional.meta(name: :int4range), - int8range: ROM::SQL::Types::PG::Int8Range.optional.meta(name: :int8range), - numrange: ROM::SQL::Types::PG::NumRange.optional.meta(name: :numrange), - tsrange: ROM::SQL::Types::PG::TsRange.optional.meta(name: :tsrange), - tstzrange: ROM::SQL::Types::PG::TsTzRange.optional.meta(name: :tstzrange), - daterange: ROM::SQL::Types::PG::DateRange.optional.meta(name: :daterange) - ) - ) + expected.each do |name, attribute| + expect(schema[name]).to eql(attribute) + end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 99a98f21d..0b6d0063d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -35,19 +35,17 @@ port: Integer(ENV.fetch('ROM_ORACLE_PORT', '1521')) } -pg_port = ENV.fetch('PGPORT', '5432') - if defined? JRUBY_VERSION DB_URIS = { sqlite: 'jdbc:sqlite:', - postgres: "jdbc:postgresql://localhost:#{ pg_port }/rom_sql", + postgres: 'jdbc:postgresql://localhost/rom_sql', mysql: 'jdbc:mysql://localhost/rom_sql?user=root&sql_mode=STRICT_TRANS_TABLES,NO_ENGINE_SUBSTITUTION&useSSL=false', oracle: ENV['ROM_USE_ORACLE'] ? fail('Setup Oracle for JRuby!') : nil } else DB_URIS = { sqlite: 'sqlite::memory', - postgres: "postgres://localhost:#{ pg_port }/rom_sql", + postgres: 'postgres://localhost/rom_sql', mysql: 'mysql2://root@localhost/rom_sql?sql_mode=STRICT_TRANS_TABLES,NO_ENGINE_SUBSTITUTION', oracle: "oracle://#{ oracle_settings[:host] }:#{ oracle_settings[:port] }/" \ "#{ oracle_settings[:db_name] }?username=rom_sql&password=rom_sql&autosequence=true" diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb index 8676dc926..4fcfd3c19 100644 --- a/spec/support/helpers.rb +++ b/spec/support/helpers.rb @@ -7,14 +7,19 @@ def define_schema(name, attrs = []) relation_name = ROM::Relation::Name.new(name) ROM::SQL::Schema.define( relation_name, - attributes: attrs.map { |key, value| value.meta(name: key, source: relation_name) }, + attributes: attrs.map do |attr_name, type| + ROM::SQL::Schema.build_attribute_info( + type.meta(source: relation_name), + name: attr_name + ) + end, attr_class: ROM::SQL::Attribute ) end def define_attribute(name, id, **opts) type = id.is_a?(Symbol) ? ROM::Types.const_get(id) : id - ROM::SQL::Attribute.new(type.meta({ name: name, **opts })) + ROM::SQL::Attribute.new(type.meta(opts), name: name) end def build_assoc(type, *args) @@ -25,13 +30,15 @@ def build_assoc(type, *args) def attributes(schema) schema.each_with_object({}) do |(key, type), acc| + type = type.type if type.is_a?(ROM::SQL::Attribute) + if type.optional? - attr = ROM::SQL::Attribute.new(type.right).optional + attr = ROM::SQL::Attribute.new(type.right, name: key).optional else - attr = ROM::SQL::Attribute.new(type) + attr = ROM::SQL::Attribute.new(type, name: key) end - meta = { name: key, source: source } + meta = { source: source } acc[key] = attr.meta(meta) end diff --git a/spec/unit/function_spec.rb b/spec/unit/function_spec.rb index 8d4ee0a43..47fded9ed 100644 --- a/spec/unit/function_spec.rb +++ b/spec/unit/function_spec.rb @@ -162,4 +162,18 @@ to eql('RANK("id") WITHIN GROUP (ORDER BY "users"."name")') end end + + describe '#name' do + it 'returns name when no alias is configured' do + func = ROM::SQL::Function.new(type, name: :id) + + expect(func.name).to eq(:id) + end + + it 'returns alias when it is configured' do + func = ROM::SQL::Function.new(type, name: :id, alias: :pk) + + expect(func.name).to eq(:pk) + end + end end