Hashable Value

[tyt2y3]

Sometimes (#1238) we need to use the Value as HashKey, but we cannot impl Hash and Eq for Value. The only thing preventing this is f32/f64.

So I propose we make a new enum with a subset of Value variants (and also cannot be null). To reduce the maintainence burden, I think realistically we only need to consider the Rust built-in types (with the exception of UUID because it is likely to be used as primary key):

#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub enum HashableValue {
    Bool(bool),
    TinyInt(i8),
    SmallInt(i16),
    Int(i32),
    BigInt(i64),
    TinyUnsigned(u8),
    SmallUnsigned(u16),
    Unsigned(u32),
    BigUnsigned(u64),
    String(Box<String>),
    Char(char),
    #[allow(clippy::box_collection)]
    Bytes(Box<Vec<u8>>),
    #[cfg(feature = "with-uuid")]
    #[cfg_attr(docsrs, doc(cfg(feature = "with-uuid")))]
    Uuid(Box<Uuid>),
}

and a few conversion functions:

impl std::convert::TryFrom<Value> for HashableValue {
    fn try_from(value: Value) -> Result<Self, Self::Error> {
        match value {
            Int(Some(v)) => Ok(Self::Int(v)),
            String(v) | Bytes(v) => // these should be non-empty
            ...
            _ => Err(...),
        }
    }
}

impl std::convert::From<HashableValue> for Value {
    fn from(value: HashableValue) -> Self {
        // this should always succeed
    }
}

[tyt2y3]

@ikrivosheev what's your thought on this design?

[ikrivosheev]

@tyt2y3 can you explain, why we need HashableValue?

[tyt2y3]

The use case in SeaORM / Seaography was that we'd basically want to group objects by primary keys.

Say we have 1->N relationship for Entity A & B, and we selected a list of : A.id, B.* and we now want to use a HashMap to group all B belonging to A by primary key. Right now we convert the primary key to string, which works, but is inefficient.

[ikrivosheev]

@tyt2y3, now i understand, thank you! Well, why do you want to implement this in SeaQuery? I think this can be a part of SeaORM. If we want to change something in the future it will be easier do this in SeaORM. And SeaORM can also control this.

[tyt2y3]

Yeah can live in SeaORM as well. No preference on that.

[karatakis]

Why we just don't implement Hash for Value ?

impl std::hash::Hash for Value {
    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
        match self {
            Self::Bool(value) => {
                value.hash(state)
            },
            Self::TinyInt(value) => {
                value.hash(state)
            },
            Self::SmallInt(value) => {
                value.hash(state)
            },
            Self::Int(value) => {
                value.hash(state)
            },
            Self::BigInt(value) => {
                value.hash(state)
            },
            Self::TinyUnsigned(value) => {
                value.hash(state)
            },
            Self::SmallUnsigned(value) => {
                value.hash(state)
            },
            Self::Unsigned(value) => {
                value.hash(state)
            },
            Self::BigUnsigned(value) => {
                value.hash(state)
            },
            Self::Float(value) => {
                let value = format!("{:?}", value);
                value.hash(state)
            },
            Self::Double(value) => {
                let value = format!("{:?}", value);
                value.hash(state)
            },
            Self::String(value) => {
                value.hash(state)
            },
            Self::Char(value) => {
                value.hash(state)
            },
            Self::Bytes(value) => {
                value.hash(state)
            },
            #[cfg(feature = "with-json")]
            #[cfg_attr(docsrs, doc(cfg(feature = "with-json")))]
            Self::Json(value) => {
                value.hash(state)
            },
            #[cfg(feature = "with-chrono")]
            #[cfg_attr(docsrs, doc(cfg(feature = "with-chrono")))]
            Self::ChronoDate(value) => {
                value.hash(state)
            },
            #[cfg(feature = "with-chrono")]
            #[cfg_attr(docsrs, doc(cfg(feature = "with-chrono")))]
            Self::ChronoTime(value) => {
                value.hash(state)
            },
            #[cfg(feature = "with-chrono")]
            #[cfg_attr(docsrs, doc(cfg(feature = "with-chrono")))]
            Self::ChronoDateTime(value) => {
                value.hash(state)
            },
            #[cfg(feature = "with-chrono")]
            #[cfg_attr(docsrs, doc(cfg(feature = "with-chrono")))]
            Self::ChronoDateTimeUtc(value) => {
                value.hash(state)
            },
            #[cfg(feature = "with-chrono")]
            #[cfg_attr(docsrs, doc(cfg(feature = "with-chrono")))]
            Self::ChronoDateTimeLocal(value) => {
                value.hash(state)
            },
            #[cfg(feature = "with-chrono")]
            #[cfg_attr(docsrs, doc(cfg(feature = "with-chrono")))]
            Self::ChronoDateTimeWithTimeZone(value) => {
                value.hash(state)
            },
            #[cfg(feature = "with-time")]
            #[cfg_attr(docsrs, doc(cfg(feature = "with-time")))]
            Self::TimeDate(value) => {
                value.hash(state)
            },
            #[cfg(feature = "with-time")]
            #[cfg_attr(docsrs, doc(cfg(feature = "with-time")))]
            Self::TimeTime(value) => {
                value.hash(state)
            },
            #[cfg(feature = "with-time")]
            #[cfg_attr(docsrs, doc(cfg(feature = "with-time")))]
            Self::TimeDateTime(value) => {
                value.hash(state)
            },
            #[cfg(feature = "with-time")]
            #[cfg_attr(docsrs, doc(cfg(feature = "with-time")))]
            Self::TimeDateTimeWithTimeZone(value) => {
                value.hash(state)
            },
            #[cfg(feature = "with-uuid")]
            #[cfg_attr(docsrs, doc(cfg(feature = "with-uuid")))]
            Self::Uuid(value) => {
                value.hash(state)
            },
            #[cfg(feature = "with-rust_decimal")]
            #[cfg_attr(docsrs, doc(cfg(feature = "with-rust_decimal")))]
            Self::Decimal(value) => {
                value.hash(state)
            },
            #[cfg(feature = "with-bigdecimal")]
            #[cfg_attr(docsrs, doc(cfg(feature = "with-bigdecimal")))]
            Self::BigDecimal(value) => {
                value.hash(state)
            },
            #[cfg(feature = "postgres-array")]
            #[cfg_attr(docsrs, doc(cfg(feature = "postgres-array")))]
            Self::Array(value) => {
                value.hash(state)
            },
            #[cfg(feature = "with-ipnetwork")]
            #[cfg_attr(docsrs, doc(cfg(feature = "with-ipnetwork")))]
            Self::IpNetwork(value) => {
                value.hash(state)
            },
            #[cfg(feature = "with-mac_address")]
            #[cfg_attr(docsrs, doc(cfg(feature = "with-mac_address")))]
            Self::MacAddress(value) => {
                value.hash(state)
            },
        }
    }
}

The f32 and f64 needs some work

[tyt2y3]

That would do, although we can only put this inside sea-query (because of orphan rule)

[karatakis]

Should I open a pull request?

[tyt2y3]

And we'd need to impl Eq as well to be usable as hash key.
What's your thought on this? @ikrivosheev