PG range does not work on create #286
Comments
|
Comment from @flash-gordon: |
|
I'm not sure we should check type in Range constructor. def self.range(name, read_type)
Type(name) do
type = SQL::Types.Definition(Values::Range).constructor do |range|
format('%s%s,%s%s',
range.exclude_begin? ? :'(' : :'[',
range.lower,
range.upper,
range.exclude_end? ? :')' : :']')
end
type.meta(read: read_type)
end
endThere're also issues with format('%s%s,%s%s',
range.respond_to?(:exclude_begin?) && range.exclude_begin? ? :'(' : :'[',
range.begin,
range.end,
range.exclude_end? ? :')' : :']')Other options comes to mind:
|
|
@cutalion this should suffice type = SQL::Types.Definition(Values::Range).constructor do |range|
case range
when ::Range
# new format logic for ruby ranges
when Values::Range
format('%s%s,%s%s',
range.exclude_begin? ? :'(' : :'[',
range.lower,
range.upper,
range.exclude_end? ? :')' : :']')
else
raise TypeError, "..."
end
end |
|
I also figured out that relation returns range fields as |
|
And...also passing ROM's range to
|
|
So, did this ever work? I'd like to fix all this, but would you accept a PR which will change current behavior? |
It's not possible, ruby ranges cannot represent PG ranges, otherwise, we would do it already. As I mentioned, cases like re @v-kolesnikov can confirm? |
We could add |
|
@v-kolesnikov here's a script to reproduce an issue with query. require 'rom'
require 'rom-sql'
require 'pry'
config = ROM::Configuration.new(:sql, 'postgres://localhost/rom_experiments')
conn = config.gateways[:default].connection
conn.drop_table?(:pg_ranges)
conn.create_table(:pg_ranges) do
primary_key :id
daterange :range
end
config.relation(:pg_ranges) do
schema(:pg_ranges, infer: true)
end
rom = ROM.container(config)
pg_ranges = rom.relations.pg_ranges
record = pg_ranges.changeset(:create, range: ROM::SQL::Postgres::Values::Range.new(Date.today, Date.today + 10, :'[]')).commit
puts pg_ranges.where(range: record[:range]).to_a.inspect # works
puts pg_ranges.where { range.overlap(record[:range]) }.to_a.inspect # does not work |
|
@Culation Thank you for opened issue! |
|
@flash-gordon @v-kolesnikov Until fix is ready, I've fixed this in my project by patching core I'd like to discuss how ranges should be returned from the database. Previously I've used my custom type for reading tstzrange column, which was a thin wrapper/converter using Also found another small issue I noticed with ROM::SQL::Postgres::Values::Range and ROM::Struct. I think in that case it either should not be converted to array or should be converted to array with only 2 values - min and max. P.S. my patches: class ::Range
def exclude_begin?
false
end
def lower
self.begin
end
def upper
self.end
end
end
class ROM::SQL::Postgres::Values::Range
def begin
lower
end
def end
upper
end
def to_sequel_range
Sequel::Postgres::PGRange.new(
lower,
upper,
exclude_begin: exclude_begin?,
exclude_end: exclude_end?
)
end
def sql_literal_append(ds, sql)
to_sequel_range.sql_literal_append(ds, sql)
end
def to_hash
to_range
end
def to_range
to_sequel_range.to_range
end
end |
|
@cutalion this is a common practice for DB adapters to coerce input data to expected data types. However, since PG ranges cannot be represented as |
|
Yes, it makes sense. Sequel does exactly this. It seems for me now that we can end up with repeating everything that PGRange is doing :) |
|
IMO not really, having something beyond a plain data structure may confuse people. On the one hand, we can have whatever behavior we want, on the other hand, this will become a subject of discussion. Implementing a fully-featured range type is kind of out of scope of |
|
I have explored example with pg_ranges.where { range.overlap("[2018-04-23,2018-05-04)") }or In other words, we should represent the value as a literal in DSL. So, what exactly would be preferable to change in API in that case? |
|
I think we should coerce input value to rom-sql's range, which should implement |
|
Hey guys, any update on this? I've prepared an example with different objects (rom sql range, sequel range, ruby range) def try(name)
yield
puts "#{name} works"
rescue => e
puts "#{name} does not work"
puts e.message
ensure
puts
end
ruby_range = Range.new(Time.now, Time.now)
rom_range = ROM::SQL::Postgres::Values::Range.new(Time.now, Time.now)
sequel_range = Sequel::Postgres::PGRange.new(Time.now, Time.now)
custom_range = CustomRange.new(Time.now, Time.now)
try(:ruby_range) { pg_ranges.changeset(:create, range: ruby_range).commit }
try(:sequel_range) { pg_ranges.changeset(:create, range: sequel_range).commit }
try(:rom_range) { pg_ranges.changeset(:create, range: rom_range).commit }
try(:custom_range) { pg_ranges.changeset(:create, range: custom_range).commit }Output: |
It seems that pg_range extension does not work on inserting records to the database.
Full example
The text was updated successfully, but these errors were encountered: