fix: pgdialect range #1221
fix: pgdialect range #1221
Conversation
thecampagnards
force-pushed
the
master
branch
3 times, most recently
from
23c9808 to
e50737c
Compare
dialect/pgdialect/range.go
Outdated
| return Array(m).Scan(anySrc) | ||
| } | ||
|
|
||
| func (m MultiRange[T]) AppendQuery(fmt schema.Formatter, buf []byte) ([]byte, error) { |
There was a problem hiding this comment.
should I move this logic to not have quote for Range inside array ?
There was a problem hiding this comment.
I think we need to specifically test which form PostgreSQL can accept, as this is a complex and uncommon case.
dialect/pgdialect/range.go
Outdated
| RangeBoundInclusiveRight RangeBound = ']' | ||
| RangeBoundExclusiveLeft RangeBound = '(' | ||
| RangeBoundExclusiveRight RangeBound = ')' | ||
| RangeLowerBoundInclusive RangeLowerBound = '[' |
There was a problem hiding this comment.
Do we really have to rename these variable?
The original name may not be perfect, but changing it would break backward compatibility.
There was a problem hiding this comment.
I can keep the same name, but I think it's preferable to keep a different type between lower and upper bound
There was a problem hiding this comment.
good I rb to the previous naming
dialect/pgdialect/range.go
Outdated
|
|
||
| if len(src) == 0 { | ||
| return io.ErrUnexpectedEOF | ||
| src, err = scanElem(&r.Upper, src) |
There was a problem hiding this comment.
Do we need to consider a comma here? I’m not sure.
There was a problem hiding this comment.
I improved the tests with equality, I also switch to a driver.valuer so we don't need to remove '
|
Thank you for your contribution, especially the related test cases. |
Thx for your feedbacks ! With this new updates this should be better for multirange |
|
there is also |
Hello,
I suggest a fixes on
pgdialect.Range, on a pg14.Doc: https://www.postgresql.org/docs/current/rangetypes.html
Thx !