[BUG]: defaultNow() in date mode should save new Date()

[emdede]

What version of drizzle-orm are you using?

Latest

What version of drizzle-kit are you using?

No response

Describe the Bug

gt in combination with timestamp returns wrong result.

const table = pgTable('table', {
  name: text('name'),
  timestamp: timestamp('timestamp').defaultNow()
})

const firstThing = (await db.insert(table).values({ name: 'first' }).returning({ timestamp: table.timestamp }))[0]
const secondThing = await db.insert(table).values({ name: 'second' })

const result = await db.query.table.findFirst({ where: gt(table.timestamp, firstThing.timestamp) })
console.log(result) // logs firstThing, not secondThing

Expected behavior

gt should not behave like gte when used with timestamps. Might be true for lt, too, I haven't checked.

I have also tested with sql`` and same result:

where: sql`${table.timestamp} > ${firstThing.timestamp}`

Also, this only happens with mode: 'date' (default), mode: 'string' works fine

[emdede]

This issue was discussed in discord here: https://discord.com/channels/1043890932593987624/1146453658813333514

It looks like the conversion to a JS Date object loses precision and as such breaks expected behavior.

I don't know if there's a straightforward fix to this, but as a direct course of action, I'd suggest pointing this behavior out in the docs - "Date" mode is less precise and can lead to unexpected results.

[emdede]

A few more thoughts on this. The only real issue here is the behavior of defaultNow because it doesn't matter if you choose date or string mode, the value that is actually saved is the timestamp string returned from sql.now(). This in turn leads to the unexpected behavior exhibited in the issue description. For date mode's defaultNow it should actually be a new Date(). This would then guarantee that all operations are performed on the same kind of Date object.

Having said that: in current iteration, I would classify this as a bug rather than a limitation.

[emdede]

I've looked at the internals for this and I think this can be fairly easily fixed but since it is a change that needs to be made across several drivers and probably need to juggle around some baseclasses I feel like I'm not going to be able to produce a satisfying result.

Basically there are two steps:

1. Get mode from config
2. If string same as before - if date set this.$defaultFn instead of this.default - () => new Date()

[Angelelz]

This issue should be closed by #1659.
If microsecond precision is needed, the mode: 'string' has to be used.