Unsigned integer read as signed #119
Comments
|
I can reproduce this by manually setting the database field to a negative value but still baffled on how it could end up negative in the first place |
|
Postgres doesn't allow unsigned integers:
Because of that, Fluent's Postgres driver is using If you need to be able to store up to
@Field(key: "ram_bytes")
var _ramBytes: Int32
var ramBytes: UInt32 {
get {
.init(bitPattern: self._ramBytes)
}
set {
self._ramBytes = .init(bitPattern: newValue)
}
} |
|
Ah yeah it's an overflow, I was worried it was some weird race because I thought I tested setting numbers above 2^31 and it worked. But testing again I'm able to reliably reproduce by writing |
|
I've started work on a PR to make this limitation clearer. Would you be open to testing it on your project? If so, you can try it by doing: swift package edit postgres-nio --revision tn-int-fixOr by adding this to your Package.swift file: .package(url: "https://github.com/vapor/postgres-nio.git", .branch("tn-int-fix"))This should do a few things: 1: Deprecate PostgresData conversion methods that can cause overflow I'm interested to see if:
|
|
Debug build gives me the runtime error but only when accessing loading the model in question, could it be made a static check instead? Would it work to set Building in production mode and setting the value to a negative value in the database crashes with this error when loading the model: |
|
Thanks for taking a look at this.
Great 👍
I've already set all of the methods as deprecated, but due to how Fluent dynamically casts model types that isn't going to show up in your model. I don't think there is a way to make this a static check. Especially under the constraint that it should continue building and working when compiled in release mode. Ideally the developer will encounter this error in a development environment and not when updating production.
Hmm. That seems like a potential mis-compile to me. Try clearing the |
|
Yup, when clearing |
|
Great, thanks for testing that. With this fix your code should stop crashing and work in release mode. As far as PostgresNIO is concerned, the UInt32 is being stored correctly. It's just that PostgreSQL (and potentially other clients) don't agree on the value. The most efficient way forward would be the "Store as an .int32 and translate the bit pattern explicitly" solution mentioned above. The caveat here being that the value may look wrong in the database to other clients. The safest way forward would be to upgrade the field to an Thanks again for helping test this and for reporting. |
Sometimes my vapor + fluent application crashes with a
NIO-ELT-0-#8 (10): Fatal error: Negative value is not representableafter a db update. After this happens the app will crash every time I try to load that record until I manually remove it from the postgres db.The value is defined with the schema:
.field("ram_bytes", .uint32)and when the crash happens it looks like it tries to decode the value as a signed 32 bit integer instead of unsigned. Looking at the db record the value has somehow ended up being a negative value (-1872634973). I haven't been able to reliably reproduce this yet but I've seen it happen several times now.Stack trace
The text was updated successfully, but these errors were encountered: