Custom Type with rawDefault serializes wrong value (from returning clause / postgresql) .

[devin-fee-ah]

Describe the bug
I have a custom type (where I use Luxon for dates). I also have a createdAt column with a rawDefault: "now()".

import {
  EntityProperty,
  Platform,
  Type,
  ValidationError,
} from "@mikro-orm/core";
import { DateTime } from "luxon";

type Maybe<T> = T | null | undefined;

export type TimestampTypeOptions = {
  hasTimeZone: boolean;
};

export class TimestampTZType extends Type<
  Maybe<DateTime>,
  Maybe<string | Date>
> {
  public convertToDatabaseValue(
    value: unknown,
    _platform: Platform,
  ): Maybe<string> {
    if (value === undefined || value === null) {
      return value;
    }

    // TODO: we shouldn't need this, as we're not serializing `Date`s to strings.
    // but, it's necessary for the returning clause...
    // also, despite returning this as a string, MikroORM loads it as a Date. What?
    if (value instanceof Date) {
      return DateTime.fromJSDate(value).toISO();
    }

    if (value instanceof DateTime) {
      return value.toUTC().toISO();
    }

    throw ValidationError.invalidType(TimestampTZType, value, "JS");
  }

  public convertToJSValue(
    value: unknown,
    _platform: Platform,
  ): Maybe<DateTime> {
    if (value === undefined || value === null) {
      return value;
    }
    if (typeof value === "string" || value instanceof Date) {
      const dateTime =
        typeof value === "string"
          ? DateTime.fromISO(value)
          : DateTime.fromJSDate(value);
      if (!dateTime.isValid) {
        throw ValidationError.invalidType(TimestampTZType, value, "database");
      }
      return dateTime.toUTC();
    }

    throw ValidationError.invalidType(TimestampTZType, value, "database");
  }

  public getColumnType(
    { length }: EntityProperty,
    _platform: Platform,
  ): string {
    return `timestamptz${length ? `(${length})` : ""}`;
  }
}

And, here's my model:

import { EntitySchema } from "@mikro-orm/core";
import { DateTime } from "luxon";
import { v4 } from "uuid";

import { TimestampTZType } from "../types";
import { Maybe } from "../utility";

export type MinimalEntityOptions = {
  createdAt?: DateTime;
  deletedAt?: Maybe<DateTime>;
  updatedAt?: DateTime;
  id?: string;
};

export class MinimalEntity {
  createdAt?: DateTime;
  id: string;

  constructor({
    createdAt,
    id = v4(),
  }: MinimalEntityOptions = {}) {
    if (createdAt !== undefined) {
      this.createdAt = createdAt;
    }
    this.id = id;
  }
}

export const minimalEntitySchema = new EntitySchema<MinimalEntity>({
  class: MinimalEntity,
  properties: {
    createdAt: {
      columnType: "timestamptz",
      defaultRaw: "now()",
      index: true,
      // onCreate: (v) => v.createdAt ?? DateTime.utc(),
      type: TimestampTZType,
    },
    id: {
      columnType: "uuid",
      defaultRaw: "uuid_generate_v4()",
      primary: true,
      type: String,
    },
  },
});

When creating an instance, I get this:

  console.log
    [query] insert into "minimal_entity" ("id") values ('d5764d58-4086-40f7-ae9c-f0f615c763c6') returning "created_at", "id" [took 3 ms]

As you can see, a value for createdAt is not supplied.

However, as we can see in my test code, the result of that returning clause is a JS Date(?!) and re-selecting the item returns an expected Luxon DateTime:

describe("MinimalEntity", () => {
  const config = getConfig();
  let orm: MikroORM<PostgreSqlDriver>;

  beforeAll(async () => {
    orm = await MikroORM.init({ ...config, debug: true });
  });

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

  it("should create user", async () => {
    const entity = new MinimalEntity({});

    // transaction manager that rolls-back for testing.
    await tx({
      em: orm.em,
      // inner transaction wrapped by the outer transaction.
      inner: async (em) => {
        const userRepository = em.getRepository(MinimalEntity);
        userRepository.persist(entity);
        await userRepository.flush();

        // this is actually wrong, but for demo purposes
        expect(entity.createdAt).toBeInstanceOf(Date);
      },
      // outer transaction which wraps the inner transaction.
      outer: async (em) => {
        const retrieved = await em
          .getRepository(MinimalEntity)
          .findOneOrFail(entity.id);

        // this is correct
        expect(retrieved.createdAt).toBeInstanceOf(DateTime);

        // ok, they're the same entity
        expect(retrieved).toEqual({
          createdAt: DateTime.fromJSDate(entity.createdAt as any).toUTC(),
          id: entity.id,
        });
        // expect(retrieved).toEqual(entity);
      },
    });
  });
});

Expected behavior
The result of a returning clause should not be run through convertToJSValue (which is happening), but instead through convertToDatabaseValue (like any other select call... which isn't happening, as the Date isn't coerced to a DateTime.

Additional context
Add any other context about the problem here.

Versions

Dependency	Version
node	12.16.3
typescript	3.9.7
mikro-orm	4.0.0-alpha.12
your-driver	postgresql

[B4nan]

MikroORM loads it as a Date. What?

It's not about MikroORM, this is done by the pg connector, it returns Date objects for timestamp/tz columns. If you don't like that, you can to configure it to return strings (although not sure if it would not break some features of the ORM, but probably not).

https://github.com/brianc/node-pg-types#use

The result of a returning clause should not be run through convertToJSValue (which is happening), but instead through convertToDatabaseValue (like any other select call... which isn't happening, as the Date isn't coerced to a DateTime.

Uh, why? Result of returning clause is a value from DB and it needs to be converted to JS value. And that is what convertToJSValue method is for.

(like any other select call...

Again, I don't understand why you think so, results of select clause are ran thru convertToJSValue method.

https://github.com/mikro-orm/mikro-orm/blob/master/packages/core/src/hydration/ObjectHydrator.ts#L28

Looking at the code, the returning clause is just reassigned to the entity directly, without using hydrator, so that is the problem here (it basically ignores the custom type definition).

https://github.com/mikro-orm/mikro-orm/blob/master/packages/core/src/unit-of-work/ChangeSetPersister.ts#L115

[B4nan]

Fix will be part of rc.1 (will ship it later this week), here is the test case (I did not want to add luxon dependency so used simple DateTime wrapper, as that is not really relevant here):

https://github.com/mikro-orm/mikro-orm/blob/c5384b4a61b8638d8a2b6aa9ffc3f1219930b83b/tests/issues/GH725.test.ts

[devin-fee-ah]

Uh, why? Result of returning clause is a value from DB and it needs to be converted to JS value. And that is what convertToJSValue method is for.

I got it backwards in those words, but correct in the code, specifically:

  public convertToDatabaseValue(
    value: unknown,
    _platform: Platform,
  ): Maybe<string> {
    if (value === undefined || value === null) {
      return value;
    }

    // TODO: we shouldn't need this, as we're not serializing `Date`s to strings.
    // but, it's necessary for the returning clause...
    // also, despite returning this as a string, MikroORM loads it as a Date. What?
    if (value instanceof Date) {
      return DateTime.fromJSDate(value).toISO();
    }

    if (value instanceof DateTime) {
      return value.toUTC().toISO();
    }

    throw ValidationError.invalidType(TimestampTZType, value, "JS");
  }

[devin-fee-ah]

@B4nan also, looking at the code, I see the resolution. You weren't using the hydrator before. c5384b4#diff-3fb8c8b8cea3f5b7b71c8ba25585b38eR122

Thanks for the fix.