partialEq for value inside activevalue

[Diwakar-Gupta]

PR Info

• Closes Add contains method for ActiveValue #1533

• Dependencies:
  No Dependencies

• Dependents:
  No Dependents

New Features

• this pr enables to compare ActiveValue dosent matter set or unset

assert!(ActiveValue::Set(2) == 2);
assert!(ActiveValue::UnSet(2) == 2);
assert!(ActiveValue::<u8>::NotSet != 2);

Breaking Changes

• This is backward compatable

Changes

• added impl<U, V> PartialEq<U> for ActiveValue<V>

[Diwakar-Gupta]

Just wanted to inform that

// This will work
assert!(ActiveValue::Set(2) == 2);

// This will give compilation error
assert!(2 == ActiveValue::Set(2));

[billy1624]

I think this is the best we can get:

assert!(ActiveValue::Set(2) == 2);

We wouldn't be able to make this works:

assert!(2 == ActiveValue::Set(2));

Because we simply can't implement PartialEq<ActiveValue<V>> for U

[billy1624]

However, I think we can provide an impl<V> PartialEq<sea_query::Value> for ActiveValue<V>?

[Diwakar-Gupta]

PartialEq<sea_query::Value>

This will make sure that we are only comparing with sea_query::Value rather than something that can be converted to sea_query::Value, am i right?

If i am right then user has to convert data to sea_query::Value to compare

ActiveValue::Set(2) == Value::from(2)
// or may be this
ActiveValue::Set(2) == 2.into_value()

[billy1624]

I mean comparing sea_orm::ActiveValue<?> with sea_query::Value. e.g.

assert!(ActiveValue::Set(2) == Value::from(2));
assert!(ActiveValue::Unchanged(String::from("hi")) == Value::from("hi"));

However, after second thought, it'd be more intuitive to do... instead

assert!(ActiveValue::Set(2) == 2);
assert!(ActiveValue::Unchanged(String::from("hi")) == "hi");

[billy1624]

Thanks!! @Diwakar-Gupta

[billy1624]

Hey @Diwakar-Gupta, after second thought... I think comparing ActiveValue<V> with Option<V> make more sense. This way we don't leave out ActiveValue::NotSet.

[tyt2y3]

Hey @Diwakar-Gupta, after second thought... I think comparing ActiveValue<V> with Option<V> make more sense. This way we don't leave out ActiveValue::NotSet.

I still feel like below has a little too much type magic:

assert!(ActiveValue::Set(2) == Some(2));

Actually, the solution can be much simpler:

we only have to add a method:

fn as_ref(&self) -> Option<&V>

Such that we can:

assert!(ActiveValue::Set(2).as_ref() == Some(2).as_ref());

this will work right?

[Diwakar-Gupta]

based on the requirement from issue this will work.

Never thought it can be solved with this simplicity.

[Diwakar-Gupta]

Can we give thought to possible solution once again

below only utilizes Set and Unchanged only but client api is very simple

assert!(ActiveValue::Set(2) == 2);

this utilizes Set and Unchanged and NotSet

assert!(ActiveValue::Set(2) == Some(2));

and the third one which get's converetd to Option<>

assert!(ActiveValue::Set(2).as_ref() == Some(2).as_ref());