Electron100 / butane

An ORM for Rust with a focus on simplicity and on writing Rust, not SQL
Apache License 2.0
91 stars 13 forks source link

HashMap support #32

Closed jayvdb closed 1 year ago

jayvdb commented 1 year ago

HashMap's are not supported yet. In postgres they can be implemented as a hstore. In sqlite, they could be a separate table, e.g. roughly like Many, or serialised to a single field using JSON, i.e. https://github.com/Electron100/butane/issues/27

Electron100 commented 1 year ago

Hmm, I'm a little torn on this one. I'm sure the folks at Postgres had good reasons for adding the hstore type but on the other hand Postgres has a lot of non-standard SQL types and I'm a bit reluctant to support them all out of the box.

I think ideally what I'd like to do here is improve support for custom types and allow ToSql/FromSql to provide different types based on the backend. That would make it possible to write a SQL transformations for things like this that used Postgres's extra types for that backend but didn't push a new core type into other backends that they needed to implement. I haven't fleshed out the details there though I'll admit.

I'm not necessarily opposed to this, but I would be interested to hear a little bit more about your use case if it's something you're able to talk about or summarize publicly -- when do you store HashMaps in a database?

jayvdb commented 1 year ago

A HashMap is one of the OpenAPI/JSONSchema container types. c.f. https://stackoverflow.com/questions/40750340/how-to-define-json-schema-for-mapstring-integer This container type offers a uniqueness property not available elsewhere in OpenAPI/JSONSchema. It is supported by https://github.com/oxidecomputer/progenitor and https://github.com/juhaku/utoipa . It would be nice to be able to store these objects in the database.

All backends could implement the HashMap storage using JSON. That seems to be the approach taken at https://github.com/SeaQL/sea-orm/issues/279 and https://github.com/diesel-rs/diesel/issues/970 . Always using JSON will avoid any limitations of hstore.

Electron100 commented 1 year ago

Hmm, part of what I'm asking is: does the underlying Postgres hstore type matter to you? Do you want to be able to write SQL queries that depends on that underlying structure? Or do you just want to read/write a HashMap from/to a database as-is and the representation doesn't matter: could be json, could be a blob, or whatever?

If the latter then I think we can just do a straight ToSql/FromSql impl without regard to the backend.

jayvdb commented 1 year ago

I dont mind what the underlying data storage is. Happy to be avoiding hstore to be honest.

jayvdb commented 1 year ago

With https://github.com/Electron100/butane/pull/43 , the following badly written test passes.

#[butane_type(Json)]
#[derive(PartialEq, Eq, Debug, Clone)]
struct StringMap(HashMap<String, String>);

impl StringMap {
    fn new() -> Self {
        StringMap {
            0: HashMap::<String, String>::new(),
        }
    }
}

impl Default for StringMap {
    fn default() -> Self {
        StringMap {
            0: HashMap::<String, String>::default(),
        }
    }
}

impl ToSql for StringMap {
    fn to_sql(&self) -> SqlVal {
        self.to_sql_ref().into()
    }
    fn to_sql_ref(&self) -> SqlValRef<'_> {
        SqlValRef::Json(serde_json::Value::Object(serde_json::Map::from_iter(
            self.0
                .iter()
                .map(|x| (x.0.to_owned(), serde_json::Value::from(x.1.to_owned()))),
        )))
    }
}
impl FromSql for StringMap {
    fn from_sql_ref(val: SqlValRef) -> Result<Self, butane::Error> {
        match val {
            SqlValRef::Json(s) => {
                let mut rv = StringMap::default();
                if let serde_json::Value::Object(m) = s {
                    for (k, v) in m {
                        if let serde_json::Value::String(v) = v {
                            rv.0.insert(k, v);
                        }
                    }
                }
                Ok(rv)
            }
            _ => Err(butane::Error::CannotConvertSqlVal(
                SqlType::Json,
                val.into(),
            )),
        }
    }
}
impl FieldType for StringMap {
    type RefType = Self;
    const SQLTYPE: SqlType = SqlType::Json;
}

#[model]
#[derive(PartialEq, Eq, Debug, Clone)]
struct FooHH {
    id: i64,
    val: StringMap,
    bar: u32,
}
impl FooHH {
    fn new(id: i64) -> Self {
        FooHH {
            id,
            val: StringMap::default(),
            bar: 0,
            state: ObjectState::default(),
        }
    }
}
fn hashmap_json(conn: Connection) {
    //create
    let id = 4;
    let mut foo = FooHH::new(id);
    let mut data = StringMap::new();
    data.0.insert("a".to_string(), "1".to_string());

    foo.val = data;
    foo.save(&conn).unwrap();

    // read
    let mut foo2 = FooHH::get(&conn, id).unwrap();
    assert_eq!(foo, foo2);

    // update
    foo2.bar = 43;
    foo2.save(&conn).unwrap();
    let foo3 = FooHH::get(&conn, id).unwrap();
    assert_eq!(foo2, foo3);
}
testall!(hashmap_json);

I've got a partial implementation which doesnt need the newtype and all of the boilerplate to go with it, so "HashMap"'s "just work", but it fails because the custom type HashMap<String, String> isn't added to types.json, as that needs #[butane_type(Json)], and that needs a newtype.