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

Make QueryResult fields and QueryResultRow public #336

Closed
qyihua opened this issue Nov 27, 2021 · 12 comments
Closed

Make QueryResult fields and QueryResultRow public #336

qyihua opened this issue Nov 27, 2021 · 12 comments

Comments

@qyihua
Copy link

qyihua commented Nov 27, 2021

Hello team,
I want to use postgis types with postgis crate, but the geometry type is not support currently

So, I use custom type like this:

use postgis::ewkb::Point as EwkbPoint;

#[derive(Clone, Debug)]
pub struct Point(EwkbPoint);

impl sqlx::Type<Postgres> for Point {
    fn type_info() -> PgTypeInfo {
        PgTypeInfo::with_name("geometry")
    }
}

impl<'de> sqlx::Decode<'de, Postgres> for Point {
    fn decode(value: PgValueRef<'de>) -> Result<Self, BoxDynError> {
        if value.is_null() {
            // Return empty geometry
            return Ok(Point(EwkbPoint::new(0.0, 0.0, None)));
        }
        let blob = <&[u8] as Decode<Postgres>>::decode(value)?;
        let mut file = Cursor::new(blob);
        let p = EwkbPoint::read_ewkb(&mut file).unwrap();
        return Ok(Point(p));
    }
}

impl<'q, DB> sqlx::Encode<'q, DB> for Point {
...
}

impl Serialize for Point {
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
    where
        S: serde::Serializer,
    {
        let s = format!("{} {}", (*self).x, (*self).y);
        serializer.serialize_str(s.as_str())
    }
}

impl<'de> Deserialize<'de> for Point {
    fn deserialize<D>(deserializer: D) -> Result<Point, D::Error>
    where
        D: Deserializer<'de>,
    {
        deserializer.deserialize_bytes(PointVisitor)
    }
}

impl PartialEq for Point {
   ...
}

impl Nullable for Point {
    fn null() -> Value {
        Value::Bytes(None)
    }
}

impl From<Point> for Value {
    fn from(p: Point) -> Self {
        let buf: Vec<u8> = Vec::new();
        let mut buf = Cursor::new(buf);
        (*p).as_ewkb().write_ewkb_body(&mut buf).unwrap();
        Value::Bytes(Some(Box::new(buf.into_inner())))
    }
}

impl ValueType for Point {
    fn try_from(v: Value) -> Result<Self, database::sea_query::ValueTypeErr> {
        match v {
            Value::Bytes(Some(buf)) => {
                let mut file = Cursor::new(*buf);
                let p = EwkbPoint::read_ewkb(&mut file).unwrap();
                Ok(Point(p))
            }
            _ => Err(sea_query::ValueTypeErr),
        }
    }

    fn type_name() -> String {
        stringify!(Point).to_owned()
    }

    fn column_type() -> database::sea_query::ColumnType {
        sea_query::ColumnType::Custom(NameIden("geometry").into_iden())
    }
}

impl TryGetable for Point {
    fn try_get(res: &QueryResult, pre: &str, col: &str) -> Result<Self, sea_orm::TryGetError> {
        let column = format!("{}{}", pre, col);
        match &res.row {
            sea_orm::QueryResultRow::SqlxPostgres(row) => {
                use sqlx::Row;
                row.try_get::<Option<Point>, _>(column.as_str())
                    .map_err(|e| sea_orm::TryGetError::DbErr(sea_orm::sqlx_error_to_query_err(e)))
                    .and_then(|opt| opt.ok_or(sea_orm::TryGetError::Null))
            }
            _ => Err(sea_orm::TryGetError::DbErr(
                DbErr::Custom("only unsupport postgresql".to_owned()).into(),
            )),
        }
    }
}

convert geometry to point need impl sqlx::Decode, So I need QueryResultRow public

Would it be possible for these fields to be changed to be public similar to the relationship data? Alternatively could getter methods be implemented to fetch the row stored within QueryResult?

If they are public, it's able to impl any custom types in database by user.
I use sea-orm in rust. sea-orm and sea-query is wonderful! Thanks yours work!

