All committers have signed the CLA.

Absolutely boss 🚀

CodSpeed Performance Report

Merging #4208 will not alter performance

_{Comparing Oreilles:feat/geometry (943c80d) with main (00f0d73)}

Summary

✅ 11 untouched benchmarks

Amazing.

So that the tests are automatically run going forward:
Can you quickly do a PR to main where you change line 5 to [![Cargo docs](https://github.com/prisma/prisma-engines/actions/workflows/on-push-to-main.yml/badge.svg)](https://github.com/prisma/prisma-engines/actions/workflows/on-push-to-main.yml)? I can then merge that, and the repository will remember that we trust you and run the tests without me clicking a button. (And that button needs to be fixed anyway 😆)

So there was quite a lot of things I had overlooked when testing on my machine..! I squashed all the fixes and we now have a mostly green test suite 👍

🚀

Two test failures left for now:

1. Something with 1 PostgreSQL metrics test, 7 v.s 9 metrics recorded or something. Could your PR cause this? (We recently change something in metrics, so you might want to rebase (Update branch button in GH UI) to get the most recent version of that code).
2. A lot of unhappiness in the Vitess 8 tests with it not being able to introspect the tables because of permission problems. Strange. That looks a bit ... weird, even flaky maybe? I retriggered a test run there. Maybe it just goes green.

PS: No need from our side to combine your PRs and force push, we only merge via "Squash and merge" which combines all the commits, and with the individual commits of the fixes it might be easier to understand what test failures where fixed where - or introduced.

So the two additional queries breaking the tests are due to:

CREATE EXTENSION IF NOT EXISTS postgis
https://github.com/Oreilles/prisma-engines/blob/a588e1296288b79df0204397a441f2b3d891b9d9/query-engine/connector-test-kit-rs/qe-setup/src/postgres.rs#L42

and

SELECT PostGIS_version()
https://github.com/Oreilles/prisma-engines/blob/a588e1296288b79df0204397a441f2b3d891b9d9/schema-engine/connectors/sql-schema-connector/src/flavour/postgres.rs#L628-L630

Both are needed to run the tests, I updated the counter to 9 to make the test succeed.
( Also rebased on main just in case... )

No clue about the Vitess issue though...

EDIT: I tried running the vitess tests on prisma:main on my machine and the result is the same, which makes me assume that this is not due to a code change in this PR...

EDIT: it's now all fixed 🙌

How can I help to merge it?

Test it, figure out how it would fit into Prisma, let the author and us know what else is needed to make this a complete feature.

@janpio @oreilles thanks for all you do guys. I am at the point in my project to start introducing PostGIS and trying to figure out how to test this PR. I cloned @oreilles feature branch and compiled debug targets of prisma-engines how do i tie it into existing prisma to run test migration/introspection ? thank you

@okonon Have a look at the documentation: https://www.prisma.io/docs/concepts/components/prisma-engines#using-custom-engine-libraries-or-binaries

@RafaelKr thanks a lot for that sorry i missed that section.
I was able to test introspection from existing database (prisma db pull):

Looks like it is introspecting correctly.

Adding a comment to voice support for this to be added to Prisma! Looking forward to seeing this merged; keep up the good work.

Definitely would love that to be merged as well 👍

Sorry, but this will definitely not be merged as it is only part of the full feature that needs to be implemented.

But we are interested in bringing it there, so let's see what we can do here.

For now I created a branch based on the commit this was forked from, so that there are no conflicts and @oreilles can keep committing if they want.

I also created a local branch of the code that runs all our CI (Buildkite is not run on external contributions), with the goal of publishing the engines to our infrastructure, so we can build a Prisma CLI and Client with them for further testing.

But unfortunately some tests are failing (results only internally visible): #4427

Turns out GitHub Actions do not run for these databases, and seems the current code has some problems - and that is blocking my plan to release these engine files :/

Random example of a SQLite failure, our of many ([2023-11-08T21:25:35Z] test result: FAILED. 622 passed; 970 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.73s):

[2023-11-08T21:25:35Z] thread 'writes::top_level_mutations::delete_many_relations::delete_many_rels::pm_c1_no_children' panicked at 'called `Result::unwrap()` on an `Err` value: Error { kind: QueryError(SqlInputError { error: Error { code: Unknown, extended_code: 1 }, msg: "no such function: InitSpatialMetaData", sql: "SELECT InitSpatialMetaData()", offset: 7 }), original_code: Some("1"), original_message: Some("no such function: InitSpatialMetaData") }', /root/build/query-engine/connector-test-kit-rs/qe-setup/src/sqlite.rs:9:65

MySQL 5.6 only fails 2:

[2023-11-08T21:35:48Z] failures:
[2023-11-08T21:35:48Z]     queries::filters::geometry_filter::geometry_filter_spec::geometric_comparison_filters
[2023-11-08T21:35:48Z]     writes::data_types::native_types::mysql::mysql::native_geojson_srid_geometry_types

5.7 only 1 even:

[2023-11-08T21:39:12Z] failures:
[2023-11-08T21:39:12Z]     writes::data_types::native_types::mysql::mysql::native_geojson_srid_geometry_types

MariaDB the same as 5.6:

[2023-11-08T21:31:19Z] failures:
[2023-11-08T21:31:19Z]     queries::filters::geometry_filter::geometry_filter_spec::geometric_comparison_filters
[2023-11-08T21:31:19Z]     writes::data_types::native_types::mysql::mysql::native_geojson_srid_geometry_types

Trying to get these tests to run on GitHub Actions now. But maybe you have speculative fixes @oreilles?

Interesting, as far as I can remember (almost) all tests were succeeding the last time we discussed the PR, and we did have tests for all MySQL versions... though I might remember wrong. Did you resolve merge conflicts when you merged main, which could explain tests failing afterwards ?

Anyway, I'll have a look as soon as possible. @janpio if that's something you can do, please send me the test cli commands to reproduce the failing tests.

Edit: nevermind, I had not seen that they were already mentioned in your message!

Got back at it, now up to date with main!

Made many changes to make it easier to digest (and maintain), specifically:

• Drop support for types incompatible with GeoJSON (M / ZM geometries and extended types (CircularString etc))
• Drop support for (E)WKT and make GeoJSON the only io format
• Therefore, only a single new Geometry prisma type identifier is needed

I had to disable the schema engine tests CI for Spatialite, since libspatialite >= 5.0.0 is required and that only ships starting with Ubuntu 22.04. (the CI fail with an SSL error if we try to update the runner).

I will be so happy when this feature will be merged! Thank you for your work and effort ❤️

This feature could be a real life changer for me! Thanks for all the work💚💚💚

Please push this forward! 👍

Is development still ongoing for this?

Please push this forward!

Any updates on this?

Any update?

Updates?