Custom types return null when using postgres with a type set on the property definition

[sylv]

Describe the bug

Using @mikro-orm/postgresql, with a custom type, with an explicit type annotation

@Property({ type: TimestampType })
createdAt: Date = new Date();

will cause the property to always be null, even when it is non-nullable and is set in the database.
However, removing the explicit : Date type

@Property({ type: TimestampType })
createdAt = new Date();

makes it work fine.

TimestampType is based on BigIntType, but using BigIntType instead did not cause the same issue with/without the type annotation.

@mikro-orm/better-sqlite seems to work fine in both scenarios.

This started happening sometime between v6.0.2 and v6.0.7, I pinned to v6.0.2 to work around it until now.

Reproduction

https://github.com/sylv/mikro-orm-custom-types-null-issue (I didn't see the repro template, sorry)

pnpm install
DB_URI=postgres://... pnpm watch

The output at first will be UserEntity { createdAt: null, id: 1 }, which should not be possible.

Opening user.entity.ts and changing createdAt: Date to createdAt will output UserEntity { createdAt: 2024-05-06T08:00:07.812Z, id: 1 }, which is what is expected.

What driver are you using?

@mikro-orm/postgresql

MikroORM version

6.2.5

Node.js version

20.11.1 (ts 5.4.5)

Operating system

Ubuntu 22.04.4

Validations

• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Slack.
• The provided reproduction is a minimal reproducible example of the bug.

[B4nan]

The problem here is that Date properties are mapped on a lower level automatically, and only then the mapped value is exposed to the custom type (if its not null). And since you store the value as bigint, it is in fact a string, and new Date(string timestamp) results in invalid date. Changing that would be breaking, even if we skip this step for explicitly defined custom types as you have.

May I ask why are you even trying to do this? Why not use the type that is expected here instead of bigint (which is surely meant for something else than timestamps). Both timestamptz and bigint will take 8 bytes of storage, so its not like you would spare some storage size by doing this. And I am rather sure that dates are technically stored as timestamps (numbers) in the actual storage.

Btw it works with sqlite since there is no native bigint type, the value is stored as a number (which works with new Date(val) just fine). I thought its your type implementation that does that (since you return number from the db convertor, which is technically wrong if you want bigint as a backend), but it stores number even if I fix that.

I will try to think of some workaround, 6.2.5 introduced an option to explicitly specify the runtimeType which should help here, but its apparently not working correctly.

[B4nan]

Why is it processing the value at all?

It's mostly for historical reasons, as many libraries like mysql2 or node-postgres do this out of the box. In v6, this is now handled in the ORM to have a unified experience among different drivers - e.g. sqlite won't do this at all, but now as it's handled on the ORM level, both drivers behave the same. I think the main reason why I kept the conversion in place was that using raw SQL would then end up returning strings instead of Date objects, and that feels like a worse experience to me than the current state.

There are no timezone issues

There won't be any timezone issues with timestamptz either, it's literally the same, you store a UNIX timestamp internally, and map it to Date object with a correct timezone (as the Date object always has a timezone).

Or maybe I just like making things harder for myself lol.

That's kinda my point, seems like you are trying to be the smart one here, which goes against the ORM trying the same :]

[sylv]

Yeah that sounds.. not fun, I'm glad I don't have to deal with it :p
I think for now I'll just downgrade back to v6.0.2 instead of trying to update and I'll come back another time to resolve this. Now that I know what it is I'll figure something out. I think I'll just switch to timestamptz, it seems like the easiest option but I'm still wary of it - something about it is just icky, like theres going to be some catch or legacy behaviour that causes issues. Maybe I'm paranoid, we'll see.
Anyway, thanks!

[B4nan]

As I said, I will have a workaround for this later today. Staying on 6.0.2 is quite a bad idea in general, there are lots of fixes in place and you are missing out on a few features too.

like theres going to be some catch or legacy behaviour that causes issues

Since you are mapping to the Date object already, I wouldn't be that scared, it should all work the same with timestamptz.

[B4nan]

Here you go 05c802b

[sylv]

Oh, thanks! I'll try updating again later today. I'll switch to timestamptz as well, I thought there would be more catches but if its a drop-in replacement I don't see a reason not to.