postgres-nio
postgres-nio copied to clipboard
Fix PostgresNumeric.init crash at large number
Fix #186
Fix reverseChunked
to comply with maxSize
.
Hi, I am coworker with @sidepelican .
We are using vapor and postgres for our company business.
We depends on numeric
type of postgres for accurate amount of money processing on DB.
I wish to grow vapor on enterprise use-cases 😁
String.Index
cannot be Strideable
because it needs to refer to the parent storage to know next index, so I cannot use stride
.
I tried to write with sequence
likestride
. What do you think?
I tried next patterns:
let aaa: [Substring] = [
""[...],
"1"[...],
"1234"[...],
"12345"[...],
"12345678"[...],
"1234567890abcd"[...],
]
for v in aaa {
print(v.chunked(by: 4), v.reverseChunked(by: 4))
}
[""] [""]
["1"] ["1"]
["1234"] ["1234"]
["1234", "5"] ["1", "2345"]
["1234", "5678"] ["1234", "5678"]
["1234", "5678", "90ab", "cd"] ["12", "3456", "7890", "abcd"]
String.Index
cannot beStrideable
because it needs to refer to the parent storage to know next index, so I cannot usestride
. I tried to write withsequence
likestride
. What do you think?
Chunking a String
doesn't make sense in the first place - maxSize is 3? 3 what? UTF-8 bytes? Unicode code points? Basic or extended grapheme clusters? Composed or decomposed clusters? String.Index
isn't Strideable
for this reason as well. Honestly, I wouldn't want these methods to work with a String
or Substring
. (Also, with the stride()
-based implementation, you don't have to store the complete set of indices in memory (per Array(sequence(...))
) before using them, though I suspect the optimizer would end up wiping out the difference anyway.)
And there's also a mistake we both made in reverseChunked()
:
/// - distance: The distance to offset `i`. `distance` must not be negative
/// unless the collection conforms to the `BidirectionalCollection`
/// protocol.
func index(_ i: Self.Index, offsetBy distance: Int, limitedBy limit: Self.Index) -> Self.Index?
There is no check for BidirectionalCollection
conformance, as rare as it is to have a Collection
that isn't bidirectional. We also need to make the Strideable
requirement explicit. How about this?
extension Collection where Index: Strideable, Index.Stride == Int {
/// Split the collection into chunks of `maxSize` length each. The last chunk may contain anywhere between 1 and maxSize elements. `maxSize` must be greater than zero.
///
/// Example:
/// ```swift
/// print(Array(1...10).chunked(by: 3)) // [[1, 2, 3], [4, 5, 6], [7, 8, 9], [10]]
/// ```
func chunked(by maxSize: Int) -> [SubSequence] {
assert(maxSize > 0, "Can not chunk less than one element at a time.")
return stride(from: self.startIndex, to: self.endIndex, by: maxSize).map {
self[$0 ..< (self.index($0, offsetBy: maxSize, limitedBy: self.endIndex) ?? self.endIndex)]
}
}
}
extension BidirectionalCollection where Index: Strideable, Index.Stride == Int {
/// Split the collection into chunks of `maxSize` length each. The **first** chunk may contain anywhere between 1 and maxSize elements. `maxSize` must be greater than zero.
///
/// Example:
/// ```swift
/// print(Array(1...10).reverseChunked(by: 3)) // [[1], [2, 3, 4], [5, 6, 7], [8, 9, 10]]
/// print(Array(1...11).reverseChunked(by: 3)) // [[1, 2], [3, 4, 5], [6, 7, 8], [9, 10, 11]]
/// ```
func reverseChunked(by maxSize: Int) -> [SubSequence] {
assert(maxSize > 0, "Can not chunk less than one element at a time.")
return stride(from: self.endIndex, to: self.startIndex, by: -maxSize).map {
self[(self.index($0, offsetBy: -maxSize, limitedBy: self.startIndex) ?? self.startIndex) ..< $0]
}.reversed()
}
}
Of course, it's possible to write versions that work without the constraints, and a reversed version that doesn't use a negative offset, but I don't think it's worthwhile to do so.
Sorry, I can't understand what you mean in the first sentence.
I think there is some misunderstanding.
At this PR, It doesn't matter whether indices in memory or calculating every time. (If you are particular about it, I will obey.)
The only thing I can say is that your code using stride
does not compile in my environment. Did you compile it in postgres-nio
?
There is no check for BidirectionalCollection conformance,
That makes sense. nice review.