sea-orm icon indicating copy to clipboard operation
sea-orm copied to clipboard

partialEq for value inside activevalue

Open Diwakar-Gupta opened this issue 1 year ago • 8 comments

PR Info

  • Closes #1533

  • Dependencies: No Dependencies

  • Dependents: No Dependents

New Features

  • [x] 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

  • [x] This is backward compatable

Changes

  • [x] added impl<U, V> PartialEq<U> for ActiveValue<V>

Diwakar-Gupta avatar Apr 25 '23 14:04 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));

Diwakar-Gupta avatar Apr 25 '23 14:04 Diwakar-Gupta

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 avatar Apr 25 '23 15:04 billy1624

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

billy1624 avatar Apr 25 '23 15:04 billy1624

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()

Diwakar-Gupta avatar Apr 26 '23 04:04 Diwakar-Gupta

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 avatar May 03 '23 03:05 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.

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?

tyt2y3 avatar May 17 '23 07:05 tyt2y3

based on the requirement from issue this will work.

Never thought it can be solved with this simplicity.

Diwakar-Gupta avatar May 17 '23 08:05 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());

Diwakar-Gupta avatar May 24 '23 08:05 Diwakar-Gupta