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

[Fix] not to make typeorm generate alter query on geometry column when that column was not changed #5525

Merged
merged 2 commits into from Feb 17, 2020

Conversation

ohjeyong
Copy link
Contributor

Problem

Let's assume the entity like this.

import { Point, Polygon } from 'geojson';

class SomeTable1 {
    ...
    @Column({type: 'geometry', srid: 4326, spatialFeatureType: 'Polygon'})
    my_polygon!: Polygon;
}

class SomeTable2 {
    ...
    @Column({type: 'geometry', srid: 4326, spatialFeatureType: 'Point'})
    my_place!: Point;
}

Typeorm always think the geometry field has been changed even though I did not change it.
If synchronize option is true, typeorm always run below query whenever server started.

query: ALTER TABLE "some_table2" ALTER COLUMN "my_place" TYPE geometry(Point,4326)

If synchronize option is false, when I execute typeorm migration:generate, it "always" make new migration file below even after I execute typeorm migration:run.

    public async up(queryRunner: QueryRunner): Promise<any> {
        await queryRunner.query('ALTER TABLE "some_table2" ALTER COLUMN "my_place" TYPE geometry(Point,4326)')
    }

    public async down(queryRunner: QueryRunner): Promise<any> {
        await queryRunner.query('ALTER TABLE "some_table2" ALTER COLUMN "my_place" TYPE geometry(POLYGON,4326)')
    }

When I run query below,

SELECT type 
FROM geometry_columns 
WHERE f_table_schema = 'public' 
AND f_table_name = 'some_table2 
and f_geometry_column = 'my_place';

it returns

type
POINT

It means, my_place column's geometry type is POINT on postgresql which is correct, but as we can see in the result of typeorm migration:generate, typeorm think this column is not POINT but POLYGON.
Therefore, typeorm keep trying to alter column's geometry type even though it set correctly.
This is not a critical bug, but typeorm execute useless alter query if synchronize option is true. It is pretty annoying when execute typeorm migration:generate if synchronize option is false.

Cause

Let's see the code of loadTables method in PostgresQueryRunner.ts.

if (tableColumn.type === "geometry") {
    const geometryColumnSql = `SELECT * FROM (
    SELECT
        "f_table_schema" "table_schema",
        "f_table_name" "table_name",
        "f_geometry_column" "column_name",
        "srid",
        "type"
        FROM "geometry_columns"
) AS _
    WHERE ${tablesCondition} AND "column_name" = '${tableColumn.name}'`;
        const results: ObjectLiteral[] = await this.query(geometryColumnSql);
        tableColumn.spatialFeatureType = results[0].type;
        tableColumn.srid = results[0].srid;
}

The geometryColumnSql execute the query like this.

SELECT * FROM (
    SELECT
    "f_table_schema" "table_schema",
    "f_table_name" "table_name",
    "f_geometry_column" "column_name",
    "srid",
    "type"
    FROM "geometry_columns"
    ) AS _
WHERE (blahblah) OR (blahblah) OR ... OR ("table_schema" = 'public' AND "table_name" = 'some_table1') OR ("table_schema" = 'public' AND "table_name" = 'some_table2') AND "column_name" = 'my_place'

The result is

table_schema table_name column_name srid type
public some_table1 my_polygon 4326 POLYGON
public some_table2 my_point 4326 POINT

This is the problem.
Let's go back to the code. results[0].type is always POLYGON. Therefore even though the column my_pointgeometry type is POINT, typeorm always think it's type is POLYGON.

Solution

First, we need to fix WHERE condition by wrapping OR condition in a parenthesis like this.

WHERE
    (
        (blahblah) OR (blahblah) OR ... OR ("table_schema" = 'public' AND "table_name" = 'some_table1') OR ("table_schema" = 'public' AND "table_name" = 'some_table2')
    )
    AND "column_name" = 'my_place'

Second, add table name on WHERE condition so that if we define same column name with different geometry type, the queryRunner can handle this properly.

WHERE
    (
        (blahblah) OR (blahblah) OR ... OR ("table_schema" = 'public' AND "table_name" = 'some_table1') OR ("table_schema" = 'public' AND "table_name" = 'some_table2')
    )
    AND "column_name" = 'my_place' AND "table_name" = 'some_table2'

Then, this query will always return only one correct column info.

table_schema table_name column_name srid type
public some_table2 my_point 4326 POINT

Extra

The default postgis srid value is 0, which means, if you omit the Column option srid like this,

    @Column({type: 'geometry', spatialFeatureType: 'Point'})
    my_place!: Point;

postgis set this column's srid to 0.
However in ColumnMetadata.ts,

if (options.args.options.srid)
    this.srid = options.args.options.srid;

srid value zero can't pass this if statement, so that migration process think there are changes.
Related issue

By fixing the condition of if statements like this options.args.options.srid !== undefined we can solve this problem.

@ohjeyong ohjeyong changed the title [Fix] not to make alter geometry column when that column was not changed [Fix] not to make typeorm generate alter query on geometry column when that column was not changed Feb 14, 2020
@pleerock
Copy link
Member

Wow haven't seen such a detailed PR description for a while! Thanks!

@krazibit
Copy link

krazibit commented Mar 4, 2020

This fix created a new issue.
The new clause added AND "table_name" = '${table.name}'
But table.name includes the schema name as prefix making the query return no result

@thynson
Copy link

thynson commented Apr 9, 2020

This PR has not been included in v0.3.0-rc.19 yet. Any plan to make a new rc release? @pleerock

@pleerock
Copy link
Member

@thynson I'll do. Just need to merge master into next first.

@ohjeyong
Copy link
Contributor Author

@pleerock
This PR cause problem when schema is set in ormconfig.
The PR(#5614) can fix this issue without any problem. Please check. Thanks.

@thynson
Copy link

thynson commented May 25, 2020

I'll do. Just need to merge merge master into next first.

Pinging for that 0.2.25 released. 🚀🚀🚀

@BlakeGardner
Copy link

For anyone else looking for a solution to this issue, this was affecting me with Postgres using the following code:

  @Column({
    type: 'point',
    spatialFeatureType: 'Point',
    srid: 4326,
  })
  point: string;

For whatever reason, changing my column to this fixed the issue for me:

import { Point } from 'typeorm';


  @Column('geometry', {
    spatialFeatureType: 'Point',
    srid: 4326,
  })
  point: Point;

Not entirely sure if this is still considered a bug, but it works.

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

Successfully merging this pull request may close these issues.

None yet

6 participants