search

noam-honig / remult-kit

3 stars 0 forks source link

sql data types #7

Closed shavitohad closed 2 months ago

shavitohad commented 9 months ago

for a decimal(8,2) field the kit generated @Fields.integer() it should generate @Fields.number() to support decimals.

It would be perfect id the kit can have an option to generate the fieldTypeInDb  valueconverter for each field for cases in which remult creates the tables on another compter.

jycouet commented 9 months ago

I guess it's here that we can have a more optimal detection: https://github.com/noam-honig/remult-kit/blob/4913c9d3c927c90cf2739d243659b7cd57893f0d/src/lib/cli/db/processColumnType.ts#L126-L131

I let @noam-honig looking at it :)

jycouet commented 2 months ago

The current version should already pick @Fields.number(). You mind checking ?

For the exact type in db, you can set fieldTypeInDb: 

@Fields.number({
  valueConverter: {
    fieldTypeInDb: 'numeric(18,8)'
  }
})
price=0;

Today remult-kit is not doing it. Maybe with a flag one day ?

noam-honig commented 2 months ago

Not sure that reading the exact type from the db into fieldTypeInDb makes sense - that type is used to create the table - and in that schema the table is already created

On Tue, Sep 10, 2024 at 12:37 AM JYC @.***> wrote:

The current version should already pick @Fields.number(). You mind checking ?

For the exact type in db, you can set fieldTypeInDb https://remult.dev/docs/ref_valueconverter#fieldtypeindb:

@Fields.number({ valueConverter: { fieldTypeInDb: 'numeric(18,8)' }})price=0;

Today remult-kit is not doing it. Maybe with a flag one day ?

— Reply to this email directly, view it on GitHub https://github.com/noam-honig/remult-kit/issues/7#issuecomment-2339198104, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD65PU7T45UYO56IJVLSNSTZVYIH7AVCNFSM6AAAAABCAG366OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZZGE4TQMJQGQ . You are receiving this because you were assigned.Message ID: @.***>

noam-honig commented 2 months ago

The original issue is fixed and covered by tests: https://github.com/noam-honig/remult-kit/blob/bf9c7f19ef9e3282818f7b8c4ac49da71c32b6a1/src/lib/cli/getEntity.test.ts#L312