ValidationError when mapping double type data with query builder in v6

[atfera]

Describe the bug

Hi,

There is a weird bug in v6 using query builder. I have double type data in my schema, and when using query builder, I got for example:
ValidationError: Trying to set Object.randomDouble of type 'string' to '0.022' of type 'number'

What is weird is that it works fine using em find method, and in v5 I don't have this error.

Reproduction

user.entity.ts

import { Embeddable, Embedded, Entity, PrimaryKey, Property, types } from '@mikro-orm/core';

@Entity()
export class User {

  @PrimaryKey()
  id!: number;

  @Property()
  age!: number;

  @Property({ type: 'double' })
  randomDouble!: number;
}

test.ts

import { MikroORM } from '@mikro-orm/sqlite';
import { User } from './user.entity';
import { faker } from '@faker-js/faker';

let orm: MikroORM;

beforeAll(async () => {
  orm = await MikroORM.init({
    dbName: 'sqlite.db',
    entities: ['dist/**/*.entity.js'],
    entitiesTs: ['src/**/*.entity.ts'],
    debug: ['query', 'query-params'],
    allowGlobalContext: true, // only for testing
  });
  await orm.schema.refreshDatabase();
});

afterAll(async () => {
  await orm.close(true);
});

test('test', async () => {
  Array.from({ length: 100 }).forEach(() => {
    orm.em.create(User, {
      age: faker.number.int({ min: 1, max: 100 }),
      randomDouble: faker.number.float({ precision: 0.001 })
    });
  });

  await orm.em.flush();

  const qb = orm.em.createQueryBuilder(User);

  await orm.em.find(User, {});

  await qb.where({})
});

logs

console.log
    [query] commit

      at DefaultLogger.log (node_modules/@mikro-orm/core/logging/DefaultLogger.js:38:14)

  console.log
    [query] select `u0`.* from `user` as `u0` [took 1 ms, 100 results]

      at DefaultLogger.log (node_modules/@mikro-orm/core/logging/DefaultLogger.js:38:14)

  console.log
    [query] select `u0`.* from `user` as `u0` [took 2 ms, 100 results]

      at DefaultLogger.log (node_modules/@mikro-orm/core/logging/DefaultLogger.js:38:14)

 FAIL  src/example.test.ts (6.104 s)
  ✕ test (86 ms)

  ● test

    ValidationError: Trying to set Object.randomDouble of type 'string' to '0.836' of type 'number'

      34 |   await orm.em.find(User, {});
      35 |
    > 36 |   await qb.where({})
         |   ^
      37 | });
      38 |

      at Function.fromWrongPropertyType (node_modules/@mikro-orm/core/errors.js:23:16)
      at EntityValidator.validateProperty (node_modules/@mikro-orm/core/entity/EntityValidator.js:67:44)
      at node_modules/@mikro-orm/core/EntityManager.js:1114:42
          at Array.forEach (<anonymous>)
      at SqlEntityManager.map (node_modules/@mikro-orm/core/EntityManager.js:1111:27)
      at QueryBuilder.getResultList (node_modules/@mikro-orm/knex/query/QueryBuilder.js:647:36)
      at Object.<anonymous> (src/example.test.ts:36:3)

What driver are you using?

@mikro-orm/sqlite

MikroORM version

6.0.2

Node.js version

20

Operating system

mcr.microsoft.com/devcontainers/typescript-node:1-20-bullseye

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]

That's because doubles are mapped to string, not to number (at least I think so), your entity definition is not aligned with how it works and you get this as a result. If you want to map to number, provide a type that handles it, as the data from database for double/decimal/numeric/bigint will always be a string (and the ORM wont convert that unless you provide a type implementation that would do so).

This worked the same in v5, such property was mapped to string there too, probably just the type in your entity def was ignored/overridden by the ORM automatically, now its respected (but wrong).

We should probably handle this the same way as bigints now, allowing to map to number explicitly (and maybe even infer the type if available)

edit: uh wait a sec, you say it works with em.find but not with QB? just to back up what I said above, the DoubleType used in the ORM explicitly says its runtime type is string.

[atfera]

Yep I tried to debug and I saw that it was mapped to string. I can understand the why for decimal/numeric/bigint, but for double I don't think we lose in precision ? (atleast in postgres that is what we are using). I thought it was a bug because like I said I don't have ValidationError with em.find, knowing that it's probably also using QueryBuilder behind, and randomDouble in my repro is converted correctly.

Now I'm curious how I need to handle my double data. For the moment, I replaced all the querybuilder that were throwing this error by em.find. The best solution if I understand is to use a custom type?

[B4nan]

Hmm, I guess you are right, JS numbers are also double precision so it should be safe to map that. Ideally, this should also reflect the type you set in your entity definition, that way it will be non-breaking. Weird that the db drivers are returning strings here.

Anyway, before this gets resolved, you should be able to do this to get the mapping to number consistently:

import { Type } from './Type';
import type { Platform } from '../platforms';
import type { EntityProperty } from '../typings';

export class MyDoubleType extends Type<number, string> {

  override getColumnType(prop: EntityProperty, platform: Platform) {
    return platform.getDoubleDeclarationSQL();
  }

  override convertToJSValue(value: string): number {
    return +value;
  }

  override compareAsType(): string {
    return 'number';
  }

}

And use that:

@Property({ type: MyDoubleType })
randomDouble!: number;

[atfera]

Hey sry,
I tried last week with the custom type solution, but it didn't work. Seems like the runtimeType is infered directly from columnTypes in MetadataDiscovery if possible:

https://github.com/mikro-orm/mikro-orm/blob/e87871442a7e64cd24bab99d2e43e3b00c081457/packages/core/src/metadata/MetadataDiscovery.ts#L1291C5-L1294C13

Maybe it's a normal behavior, but just to let you know.

It works well now with your fix. Thank you for that 👍

[B4nan]

It's set by the metadata provider if available, the code you linked uses ??=, it won't override the value if it can be inferred. Note that this part changed a bit in the latest version too, before it was ignoring columns that had an explicit type option (which you had if you did @Property({ type: MyDoubleType })).

But I don't understand how is the runtimeType relevant here, when you use the custom type I suggested, it shouldn't be relevant. It's about the convertToJSValue converting to numbers, not about runtimeType.

Maybe retest that approach with 6.0.4 just for the sake of it, as I feel like both will work correctly now.

[atfera]

Yep, with or without the customType solution, it works well with the ^6.0.3 versions, but not with 6.0.2. I was still having a runtimeType string with my customType, and thought that was the problem, but maybe i'm wrong.