@tyt2y3
Copy link
Member

tyt2y3 commented Nov 27, 2021

I don't think so as we do not want to 'leak' the SQLx types through the API.

Although it seems that these integration could be built into the library behind a feature gate. It benefits everyone then.

PR would be welcome. Let us know your thoughts?

@qyihua
Copy link
Author

qyihua commented Nov 27, 2021

I don't think so as we do not want to 'leak' the SQLx types through the API.

Although it seems that these integration could be built into the library behind a feature gate. It benefits everyone then.

PR would be welcome. Let us know your thoughts?

Thank you for your reply quickly.

I am new to rust, and not very familiar with rust, so I couldn't make a right PR now.
I have a thought,
defined a struct just like named RawType that contain Vec , the raw binary data from database, then everyone can convert it to another type in rust within TryGetable trait.And it don't need to 'leak' sqlx types.
The RawType need impl sqlx::Type , and fn compatible() return true always.
I don't know if it will work?
I want to have a try 😀

@tyt2y3
Copy link
Member

tyt2y3 commented Nov 28, 2021

Yeah that's one solution. But we can also save all the hassle.

I was suggesting may be we can instead provide postgis support inside SeaORM. Such that we can simply dump all the boilerplate into a separate file which can be enabled with a sqlx-postgis feature flag.

@qyihua
Copy link
Author

qyihua commented Nov 28, 2021

Yeah that's one solution. But we can also save all the hassle.

I was suggesting may be we can instead provide postgis support inside SeaORM. Such that we can simply dump all the boilerplate into a separate file which can be enabled with a sqlx-postgis feature flag.

Oh, I see.But I am sorry I am incapable of implement the features now.

@billy1624
Copy link
Member

billy1624 commented Nov 28, 2021

Hey @qyihua, I think SQLx did try to support postgis crate but it's still WIP. The first comment you made might be helpful for them.

Just a side note loll

@qyihua
Copy link
Author

qyihua commented Nov 29, 2021

I have an issue:
postgis type need convert to json value, I found crate geo-json and geozero::wkt, but they haven't support 3d point in postgis yet.
I don't know how to deal with the serialize of postgis type. And it will have a lot of work without geo-json or geozero.
I am not able to save all the hassle now.
Can anyone help? @tyt2y3 @billy1624

@billy1624
Copy link
Member

Related issue: #559

@billy1624
Copy link
Member

I have an issue: postgis type need convert to json value, I found crate geo-json and geozero::wkt, but they haven't support 3d point in postgis yet. I don't know how to deal with the serialize of postgis type. And it will have a lot of work without geo-json or geozero. I am not able to save all the hassle now. Can anyone help? @tyt2y3 @billy1624

Hey @qyihua, sorry for the delay. I think you can use newtype pattern to define a custom serializer / deserializer?

pub struct NewStruct(postgis::SomeType);

// implement custom serializer and deserializer for NewStruct
impl ... for NewStruct { ... }

@billy1624
Copy link
Member

I'll close this for now. If you still have questions please reopen this :)

@Zizico2
Copy link

Zizico2 commented Jan 11, 2023

Is this any further along?

Even this

I don't think so as we do not want to 'leak' the SQLx types through the API.
Although it seems that these integration could be built into the library behind a feature gate. It benefits everyone then.
PR would be welcome. Let us know your thoughts?

Thank you for your reply quickly.

I am new to rust, and not very familiar with rust, so I couldn't make a right PR now. I have a thought, defined a struct just like named RawType that contain Vec , the raw binary data from database, then everyone can convert it to another type in rust within TryGetable trait.And it don't need to 'leak' sqlx types. The RawType need impl sqlx::Type , and fn compatible() return true always. I don't know if it will work? I want to have a try 😀

as a stop gap solution?

@billy1624
Copy link
Member

billy1624 commented Jan 11, 2023

Hey @Zizico2, would you be interested to implement that stopgap solution?

@Zizico2
Copy link

Zizico2 commented Jan 11, 2023

Yes!
Could you point me in the general direction?

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

4 participants