search

SeaQL / sea-query

🔱 A dynamic SQL query builder for MySQL, Postgres and SQLite
https://www.sea-ql.org
Other
1.12k stars 184 forks source link

postgres: silently uses signed integers when unsigned integers are specified in schema #731

Open tkporter opened 8 months ago

tkporter commented 8 months ago

Description

Hi!

We have a table whose columns were created as unsigned integers, which we had originally expected to be a 4 byte unsigned integer. At the time, I don't think we realized that postgres doesn't support unsigned integers. Only now upon trying to insert a value > (2^31) - 1 did I realize that the real column data type is a signed integer.

I'm not sure if this is documented behavior where sea-query will silently use signed integers under the hood if using Postgres. Have you considered making the use of unsigned integers prohibited if using postgres? Maybe it could be put behind a feature flag, or some other compile time check? This is unfortunately now a pain for us to change so we can use higher value u32s.

Poking around, it seems we're not the first to need to hack around this https://github.com/SeaQL/sea-query/pull/275#issuecomment-1075383107

Steps to Reproduce

  1. Create a table with a column ColumnDef::new(Foo::Bar).unsigned() using postgres under the hood
  2. \d my_table, which uses a normal integer

Expected Behavior

Would've expected to not be able to create an unsigned integer, or some assurances about the number range I can use

Actual Behavior

uses a signed integer under the hood

Reproduces How Often

every time w/ postgres

Versions

├── sea-orm v0.11.3
│   ├── sea-orm-macros v0.11.3 (proc-macro)
│   ├── sea-query v0.28.5
│   │   ├── sea-query-derive v0.3.0 (proc-macro)
│   ├── sea-query-binder v0.3.1
│   │   ├── sea-query v0.28.5 (*)
│   ├── sea-strum v0.23.0
│   │   └── sea-strum_macros v0.23.0 (proc-macro)
├── sea-orm-migration v0.11.3
│   ├── sea-orm v0.11.3 (*)
│   ├── sea-orm-cli v0.11.3
│   │   ├── sea-schema v0.11.0
│   │   │   ├── sea-query v0.28.5 (*)
│   │   │   └── sea-schema-derive v0.1.0 (proc-macro)
│   ├── sea-schema v0.11.0 (*)
├── sea-orm v0.11.3 (*)

db: PostgreSQL 14

Additional Information

tyt2y3 commented 8 months ago

Yes, we can add a feature flag to panic, if that's what you wanted. For example, https://github.com/SeaQL/sea-query/pull/723/files

tkporter commented 8 months ago

thank you :) to be transparent, I don't believe I'll have the bandwidth to contribute this, but I'd love to see this happen. In the meantime we'll likely migrate to some larger data types