Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when updating an entity with a custom type #5176

Closed
5 tasks done
skaltus opened this issue Jan 28, 2024 · 4 comments
Closed
5 tasks done

Error when updating an entity with a custom type #5176

skaltus opened this issue Jan 28, 2024 · 4 comments

Comments

@skaltus
Copy link

skaltus commented Jan 28, 2024

Describe the bug

Error when updating an entity with a custom type:

Custom type:

export interface GeoJSONPolygon {
  type: 'Polygon';
  coordinates: number[][][];
}

export class GeoJSONPolygonType extends Type<GeoJSONPolygon | undefined, string | undefined> {
  convertToDatabaseValue(polygon: GeoJSONPolygon | undefined): string | undefined {
    if (!polygon) {
      return undefined;
    }
    return JSON.stringify(polygon);
  }

  convertToJSValue(value: string | undefined): GeoJSONPolygon | undefined {
    if (!value) {
      return undefined;
    }

    const polygon = JSON.parse(value) as GeoJSONPolygon;
    return polygon;
  }

  convertToJSValueSQL(key: string) {
    return `ST_AsGeoJSON(${key},15)`;
  }

  convertToDatabaseValueSQL(key: string) {
    return `ST_GeomFromGeoJSON(${key})`;
  }

  getColumnType(): string {
    return 'GEOGRAPHY(POLYGON,4326)';
  }
}

Entity:

@Entity()
export class DeliveryZone {
  @PrimaryKey()
  id!: number;

  @Property({ type: GeoJSONPolygonType })
  polygon!: GeoJSONPolygon;
}

Entity update code:

const deliveryZone = await orm.em.findOneOrFail(DeliveryZone, { id });
deliveryZone.polygon = newPolygon;
await orm.em.flush();

Update query:

update "delivery_zone" set "polygon" = ST_GeomFromGeoJSON('{"type":"Polygon","coordinates":[[[44.27741512978747,46.309845946384456],[44.277767041287746,46.29635302500847],[44.2893214688815,46.29805501817512],[44.29882307938891,46.29254361051457],[44.30908716481349,46.29323256680655],[44.31436583731761,46.297649786505275],[44.31694652165373,46.29748769299738],[44.318002256154585,46.300283738754416],[44.32492318232704,46.29943278342128],[44.33403003241864,46.31647378878097],[44.32888721608958,46.32346636590617],[44.321333704606445,46.32635193087927],[44.308396307279764,46.32829405237257],[44.30164636084854,46.31319917959189],[44.27741512978747,46.309845946384456]]]}') where "id" = 1 returning "polygon"

Error:

JIT runtime error: Unexpected number in JSON at position 1
    
      function(entity, data, factory, newEntity, convertCustomTypes, schema) {
        if (data.id === null) {
          entity.id = null;
        } else if (typeof data.id !== 'undefined') {
          entity.id = data.id;
        }
        if (data.polygon === null) {
          entity.polygon = null;
        } else if (typeof data.polygon !== 'undefined') {
          if (convertCustomTypes) {
    >       const value = convertToJSValue_polygon(data.polygon);
                         ^
            data.polygon = convertToDatabaseValue_polygon(value);
            entity.polygon = value;
          } else {
            entity.polygon = data.polygon;
          }
        }
      }

I'm not sure, but it looks like convertToJSValueSQL is not used in the update query or update query should not return polygon

Reproduction

https://github.com/skaltus/reproduction

What driver are you using?

@mikro-orm/postgresql

MikroORM version

6.0.5

Node.js version

18

Operating system

Ubuntu 20.04

Validations

@B4nan
Copy link
Member

B4nan commented Jan 29, 2024

You might need to extend the JsonType for the ORM to understand what you want, as your type is basically doing just that (plus the custom SQL convertors). JSON columns are handled a bit differently, and the JsonType is needed.

Try this:

export class GeoJSONPolygonType extends JsonType {
  convertToJSValueSQL(key: string) {
    return `ST_AsGeoJSON(${key},15)`;
  }

  convertToDatabaseValueSQL(key: string) {
    return `ST_GeomFromGeoJSON(${key})`;
  }

  getColumnType(): string {
    return 'GEOGRAPHY(POLYGON,4326)';
  }
}

@skaltus
Copy link
Author

skaltus commented Jan 29, 2024

I used this type and added new tests to the repository - https://github.com/skaltus/reproduction/blob/master/src/example.test.ts
Now, when updating, the polygon remains unparsed.

Native update, create, fetch entity work fine.

Btw, I forgot to mention that the error appeared after updating to 6.0.5, in 5.9.7 everything worked

image

@B4nan
Copy link
Member

B4nan commented Feb 2, 2024

Now, when updating, the polygon remains unparsed.

The problem is not that it remains unparsed, the update works correctly, but it has returning clause (which is there because its using a raw query under the hood) and its results are not parsed, so you end up with the raw value. So technically two problems, fixing the second one is pretty simple and that should be generally enough, the first one is rather optimization.

edit: to be precise, the problem is support for convertToJSValueSQL in returning statements

@B4nan B4nan closed this as completed in 2e1d6c8 Feb 3, 2024
@B4nan
Copy link
Member

B4nan commented Feb 3, 2024

The fix is not very complete, but I wasn't able to come up with any examples on how to break things the same way for insert+returning or for the insert/update many methods, so leaving that for future if someone actually comes up with a breaking repro.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants