Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BigDecimal is not supported by codegen #1530

Open
mattiekat opened this issue Mar 9, 2023 · 1 comment · May be fixed by #1700
Open

BigDecimal is not supported by codegen #1530

mattiekat opened this issue Mar 9, 2023 · 1 comment · May be fixed by #1700

Comments

@mattiekat
Copy link

Description

The auto-generated Model defaults to Decimal which breaks when with-rust_decimal is disabled and with-bigdecimal is enabled.

Also the docs do not list BigDecimal as a valid numeric/decimal type.
https://www.sea-ql.org/SeaORM/docs/generate-entity/entity-structure/#column-type

Steps to Reproduce

I have a postgres table defined with a migration

.col(ColumnDef::new(Table::Column).decimal_len(78, 0))

that works without issue but then when I generate an entity using the sea-orm-cli it generates an invalid Model implementation of

#[derive(Clone, Debug, PartialEq, DeriveModel, DeriveActiveModel, Eq)]
pub struct Model {
    ...
    pub column: Decimal,
}

Expected Behavior

Fundamentally I expect it to generate code that compiles, however, I am not sure if the right approach here is to figure out somehow that it should use BigDecimal during codegen or if it should alias BigDecimal as Decimal when only the one feature flag is enabled.

If both feature flags are enabled I think it would be appropriate to default to one or the other and leave it up to the user to manually change them as needed. I would lean towards defaulting to BigDecimal as it is not a default feature flag so having both active implies it is the preference. I don't have particularly strong feelings on this case since I think most people (myself for sure) will decide to use just one or the other exclusively, though, it is not improbable that someone will accidentally not disable the default feature flag.

Actual Behavior

Generated code that does not compile with the chosen feature flags.

Reproduces How Often

Reliably

Workarounds

For now I am just manually replacing all generated Decimal references with BigDecimal.

Reproducible Example

Non-trivial as it requires generating code from a live database.

Versions

postgres 15

├── sea-orm v0.11.0
│   ├── sea-orm-macros v0.11.0 (proc-macro)
│   ├── sea-query v0.28.3
│   │   ├── sea-query-derive v0.3.0 (proc-macro)
│   ├── sea-query-binder v0.3.0
│   │   ├── sea-query v0.28.3 (*)
│   ├── sea-strum v0.23.0
│   │   └── sea-strum_macros v0.23.0 (proc-macro)
├── sea-orm-migration v0.11.0
│   ├── sea-orm v0.11.0 (*)
│   ├── sea-orm-cli v0.11.0
│   │   ├── sea-schema v0.11.0
│   │   │   ├── sea-query v0.28.3 (*)
│   │   │   └── sea-schema-derive v0.1.0 (proc-macro)
│   ├── sea-schema v0.11.0 (*)
├── sea-orm v0.11.0 (*)
@mattiekat mattiekat changed the title BigDecimal with codegen BigDecimal is not supported by codegen Mar 9, 2023
@tyt2y3
Copy link
Member

tyt2y3 commented May 16, 2023

There is a date_time_crate option in sea-orm-cli, so we can have a decimal_crate as well.

PR is welcomed

@ProbablyClem ProbablyClem linked a pull request Jun 11, 2023 that will close this issue
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants