vitaly-m / postgis-diesel

Postgis types extension for Diesel framework
MIT License
37 stars 15 forks source link

Diesel Generates Geometry Type #10

Closed atwoodjw closed 1 year ago

atwoodjw commented 2 years ago

I have a migration that creates a stops table.

up.sql

CREATE TABLE "stops" (
    "stop_id" varchar NOT NULL,
    "stop_lat_lon" geometry(Point,4326),
    PRIMARY KEY ("stop_id")
);

However, when run the migration (diesel migration run), Diesel generates the schema with new Geometry struct that overrides postgis_diesel::sql_types::Geometry.

schema.rs

// @generated automatically by Diesel CLI.

pub mod sql_types {
    #[derive(diesel::sql_types::SqlType)]
    #[diesel(postgres_type(name = "geometry"))]
    pub struct Geometry;
}

diesel::table! {
    use diesel::sql_types::*;
    use postgis_diesel::sql_types::*;
    use super::sql_types::Geometry;

    stops (stop_id) {
        stop_id -> Varchar,
        stop_lat_lon -> Nullable<Geometry>,
    }
}

This causes a build error.

the trait bound `postgis_diesel::types::Point: diesel::Expression` is not satisfied
the following other types implement trait `diesel::Expression`:
  &'a T
  (T0, T1)
  (T0, T1, T2)
  (T0, T1, T2, T3)
  (T0, T1, T2, T3, T4)
  (T0, T1, T2, T3, T4, T5)
  (T0, T1, T2, T3, T4, T5, T6)
  (T0, T1, T2, T3, T4, T5, T6, T7)
and 211 others
required because of the requirements on the impl of `diesel::Expression` for `&'insert postgis_diesel::types::Point`
required because of the requirements on the impl of `AsExpression<Nullable<commuter::schema::sql_types::Geometry>>` for `&'insert postgis_diesel::types::Point`rustc[E0277](https://doc.rust-lang.org/error-index.html#E0277)

Commenting out use super::sql_types::Geometry; resolves the issue.

Any idea what's happening here? I don't see this issue in integration_test.rs, but that schema code isn't dynamically generated.

vitaly-m commented 2 years ago

Hi, I think that is because diesel doesn't know about that type out of the box and creates custom type for geometry. I don't have hands-on experience in it unfortunately. Maybe it is possible to defined which type to use somehow in arguments.

myaple commented 2 years ago

@atwoodjw In case you haven't figured this out yet, and for all the future folks out there, in your diesel.toml set

generate_missing_sql_type_definitions = false

This will stop diesel from filling in SQL types that it doesn't have internally with the skeleton type.

https://diesel.rs/guides/configuring-diesel-cli.html

ThorbenStaufer commented 1 year ago

I have a similar issue as OC, but the proposed fix is unfortunately not suitable for my use case, since we rely on the auto-generation of SQL types in other places. Is there any other way to have diesel prioritize the already existing geometry/geography type?

myaple commented 1 year ago

If you're using a diesel.toml for config, the import_types field is in order. Things that are imported later overwrite things that are imported earlier. If you import specific names, it should overwrite the previous ones I think.

ThorbenStaufer commented 1 year ago

Specifying the exact type instead of using the *-operator in the import_types field worked actually. It is weird that diesel still generates it's own Geography struct, but hovering over the definition in schema.rs reveals that the postgis_diesel type is used. Thank you very much for the help.

ThorbenStaufer commented 1 year ago

One additional problem just came up. It now uses the correct Geography type, but since Diesel still generates its own, I get an error for having multiple definitions for Geography.

vitaly-m commented 1 year ago

I have this configuration in diesel.toml in my project, it works for me.

 [print_schema]
file = "src/schema.rs"
import_types = ["diesel::sql_types::*", "postgis_diesel::sql_types::*", "crate::sql_types::*"]
GHOSCHT commented 6 months ago

One additional problem just came up. It now uses the correct Geography type, but since Diesel still generates its own, I get an error for having multiple definitions for Geography.

How exactly did you fix your problem @ThorbenStaufer ? I'm currently facing exactly the same reimport error due to multiple definitions for Geography with import_types = ["diesel::sql_types::*", "postgis_diesel::sql_types::Geometry"].

Sadly the previously mentioned import_types = ["diesel::sql_types::*", "postgis_diesel::sql_types::*", "crate::sql_types::*"] doesn't work for me either.

vitaly-m commented 6 months ago

Hi @GHOSCHT,

Try to generate patch file for the generated schema which will remove redefinition of custom types. See https://diesel.rs/guides/configuring-diesel-cli.html patch_file.

  1. Generate schema file with diesel print-schema > src/full_schema.rs.
  2. Remove not required SQL types from it and save to src/schema.rs.
  3. Run diff -U6 src/full_schema.rs src/schema.rs > src/schema.patch.
  4. Add patch_file = "src/schema.patch" to diesel.toml.
  5. Remove src/full_schema.rs, check that diesel print-schema > src/schema.rs will not add Geometry type.

I did the same for my project the final patch file looks like:

@@ -1,12 +1,9 @@
 // @generated automatically by Diesel CLI.

 pub mod sql_types {
-    #[derive(diesel::query_builder::QueryId, diesel::sql_types::SqlType)]
-    #[diesel(postgres_type(name = "geometry"))]
-    pub struct Geometry;

     #[derive(diesel::query_builder::QueryId, diesel::sql_types::SqlType)]
     #[diesel(postgres_type(name = "intensity"))]
     pub struct Intensity;

     #[derive(diesel::query_builder::QueryId, diesel::sql_types::SqlType)]
@@ -52,13 +49,12 @@

 diesel::table! {
     use diesel::sql_types::*;
     use postgis_diesel::sql_types::*;
     use super::sql_types::Intensity;
     use super::sql_types::Triggermethod;
-    use super::sql_types::Geometry;

     laps (activity_id, started_at, manual_track) {
         activity_id -> Uuid,
         started_at -> Timestamptz,
         total_time_seconds -> Float8,
         distance_meters -> Float8,
rj76 commented 6 months ago

Oh, this is great. I used to this by hand each time :)

GHOSCHT commented 6 months ago

@vitaly-m Thanks for the quick and precise answer! It worked like a charm. Using patches would have also been my approach if I didn't get any reply, but I wasn't sure if there was a more elegant way of doing it.

I think adding this to the documentation might be worth it, since most newcomers will probably encounter this issue.