surrealdb / surrealdb.js

SurrealDB SDK for JavaScript
https://surrealdb.com
Apache License 2.0
271 stars 46 forks source link

Bug: Type mismatch in geometries #263

Closed tai-kun closed 1 month ago

tai-kun commented 1 month ago

Describe the bug

The type of line in GeometryLine is defined as follows:

export class GeometryLine extends Geometry {
    readonly line: [GeometryPoint, GeometryPoint];
}

GeometryLine is managed as a LineString type, and according to the geo crate, it is defined as "a series of continuous line segments represented by two or more coordinates." (ref). However, geo ignores this definition unless necessary, so in reality, it can have fewer than two coordinates. Therefore, I think the definition should be as follows:

export class GeometryLine extends Geometry {
    readonly line: GeometryPoint[];
}
Running in Rust ![Screenshot from 2024-05-15 11-22-27](https://github.com/surrealdb/surrealdb.js/assets/87357925/85c428a1-bde3-436d-8485-14a8f2f33a29)

Upon examining other geometries, classes requiring similar modifications seem to be GeometryMultiPoint, GeometryMultiLine, and GeometryMultiPolygon.

It was expected that Point and Polygon would require one or more coordinates, but I'm unsure why GeometryCollection geometries would also need one or more.

Running in Rust ![Screenshot from 2024-05-15 11-40-03](https://github.com/surrealdb/surrealdb.js/assets/87357925/f78bebd1-ca4b-45b3-bede-e7173e7fcb87)

P.S. Since GeometryPolygon requires at least one GeometryLine, I think you also need to modify it to readonly polygon: [GeometryLine, ...GeometryLine[]];.

Steps to reproduce

Each ring of the polygon is a LineString, and they must be closed and simple LineStrings. When an incomplete LineString is given to the polygon, geo tries to close it. Therefore, it fails the following test:

Deno.test("a", async () => {
    const surreal = await createSurreal();
    const input = new GeometryPolygon([
        new GeometryLine([
            new GeometryPoint([1, 2]),
            new GeometryPoint([3, 4]),
        ]),
        new GeometryLine([
            new GeometryPoint([5, 6]),
            new GeometryPoint([7, 8]),
        ]),
    ]);
    const [output] = await surreal.query<[typeof input]>(
        /* surql */ `$input`,
        { input },
    );

    assertEquals(JSON.parse(JSON.stringify(output)), {
        type: "Polygon",
        coordinates: [
            [["1", "2"], ["3", "4"], ["1", "2"]],
            [["5", "6"], ["7", "8"], ["5", "6"]],
        ],
    });

    await surreal.close();
});
Running in Rust ![Screenshot from 2024-05-15 12-33-02](https://github.com/surrealdb/surrealdb.js/assets/87357925/5a54805f-72e2-4b63-ac5e-a84c4447cdc8)

Test result:

a => ./tests/integration/tests/querying.ts:303:6
error: AssertionError: Values are not equal.

    [Diff] Actual / Expected

    {
      coordinates: [
        [
          [
            "1",
            "2",
          ],
          [
            "3",
            "4",
          ],
+         [
+           "1",
+           "2",
+         ],
        ],
        [
          [
            "5",
            "6",
          ],
          [
            "7",
            "8",
+         ],
+         [
+           "5",
+           "6",
          ],
        ],
      ],
      type: "Polygon",
    }

Expected behaviour

It is expected that the type of the geometry class matches the actual value.

SurrealDB version

1.5.0 for linux on x86_64

SurrealDB.js version

v1.0.0-beta.6

Contact Details

No response

Is there an existing issue for this?

Code of Conduct

tai-kun commented 1 month ago

This test might be more straightforward in terms of semantics:

Deno.test("issue #263", async () => {
    const surreal = await createSurreal();

    const input = new GeometryPolygon([
        new GeometryLine([
            new GeometryPoint([1, 2]),
            new GeometryPoint([3, 4]),
        ]),
        new GeometryLine([
            new GeometryPoint([5, 6]),
            new GeometryPoint([7, 8]),
        ]),
    ]);
    const [output, expected] = await surreal.query<[
        typeof input,
        GeometryPolygon,
    ]>(
        /* surql */ `
            $input;
            {
                type: "Polygon",
                coordinates: [
                    [ [1, 2], [3, 4] ],
                    [ [5, 6], [7, 8] ],
                ],
            };
        `,
        { input },
    );

    assertInstanceOf(expected, GeometryPolygon);

    const expectedJson = JSON.parse(JSON.stringify(expected));

    assertEquals(expectedJson, {
        type: "Polygon",
        coordinates: [
            [["1", "2"], ["3", "4"], ["1", "2"]],
            [["5", "6"], ["7", "8"], ["5", "6"]],
        ],
    });
    assertEquals(JSON.parse(JSON.stringify(output)), expectedJson);

    await surreal.close();
});

P.S: The test results are the same.

kearfy commented 1 month ago

Hey @tai-kun, I'm not sure if I understand the issue correctly. In the second assertion, only in the test, your added a third point in the lines of the polygon, but you have not added those third points anywhere else. In that sense it's expected that the test fails, right?

kearfy commented 1 month ago

Ah it's just the type that's incorrect! The test you added here threw me off a bit 😅

tai-kun commented 1 month ago

I'm sorry for the confusion. I couldn't figure out a good way to reproduce the inconsistency between the type and the actual value. I attempted to recreate it by exploiting the Geo crate to add a GeometryPoint to a GeometryLine in specific cases, but it was a desperate attempt. Please discard that test...

kearfy commented 1 month ago

I think the confusion is happening because what SurrealDB represents as a line (two points) is actually a linestring (two or more points). They're different datatypes, but we just simply use only linestrings.